From 907297341ce2b73baf0af28843ec7edbc0b6a522 Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Thu, 11 Jun 2026 15:47:26 -0400 Subject: [PATCH] Fix initrd PPI reboot loop by comparing correct image URL When a PreprovisioningImage uses InitRD format, the image-updated check was always comparing the current ImageUrl against ISODownloadURL instead of the InitrdURL. This caused the BMH to be rebooted on every reconcile even when the initrd image had not changed. Extract getExpectedImageURL to select the correct URL (ISO or initrd) based on the PPI's AcceptFormats, and use it in the imageUpdated check. Resolves https://redhat.atlassian.net/browse/MGMT-24571 Assisted-by: Claude Code --- .../preprovisioningimage_controller.go | 13 ++- .../preprovisioningimage_controller_test.go | 106 ++++++++++++++++++ 2 files changed, 118 insertions(+), 1 deletion(-) diff --git a/internal/controller/controllers/preprovisioningimage_controller.go b/internal/controller/controllers/preprovisioningimage_controller.go index 39cd162d47f5..638af7849729 100644 --- a/internal/controller/controllers/preprovisioningimage_controller.go +++ b/internal/controller/controllers/preprovisioningimage_controller.go @@ -194,13 +194,24 @@ func clearImageStatus(image *metal3_v1alpha1.PreprovisioningImage) { image.Status.Architecture = "" } +func getExpectedImageURL(image *metal3_v1alpha1.PreprovisioningImage, infraEnv *aiv1beta1.InfraEnv) string { + if funk.Contains(image.Spec.AcceptFormats, metal3_v1alpha1.ImageFormatISO) { + return infraEnv.Status.ISODownloadURL + } + if funk.Contains(image.Spec.AcceptFormats, metal3_v1alpha1.ImageFormatInitRD) { + return infraEnv.Status.BootArtifacts.InitrdURL + } + return "" +} + func (r *PreprovisioningImageReconciler) handleImageUpdate(ctx context.Context, log logrus.FieldLogger, image *metal3_v1alpha1.PreprovisioningImage, infraEnv *aiv1beta1.InfraEnv) error { bmh, err := r.getBMH(ctx, image) if err != nil { return err } - imageUpdated := image.Status.ImageUrl != "" && image.Status.ImageUrl != infraEnv.Status.ISODownloadURL + expectedURL := getExpectedImageURL(image, infraEnv) + imageUpdated := image.Status.ImageUrl != "" && image.Status.ImageUrl != expectedURL log.Info("Setting images in PreprovisioningImage status") err = r.patchImageStatus(ctx, log, image, func(img *metal3_v1alpha1.PreprovisioningImage) { r.setImage(img, *infraEnv) diff --git a/internal/controller/controllers/preprovisioningimage_controller_test.go b/internal/controller/controllers/preprovisioningimage_controller_test.go index 7fc6d0ebb9b0..685299b0190c 100644 --- a/internal/controller/controllers/preprovisioningimage_controller_test.go +++ b/internal/controller/controllers/preprovisioningimage_controller_test.go @@ -653,6 +653,112 @@ var _ = Describe("PreprovisioningImage reconcile", func() { Expect(ppi.Status.ExtraKernelParams).To(Equal(fmt.Sprintf("coreos.live.rootfs_url=%s rd.bootif=0 arg=thing other.arg", rootfsURL))) }) + It("doesn't reboot the host when initrd PreprovisioningImage ImageUrl is up to date", func() { + ppi.Spec.AcceptFormats = []metal3_v1alpha1.ImageFormat{metal3_v1alpha1.ImageFormatInitRD} + Expect(c.Update(ctx, ppi)).To(BeNil()) + + infraEnv.Status.ISODownloadURL = downloadURL + infraEnv.Status.BootArtifacts.InitrdURL = initrdURL + infraEnv.Status.BootArtifacts.KernelURL = kernelURL + infraEnv.Status.BootArtifacts.RootfsURL = rootfsURL + createdAt := metav1.Now().Add(-InfraEnvImageCooldownPeriod) + infraEnv.Status.CreatedTime = &metav1.Time{Time: createdAt} + infraEnv.Status.Conditions = []conditionsv1.Condition{{Type: aiv1beta1.ImageCreatedCondition, + Status: corev1.ConditionTrue, + Reason: "some reason", + Message: "Some message", + }} + Expect(c.Create(ctx, infraEnv)).To(BeNil()) + + ppi.Status.ImageUrl = initrdURL + ppi.Status.Format = metal3_v1alpha1.ImageFormatInitRD + ppi.Status.KernelUrl = kernelURL + ppi.Status.Conditions = []metav1.Condition{ + {Type: string(metal3_v1alpha1.ConditionImageReady), + Reason: "some reason", + Message: "Some message", + Status: metav1.ConditionTrue}, + {Type: string(metal3_v1alpha1.ConditionImageError), + Reason: "some reason", + Message: "Some message", + Status: metav1.ConditionFalse}, + } + Expect(c.Status().Update(ctx, ppi)).To(BeNil()) + + setInfraEnvIronicConfig() + mockBMOUtils.EXPECT().getICCConfig(gomock.Any()).Times(1).Return(nil, errors.Errorf("ICC configuration is not available")) + + res, err := pr.Reconcile(ctx, newPreprovisioningImageRequest(ppi)) + Expect(err).To(BeNil()) + Expect(res).To(Equal(ctrl.Result{})) + key := types.NamespacedName{ + Namespace: testNamespace, + Name: "testPPI", + } + Expect(c.Get(ctx, key, ppi)).To(BeNil()) + validateStatus(initrdURL, conditionsv1.FindStatusCondition(infraEnv.Status.Conditions, aiv1beta1.ImageCreatedCondition), ppi) + bmhKey := types.NamespacedName{ + Namespace: bmh.Namespace, + Name: bmh.Name, + } + Expect(c.Get(ctx, bmhKey, bmh)).To(BeNil()) + Expect(bmh.Annotations).NotTo(HaveKey("reboot.metal3.io")) + }) + + It("reboots the host when the initrd image is updated", func() { + ppi.Spec.AcceptFormats = []metal3_v1alpha1.ImageFormat{metal3_v1alpha1.ImageFormatInitRD} + Expect(c.Update(ctx, ppi)).To(BeNil()) + + oldInitrdURL := "https://old-initrd.example.com" + infraEnv.Status.ISODownloadURL = downloadURL + infraEnv.Status.BootArtifacts.InitrdURL = oldInitrdURL + infraEnv.Status.BootArtifacts.KernelURL = kernelURL + infraEnv.Status.BootArtifacts.RootfsURL = rootfsURL + infraEnv.Status.CreatedTime = &metav1.Time{Time: metav1.Now().Add(-InfraEnvImageCooldownPeriod)} + infraEnv.Status.Conditions = []conditionsv1.Condition{{Type: aiv1beta1.ImageCreatedCondition, + Status: corev1.ConditionTrue, + Reason: "some reason", + Message: "Some message", + }} + + ppi.Status.ImageUrl = oldInitrdURL + ppi.Status.Format = metal3_v1alpha1.ImageFormatInitRD + ppi.Status.KernelUrl = kernelURL + ppi.Status.Conditions = []metav1.Condition{ + {Type: string(metal3_v1alpha1.ConditionImageReady), + Reason: "some reason", + Message: "Some message", + Status: metav1.ConditionTrue}, + {Type: string(metal3_v1alpha1.ConditionImageError), + Reason: "some reason", + Message: "Some message", + Status: metav1.ConditionFalse}, + } + Expect(c.Status().Update(ctx, ppi)).To(BeNil()) + + infraEnv.Status.BootArtifacts.InitrdURL = initrdURL + Expect(c.Create(ctx, infraEnv)).To(BeNil()) + + setInfraEnvIronicConfig() + mockBMOUtils.EXPECT().getICCConfig(gomock.Any()).Times(1).Return(nil, errors.Errorf("ICC configuration is not available")) + + res, err := pr.Reconcile(ctx, newPreprovisioningImageRequest(ppi)) + Expect(err).To(BeNil()) + Expect(res).To(Equal(ctrl.Result{})) + key := types.NamespacedName{ + Namespace: testNamespace, + Name: "testPPI", + } + Expect(c.Get(ctx, key, ppi)).To(BeNil()) + validateStatus(initrdURL, conditionsv1.FindStatusCondition(infraEnv.Status.Conditions, aiv1beta1.ImageCreatedCondition), ppi) + bmhKey := types.NamespacedName{ + Namespace: bmh.Namespace, + Name: bmh.Name, + } + Expect(c.Get(ctx, bmhKey, bmh)).To(BeNil()) + Expect(bmh.Annotations).To(HaveKey("reboot.metal3.io")) + }) + It("doesn't reboot the host when PreprovisioningImage ImageUrl is up to date", func() { infraEnv.Status.ISODownloadURL = downloadURL createdAt := metav1.Now().Add(-InfraEnvImageCooldownPeriod)