fix: use volume state to check all detached

Signed-off-by: James Munson <[email protected]>
longhorn · Jan 31, 2025 · cbb72cd · cbb72cd
1 parent 2820078
commit cbb72cd
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 14 deletions.
diff --git a/controller/setting_controller.go b/controller/setting_controller.go
@@ -300,7 +300,7 @@ func (sc *SettingController) syncDangerZoneSettingsForManagedComponents(settingN
 			}
 		case types.SettingNameStorageNetwork:
 			funcPreupdate := func() error {
-				detached, _, err := sc.ds.AreAllVolumesDetached(longhorn.DataEngineTypeAll)
+				detached, err := sc.ds.AreAllVolumesDetachedState()
 				if err != nil {
 					return errors.Wrapf(err, "failed to check volume detachment for %v setting update", types.SettingNameStorageNetwork)
 				}
@@ -422,7 +422,7 @@ func (sc *SettingController) updateTaintToleration() error {
 		return nil
 	}
 
-	detached, _, err := sc.ds.AreAllVolumesDetached(longhorn.DataEngineTypeAll)
+	detached, err := sc.ds.AreAllVolumesDetachedState()
 	if err != nil {
 		return errors.Wrapf(err, "failed to check volume detachment for %v setting update", types.SettingNameTaintToleration)
 	}
@@ -593,7 +593,7 @@ func (sc *SettingController) updatePriorityClass() error {
 		return nil
 	}
 
-	detached, _, err := sc.ds.AreAllVolumesDetached(longhorn.DataEngineTypeAll)
+	detached, err := sc.ds.AreAllVolumesDetachedState()
 	if err != nil {
 		return errors.Wrapf(err, "failed to check volume detachment for %v setting update", types.SettingNamePriorityClass)
 	}
@@ -994,7 +994,7 @@ func (sc *SettingController) updateNodeSelector() error {
 		return nil
 	}
 
-	detached, _, err := sc.ds.AreAllVolumesDetached(longhorn.DataEngineTypeAll)
+	detached, err := sc.ds.AreAllVolumesDetachedState()
 	if err != nil {
 		return errors.Wrapf(err, "failed to check volume detachment for %v setting update", types.SettingNameSystemManagedComponentsNodeSelector)
 	}
@@ -1258,12 +1258,12 @@ func (sc *SettingController) updateInstanceManagerCPURequest(dataEngine longhorn
 		return nil
 	}
 
-	detached, _, err := sc.ds.AreAllVolumesDetached(dataEngine)
+	stopped, _, err := sc.ds.AreAllEngineInstancesStopped(dataEngine)
 	if err != nil {
-		return errors.Wrapf(err, "failed to check volume detachment for %v setting update", settingName)
+		return errors.Wrapf(err, "failed to check engine instances for %v setting update", settingName)
 	}
-	if !detached {
-		return &types.ErrorInvalidState{Reason: fmt.Sprintf("failed to apply %v setting to Longhorn components when there are attached volumes. It will be eventually applied", settingName)}
+	if !stopped {
+		return &types.ErrorInvalidState{Reason: fmt.Sprintf("failed to apply %v setting to Longhorn components when there are running engine instances. It will be eventually applied", settingName)}
 	}
 
 	for _, pod := range notUpdatedPods {

diff --git a/datastore/longhorn.go b/datastore/longhorn.go
@@ -394,13 +394,13 @@ func (s *DataStore) ValidateSetting(name, value string) (err error) {
 
 func (s *DataStore) ValidateV1DataEngineEnabled(dataEngineEnabled bool) (ims []*longhorn.InstanceManager, err error) {
 	if !dataEngineEnabled {
-		allVolumesDetached, _ims, err := s.AreAllVolumesDetached(longhorn.DataEngineTypeV1)
+		allV1VolumesDetached, _ims, err := s.AreAllEngineInstancesStopped(longhorn.DataEngineTypeV1)
 		if err != nil {
 			return nil, errors.Wrapf(err, "failed to check volume detachment for %v setting update", types.SettingNameV1DataEngine)
 		}
 		ims = _ims
 
-		if !allVolumesDetached {
+		if !allV1VolumesDetached {
 			return nil, &types.ErrorInvalidState{Reason: fmt.Sprintf("cannot apply %v setting to Longhorn workloads when there are attached v1 volumes", types.SettingNameV1DataEngine)}
 		}
 	}
@@ -410,13 +410,13 @@ func (s *DataStore) ValidateV1DataEngineEnabled(dataEngineEnabled bool) (ims []*
 
 func (s *DataStore) ValidateV2DataEngineEnabled(dataEngineEnabled bool) (ims []*longhorn.InstanceManager, err error) {
 	if !dataEngineEnabled {
-		allVolumesDetached, _ims, err := s.AreAllVolumesDetached(longhorn.DataEngineTypeV2)
+		allV2VolumesDetached, _ims, err := s.AreAllEngineInstancesStopped(longhorn.DataEngineTypeV2)
 		if err != nil {
 			return nil, errors.Wrapf(err, "failed to check volume detachment for %v setting update", types.SettingNameV2DataEngine)
 		}
 		ims = _ims
 
-		if !allVolumesDetached {
+		if !allV2VolumesDetached {
 			return nil, &types.ErrorInvalidState{Reason: fmt.Sprintf("cannot apply %v setting to Longhorn workloads when there are attached v2 volumes", types.SettingNameV2DataEngine)}
 		}
 	}
@@ -534,7 +534,7 @@ func (s *DataStore) AreAllRWXVolumesDetached() (bool, error) {
 	return true, nil
 }
 
-func (s *DataStore) AreAllVolumesDetached(dataEngine longhorn.DataEngineType) (bool, []*longhorn.InstanceManager, error) {
+func (s *DataStore) AreAllEngineInstancesStopped(dataEngine longhorn.DataEngineType) (bool, []*longhorn.InstanceManager, error) {
 	var ims []*longhorn.InstanceManager
 
 	nodes, err := s.ListNodes()

diff --git a/webhook/resources/systemrestore/validator.go b/webhook/resources/systemrestore/validator.go
@@ -42,7 +42,7 @@ func (v *systemRestoreValidator) Create(request *admission.Request, newObj runti
 		return werror.NewInvalidError(fmt.Sprintf("%v is not a *longhorn.SystemRestore", newObj), "")
 	}
 
-	areAllVolumesDetached, _, err := v.ds.AreAllVolumesDetached(longhorn.DataEngineTypeAll)
+	areAllVolumesDetached, err := v.ds.AreAllVolumesDetachedState()
 	if err != nil {
 		return werror.NewInvalidError(err.Error(), "")
 	}