Repository: metal3-io/baremetal-operator
Pull request #1230 (2023-03-07 15:19:36)

:seedling: Delay deprovision/delete when multiple finalizers exist
Delaying moving the host to deprovisioning or deleting allows other components to do whatever logic they need to do before BMO decides if the host should be deprovisioned or deleted.

In one specific case a finalizer could be added to a detached host which will attach the host to allow it to be cleaned at the time it is deleted. This logic needs to happen before BMO proceeds with its own delete logic.

@dtantsur PTAL


Patch:
diff --git a/controllers/metal3.io/baremetalhost_controller.go b/controllers/metal3.io/baremetalhost_controller.go
index 6bac3c94..0bad0678 100644
--- a/controllers/metal3.io/baremetalhost_controller.go
+++ b/controllers/metal3.io/baremetalhost_controller.go
@@ -838,7 +838,7 @@ func (r *BareMetalHostReconciler) registerHost(prov provisioner.Provisioner, inf
 	// Create the hostFirmwareSettings resource with same host name/namespace if it doesn't exist
 	if info.host.Name != "" {
 		if !info.host.DeletionTimestamp.IsZero() {
-			r.Log.Info(fmt.Sprintf("will not attempt to create new hostFirmwareSettings in %s", info.host.Namespace))
+			info.log.Info(fmt.Sprintf("will not attempt to create new hostFirmwareSettings in %s", info.host.Namespace))
 		} else {
 			if err = r.createHostFirmwareSettings(info); err != nil {
 				info.log.Info("failed creating hostfirmwaresettings")
diff --git a/controllers/metal3.io/host_state_machine.go b/controllers/metal3.io/host_state_machine.go
index 40891f19..e97317ba 100644
--- a/controllers/metal3.io/host_state_machine.go
+++ b/controllers/metal3.io/host_state_machine.go
@@ -223,6 +223,10 @@ func (hsm *hostStateMachine) checkInitiateDelete() bool {
 	default:
 		hsm.NextState = metal3v1alpha1.StateDeleting
 	case metal3v1alpha1.StateProvisioning, metal3v1alpha1.StateProvisioned:
+		if len(hsm.Host.Finalizers) > 1 {
+			// Allow other finalizers to run before changing state
+			return false
+		}
 		if hsm.Host.OperationalStatus() == metal3v1alpha1.OperationalStatusDetached {
 			hsm.NextState = metal3v1alpha1.StateDeleting
 		} else {


Test patch:
diff --git a/controllers/metal3.io/host_state_machine_test.go b/controllers/metal3.io/host_state_machine_test.go
index f7901c8c..1038bd9c 100644
--- a/controllers/metal3.io/host_state_machine_test.go
+++ b/controllers/metal3.io/host_state_machine_test.go
@@ -1075,6 +1075,60 @@ func TestErrorClean(t *testing.T) {
 	}
 }
 
+func TestDeleteWaitsForFinalizers(t *testing.T) {
+	tests := []struct {
+		Scenario      string
+		Host          *metal3v1alpha1.BareMetalHost
+		ExpectedState metal3v1alpha1.ProvisioningState
+	}{
+		{
+			Scenario: "provisioning-2-finalizers",
+			Host: host(metal3v1alpha1.StateProvisioning).
+				setDeletion().
+				setFinalizers([]string{metal3v1alpha1.BareMetalHostFinalizer, "other-finalizer"}).
+				build(),
+			ExpectedState: metal3v1alpha1.StateProvisioned,
+		},
+		{
+			Scenario: "provisioned-2-finalizers",
+			Host: host(metal3v1alpha1.StateProvisioned).
+				setDeletion().
+				setFinalizers([]string{metal3v1alpha1.BareMetalHostFinalizer, "other-finalizer"}).
+				build(),
+			ExpectedState: metal3v1alpha1.StateProvisioned,
+		},
+		{
+			Scenario: "provisioning-1-finalizer",
+			Host: host(metal3v1alpha1.StateProvisioning).
+				setDeletion().
+				setFinalizers([]string{metal3v1alpha1.BareMetalHostFinalizer}).
+				build(),
+			ExpectedState: metal3v1alpha1.StateDeprovisioning,
+		},
+		{
+			Scenario: "provisioned-1-finalizer",
+			Host: host(metal3v1alpha1.StateProvisioned).
+				setDeletion().
+				setFinalizers([]string{metal3v1alpha1.BareMetalHostFinalizer}).
+				build(),
+			ExpectedState: metal3v1alpha1.StateDeprovisioning,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.Scenario, func(t *testing.T) {
+			prov := newMockProvisioner()
+			hsm := newHostStateMachine(tt.Host, &BareMetalHostReconciler{
+				Client: fakeclient.NewFakeClient(),
+			}, prov, true)
+
+			info := makeDefaultReconcileInfo(tt.Host)
+			hsm.ReconcileState(info)
+
+			assert.Equal(t, tt.ExpectedState, tt.Host.Status.Provisioning.State)
+		})
+	}
+}
+
 type hostBuilder struct {
 	metal3v1alpha1.BareMetalHost
 }
@@ -1194,6 +1248,11 @@ func (hb *hostBuilder) setDeletion() *hostBuilder {
 	return hb
 }
 
+func (hb *hostBuilder) setFinalizers(finalizers []string) *hostBuilder {
+	hb.Finalizers = finalizers
+	return hb
+}
+
 func makeDefaultReconcileInfo(host *metal3v1alpha1.BareMetalHost) *reconcileInfo {
 	return &reconcileInfo{
 		log:     logf.Log.WithName("controllers").WithName("BareMetalHost").WithName("host_state_machine"),