🐛 Enable removal of ExternallyProvisioned attribute from BareMetalHosts

 Signed-off-by: Ethan J. Gallant <[email protected]>

Author: ethan-gallant

internal/controller/metal3.io/host_state_machine.go

@@ -408,6 +408,9 @@ func (hsm *hostStateMachine) handleRegistering(_ *reconcileInfo) actionResult {
 	// if the credentials change and the Host must be re-registered.
 	if hsm.Host.Spec.ExternallyProvisioned {
 		hsm.NextState = metal3api.StateExternallyProvisioned
+	} else if hsm.Host.Status.Provisioning.State == metal3api.StateExternallyProvisioned {
+		// Removing externallyManaged moves hosts to provisioned state
+		hsm.NextState = metal3api.StateProvisioned
 	} else if inspectionDisabled(hsm.Host) {
 		hsm.NextState = metal3api.StatePreparing
 	} else {

internal/webhooks/metal3.io/v1alpha1/baremetalhost_validation.go

@@ -109,10 +109,6 @@ func (webhook *BareMetalHost) validateChanges(oldObj *metal3api.BareMetalHost, n
 		errs = append(errs, errors.New("bootMACAddress can not be changed once it is set"))
 	}
 
-	if oldObj.Spec.ExternallyProvisioned != newObj.Spec.ExternallyProvisioned {
-		errs = append(errs, errors.New("externallyProvisioned can not be changed"))
-	}
-
 	return errs
 }
 

internal/webhooks/metal3.io/v1alpha1/baremetalhost_validation_test.go

@@ -970,14 +970,6 @@ func TestValidateUpdate(t *testing.T) {
 				TypeMeta: tm, ObjectMeta: om, Spec: metal3api.BareMetalHostSpec{BootMACAddress: "test-mac"}},
 			wantedErr: "bootMACAddress can not be changed once it is set",
 		},
-		{
-			name: "updateExternallyProvisioned",
-			newBMH: &metal3api.BareMetalHost{
-				TypeMeta: tm, ObjectMeta: om, Spec: metal3api.BareMetalHostSpec{}},
-			oldBMH: &metal3api.BareMetalHost{
-				TypeMeta: tm, ObjectMeta: om, Spec: metal3api.BareMetalHostSpec{ExternallyProvisioned: true}},
-			wantedErr: "externallyProvisioned can not be changed",
-		},
 	}
 
 	for _, tt := range tests {

test/e2e/externally_provisioned_test.go

@@ -18,6 +18,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/cluster-api/test/framework"
 	"sigs.k8s.io/cluster-api/util"
+	kclient "sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 var _ = Describe("Create as externally provisioned, deprovision", Label("required", "provision", "deprovision"),
@@ -79,14 +80,28 @@ var _ = Describe("Create as externally provisioned, deprovision", Label("require
 			}, &bmh)
 			Expect(err).NotTo(HaveOccurred())
 
-			By("checking that the BMH was not inspected or deployed")
+			By("Checking that the BMH was not inspected or deployed")
 			Expect(bmh.Status.OperationHistory.Inspect.Start.IsZero()).To(BeTrue())
 			Expect(bmh.Status.OperationHistory.Provision.Start.IsZero()).To(BeTrue())
 
+			By("Removing the ExternallyProvisioned field")
+			bmh.Spec.ExternallyProvisioned = false
+
+			err = clusterProxy.GetClient().Update(ctx, &bmh)
+			Expect(err).NotTo(HaveOccurred())
+
+			Eventually(func(g Gomega) {
+				err = clusterProxy.GetClient().Get(ctx, kclient.ObjectKeyFromObject(&bmh), &bmh)
+				g.Expect(bmh.Status.Provisioning.State).To(Equal(metal3api.StateProvisioned))
+			}, 2*time.Second, 100*time.Millisecond).Should(Succeed())
+
 			By("Deleting the BMH")
-			// Wait for 2 seconds to allow time to confirm annotation is set
-			// TODO: fix this so we do not need the sleep
-			time.Sleep(2 * time.Second)
+			// Wait for the finalizer to be there, otherwise cleanup logic can't hold the resource
+			Eventually(func(g Gomega) {
+				err = clusterProxy.GetClient().Get(ctx, kclient.ObjectKeyFromObject(&bmh), &bmh)
+				g.Expect(err).NotTo(HaveOccurred())
+				g.Expect(bmh.Finalizers).To(ContainElement(metal3api.BareMetalHostFinalizer))
+			}, 2*time.Second, 100*time.Millisecond).Should(Succeed())
 
 			err = clusterProxy.GetClient().Delete(ctx, &bmh)
 			Expect(err).NotTo(HaveOccurred())