Skip to content

fix: harden OpenShift GIM/VFIO operator path - #1

Closed
johnahull wants to merge 14 commits into
mainfrom
fix/gim-vfio-vm-e2e
Closed

johnahull wants to merge 14 commits into
mainfrom
fix/gim-vfio-vm-e2e

Conversation

@johnahull

@johnahull johnahull commented Nov 5, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Make OpenShift MCO the owner of GIM host configuration and stop the node-labeller from mutating host modprobe files.
  • Add narrowly scoped SCC bindings for GIM/VFIO operand service accounts.
  • Gate VF passthrough readiness on the current device-plugin DaemonSet rollout and AMD Node allocatable resources, with status/resource watchers.
  • Document the OpenShift and KubeVirt GIM/VFIO prerequisites.

Validation

  • Focused controller, watcher, and node-labeller tests pass.
  • Non-E2E Go tests, go vet ./..., manager build, Helm lint/render, and whitespace checks pass.
  • Live OpenShift validation used the built operator image and confirmed DevicePluginPending when no amd.com/gpu resource was advertised.
  • The existing KubeVirt VFIO smoke VM remains Running/Ready through the built-in amd.com/kvgpu path.

Scope and limitations

  • This PR contains operator-only changes; the AMD device-plugin repository is unchanged.
  • Full E2E tests remain environment-blocked by missing E2E_UBUNTU_BASE_IMAGE, GPU_OPERATOR_CHART, and the local test API endpoint.
  • Markdown lint could not run because markdownlint-cli2 is not installed.

Draft PR for review of the GIM/VFIO operator path.

Summary by Sourcery

Harden the OpenShift GIM/VFIO operator path with MCO-owned host configuration, scoped security permissions, reliable VFIO processing, and resource-aware readiness reporting.

New Features:

  • Add readiness tracking for VFIO passthrough that verifies KMM, VFIO binding, device-plugin rollout, and advertised AMD resources.
  • Support current and legacy GIM release artifact layouts for passthrough driver builds.
  • Add narrowly scoped OpenShift SCC bindings for GPU operator operand service accounts.

Bug Fixes:

  • Prevent OpenShift node-labeller workloads from modifying host modprobe configuration managed by the Machine Config Operator.
  • Improve VFIO worker recovery and fail when device binding or unbinding does not take effect.
  • Require explicit passthrough configuration and clean up driver labels when driver management is disabled.

Enhancements:

  • Make controller status react to device-plugin DaemonSet and node resource changes, including failed VFIO worker handling.
  • Expand supported AMD VF device IDs and harden worker device detection and command handling.
  • Update GIM/VFIO and KubeVirt documentation with OpenShift, IOMMU, device-plugin, and resource prerequisites.

Build:

  • Preserve required OpenShift KMM module-loader RBAC while generating Kubernetes Helm charts.

Documentation:

  • Document OpenShift Machine Config Operator ownership of host configuration and prerequisites for GIM/VFIO passthrough.
  • Document pinned VF-aware device-plugin requirements and KubeVirt resource naming configuration.

Tests:

  • Add coverage for passthrough readiness, GIM artifact resolution, watcher-triggered reconciliation, node-labeller behavior, VFIO worker command safety, and validation requirements.

@sourcery-ai

sourcery-ai Bot commented Nov 5, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR hardens the OpenShift GIM/VFIO path by delegating host configuration to MCO, scoping privileged SCC access, making VFIO workers safer and retryable, and gating passthrough readiness on verified KMM/VFIO/device-plugin state and AMD resources. It also fixes version-specific GIM artifact packaging, tightens validation, and documents the required OpenShift and KubeVirt setup.

Sequence diagram for VF passthrough readiness reconciliation

sequenceDiagram
    participant Node as Kubernetes Node
    participant DS as DevicePlugin DaemonSet
    participant Pod as VFIO worker pod
    participant Reconciler as DeviceConfigReconciler
    participant Status as DeviceConfig status

    Reconciler->>Pod: ensureVFIOWorkers()
    Pod-->>Reconciler: VFIO binding result
    Reconciler->>DS: fetch current DaemonSet
    Reconciler->>Node: inspect labels and allocatable resources
    Node-->>Reconciler: VFIO label and amd.com/gpu
    Reconciler->>Status: deviceConfigReadiness()
    alt rollout, VFIO binding, or AMD resource is pending
        Status-->>Reconciler: DevicePluginPending or VFIOBindingPending
        Reconciler->>Status: SetReadyCondition(false)
    else all selected nodes are ready
        Reconciler->>Status: SetReadyCondition(true)
    end
Loading

Sequence diagram for VFIO worker retry and failure handling

