detach dataImage before normal bmh deletion, remove finalizer for bmh…

… detach and delete

Signed-off-by: Himanshu Roy <hroy@redhat.com>

controllers/metal3.io/baremetalhost_controller.go

@@ -510,6 +510,12 @@ func (r *BareMetalHostReconciler) actionPowerOffBeforeDeleting(prov provisioner.
 		return result
 	}
 
+	// Detach the DataImage when the BMH has been powered off before deletion.
+	if result := r.handleDataImageActions(prov, info); result != nil {
+		info.log.Info("Detach DataImage if it exists, before deleting BMH")
+		return result
+	}
+
 	return actionComplete{}
 }
 
@@ -520,6 +526,11 @@ func (r *BareMetalHostReconciler) actionDeleting(prov provisioner.Provisioner, i
 		"timestamp", info.host.DeletionTimestamp,
 	)
 
+	// Deleting DataImage if it exists for the BMH
+	if err := r.removeFinalizerDataImage(info); err != nil {
+		info.log.Error(err, "Failed to delete DataImage")
+	}
+
 	// no-op if finalizer has been removed.
 	if !utils.StringInList(info.host.Finalizers, metal3api.BareMetalHostFinalizer) {
 		info.log.Info("ready to be deleted")
@@ -593,6 +604,11 @@ func hasCustomDeploy(host *metal3api.BareMetalHost) bool {
 
 // detachHost() detaches the host from the Provisioner.
 func (r *BareMetalHostReconciler) detachHost(prov provisioner.Provisioner, info *reconcileInfo) actionResult {
+	// Remove DataImage finalizer if the host has been detached
+	if err := r.removeFinalizerDataImage(info); err != nil {
+		info.log.Info("Failed to remove DataImage finalizer, %w", "Error", err)
+	}
+
 	provResult, err := prov.Detach()
 	if err != nil {
 		return actionError{errors.Wrap(err, "failed to detach")}
@@ -1394,6 +1410,8 @@ func (r *BareMetalHostReconciler) manageHostPower(prov provisioner.Provisioner,
 	// Normal reboots only work in provisioned states, changing online is also possible for available hosts.
 	isProvisioned := provState == metal3api.StateProvisioned || provState == metal3api.StateExternallyProvisioned
 
+	isAvailable := provState == metal3api.StateAvailable
+
 	desiredReboot, desiredRebootMode := hasRebootAnnotation(info, !isProvisioned)
 
 	if desiredReboot {
@@ -1415,7 +1433,9 @@ func (r *BareMetalHostReconciler) manageHostPower(prov provisioner.Provisioner,
 		"reboot process", desiredPowerOnState != info.host.Spec.Online)
 
 	if desiredPowerOnState {
-		if isProvisioned {
+		// If the host is provisioned/externallyProvisioned or if its in
+		// the available state after deprovisioning.
+		if isProvisioned || isAvailable {
 			// If DataImage exists, handle attachment/detachment
 			dataImageResult := r.handleDataImageActions(prov, info)
 			if dataImageResult != nil {
@@ -1493,6 +1513,21 @@ func (r *BareMetalHostReconciler) handleDataImageActions(prov provisioner.Provis
 		return actionContinue{}
 	}
 
+	bmhProvState := info.host.Status.Provisioning.State
+	isBMHAvailable := bmhProvState == metal3api.StateAvailable
+	hostDeleteTriggered := !info.host.DeletionTimestamp.IsZero()
+	// If host has been deprovisioned and is in Available state, Or if
+	// BMH delete has been triggered and its in PowerOffBeforeDelete
+	// state, then detach already attached image or return empty.
+	detachDataImage := false
+	if isBMHAvailable || hostDeleteTriggered {
+		if dataImage.Status.AttachedImage.URL != "" {
+			detachDataImage = true
+		} else {
+			return nil
+		}
+	}
+
 	// Update reconciliation timestamp for dataImage
 	dataImage.Status.LastReconciled = &metav1.Time{Time: time.Now()}
 
@@ -1547,10 +1582,20 @@ func (r *BareMetalHostReconciler) handleDataImageActions(prov provisioner.Provis
 
 	attachedURL := dataImage.Status.AttachedImage.URL
 
-	if deleteDataImage {
-		info.log.Info("DataImage requested for deletion")
+	if deleteDataImage || detachDataImage {
+		detachReason := ""
+		if deleteDataImage {
+			detachReason = "Delete DataImage"
+		}
+		if isBMHAvailable {
+			detachReason = "BMH Deprovision finished, node in available state"
+		}
+		if hostDeleteTriggered {
+			detachReason = "BMH requested to be deleted"
+		}
+		info.log.Info("%w, requesting DataImage detach", "Reason", detachReason)
 		if attachedURL != "" || dirty {
-			info.log.Info("Detaching DataImage as it was deleted")
+			info.log.Info("Detaching DataImage, Reason = %w", "Reason", detachReason)
 			err := r.detachDataImage(prov, info, dataImage)
 			if err != nil {
 				return actionError{fmt.Errorf("failed to detach, %w", err)}
@@ -1677,6 +1722,29 @@ func (r *BareMetalHostReconciler) detachDataImage(prov provisioner.Provisioner,
 	return nil
 }
 
+// Delete DataImage when BareMetalHost is deleted.
+func (r *BareMetalHostReconciler) removeFinalizerDataImage(info *reconcileInfo) error {
+	dataImage := &metal3api.DataImage{}
+	if err := r.Get(info.ctx, info.request.NamespacedName, dataImage); err != nil {
+		// The DataImage resource may have been deleted
+		if k8serrors.IsNotFound(err) {
+			info.log.Info("No dataImage found for the bareMetalHost")
+			return nil
+		}
+		// Error reading the object - requeue the request.
+		return fmt.Errorf("could not load dataImage, %w", err)
+	}
+
+	info.log.Info("Remove finalizer for DataImage, since BareMetalHost has been deleted")
+	dataImage.Finalizers = utils.FilterStringFromList(
+		dataImage.Finalizers, metal3api.DataImageFinalizer)
+
+	if err := r.Update(info.ctx, dataImage); err != nil {
+		return fmt.Errorf("failed to update resource after remove finalizer, %w", err)
+	}
+	return nil
+}
+
 // A host reaching this action handler should be provisioned or externally
 // provisioned -- a state that it will stay in until the user takes further
 // action. We use the Adopt() API to make sure that the provisioner is aware of