fix: add instance status.starting for instance cleanup

Longhorn 11330

Signed-off-by: Shuo Wu <shuo.wu@suse.com>

Author: shuo-wu

controller/engine_controller.go

@@ -494,6 +494,11 @@ func (ec *EngineController) CreateInstance(obj interface{}) (*longhorn.InstanceP
 
 	instanceManagerStorageIP := ec.ds.GetStorageIPFromPod(instanceManagerPod)
 
+	e.Status.Starting = true
+	if e, err = ec.ds.UpdateEngineStatus(e); err != nil {
+		return nil, errors.Wrapf(err, "failed to update engine %v status.starting to true before sending instance create request", e.Name)
+	}
+
 	return c.EngineInstanceCreate(&engineapi.EngineInstanceCreateRequest{
 		Engine:                           e,
 		VolumeFrontend:                   frontend,

controller/instance_handler.go

@@ -368,6 +368,7 @@ func (h *InstanceHandler) ReconcileInstanceState(obj interface{}, spec *longhorn
 
 		if i, exists := instances[instanceName]; exists && i.Status.State == longhorn.InstanceStateRunning {
 			status.Started = true
+			status.Starting = false
 			break
 		}
 
@@ -388,18 +389,24 @@ func (h *InstanceHandler) ReconcileInstanceState(obj interface{}, spec *longhorn
 		}
 
 	case longhorn.InstanceStateStopped:
+		shouldDelete := false
 		if im != nil && im.DeletionTimestamp == nil {
+			if _, exists := instances[instanceName]; exists {
+				shouldDelete = true
+			}
+		}
+		if status.Starting {
+			shouldDelete = true
+		}
+		if shouldDelete {
 			// there is a delay between deleteInstance() invocation and state/InstanceManager update,
 			// deleteInstance() may be called multiple times.
-			if instance, exists := instances[instanceName]; exists {
-				if shouldDeleteInstance(&instance) {
-					if err := h.deleteInstance(instanceName, runtimeObj); err != nil {
-						return err
-					}
-				}
+			if err := h.deleteInstance(instanceName, runtimeObj); err != nil {
+				return err
 			}
 		}
 		status.Started = false
+		status.Starting = false
 	default:
 		return fmt.Errorf("unknown instance desire state: desire %v", spec.DesireState)
 	}
@@ -445,17 +452,6 @@ func (h *InstanceHandler) ReconcileInstanceState(obj interface{}, spec *longhorn
 	return nil
 }
 
-func shouldDeleteInstance(instance *longhorn.InstanceProcess) bool {
-	// For a replica of a SPDK volume, a stopped replica means the lvol is not exposed,
-	// but the lvol is still there. We don't need to delete it.
-	if types.IsDataEngineV2(instance.Spec.DataEngine) {
-		if instance.Status.State == longhorn.InstanceStateStopped {
-			return false
-		}
-	}
-	return true
-}
-
 func (h *InstanceHandler) getInstancesFromInstanceManager(obj runtime.Object, instanceManager *longhorn.InstanceManager) (map[string]longhorn.InstanceProcess, error) {
 	switch obj.(type) {
 	case *longhorn.Engine:

controller/replica_controller.go

@@ -380,6 +380,11 @@ func (rc *ReplicaController) CreateInstance(obj interface{}) (*longhorn.Instance
 		return nil, err
 	}
 
+	r.Status.Starting = true
+	if r, err = rc.ds.UpdateReplicaStatus(r); err != nil {
+		return nil, errors.Wrapf(err, "failed to update replica %v status.starting to true before sending instance create request", r.Name)
+	}
+
 	return c.ReplicaInstanceCreate(&engineapi.ReplicaInstanceCreateRequest{
 		Replica:             r,
 		DiskName:            diskName,

k8s/crds.yaml

@@ -1551,6 +1551,8 @@ spec:
                 type: string
               started:
                 type: boolean
+              starting:
+                type: boolean
               storageIP:
                 type: string
               ublkID:
@@ -2555,6 +2557,8 @@ spec:
                 type: boolean
               started:
                 type: boolean
+              starting:
+                type: boolean
               storageIP:
                 type: string
               ublkID:

k8s/pkg/apis/longhorn/v1beta2/instancemanager.go

@@ -102,6 +102,8 @@ type InstanceStatus struct {
 	// +optional
 	Port int `json:"port"`
 	// +optional
+	Starting bool `json:"starting"`
+	// +optional
 	Started bool `json:"started"`
 	// +optional
 	LogFetched bool `json:"logFetched"`