sequenceDiagram
    participant Worker as VFIO worker pod
    participant Watcher as PodEventHandler
    participant Manager as workerMgr
    participant Node as Worker node
    participant Reconciler as DeviceConfigReconciler

    Worker-->>Watcher: failed or unknown pod event
    Watcher->>Manager: Work() or Cleanup()
    Manager->>Node: bind or unbind vfio-pci
    Node-->>Manager: verified driver binding result
    Manager-->>Watcher: worker pod recreated or cleanup requested
    Watcher->>Reconciler: enqueue DeviceConfig
    Reconciler->>Reconciler: getFailedWorkerNode()
    alt retry remains failed
        Reconciler->>Reconciler: SetErrorCondition(VFIOWorkerFailed)
    else worker succeeds
        Reconciler->>Reconciler: publish ready status
    end
Loading

File-Level Changes

Change Details Files
Move OpenShift host kernel configuration ownership to the Machine Config Operator and prevent node-labeller host-file mutation.
  • Remove OpenShift /etc modprobe mounts and blacklist writes while retaining driver-readiness waits.
  • Clean up driver-type labels when driver management is disabled and update documentation for MCO prerequisites.
internal/nodelabeller/nodelabeller.go
internal/nodelabeller/nodelabeller_test.go
internal/utils.go
internal/utils_test.go
internal/controllers/device_config_reconciler.go
docs/drivers/installation.md
docs/installation/openshift-olm.md
docs/kubevirt/kubevirt.md
Add narrowly scoped OpenShift privileged SCC usage for operand service accounts.
  • Bind only the KMM device-plugin, KMM module-loader, and node-labeller service accounts to the privileged SCC.
  • Preserve OpenShift-only rendering and keep vanilla Kubernetes chart generation behavior aligned.
hack/k8s-patch/template-patch/kmm-device-plugin-scc-rbac.yaml
hack/k8s-patch/template-patch/kmm-module-loader-rbac.yaml
hack/k8s-patch/template-patch/node-labeller-scc-rbac.yaml
helm-charts-k8s/templates/kmm-device-plugin-scc-rbac.yaml
helm-charts-k8s/templates/kmm-module-loader-rbac.yaml
helm-charts-k8s/templates/node-labeller-scc-rbac.yaml
Makefile
Harden VFIO passthrough orchestration and readiness reporting around KMM, VFIO workers, the current device-plugin rollout, and node resources.
  • Ensure VFIO worker pods are created, retried, and reported as failed for passthrough configurations.
  • Require KMM readiness, VFIO-ready labels, current DaemonSet rollout status, and AMD allocatable resources before VF passthrough becomes ready.
  • Reconcile on DaemonSet status and node allocatable-resource changes, and expose specific pending/error conditions.
internal/controllers/readiness.go
internal/controllers/readiness_test.go
internal/controllers/device_config_reconciler.go
internal/controllers/watchers/daemonset.go
internal/controllers/watchers/daemonset_test.go
internal/controllers/watchers/node.go
internal/controllers/watchers/node_test.go
internal/controllers/watchers/pod.go
internal/controllers/watchers/pod_test.go
internal/conditions/deviceconfig.go
Make VFIO worker execution safer and more deterministic.
  • Quote configured PCI IDs and bound VF detection commands to avoid shell injection and restrict detection to AMD devices.
  • Bound device discovery, use non-restarting worker pods, and verify bind/unbind operations through lspci.
  • Use the dedicated utility-container service account and support cleanup during configuration deletion.
internal/controllers/workermgr/workermgr.go
internal/controllers/workermgr/workermgr_test.go
internal/controllers/workermgr/scripts/vfio_bind.sh
internal/controllers/workermgr/scripts/vfio_unbind.sh
Correct GIM artifact selection and build inputs across legacy and current releases.
  • Resolve release tags and Debian/RPM filenames by GIM version, including the 9.3.0.K RPM layout.
  • Pass resolved artifact metadata into Ubuntu and CoreOS KMM image builds and update generated-output tests.
internal/kmmmodule/kmmmodule.go
internal/kmmmodule/kmmmodule_test.go
internal/kmmmodule/dockerfiles/DockerfileTemplate.coreos.gim
internal/kmmmodule/dockerfiles/DockerfileTemplate.ubuntu.gim
cmd/prepareimage/output_test.go
Tighten passthrough configuration validation and document device-plugin/KubeVirt integration requirements.
  • Require an explicit VF passthrough driver version and node selector for enabled passthrough modes.
  • Document pinned VF-aware device-plugin images, resource naming, KubeVirt prerequisites, IOMMU, and operator limitations.
  • Expand the supported default VF device ID list.
internal/validator/specValidators.go
internal/validator/specValidators_test.go
internal/utils.go
docs/device_plugin/device-plugin.md
docs/kubevirt/kubevirt.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@johnahull

Copy link
Copy Markdown
Owner Author

Superseded by the upstream draft PR: ROCm#680

@johnahull johnahull closed this Nov 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant