[release-ocm-2.13] ACM-26035: Introduce PreprovisioningImage finalizer

internal/controller/controllers/preprovisioningimage_controller.go

@@ -48,13 +48,17 @@
 	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 )
 
 type imageConditionReason string
 
-const archMismatchReason = "InfraEnvArchMismatch"
+const (
+	archMismatchReason                = "InfraEnvArchMismatch"
+	PreprovisioningImageFinalizerName = "preprovisioningimage." + aiv1beta1.Group + "/ai-deprovision"
+)
 
 type PreprovisioningImageControllerConfig struct {
 	// The default ironic agent image was obtained by running "oc adm release info --image-for=ironic-agent  quay.io/openshift-release-dev/ocp-release:4.11.0-fc.0-x86_64"
@@ -105,6 +109,15 @@
 	if err != nil {
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
+
+	if !image.ObjectMeta.DeletionTimestamp.IsZero() {
+		return r.handlePreprovisioningImageDeletion(ctx, log, image)
+	}
+
+	if !funk.ContainsString(image.GetFinalizers(), PreprovisioningImageFinalizerName) {
+		return r.ensurePreprovisioningImageFinalizer(ctx, log, image)
+	}
+
 	if !funk.Some(image.Spec.AcceptFormats, metal3_v1alpha1.ImageFormatISO, metal3_v1alpha1.ImageFormatInitRD) {
 		// Currently, the PreprovisioningImageController only support ISO and InitRD image
 		log.Infof("Unsupported image format: %s", image.Spec.AcceptFormats)
@@ -115,7 +128,7 @@
 		}
 		return ctrl.Result{}, err
 	}
-	// Consider adding finalizer in case we need to clean up resources
+
 	// Retrieve InfraEnv
 	infraEnv, err := r.findInfraEnvForPreprovisioningImage(ctx, log, image)
 	if err != nil {
@@ -639,6 +652,58 @@
 	return nil
 }
 
+// ensurePreprovisioningImageFinalizer adds a finalizer to the PreprovisioningImage
+func (r *PreprovisioningImageReconciler) ensurePreprovisioningImageFinalizer(ctx context.Context, log logrus.FieldLogger, image *metal3_v1alpha1.PreprovisioningImage) (ctrl.Result, error) {
+	controllerutil.AddFinalizer(image, PreprovisioningImageFinalizerName)
+	if err := r.Update(ctx, image); err != nil {
+		log.WithError(err).Errorf("failed to add finalizer %s to PreprovisioningImage %s/%s", PreprovisioningImageFinalizerName, image.Namespace, image.Name)
+		return ctrl.Result{Requeue: true}, err
+	}
+	return ctrl.Result{Requeue: true}, nil
+}
+
+// handlePreprovisioningImageDeletion handles the deletion of a PreprovisioningImage by checking if any BMHs
+// with automated cleaning enabled still reference it.
+func (r *PreprovisioningImageReconciler) handlePreprovisioningImageDeletion(ctx context.Context, log logrus.FieldLogger, image *metal3_v1alpha1.PreprovisioningImage) (ctrl.Result, error) {
+	if !funk.ContainsString(image.GetFinalizers(), PreprovisioningImageFinalizerName) {
+		// Allow deletion of the PreprovisioningImage if the finalizer is not present
+		return ctrl.Result{}, nil
+	}
+
+	// Get the BMH that owns this PreprovisioningImage
+	bmh, err := r.getBMH(ctx, image)
+	if err != nil {
+		if client.IgnoreNotFound(err) == nil || strings.Contains(err.Error(), "failed to find BMH owner") {
+			// BMH not found or this preprovisioningimage is not owned by a BMH, allow deletion of the PreprovisioningImage
+			log.Info("BMH not found, removing PreprovisioningImage finalizer")
+			controllerutil.RemoveFinalizer(image, PreprovisioningImageFinalizerName)
+			if err = r.Update(ctx, image); err != nil {
+				log.WithError(err).Errorf("failed to remove finalizer %s from PreprovisioningImage %s/%s", PreprovisioningImageFinalizerName, image.Namespace, image.Name)
+				return ctrl.Result{Requeue: true}, err
+			}
+			return ctrl.Result{}, nil
+		}
+		log.WithError(err).Error("failed to get BMH for PreprovisioningImage")
+		return ctrl.Result{RequeueAfter: longerRequeueAfterOnError}, err
+	}
+
+	// PreprovisioningImage should wait for a BMH with automated cleaning enabled to be deleted
+	if bmh.Spec.AutomatedCleaningMode != metal3_v1alpha1.CleaningModeDisabled {
+		log.Infof("Cannot delete PreprovisioningImage yet: BMH %s/%s with automatedCleaningMode=%s exists and requires the image for deprovisioning",
+			bmh.Namespace, bmh.Name, bmh.Spec.AutomatedCleaningMode)
+		return ctrl.Result{Requeue: true}, nil
+	}
+
+	// Safe to delete, remove finalizer
+	log.Info("Removing finalizer from PreprovisioningImage")
+	controllerutil.RemoveFinalizer(image, PreprovisioningImageFinalizerName)
+	if err := r.Update(ctx, image); err != nil {
+		log.WithError(err).Errorf("failed to remove finalizer %s from PreprovisioningImage %s/%s", PreprovisioningImageFinalizerName, image.Namespace, image.Name)
+		return ctrl.Result{Requeue: true}, err
+	}
+	return ctrl.Result{}, nil
+}
+
 // processMirrorRegistryConfig retrieves the mirror registry configuration from the referenced ConfigMap
 func (r *PreprovisioningImageReconciler) processMirrorRegistryConfig(ctx context.Context, log logrus.FieldLogger, infraEnv *aiv1beta1.InfraEnv) (*common.MirrorRegistryConfiguration, error) {
 	mirrorRegistryConfiguration, userTomlConfigMap, err := mirrorregistry.ProcessMirrorRegistryConfig(ctx, log, r.Client, infraEnv.Spec.MirrorRegistryRef)

internal/controller/controllers/preprovisioningimage_controller_test.go

@@ -24,6 +24,7 @@ import (
 	conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1"
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -52,6 +53,7 @@ func newPreprovisioningImage(name, namespace, labelKey, labelValue, bmhName stri
 				Kind:       "BareMetalHost",
 				Name:       bmhName,
 			}},
+			Finalizers: []string{PreprovisioningImageFinalizerName},
 		},
 		Spec: metal3_v1alpha1.PreprovisioningImageSpec{
 			AcceptFormats: []metal3_v1alpha1.ImageFormat{
@@ -1030,3 +1032,186 @@ func validateStatus(imageURL string, ExpectedImageReadyCondition *conditionsv1.C
 		meta.FindStatusCondition(ppi.Status.Conditions, string(metal3_v1alpha1.ConditionImageError)).Status)))
 
 }
