From 5d82935a84a57aa65522f3c349f19e04363bdc16 Mon Sep 17 00:00:00 2001 From: Tuomas Katila Date: Thu, 6 Aug 2026 15:40:58 +0300 Subject: [PATCH 1/2] fwupdate: amc doesn't require user/pass with xpu-smi 2.0 Also note that the update goes through an auxiliary interface. Signed-off-by: Tuomas Katila --- FWUPDATE.md | 30 ++------- api/v1alpha1/gpufirmwareupdate_types.go | 6 -- api/v1alpha1/gpufirmwareupdate_webhook.go | 1 - .../crds/gpufirmwareupdates.yaml | 6 -- .../bases/intel.com_gpufirmwareupdates.yaml | 6 -- .../samples/fwupdate-amc/kustomization.yaml | 1 - config/samples/fwupdate-amc/secret.yaml | 9 --- .../v1alpha1_gpufirmwareupdate_amc.yaml | 4 -- .../gpufirmwareupdate_controller.go | 27 -------- .../gpufirmwareupdate_controller_test.go | 64 +------------------ scripts/update.sh | 4 +- 11 files changed, 8 insertions(+), 150 deletions(-) delete mode 100644 config/samples/fwupdate-amc/secret.yaml diff --git a/FWUPDATE.md b/FWUPDATE.md index c6392c4..e99f2b5 100644 --- a/FWUPDATE.md +++ b/FWUPDATE.md @@ -4,7 +4,7 @@ The operator manages firmware updates for Intel GPUs through the `GPUFirmwareUpd Two underlying update methods are supported, selected per firmware file type: - **igsc** – updates through the MEI (Management Engine Interface) on the host; requires detecting the GPU's BDF address. -- **AMC** – updates through a Redfish interface via an external component; no host-side device detection needed, but requires credentials. +- **AMC** – updates through an auxiliary interface; no host-side device detection needed. ## Rough flow for executing a GPU firmware update @@ -334,30 +334,12 @@ kubectl patch gpufirmwareupdate gfx-update --type=merge -p '{"spec":{"holdAfterC ## AMC update -AMC updates go through a Redfish interface on an external component, so there is no need to detect the GPU's BDF address on the host. The `xpu-smi` tool handles device targeting internally. Because the Redfish interface requires authentication, a Kubernetes Secret must be provided. - -### Secret format - -Create a Secret in the **operator namespace** with keys `username` and `password`: - -```yaml -apiVersion: v1 -kind: Secret -metadata: - name: amc-credentials - namespace: -type: Opaque -stringData: - username: admin - password: s3cr3t -``` - -The operator injects these into the update Job as the environment variables `AMC_USERNAME` and `AMC_PASSWORD`. +AMC updates go through an auxiliary interface that is attached the the GPU. The `xpu-smi` tool handles device targeting internally, and `xpu-smi updatefw -t AMC`. The update command run inside the Job container is: ```sh -xpu-smi updatefw -y -t AMC -f -u $AMC_USERNAME -p $AMC_PASSWORD +xpu-smi updatefw -y -t AMC -f ``` ### Example CR @@ -373,7 +355,6 @@ spec: updateTaint: gpufirmware-update nodeSelector: gpu-update: "true" - amcCredentialsSecret: amc-credentials # must be in the operator namespace updaterImage: intel/xpumanager:v1.3.4 content: containerImage: registry.example.com/gpu-firmware@sha256:abcd1234... @@ -383,8 +364,6 @@ spec: checksum: "sha256:3739b855dd6b92df2f905a1c2cf6127efa03efc4862e063074e0ec8930bb5bb4" ``` -> **Note:** `amcCredentialsSecret` is required whenever any entry in `spec.content.files` has `type: AMC`. The operator will reject the CR (set status to `error`) if the field is missing. - --- ## Advanced topics @@ -399,7 +378,7 @@ The following firmware file types are supported (set in `spec.content.files[].ty | `GFX_DATA` | igsc | | | `GFX_CODE_DATA` | igsc | | | `GFX_PSCBIN` | igsc | | -| `AMC` | AMC (Redfish) | Requires `spec.amcCredentialsSecret` | +| `AMC` | AMC | No `-d` | | `FAN_TABLE` | igsc | | | `VR_CONFIG` | igsc | | | `OPROM_CODE` | igsc | | @@ -416,7 +395,6 @@ A validating admission webhook prevents editing structural spec fields while an | `spec.nodeSelector` | Node selection has already happened | | `spec.pciDeviceID` | Already encoded in running Jobs | | `spec.content` | Firmware files and image already in use by running Jobs | -| `spec.amcCredentialsSecret` | Credentials already injected into running Jobs | | `spec.updateTaint` | Taint key already applied to nodes; cleanup depends on it | The following fields remain mutable at any time: diff --git a/api/v1alpha1/gpufirmwareupdate_types.go b/api/v1alpha1/gpufirmwareupdate_types.go index 9d04707..825cb32 100644 --- a/api/v1alpha1/gpufirmwareupdate_types.go +++ b/api/v1alpha1/gpufirmwareupdate_types.go @@ -51,12 +51,6 @@ type GPUFirmwareUpdateSpec struct { // Node selector to target specific nodes for firmware update. NodeSelector map[string]string `json:"nodeSelector,omitempty"` - // AMCCredentialsSecret is the name of a Kubernetes Secret (in the operator namespace) - // containing 'username' and 'password' keys used for the AMC firmware update method - // (redfish interface). Required when any firmware file has type AMC. - // +optional - AMCCredentialsSecret string `json:"amcCredentialsSecret,omitempty"` - // InsecureSkipTLSVerify disables TLS certificate verification when the operator // contacts the content image registry to check reachability or verify file checksums. // Use this when the registry uses a self-signed or private-CA certificate. diff --git a/api/v1alpha1/gpufirmwareupdate_webhook.go b/api/v1alpha1/gpufirmwareupdate_webhook.go index 3ef0d2b..9316a59 100644 --- a/api/v1alpha1/gpufirmwareupdate_webhook.go +++ b/api/v1alpha1/gpufirmwareupdate_webhook.go @@ -145,7 +145,6 @@ func (v *GPUFirmwareUpdateCustomValidator) ValidateUpdate(_ context.Context, old {"spec.updaterImage", oldFU.Spec.UpdaterImage, newFU.Spec.UpdaterImage}, {"spec.pciDeviceID", oldFU.Spec.PCIDeviceID, newFU.Spec.PCIDeviceID}, {"spec.content", oldFU.Spec.Content, newFU.Spec.Content}, - {"spec.amcCredentialsSecret", oldFU.Spec.AMCCredentialsSecret, newFU.Spec.AMCCredentialsSecret}, {"spec.updateTaint", oldFU.Spec.UpdateTaint, newFU.Spec.UpdateTaint}, } diff --git a/charts/gpu-base-operator/crds/gpufirmwareupdates.yaml b/charts/gpu-base-operator/crds/gpufirmwareupdates.yaml index d8a4a76..9251844 100644 --- a/charts/gpu-base-operator/crds/gpufirmwareupdates.yaml +++ b/charts/gpu-base-operator/crds/gpufirmwareupdates.yaml @@ -39,12 +39,6 @@ spec: spec: description: GPUFirmwareUpdateSpec defines the desired state of GPUFirmwareUpdate. properties: - amcCredentialsSecret: - description: |- - AMCCredentialsSecret is the name of a Kubernetes Secret (in the operator namespace) - containing 'username' and 'password' keys used for the AMC firmware update method - (redfish interface). Required when any firmware file has type AMC. - type: string content: description: List of GPU firmwares to be updated. properties: diff --git a/config/crd/bases/intel.com_gpufirmwareupdates.yaml b/config/crd/bases/intel.com_gpufirmwareupdates.yaml index d8a4a76..9251844 100644 --- a/config/crd/bases/intel.com_gpufirmwareupdates.yaml +++ b/config/crd/bases/intel.com_gpufirmwareupdates.yaml @@ -39,12 +39,6 @@ spec: spec: description: GPUFirmwareUpdateSpec defines the desired state of GPUFirmwareUpdate. properties: - amcCredentialsSecret: - description: |- - AMCCredentialsSecret is the name of a Kubernetes Secret (in the operator namespace) - containing 'username' and 'password' keys used for the AMC firmware update method - (redfish interface). Required when any firmware file has type AMC. - type: string content: description: List of GPU firmwares to be updated. properties: diff --git a/config/samples/fwupdate-amc/kustomization.yaml b/config/samples/fwupdate-amc/kustomization.yaml index 5fc4332..4ed4b02 100644 --- a/config/samples/fwupdate-amc/kustomization.yaml +++ b/config/samples/fwupdate-amc/kustomization.yaml @@ -1,5 +1,4 @@ apiVersion: kustomize.config.k8s.io/v1beta1 kind: Kustomization resources: -- secret.yaml - v1alpha1_gpufirmwareupdate_amc.yaml diff --git a/config/samples/fwupdate-amc/secret.yaml b/config/samples/fwupdate-amc/secret.yaml deleted file mode 100644 index 99693e1..0000000 --- a/config/samples/fwupdate-amc/secret.yaml +++ /dev/null @@ -1,9 +0,0 @@ -apiVersion: v1 -kind: Secret -metadata: - name: amc-credentials - namespace: intel-gpu-base-operator -type: Opaque -stringData: - username: admin - password: changeme diff --git a/config/samples/fwupdate-amc/v1alpha1_gpufirmwareupdate_amc.yaml b/config/samples/fwupdate-amc/v1alpha1_gpufirmwareupdate_amc.yaml index 898bb9e..226255d 100644 --- a/config/samples/fwupdate-amc/v1alpha1_gpufirmwareupdate_amc.yaml +++ b/config/samples/fwupdate-amc/v1alpha1_gpufirmwareupdate_amc.yaml @@ -16,10 +16,6 @@ spec: updateMethod: "direct" updateTaint: "gpu-update-in-progress" - # Reference to the Secret holding the Redfish credentials (username/password). - # The Secret must exist in the operator namespace before applying this CR. - amcCredentialsSecret: amc-credentials - content: containerImage: ghcr.io/intel/gpu-firmware-update-files:devel diff --git a/internal/controller/gpufirmwareupdate_controller.go b/internal/controller/gpufirmwareupdate_controller.go index a242221..07c08f9 100644 --- a/internal/controller/gpufirmwareupdate_controller.go +++ b/internal/controller/gpufirmwareupdate_controller.go @@ -202,9 +202,6 @@ func (r *GPUFirmwareUpdateReconciler) verifyGivenParameters(fu *intelcomv1alpha1 return fmt.Errorf("unsupported firmware type %s", fw.Type) } - if fw.Type == FirmwareTypeAMC && fu.Spec.AMCCredentialsSecret == "" { - return fmt.Errorf("amcCredentialsSecret must be specified when firmware type AMC is requested") - } } return nil @@ -625,30 +622,6 @@ func (r *GPUFirmwareUpdateReconciler) createUpdateJobObjForNode(nodeName string, job.Spec.Template.Spec.InitContainers[0].Image = fu.Spec.Content.ContainerImage - if fu.Spec.AMCCredentialsSecret != "" { - job.Spec.Template.Spec.Containers[0].Env = append( - job.Spec.Template.Spec.Containers[0].Env, - core.EnvVar{ - Name: "AMC_USERNAME", - ValueFrom: &core.EnvVarSource{ - SecretKeyRef: &core.SecretKeySelector{ - LocalObjectReference: core.LocalObjectReference{Name: fu.Spec.AMCCredentialsSecret}, - Key: "username", - }, - }, - }, - core.EnvVar{ - Name: "AMC_PASSWORD", - ValueFrom: &core.EnvVarSource{ - SecretKeyRef: &core.SecretKeySelector{ - LocalObjectReference: core.LocalObjectReference{Name: fu.Spec.AMCCredentialsSecret}, - Key: "password", - }, - }, - }, - ) - } - updateCommands := []string{fmt.Sprintf("echo \"Starting firmware update process on node %s\"", nodeName)} for _, fw := range fu.Spec.Content.Files { diff --git a/internal/controller/gpufirmwareupdate_controller_test.go b/internal/controller/gpufirmwareupdate_controller_test.go index 6b2de65..57f4186 100644 --- a/internal/controller/gpufirmwareupdate_controller_test.go +++ b/internal/controller/gpufirmwareupdate_controller_test.go @@ -573,22 +573,9 @@ var _ = Describe("GPUFirmwareUpdate Controller", func() { Expect(node.Spec.Taints).To(HaveLen(1)) }) - It("it should complete AMC update flow with credentials from secret", func() { - amcSecret := &core.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "amc-creds", - Namespace: "default", - }, - Data: map[string][]byte{ - "username": []byte("admin"), - "password": []byte("s3cr3t"), - }, - } - Expect(fakeClient.Create(ctx, amcSecret)).To(Succeed()) - + It("it should complete AMC update flow", func() { resource = baseCr() resource.Spec.UpdateMethod = "direct" - resource.Spec.AMCCredentialsSecret = "amc-creds" resource.Spec.Content.Files = []intelcomv1alpha1.GPUFirmwareFile{ {Type: "AMC", FileName: "amc_firmware.bin"}, } @@ -608,7 +595,7 @@ var _ = Describe("GPUFirmwareUpdate Controller", func() { Expect(err).NotTo(HaveOccurred()) Expect(resource.Status.State).To(Equal(stateUpdating)) - By("Updating reconcile: create Jobs and verify AMC env vars from secret") + By("Updating reconcile: create Jobs") ret, err = reconcileAndGet(fakeClient, controllerReconciler, resource) Expect(err).NotTo(HaveOccurred()) Expect(resource.Status.State).To(Equal(stateUpdating)) @@ -618,31 +605,6 @@ var _ = Describe("GPUFirmwareUpdate Controller", func() { Expect(fakeClient.List(ctx, jobs)).To(Succeed()) Expect(jobs.Items).To(HaveLen(2)) - for i := range jobs.Items { - containers := jobs.Items[i].Spec.Template.Spec.Containers - Expect(containers).To(HaveLen(1)) - - envVars := containers[0].Env - var usernameEnv, passwordEnv *core.EnvVar - for j := range envVars { - if envVars[j].Name == "AMC_USERNAME" { - usernameEnv = &envVars[j] - } - if envVars[j].Name == "AMC_PASSWORD" { - passwordEnv = &envVars[j] - } - } - Expect(usernameEnv).NotTo(BeNil(), "AMC_USERNAME env var must be present") - Expect(usernameEnv.ValueFrom).NotTo(BeNil()) - Expect(usernameEnv.ValueFrom.SecretKeyRef.Name).To(Equal("amc-creds")) - Expect(usernameEnv.ValueFrom.SecretKeyRef.Key).To(Equal("username")) - - Expect(passwordEnv).NotTo(BeNil(), "AMC_PASSWORD env var must be present") - Expect(passwordEnv.ValueFrom).NotTo(BeNil()) - Expect(passwordEnv.ValueFrom.SecretKeyRef.Name).To(Equal("amc-creds")) - Expect(passwordEnv.ValueFrom.SecretKeyRef.Key).To(Equal("password")) - } - createPodsForJobs(fakeClient, jobs) By("Mark all jobs succeeded → expect cleanup") @@ -943,28 +905,6 @@ var _ = Describe("GPUFirmwareUpdate Controller Errors", func() { Expect(resource.Status.State).To(Equal(stateError)) }) - It("should detect missing AMC credentials and fail", func() { - resource = baseCr() - - resource.Spec.Content.Files = []intelcomv1alpha1.GPUFirmwareFile{ - {Type: FirmwareTypeAMC, FileName: "amc.bin"}, - } - - Expect(fakeClient.Create(ctx, resource)).To(Succeed()) - - _, err := controllerReconciler.Reconcile(ctx, reconcile.Request{ - NamespacedName: typeNamespacedName, - }) - - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("amcCredentialsSecret must be specified")) - - err = fakeClient.Get(ctx, typeNamespacedName, resource) - Expect(err).NotTo(HaveOccurred()) - - Expect(resource.Status.State).To(Equal(stateError)) - }) - It("should bail out properly when no nodes are available", func() { resource = baseCr() diff --git a/scripts/update.sh b/scripts/update.sh index ba15a00..299736d 100644 --- a/scripts/update.sh +++ b/scripts/update.sh @@ -57,9 +57,9 @@ update_firmware_amc() { local type=$1 local filepath=$2 - echo "Calling xpu-smi: xpu-smi updatefw -y -t $type -f $filepath -u *** -p ***" + echo "Calling xpu-smi: xpu-smi updatefw -y -t $type -f $filepath" - xpu-smi updatefw -y -t "$type" -f "$filepath" -u "$AMC_USERNAME" -p "$AMC_PASSWORD" || { + xpu-smi updatefw -y -t "$type" -f "$filepath" || { echoerr "AMC firmware update failed" return 1 } From a9df0084c79b36acfc9ed7baa6c5536f8bc6a189 Mon Sep 17 00:00:00 2001 From: Tuomas Katila Date: Sun, 30 Aug 2026 10:43:49 +0300 Subject: [PATCH 2/2] test: fix crd inclusion when moving from branch to another Signed-off-by: Tuomas Katila --- internal/controller/suite_test.go | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 9bf4bc6..93b048a 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -126,6 +126,23 @@ var _ = AfterSuite(func() { Expect(err).NotTo(HaveOccurred()) }) +// goModDir returns the module cache directory of the given module. '-mod=mod' is +// required because 'go list -m' reports an empty directory in vendor mode, and the +// module's non-Go files (e.g. CRD manifests) are not copied into 'vendor/' anyway. +func goModDir(module string) string { + out, err := exec.Command("go", "list", "-mod=mod", "-m", "-f", "{{.Dir}}", module).Output() + if err != nil { + panic(fmt.Sprintf("failed to resolve module directory for %s: %v", module, err)) + } + + dir := strings.TrimSpace(string(out)) + if dir == "" { + panic(fmt.Sprintf("empty module directory for %s, run 'go mod download %s'", module, module)) + } + + return dir +} + // getFirstFoundEnvTestBinaryDir locates the first binary in the specified path. // ENVTEST-based tests depend on specific binaries, usually located in paths set by // controller-runtime. When running tests directly (e.g., via an IDE) without using @@ -134,14 +151,6 @@ var _ = AfterSuite(func() { // This function streamlines the process by finding the required binaries, similar to // setting the 'KUBEBUILDER_ASSETS' environment variable. To ensure the binaries are // properly set up, run 'make setup-envtest' beforehand. -func goModDir(module string) string { - out, err := exec.Command("go", "list", "-m", "-f", "{{.Dir}}", module).Output() - if err != nil { - panic(fmt.Sprintf("failed to resolve module directory for %s: %v", module, err)) - } - return strings.TrimSpace(string(out)) -} - func getFirstFoundEnvTestBinaryDir() string { basePath := filepath.Join("..", "..", "bin", "k8s") entries, err := os.ReadDir(basePath)