Repository: metal3-io/baremetal-operator
Pull request #745 (2020-12-08 22:01:54)

Fix issues with deprovisioning after an error/failover
If provisioning fails and the node ends up in the `deploy failed` state,
we try to recover by deprovisioning. That would work better if we
actually handled `deploy failed` in `Deprovision()`.

Only call `Adopt()` if the previous provisioning completed and recorded
the requisite image data.

metal3-io-bot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/metal3-io/baremetal-operator/pull/745#" title="Author self-approved">zaneb</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=metal3-io%2Fbaremetal-operator).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/metal3-io/baremetal-operator/blob/master/OWNERS)~~ [zaneb]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
zaneb: /test-integration
metal3-io-bot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/metal3-io/baremetal-operator/pull/745#pullrequestreview-561885524" title="Approved">dhellmann</a>*, *<a href="https://github.com/metal3-io/baremetal-operator/pull/745#" title="Author self-approved">zaneb</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=metal3-io%2Fbaremetal-operator).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/metal3-io/baremetal-operator/blob/master/OWNERS)~~ [dhellmann,zaneb]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
zaneb: /test-integration

Patch:
diff --git a/controllers/metal3.io/baremetalhost_controller.go b/controllers/metal3.io/baremetalhost_controller.go
index 502116e2..d940abbf 100644
--- a/controllers/metal3.io/baremetalhost_controller.go
+++ b/controllers/metal3.io/baremetalhost_controller.go
@@ -612,26 +612,28 @@ func clearHostProvisioningSettings(host *metal3v1alpha1.BareMetalHost) {
 }
 
 func (r *BareMetalHostReconciler) actionDeprovisioning(prov provisioner.Provisioner, info *reconcileInfo) actionResult {
-	// Adopt the host in case it has been re-registered during the
-	// deprovisioning process before it completed
-	provResult, err := prov.Adopt(info.host.Status.ErrorType == metal3v1alpha1.RegistrationError)
-	if err != nil {
-		return actionError{err}
-	}
-	if provResult.ErrorMessage != "" {
-		return recordActionFailure(info, metal3v1alpha1.RegistrationError, provResult.ErrorMessage)
-	}
-	if provResult.Dirty {
-		result := actionContinue{provResult.RequeueAfter}
-		if clearError(info.host) {
-			return actionUpdate{result}
+	if info.host.Status.Provisioning.Image.URL != "" {
+		// Adopt the host in case it has been re-registered during the
+		// deprovisioning process before it completed
+		provResult, err := prov.Adopt(info.host.Status.ErrorType == metal3v1alpha1.RegistrationError)
+		if err != nil {
+			return actionError{err}
+		}
+		if provResult.ErrorMessage != "" {
+			return recordActionFailure(info, metal3v1alpha1.RegistrationError, provResult.ErrorMessage)
+		}
+		if provResult.Dirty {
+			result := actionContinue{provResult.RequeueAfter}
+			if clearError(info.host) {
+				return actionUpdate{result}
+			}
+			return result
 		}
-		return result
 	}
 
 	info.log.Info("deprovisioning")
 
-	provResult, err = prov.Deprovision(info.host.Status.ErrorType == metal3v1alpha1.ProvisioningError)
+	provResult, err := prov.Deprovision(info.host.Status.ErrorType == metal3v1alpha1.ProvisioningError)
 	if err != nil {
 		return actionError{errors.Wrap(err, "failed to deprovision")}
 	}
diff --git a/pkg/provisioner/ironic/ironic.go b/pkg/provisioner/ironic/ironic.go
index 8dc7c6fe..22379f70 100644
--- a/pkg/provisioner/ironic/ironic.go
+++ b/pkg/provisioner/ironic/ironic.go
@@ -1329,7 +1329,7 @@ func (p *ironicProvisioner) Deprovision(force bool) (result provisioner.Result,
 		p.log.Info("cleaning")
 		return operationContinuing(deprovisionRequeueDelay)
 
-	case nodes.Active:
+	case nodes.Active, nodes.DeployFail:
 		p.log.Info("starting deprovisioning")
 		p.publisher("DeprovisioningStarted", "Image deprovisioning started")
 		return p.changeNodeProvisionState(


Test patch:
diff --git a/pkg/provisioner/ironic/provision_test.go b/pkg/provisioner/ironic/provision_test.go
index 947cecab..a8f3d693 100644
--- a/pkg/provisioner/ironic/provision_test.go
+++ b/pkg/provisioner/ironic/provision_test.go
@@ -145,6 +145,15 @@ func TestDeprovision(t *testing.T) {
 			expectedRequestAfter: 10,
 			expectedDirty:        true,
 		},
+		{
+			name: "deploy failed state",
+			ironic: testserver.NewIronic(t).WithDefaultResponses().Node(nodes.Node{
+				ProvisionState: string(nodes.DeployFail),
+				UUID:           nodeUUID,
+			}),
+			expectedRequestAfter: 10,
+			expectedDirty:        true,
+		},
 		{
 			name: "error state",
 			ironic: testserver.NewIronic(t).WithDefaultResponses().Node(nodes.Node{