+
+var _ = Describe("PreprovisioningImage deletion protection", func() {
+	var (
+		c                     client.Client
+		pr                    *PreprovisioningImageReconciler
+		mockCtrl              *gomock.Controller
+		mockInstallerInternal *bminventory.MockInstallerInternals
+		mockCRDEventsHandler  *MockCRDEventsHandler
+		mockVersionHandler    *versions.MockHandler
+		mockOcRelease         *oc.MockRelease
+		mockBMOUtils          *MockBMOUtils
+		ctx                   = context.Background()
+		ppi                   *metal3_v1alpha1.PreprovisioningImage
+		bmh                   *metal3_v1alpha1.BareMetalHost
+	)
+
+	BeforeEach(func() {
+		schemes := runtime.NewScheme()
+		Expect(configv1.AddToScheme(schemes)).To(Succeed())
+		Expect(metal3_v1alpha1.AddToScheme(schemes)).To(Succeed())
+		Expect(aiv1beta1.AddToScheme(schemes)).To(Succeed())
+		c = fakeclient.NewClientBuilder().WithScheme(schemes).
+			WithStatusSubresource(&metal3_v1alpha1.PreprovisioningImage{}, &metal3_v1alpha1.BareMetalHost{}).Build()
+		mockCtrl = gomock.NewController(GinkgoT())
+		mockInstallerInternal = bminventory.NewMockInstallerInternals(mockCtrl)
+		mockCRDEventsHandler = NewMockCRDEventsHandler(mockCtrl)
+		mockVersionHandler = versions.NewMockHandler(mockCtrl)
+		mockOcRelease = oc.NewMockRelease(mockCtrl)
+		mockBMOUtils = NewMockBMOUtils(mockCtrl)
+
+		pr = &PreprovisioningImageReconciler{
+			Client:           c,
+			Log:              common.GetTestLog(),
+			Installer:        mockInstallerInternal,
+			CRDEventsHandler: mockCRDEventsHandler,
+			VersionsHandler:  mockVersionHandler,
+			OcRelease:        mockOcRelease,
+			BMOUtils:         mockBMOUtils,
+		}
+
+		bmh = &metal3_v1alpha1.BareMetalHost{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "testBMH",
+				Namespace: testNamespace,
+			},
+			Spec: metal3_v1alpha1.BareMetalHostSpec{
+				AutomatedCleaningMode: metal3_v1alpha1.CleaningModeMetadata,
+			},
+		}
+		Expect(c.Create(ctx, bmh)).To(Succeed())
+
+		ppi = newPreprovisioningImage("testPPI", testNamespace, InfraEnvLabel, "testInfraEnv", bmh.Name)
+	})
+
+	AfterEach(func() {
+		mockCtrl.Finish()
+	})
+
+	Context("ensurePreprovisioningImageFinalizer", func() {
+		It("adds finalizer when not present on PreprovisioningImage", func() {
+			ppi.Finalizers = []string{}
+			Expect(c.Create(ctx, ppi)).To(Succeed())
+
+			// Reconcile and verify finalizer was added
+			result, err := pr.Reconcile(ctx, newPreprovisioningImageRequest(ppi))
+			Expect(err).To(BeNil())
+			Expect(result).To(Equal(ctrl.Result{Requeue: true}))
+
+			key := types.NamespacedName{Name: ppi.Name, Namespace: ppi.Namespace}
+			Expect(c.Get(ctx, key, ppi)).To(Succeed())
+			Expect(ppi.GetFinalizers()).To(ContainElement(PreprovisioningImageFinalizerName))
+		})
+
+		It("does nothing when finalizer is already present on PreprovisioningImage", func() {
+			Expect(c.Create(ctx, ppi)).To(Succeed())
+
+			// Reconcile and verify finalizer was not added again
+			result, err := pr.Reconcile(ctx, newPreprovisioningImageRequest(ppi))
+			Expect(err).To(BeNil())
+			Expect(result).To(Equal(ctrl.Result{}))
+
+			key := types.NamespacedName{Name: ppi.Name, Namespace: ppi.Namespace}
+			Expect(c.Get(ctx, key, ppi)).To(Succeed())
+			Expect(ppi.GetFinalizers()).To(HaveLen(1))
+			Expect(ppi.GetFinalizers()).To(ContainElement(PreprovisioningImageFinalizerName))
+		})
+	})
+
+	Context("handlePreprovisioningImageDeletion", func() {
+		It("allows deletion when finalizer is not present", func() {
+			ppi.OwnerReferences = []metav1.OwnerReference{}
+			Expect(c.Create(ctx, ppi)).To(Succeed())
+			// Delete ppi
+			Expect(c.Delete(ctx, ppi)).To(Succeed())
+
+			result, err := pr.Reconcile(ctx, newPreprovisioningImageRequest(ppi))
+			Expect(err).To(BeNil())
+			Expect(result).To(Equal(ctrl.Result{}))
+
+			// Verify ppi was deleted
+			key := types.NamespacedName{Name: ppi.Name, Namespace: ppi.Namespace}
+			Expect(k8serrors.IsNotFound(c.Get(ctx, key, ppi))).To(BeTrue())
+		})
+
+		It("removes finalizer and allows deletion when BMH not found", func() {
+			// Set owner reference to a non-existent BMH
+			ppi.OwnerReferences = []metav1.OwnerReference{{
+				APIVersion: "metal3.io/v1alpha1",
+				Kind:       "BareMetalHost",
+				Name:       "nonExistentBMH",
+			}}
+			Expect(c.Create(ctx, ppi)).To(Succeed())
+			// Delete ppi
+			Expect(c.Delete(ctx, ppi)).To(Succeed())
+			// Reconcile and verify ppi was deleted
+			result, err := pr.Reconcile(ctx, newPreprovisioningImageRequest(ppi))
+			Expect(err).To(BeNil())
+			Expect(result).To(Equal(ctrl.Result{}))
+
+			// Verify ppi was deleted
+			key := types.NamespacedName{Name: ppi.Name, Namespace: ppi.Namespace}
+			Expect(k8serrors.IsNotFound(c.Get(ctx, key, ppi))).To(BeTrue())
+		})
+
+		It("blocks deletion when BMH has metadata cleaning enabled and is being deleted", func() {
+			Expect(c.Create(ctx, ppi)).To(Succeed())
+			// Delete ppi
+			Expect(c.Delete(ctx, ppi)).To(Succeed())
+
+			// UpdateBMH to set metadata cleaning enabled
+			bmh.Spec.AutomatedCleaningMode = metal3_v1alpha1.CleaningModeMetadata
+			bmh.Finalizers = []string{"arbitraryfinalizer"}
+			Expect(c.Update(ctx, bmh)).To(Succeed())
+			// Set BMH to be deleting
+			Expect(c.Delete(ctx, bmh)).To(Succeed())
+
+			result, err := pr.Reconcile(ctx, newPreprovisioningImageRequest(ppi))
+
+			Expect(err).To(BeNil())
+			Expect(result.Requeue).To(BeTrue())
+
+			// Verify finalizer was NOT removed
+			key := types.NamespacedName{Name: ppi.Name, Namespace: ppi.Namespace}
+			Expect(c.Get(ctx, key, ppi)).To(Succeed())
+			Expect(ppi.GetFinalizers()).To(ContainElement(PreprovisioningImageFinalizerName))
+		})
+
+		It("allows deletion when BMH has cleaning disabled", func() {
+			Expect(c.Create(ctx, ppi)).To(Succeed())
+			// Delete ppi
+			Expect(c.Delete(ctx, ppi)).To(Succeed())
+			// Set BMH cleaning to disabled
+			bmh.Spec.AutomatedCleaningMode = metal3_v1alpha1.CleaningModeDisabled
+			Expect(c.Update(ctx, bmh)).To(Succeed())
+			Expect(c.Delete(ctx, bmh)).To(Succeed())
+
+			result, err := pr.Reconcile(ctx, newPreprovisioningImageRequest(ppi))
+
+			Expect(err).To(BeNil())
+			Expect(result).To(Equal(ctrl.Result{}))
+
+			// Verify ppi was deleted
+			key := types.NamespacedName{Name: ppi.Name, Namespace: ppi.Namespace}
+			Expect(k8serrors.IsNotFound(c.Get(ctx, key, ppi))).To(BeTrue())
+		})
+
+		It("blocks deletion when BMH has metadata cleaning enabled but is NOT being deleted", func() {
+			Expect(c.Create(ctx, ppi)).To(Succeed())
+			// Delete ppi
+			Expect(c.Delete(ctx, ppi)).To(Succeed())
+			// BMH is not being deleted (no DeletionTimestamp)
+			result, err := pr.Reconcile(ctx, newPreprovisioningImageRequest(ppi))
+
+			Expect(err).To(BeNil())
+			Expect(result.Requeue).To(BeTrue())
+
+			// Verify ppi finalizer was not removed
+			key := types.NamespacedName{Name: ppi.Name, Namespace: ppi.Namespace}
+			Expect(c.Get(ctx, key, ppi)).To(Succeed())
+			Expect(ppi.GetFinalizers()).To(ContainElement(PreprovisioningImageFinalizerName))
+		})
+	})
+})