Allow forced detachment of a host from Ironic

Adds Force as DetachedAnnotationArgument to API.
Detach in all states if force is set to true by aborting the currently
running process.

Co-Authored-By: Dmitry Tantsur <dtantsur@protonmail.com>
Signed-off-by: CrystalChun <cchun@redhat.com>
Signed-off-by: Dmitry Tantsur <dtantsur@protonmail.com>

Author: CrystalChun

apis/metal3.io/v1alpha1/baremetalhost_types.go

@@ -685,6 +685,11 @@ const (
 type DetachedAnnotationArguments struct {
 	// DeleteAction indicates the desired delete logic when the detached annotation is present
 	DeleteAction DetachedDeleteAction `json:"deleteAction,omitempty"`
+
+	// Force indicates if detaching should be forced regardless of the host's state
+	// +optional
+	// +kubebuilder:default:=false
+	Force bool `json:"force,omitempty"`
 }
 
 // Match compares the saved status information with the name and

internal/controller/metal3.io/baremetalhost_controller.go

@@ -617,8 +617,8 @@ func hasCustomDeploy(host *metal3api.BareMetalHost) bool {
 }
 
 // detachHost() detaches the host from the Provisioner.
-func (r *BareMetalHostReconciler) detachHost(ctx context.Context, prov provisioner.Provisioner, info *reconcileInfo) actionResult {
-	provResult, err := prov.Detach(ctx)
+func (r *BareMetalHostReconciler) detachHost(ctx context.Context, prov provisioner.Provisioner, info *reconcileInfo, force bool) actionResult {
+	provResult, err := prov.Detach(ctx, force)
 	if err != nil {
 		return actionError{fmt.Errorf("failed to detach: %w", err)}
 	}

internal/controller/metal3.io/host_state_machine.go

@@ -314,11 +314,17 @@ func (hsm *hostStateMachine) checkDetachedHost(ctx context.Context, info *reconc
 	// provisioner and take no further action
 	// Note this doesn't change the current state, only the OperationalStatus
 	if hasDetachedAnnotation(hsm.Host) {
-		// Only allow detaching hosts in Provisioned/ExternallyProvisioned/Ready/Available states
+		// Only allow detaching hosts in Provisioned/ExternallyProvisioned/Ready/Available states unless forced
 		switch info.host.Status.Provisioning.State {
 		case metal3api.StateProvisioned, metal3api.StateExternallyProvisioned, metal3api.StateReady, metal3api.StateAvailable:
-			return hsm.Reconciler.detachHost(ctx, hsm.Provisioner, info)
+			return hsm.Reconciler.detachHost(ctx, hsm.Provisioner, info, false)
+		case metal3api.StateDeleting:
+			// No point in detaching a host that is being deleted already
 		default:
+			if hasForceDetachAnnotation(hsm.Host) {
+				info.log.Info("forcing detach of host", "provisioningState", info.host.Status.Provisioning.State)
+				return hsm.Reconciler.detachHost(ctx, hsm.Provisioner, info, true)
+			}
 			info.log.Info("host cannot be detached yet, waiting for the current operation to finish", "provisioningState", info.host.Status.Provisioning.State)
 		}
 	}
@@ -340,6 +346,20 @@ func (hsm *hostStateMachine) checkDetachedHost(ctx context.Context, info *reconc
 	return nil
 }
 
+func hasForceDetachAnnotation(host *metal3api.BareMetalHost) bool {
+	annotations := host.GetAnnotations()
+	if annotations != nil {
+		if val, ok := annotations[metal3api.DetachedAnnotation]; ok {
+			args := metal3api.DetachedAnnotationArguments{}
+			if err := json.Unmarshal([]byte(val), &args); err != nil {
+				return false
+			}
+			return args.Force
+		}
+	}
+	return false
+}
+
 func (hsm *hostStateMachine) ensureRegistered(ctx context.Context, info *reconcileInfo) (result actionResult) {
 	if !hsm.haveCreds {
 		// If we are in the process of deletion (which may start with

internal/controller/metal3.io/host_state_machine_test.go

@@ -1379,11 +1379,11 @@ func (m *mockProvisioner) Deprovision(_ context.Context, _ bool, _ metal3api.Aut
 	return m.getNextResultByMethod("Deprovision"), err
 }
 
-func (m *mockProvisioner) Delete(context.Context) (result provisioner.Result, err error) {
+func (m *mockProvisioner) Delete(_ context.Context) (result provisioner.Result, err error) {
 	return m.getNextResultByMethod("Delete"), err
 }
 
-func (m *mockProvisioner) Detach(context.Context) (result provisioner.Result, err error) {
+func (m *mockProvisioner) Detach(_ context.Context, _ bool) (result provisioner.Result, err error) {
 	res := m.getNextResultByMethod("Detach")
 	return res, err
 }

pkg/provisioner/demo/demo.go

@@ -287,7 +287,7 @@ func (p *demoProvisioner) Delete(_ context.Context) (result provisioner.Result,
 // for the target system.  It may be called multiple times,
 // and should return true for its dirty  flag until the
 // deletion operation is completed.
-func (p *demoProvisioner) Detach(_ context.Context) (result provisioner.Result, err error) {
+func (p *demoProvisioner) Detach(_ context.Context, _ bool) (result provisioner.Result, err error) {
 	p.log.Info("detaching host")
 	return result, nil
 }

pkg/provisioner/fixture/fixture.go

@@ -349,7 +349,7 @@ func (p *fixtureProvisioner) Delete(_ context.Context) (result provisioner.Resul
 // for the target system.  It may be called multiple times,
 // and should return true for its dirty  flag until the
 // deletion operation is completed.
-func (p *fixtureProvisioner) Detach(ctx context.Context) (result provisioner.Result, err error) {
+func (p *fixtureProvisioner) Detach(ctx context.Context, _ bool) (result provisioner.Result, err error) {
 	return p.Delete(ctx)
 }
 

pkg/provisioner/ironic/clients/features.go

@@ -56,7 +56,15 @@ func (af AvailableFeatures) HasDisablePowerOff() bool {
 	return af.MaxVersion >= 95 //nolint:mnd
 }
 
+func (af AvailableFeatures) HasDeploymentAbort() bool {
+	return af.MaxVersion >= 110 //nolint:mnd
+}
+
 func (af AvailableFeatures) ChooseMicroversion() string {
+	if af.HasDeploymentAbort() {
+		return "1.110"
+	}
+
 	if af.HasDisablePowerOff() {
 		return "1.95"
 	}

pkg/provisioner/ironic/delete_test.go

@@ -217,7 +217,7 @@ func deleteTest(t *testing.T, detach bool) {
 
 			var result provisioner.Result
 			if detach {
-				result, err = prov.Detach(t.Context())
+				result, err = prov.Detach(t.Context(), false)
 			} else {
 				result, err = prov.Delete(t.Context())
 			}

pkg/provisioner/ironic/ironic.go

@@ -1527,23 +1527,27 @@ func (p *ironicProvisioner) Delete(ctx context.Context) (result provisioner.Resu
 		"target", ironicNode.TargetProvisionState,
 		"deploy step", ironicNode.DeployStep,
 	)
+	return p.realDelete(ctx, ironicNode, false)
+}
 
+func (p *ironicProvisioner) realDelete(ctx context.Context, ironicNode *nodes.Node, force bool) (result provisioner.Result, err error) {
 	currentProvState := nodes.ProvisionState(ironicNode.ProvisionState)
 
-	// Handle verifying state specially: Ironic holds an exclusive lock during
-	// verification, so we can't set maintenance mode or delete until it completes.
-	// Just wait for the verification to finish (success or timeout).
-	if currentProvState == nodes.Verifying {
+	switch currentProvState {
+	case nodes.Verifying:
+		// Handle verifying state specially: Ironic holds an exclusive lock during
+		// verification, so we can't set maintenance mode or delete until it completes.
+		// Just wait for the verification to finish (success or timeout).
 		p.log.Info("node is verifying, waiting for verification to complete before deletion")
 		return operationContinuing(provisionRequeueDelay)
-	}
 
-	// For enroll state, the node can be deleted directly without maintenance mode
-	// since it has no Nova associations and isn't locked.
-	if currentProvState == nodes.Enroll {
+	case nodes.Enroll:
+		// For enroll state, the node can be deleted directly without maintenance mode
+		// since it has no Nova associations and isn't locked.
 		p.log.Info("node is in enroll state, proceeding to delete directly")
 		// Fall through to deletion
-	} else if currentProvState == nodes.Available || currentProvState == nodes.Manageable {
+
+	case nodes.Available, nodes.Manageable:
 		// Make sure we don't have a stale instance UUID
 		if ironicNode.InstanceUUID != "" {
 			var success bool
@@ -1555,20 +1559,51 @@ func (p *ironicProvisioner) Delete(ctx context.Context) (result provisioner.Resu
 				return result, err
 			}
 		}
-	} else if !ironicNode.Maintenance {
-		// If we see an active node and the controller doesn't think
-		// we need to deprovision it, that means the node was
-		// ExternallyProvisioned and we should remove it from Ironic
-		// without deprovisioning it.
-		//
-		// If we see a node with an error, we will have to set the
-		// maintenance flag before deleting it.
-		//
-		// Any other state requires us to use maintenance mode to
-		// delete while bypassing Ironic's internal checks related to
-		// Nova.
-		p.log.Info("setting host maintenance flag to force image delete")
-		return p.setMaintenanceFlag(ctx, ironicNode, true, "forcing deletion in baremetal-operator")
+
+	case nodes.Deploying, nodes.Cleaning, nodes.Inspecting, nodes.Servicing, nodes.Deleting:
+		p.log.Info("node is in state that does not allow deletion, waiting", "currentState", currentProvState)
+		return operationContinuing(provisionRequeueDelay)
+
+	case nodes.DeployWait:
+		if force && !p.availableFeatures.HasDeploymentAbort() {
+			p.log.Info("deprovisioning to force deletion")
+			// No new API - fall back to deprovisioning and wait for CLEANWAIT
+			return p.changeNodeProvisionState(ctx, ironicNode,
+				nodes.ProvisionStateOpts{Target: nodes.TargetDeleted},
+			)
+		}
+
+		// Otherwise, use the abort API as well
+		fallthrough
+
+	case nodes.CleanWait, nodes.ServiceWait:
+		if force {
+			p.log.Info("aborting the current operation to force deletion", "currentState", currentProvState)
+			return p.changeNodeProvisionState(ctx, ironicNode,
+				nodes.ProvisionStateOpts{Target: nodes.TargetAbort},
+			)
+		}
+
+		// Normal deletion won't work in these states, so wait
+		p.log.Info("node is in state that does not allow deletion, waiting", "currentState", currentProvState)
+		return operationContinuing(provisionRequeueDelay)
+
+	default:
+		if !ironicNode.Maintenance {
+			// If we see an active node and the controller doesn't think
+			// we need to deprovision it, that means the node was
+			// ExternallyProvisioned and we should remove it from Ironic
+			// without deprovisioning it.
+			//
+			// If we see a node with an error, we will have to set the
+			// maintenance flag before deleting it.
+			//
+			// Any other state requires us to use maintenance mode to
+			// delete while bypassing Ironic's internal checks related to
+			// Nova.
+			p.log.Info("setting host maintenance flag to force image delete", "currentState", currentProvState)
+			return p.setMaintenanceFlag(ctx, ironicNode, true, "forcing deletion in baremetal-operator")
+		}
 	}
 
 	p.log.Info("host ready to be removed")
@@ -1592,10 +1627,26 @@ func (p *ironicProvisioner) Delete(ctx context.Context) (result provisioner.Resu
 // for the target system.  It may be called multiple times,
 // and should return true for its dirty  flag until the
 // deletion operation is completed.
-func (p *ironicProvisioner) Detach(ctx context.Context) (result provisioner.Result, err error) {
+func (p *ironicProvisioner) Detach(ctx context.Context, force bool) (result provisioner.Result, err error) {
 	// Currently the same behavior as Delete()
-	p.log.Info("removing the node for detachment", "node", p.nodeID)
-	return p.Delete(ctx)
+	ironicNode, err := p.getNode(ctx)
+	if err != nil {
+		if errors.Is(err, provisioner.ErrNeedsRegistration) {
+			p.log.Info("no node found, already deleted")
+			return operationComplete()
+		}
+		return transientError(err)
+	}
+
+	p.log.Info("deleting host for detachment",
+		"ID", ironicNode.UUID,
+		"lastError", ironicNode.LastError,
+		"current", ironicNode.ProvisionState,
+		"target", ironicNode.TargetProvisionState,
+		"deploy step", ironicNode.DeployStep,
+		"force", force,
+	)
+	return p.realDelete(ctx, ironicNode, force)
 }
 
 // softPowerOffUnsupportedError is returned when the BMC does not

pkg/provisioner/provisioner.go

@@ -189,7 +189,7 @@ type Provisioner interface {
 	// for the target system.  It may be called multiple times,
 	// and should return true for its dirty  flag until the
 	// deletion operation is completed.
-	Detach(ctx context.Context) (result Result, err error)
+	Detach(ctx context.Context, force bool) (result Result, err error)
 
 	// PowerOn ensures the server is powered on independently of any image
 	// provisioning operation.