MGMT-14852: Allow AutomatedCleaningMode to be set by user (#5319)

https://issues.redhat.com/browse/MGMT-14852
Epic: https://issues.redhat.com/browse/MGMT-13017
Removes setting automated cleaning mode to disabled
by default, which lets the user set the field.
Part of the larger epic to allow wiping disks before
installing by using Ironic's automated cleaning feature.

internal/controller/controllers/bmh_agent_controller.go

@@ -868,29 +868,21 @@ func (r *BMACReconciler) reconcileBMH(ctx context.Context, log logrus.FieldLogge
 	}
 
 	dirty := false
-	// in case of converged flow set the custom deploy and cleaning mode instead of the annotations
+	// in case of converged flow set the custom deploy instead of the annotations
 	if r.ConvergedFlowEnabled {
 		if bmh.Spec.CustomDeploy == nil || bmh.Spec.CustomDeploy.Method != ASSISTED_DEPLOY_METHOD {
 			log.Infof("Updating BMH CustomDeploy to %s", ASSISTED_DEPLOY_METHOD)
 			bmh.Spec.CustomDeploy = &bmh_v1alpha1.CustomDeploy{Method: ASSISTED_DEPLOY_METHOD}
 			dirty = true
 		}
-		if bmh.Spec.AutomatedCleaningMode != bmh_v1alpha1.CleaningModeDisabled {
-			log.Infof("Updating BMH AutomatedCleaningMode to %s", bmh_v1alpha1.CleaningModeDisabled)
-			bmh.Spec.AutomatedCleaningMode = bmh_v1alpha1.CleaningModeDisabled
-			dirty = true
-		}
 		return reconcileComplete{dirty: dirty, stop: false}
 	}
 
 	annotations := bmh.ObjectMeta.GetAnnotations()
 	// Set the following parameters regardless of the state
 	// of the InfraEnv and the BMH. There is no need for
-	// inspection and cleaning to happen out of assisted
-	// service's loop.
-	if _, ok := annotations[BMH_INSPECT_ANNOTATION]; !ok || bmh.Spec.AutomatedCleaningMode != bmh_v1alpha1.CleaningModeDisabled {
-		bmh.Spec.AutomatedCleaningMode = bmh_v1alpha1.CleaningModeDisabled
-
+	// inspection to happen out of assisted service's loop.
+	if _, ok := annotations[BMH_INSPECT_ANNOTATION]; !ok {
 		// Let's make sure inspection is disabled for BMH resources
 		// that are associated with an agent-based deployment.
 		//

internal/controller/controllers/bmh_agent_controller_test.go

@@ -347,22 +347,22 @@ var _ = Describe("bmac reconcile", func() {
 				Expect(host.ObjectMeta.Annotations[BMH_INSPECT_ANNOTATION]).To(Equal("disabled"))
 				Expect(host.Spec.AutomatedCleaningMode).To(Equal(bmh_v1alpha1.CleaningModeDisabled))
 
-				// Test that only cleaning != disabled will set both parameters
+				// Test that cleaning mode stays the same
 				host.Spec.AutomatedCleaningMode = bmh_v1alpha1.CleaningModeMetadata
 				host.Status.Provisioning.State = bmh_v1alpha1.StateProvisioned
 
 				result = bmhr.reconcileBMH(ctx, bmhr.Log, host, nil)
-				Expect(result).To(Equal(reconcileComplete{dirty: true, stop: true}))
+				Expect(result).To(Equal(reconcileComplete{dirty: false, stop: true}))
 				Expect(host.ObjectMeta.Annotations).To(HaveKey(BMH_INSPECT_ANNOTATION))
 				Expect(host.ObjectMeta.Annotations[BMH_INSPECT_ANNOTATION]).To(Equal("disabled"))
-				Expect(host.Spec.AutomatedCleaningMode).To(Equal(bmh_v1alpha1.CleaningModeDisabled))
+				Expect(host.Spec.AutomatedCleaningMode).To(Equal(bmh_v1alpha1.CleaningModeMetadata))
 
 				// This should not return a dirty result because label is already set
 				result = bmhr.reconcileBMH(ctx, bmhr.Log, host, nil)
 				Expect(result).To(Equal(reconcileComplete{dirty: false, stop: true}))
 				Expect(host.ObjectMeta.Annotations).To(HaveKey(BMH_INSPECT_ANNOTATION))
 				Expect(host.ObjectMeta.Annotations[BMH_INSPECT_ANNOTATION]).To(Equal("disabled"))
-				Expect(host.Spec.AutomatedCleaningMode).To(Equal(bmh_v1alpha1.CleaningModeDisabled))
+				Expect(host.Spec.AutomatedCleaningMode).To(Equal(bmh_v1alpha1.CleaningModeMetadata))
 			})
 
 			It("should set the ISODownloadURL in the BMH", func() {
@@ -376,7 +376,7 @@ var _ = Describe("bmac reconcile", func() {
 				Expect(updatedHost.Spec.Image.URL).To(Equal(isoImageURL))
 			})
 
-			It("should disable cleaning and set online true in the BMH", func() {
+			It("should not disable cleaning and set online true in the BMH", func() {
 				result, err := bmhr.Reconcile(ctx, newBMHRequest(host))
 				Expect(err).To(BeNil())
 				Expect(result).To(Equal(ctrl.Result{}))
@@ -385,7 +385,7 @@ var _ = Describe("bmac reconcile", func() {
 				err = c.Get(ctx, types.NamespacedName{Name: "bmh-reconcile", Namespace: testNamespace}, updatedHost)
 				Expect(err).To(BeNil())
 				Expect(updatedHost.Spec.Online).To(Equal(true))
-				Expect(updatedHost.Spec.AutomatedCleaningMode).To(Equal(bmh_v1alpha1.CleaningModeDisabled))
+				Expect(updatedHost.Spec.AutomatedCleaningMode).NotTo(Equal(bmh_v1alpha1.CleaningModeDisabled))
 			})
 			It("should not reconcile BMH if the updated image has not been around longer than the grace period", func() {
 				// Reconcile with the original ISO
@@ -1556,7 +1556,7 @@ var _ = Describe("bmac reconcile - converged flow enabled", func() {
 			Expect(updatedHost.ObjectMeta.Annotations).To(BeNil())
 		})
 
-		It("should disable cleaning in the BMH", func() {
+		It("should not disable cleaning in the BMH", func() {
 
 			host.Spec.AutomatedCleaningMode = bmh_v1alpha1.CleaningModeMetadata
 			Expect(c.Update(ctx, host)).To(BeNil())
@@ -1568,7 +1568,7 @@ var _ = Describe("bmac reconcile - converged flow enabled", func() {
 			updatedHost := &bmh_v1alpha1.BareMetalHost{}
 			err = c.Get(ctx, types.NamespacedName{Name: "bmh-reconcile", Namespace: testNamespace}, updatedHost)
 			Expect(err).To(BeNil())
-			Expect(updatedHost.Spec.AutomatedCleaningMode).To(Equal(bmh_v1alpha1.CleaningModeDisabled))
+			Expect(updatedHost.Spec.AutomatedCleaningMode).To(Equal(bmh_v1alpha1.CleaningModeMetadata))
 			// check that the host isn't detached
 			Expect(updatedHost.ObjectMeta.Annotations).To(BeNil())
 		})