fix(orphan): finalize orphan instance CR when instance manager unavailable

Signed-off-by: Raphanus Lo <[email protected]>

Author: COLDTURNIP

controller/orphan_controller.go

@@ -315,7 +315,7 @@ func (oc *OrphanController) cleanupOrphanedResource(orphan *longhorn.Orphan) (is
 			longhorn.OrphanConditionTypeError, longhorn.ConditionStatusTrue, "", err.Error())
 	}()
 
-	// Make sure if the orphan nodeID and controller ID are same.
+	// Make sure if the orphan nodeID and controller ID are the same.
 	// If NO, just delete the orphan resource object and don't touch the data.
 	if orphan.Spec.NodeID != oc.controllerID {
 		log.WithFields(logrus.Fields{
@@ -370,7 +370,7 @@ func (oc *OrphanController) cleanupOrphanedEngineInstance(orphan *longhorn.Orpha
 	} else {
 		spec = &engineCR.Spec.InstanceSpec
 	}
-	return oc.cleanupOrphanedInstance(orphan, instance, imName, longhorn.InstanceManagerTypeEngine, spec)
+	return oc.cleanupOrphanedInstance(orphan, instance, imName, longhorn.InstanceManagerTypeEngine, spec), nil
 }
 
 func (oc *OrphanController) cleanupOrphanedReplicaInstance(orphan *longhorn.Orphan) (isCleanupComplete bool, err error) {
@@ -394,7 +394,7 @@ func (oc *OrphanController) cleanupOrphanedReplicaInstance(orphan *longhorn.Orph
 	} else {
 		spec = &replicaCR.Spec.InstanceSpec
 	}
-	return oc.cleanupOrphanedInstance(orphan, instance, imName, longhorn.InstanceManagerTypeReplica, spec)
+	return oc.cleanupOrphanedInstance(orphan, instance, imName, longhorn.InstanceManagerTypeReplica, spec), nil
 }
 
 func (oc *OrphanController) extractOrphanedInstanceInfo(orphan *longhorn.Orphan) (name, instanceManager string, err error) {
@@ -418,33 +418,33 @@ func (oc *OrphanController) extractOrphanedInstanceInfo(orphan *longhorn.Orphan)
 	return name, instanceManager, nil
 }
 
-func (oc *OrphanController) cleanupOrphanedInstance(orphan *longhorn.Orphan, instance, imName string, imType longhorn.InstanceManagerType, instanceCRSpec *longhorn.InstanceSpec) (isCleanupComplete bool, err error) {
+func (oc *OrphanController) cleanupOrphanedInstance(orphan *longhorn.Orphan, instance, imName string, imType longhorn.InstanceManagerType, instanceCRSpec *longhorn.InstanceSpec) bool {
 	if instanceCRSpec != nil && instanceCRSpec.NodeID == orphan.Spec.NodeID {
 		oc.logger.Infof("Orphan instance %v is scheduled back to current node %v. Skip cleaning up the instance resource and finalize the orphan CR.", instance, orphan.Spec.NodeID)
-		return true, nil
+		return true
 	}
 
+	// If the instance manager client is unavailable or failed to delete the instance, continue finalizing the orphan.
+	// Later if the orphaned instance is still reachable, the orphan will be recreated.
 	imc, err := oc.getRunningInstanceManagerClientForOrphan(orphan, imName)
 	if err != nil {
-		return false, errors.Wrapf(err, "failed to get running instance manager client for orphan %v", orphan.Name)
+		oc.logger.WithError(err).Warnf("Failed to delete orphan instance %v due to instance manager client initialization failure. Continue to finalize orphan %v", instance, orphan.Name)
+		return false
 	} else if imc == nil {
 		oc.logger.WithField("orphanInstanceNode", orphan.Spec.NodeID).Warnf("No running instance manager for deleting orphan instance %v", orphan.Name)
-		return true, nil
+		return true
 	}
 	defer func() {
 		if closeErr := imc.Close(); closeErr != nil {
 			oc.logger.WithError(closeErr).Error("Failed to close instance manager client")
 		}
 	}()
 
-	if err := oc.deleteInstance(imc, instance, imType, orphan.Spec.DataEngine); err != nil {
-		return false, err
-	}
-	isCleanupComplete, cleanupErr := oc.confirmOrphanInstanceCleanup(imc, instance, imType, orphan.Spec.DataEngine)
-	if cleanupErr == nil && !isCleanupComplete {
-		oc.logger.Infof("Orphan instance %v cleanup in progress", instance)
+	err = imc.InstanceDelete(orphan.Spec.DataEngine, instance, string(imType), "", false)
+	if err != nil && !types.ErrorIsNotFound(err) {
+		oc.logger.WithError(err).Warnf("Failed to delete orphan instance %v. Continue to finalize orphan %v", instance, orphan.Name)
 	}
-	return isCleanupComplete, cleanupErr
+	return true
 }
 
 func (oc *OrphanController) getRunningInstanceManagerClientForOrphan(orphan *longhorn.Orphan, imName string) (*engineapi.InstanceManagerClient, error) {
@@ -462,46 +462,6 @@ func (oc *OrphanController) getRunningInstanceManagerClientForOrphan(orphan *lon
 	return engineapi.NewInstanceManagerClient(im, false)
 }
 
-func (oc *OrphanController) deleteInstance(imc *engineapi.InstanceManagerClient, instanceName string, instanceKind longhorn.InstanceManagerType, engineType longhorn.DataEngineType) (err error) {
-	// There is a delay between deletion initiation and state/InstanceManager update,
-	// this function may be called multiple times before given instance exits.
-
-	oc.logger.Infof("Orphan controller deleting instance %v", instanceName)
-
-	defer func() {
-		err = errors.Wrapf(err, "failed to delete instance %v", instanceName)
-	}()
-
-	err = imc.InstanceDelete(engineType, instanceName, string(instanceKind), "", false)
-	if err != nil && !types.ErrorIsNotFound(err) {
-		return err
-	}
-
-	return nil
-}
-
-func (oc *OrphanController) confirmOrphanInstanceCleanup(imc *engineapi.InstanceManagerClient, instanceName string, imType longhorn.InstanceManagerType, engineType longhorn.DataEngineType) (isCleanupComplete bool, err error) {
-	defer func() {
-		if err != nil {
-			err = errors.Wrapf(err, "failed to confirm cleanup result of instance %v", instanceName)
-		}
-	}()
-
-	_, err = imc.InstanceGet(engineType, instanceName, string(imType))
-	switch {
-	case err == nil:
-		// Instance still exists - cleanup not complete
-		// Cleanup will continue after the instance state update.
-		return false, nil
-	case types.ErrorIsNotFound(err):
-		// instance not found - cleanup completed.
-		return true, nil
-	default:
-		// Unexpected error - cleanup status unknown.
-		return false, err
-	}
-}
-
 func (oc *OrphanController) deleteOrphanedReplicaDataStore(orphan *longhorn.Orphan) error {
 	oc.logger.Infof("Deleting orphan %v replica data store %v in disk %v on node %v",
 		orphan.Name, orphan.Spec.Parameters[longhorn.OrphanDataName],