diff --git a/controller/monitor/disk_monitor.go b/controller/monitor/disk_monitor.go index 67e7e0fb4c..2bad193fec 100644 --- a/controller/monitor/disk_monitor.go +++ b/controller/monitor/disk_monitor.go @@ -141,15 +141,22 @@ func (m *NodeMonitor) run(value interface{}) error { } func (m *NodeMonitor) getBackendStoreDrivers() []longhorn.BackendStoreDriverType { - backendStoreDrivers := []longhorn.BackendStoreDriverType{longhorn.BackendStoreDriverTypeV1} + backendStoreDrivers := []longhorn.BackendStoreDriverType{} - v2DataEngineEnabled, err := m.ds.GetSettingAsBool(types.SettingNameV2DataEngine) - if err != nil { - m.logger.WithError(err).Errorf("Failed to get setting %v", types.SettingNameV2DataEngine) - return backendStoreDrivers - } - if v2DataEngineEnabled { - backendStoreDrivers = append(backendStoreDrivers, longhorn.BackendStoreDriverTypeV2) + for _, setting := range []types.SettingName{types.SettingNameV1DataEngine, types.SettingNameV2DataEngine} { + dataEngineEnabled, err := m.ds.GetSettingAsBool(setting) + if err != nil { + m.logger.WithError(err).Warnf("Failed to get setting %v", setting) + continue + } + if dataEngineEnabled { + switch setting { + case types.SettingNameV1DataEngine: + backendStoreDrivers = append(backendStoreDrivers, longhorn.BackendStoreDriverTypeV1) + case types.SettingNameV2DataEngine: + backendStoreDrivers = append(backendStoreDrivers, longhorn.BackendStoreDriverTypeV2) + } + } } return backendStoreDrivers diff --git a/controller/node_controller.go b/controller/node_controller.go index 088bbdb72b..748cfaefdc 100644 --- a/controller/node_controller.go +++ b/controller/node_controller.go @@ -978,41 +978,39 @@ func (nc *NodeController) syncNodeStatus(pod *corev1.Pod, node *longhorn.Node) e func (nc *NodeController) getImTypeBackendStoreDrivers(node *longhorn.Node) map[longhorn.InstanceManagerType][]longhorn.BackendStoreDriverType { log := getLoggerForNode(nc.logger, node) - // Check if v1 data engine is enabled + // TODO: remove InstanceManagerTypeEngine and InstanceManagerTypeReplica in the future. backendStoreDrivers := map[longhorn.InstanceManagerType][]longhorn.BackendStoreDriverType{ - longhorn.InstanceManagerTypeAllInOne: { - longhorn.BackendStoreDriverTypeV1, - }, - longhorn.InstanceManagerTypeEngine: { - longhorn.BackendStoreDriverTypeV1, - }, - } - if len(node.Spec.Disks) != 0 { - backendStoreDrivers[longhorn.InstanceManagerTypeReplica] = []longhorn.BackendStoreDriverType{ - longhorn.BackendStoreDriverTypeV1, - } - } - - // Check if v2 data engine is enabled - v2DataEngineEnabled, err := nc.ds.GetSettingAsBool(types.SettingNameV2DataEngine) - if err != nil { - log.WithError(err).Warnf("Failed to get %v setting", types.SettingNameV2DataEngine) - return backendStoreDrivers + longhorn.InstanceManagerTypeAllInOne: {}, + longhorn.InstanceManagerTypeEngine: {}, + longhorn.InstanceManagerTypeReplica: {}, } - if !v2DataEngineEnabled { - return backendStoreDrivers - } + for _, setting := range []types.SettingName{types.SettingNameV1DataEngine, types.SettingNameV2DataEngine} { + enabled, err := nc.ds.GetSettingAsBool(setting) + if err != nil { + log.WithError(err).Warnf("Failed to get %v setting", setting) + continue + } + if !enabled { + continue + } - err = nc.ds.ValidateV2DataEngine(v2DataEngineEnabled) - if err != nil { - log.WithError(err).Warnf("Failed to validate %v setting", types.SettingNameV2DataEngine) - return backendStoreDrivers + switch setting { + case types.SettingNameV1DataEngine: + backendStoreDrivers[longhorn.InstanceManagerTypeAllInOne] = append(backendStoreDrivers[longhorn.InstanceManagerTypeAllInOne], longhorn.BackendStoreDriverTypeV1) + backendStoreDrivers[longhorn.InstanceManagerTypeEngine] = append(backendStoreDrivers[longhorn.InstanceManagerTypeEngine], longhorn.BackendStoreDriverTypeV1) + if len(node.Spec.Disks) != 0 { + backendStoreDrivers[longhorn.InstanceManagerTypeReplica] = append(backendStoreDrivers[longhorn.InstanceManagerTypeReplica], longhorn.BackendStoreDriverTypeV1) + } + case types.SettingNameV2DataEngine: + if err := nc.ds.ValidateV2DataEngineEnabled(enabled); err == nil { + backendStoreDrivers[longhorn.InstanceManagerTypeAllInOne] = append(backendStoreDrivers[longhorn.InstanceManagerTypeAllInOne], longhorn.BackendStoreDriverTypeV2) + } else { + log.WithError(err).Warnf("Failed to validate %v setting", types.SettingNameV2DataEngine) + } + } } - backendStoreDrivers[longhorn.InstanceManagerTypeAllInOne] = - append(backendStoreDrivers[longhorn.InstanceManagerTypeAllInOne], longhorn.BackendStoreDriverTypeV2) - return backendStoreDrivers } @@ -1060,7 +1058,7 @@ func (nc *NodeController) syncInstanceManagers(node *longhorn.Node) error { } for _, im := range imMap { if im.Labels[types.GetLonghornLabelKey(types.LonghornLabelNode)] != im.Spec.NodeID { - return fmt.Errorf("instance manager %v NodeID %v is not consistent with the label %v=%v", + return fmt.Errorf("instance manager %v nodeID %v is not consistent with the label %v=%v", im.Name, im.Spec.NodeID, types.GetLonghornLabelKey(types.LonghornLabelNode), im.Labels[types.GetLonghornLabelKey(types.LonghornLabelNode)]) } cleanupRequired := true diff --git a/controller/setting_controller.go b/controller/setting_controller.go index a0be3f0a65..78e759f067 100644 --- a/controller/setting_controller.go +++ b/controller/setting_controller.go @@ -259,8 +259,8 @@ func (sc *SettingController) syncSetting(key string) (err error) { if err := sc.updateLogLevel(); err != nil { return err } - case string(types.SettingNameV2DataEngine): - if err := sc.updateV2DataEngine(); err != nil { + case string(types.SettingNameV1DataEngine), string(types.SettingNameV2DataEngine): + if err := sc.updateDataEngine(types.SettingName(name)); err != nil { return err } } @@ -752,7 +752,7 @@ func (sc *SettingController) updateCNI() error { return err } - volumesDetached, err := sc.ds.AreAllVolumesDetached() + volumesDetached, err := sc.ds.AreAllVolumesDetached(longhorn.BackendStoreDriverTypeAll) if err != nil { return errors.Wrapf(err, "failed to check volume detachment for %v setting update", types.SettingNameStorageNetwork) } @@ -804,36 +804,49 @@ func (sc *SettingController) updateLogLevel() error { return nil } -func (sc *SettingController) updateV2DataEngine() error { - v2DataEngineEnabled, err := sc.ds.GetSettingAsBool(types.SettingNameV2DataEngine) +func (sc *SettingController) updateDataEngine(setting types.SettingName) error { + enabled, err := sc.ds.GetSettingAsBool(setting) if err != nil { - return err + return errors.Wrapf(err, "failed to get %v setting for updating data engine", setting) } - if err := sc.ds.ValidateV2DataEngine(v2DataEngineEnabled); err != nil { - return err + var backednStoreDriver longhorn.BackendStoreDriverType + switch setting { + case types.SettingNameV1DataEngine: + backednStoreDriver = longhorn.BackendStoreDriverTypeV1 + if err := sc.ds.ValidateV1DataEngineEnabled(enabled); err != nil { + return err + } + case types.SettingNameV2DataEngine: + backednStoreDriver = longhorn.BackendStoreDriverTypeV2 + if err := sc.ds.ValidateV2DataEngineEnabled(enabled); err != nil { + return err + } + default: + return fmt.Errorf("unknown setting %v for updating data engine", setting) } - if !v2DataEngineEnabled { - return sc.cleanupInstanceManagerForV2DataEngine() + if !enabled { + return sc.cleanupInstanceManager(backednStoreDriver) } return nil } -func (sc *SettingController) cleanupInstanceManagerForV2DataEngine() error { - imMap, err := sc.ds.ListInstanceManagersBySelectorRO("", "", longhorn.InstanceManagerTypeAllInOne, longhorn.BackendStoreDriverTypeV2) +func (sc *SettingController) cleanupInstanceManager(backendStoreDriver longhorn.BackendStoreDriverType) error { + sc.logger.Infof("Cleaning up the instance manager for %v data engine", backendStoreDriver) + imMap, err := sc.ds.ListInstanceManagersBySelectorRO("", "", longhorn.InstanceManagerTypeAllInOne, backendStoreDriver) if err != nil { - return err + return errors.Wrapf(err, "failed to list instance managers for cleaning up %v data engine", backendStoreDriver) } for _, im := range imMap { if len(im.Status.InstanceEngines) != 0 || len(im.Status.InstanceReplicas) != 0 || len(im.Status.Instances) != 0 { - sc.logger.Infof("Skipping cleaning up the instance manager %v for v2 data engine since there are still instances running on it", im.Name) + sc.logger.Infof("Skipping cleaning up the instance manager %v for %v data engine since there are still instances running on it", im.Name, backendStoreDriver) continue } - sc.logger.Infof("Cleaning up the instance manager %v for v2 data engine", im.Name) + sc.logger.Infof("Cleaning up the instance manager %v for %v data engine", im.Name, backendStoreDriver) if err := sc.ds.DeleteInstanceManager(im.Name); err != nil { return err } @@ -1473,6 +1486,7 @@ func (info *ClusterInfo) collectSettings() error { types.SettingNameStorageReservedPercentageForDefaultDisk: true, types.SettingNameSupportBundleFailedHistoryLimit: true, types.SettingNameSystemManagedPodsImagePullPolicy: true, + types.SettingNameV1DataEngine: true, types.SettingNameV2DataEngine: true, types.SettingNameV2DataEngineGuaranteedInstanceManagerCPU: true, types.SettingNameOfflineReplicaRebuilding: true, diff --git a/datastore/longhorn.go b/datastore/longhorn.go index b487289279..ef27fd3785 100644 --- a/datastore/longhorn.go +++ b/datastore/longhorn.go @@ -358,24 +358,39 @@ func (s *DataStore) ValidateSetting(name, value string) (err error) { return err } case types.SettingNameStorageNetwork: - volumesDetached, err := s.AreAllVolumesDetached() + volumesDetached, err := s.AreAllVolumesDetached(longhorn.BackendStoreDriverTypeAll) if err != nil { return errors.Wrapf(err, "failed to check volume detachment for %v setting update", name) } if !volumesDetached { return &types.ErrorInvalidState{Reason: fmt.Sprintf("cannot apply %v setting to Longhorn workloads when there are attached volumes", name)} } + case types.SettingNameV1DataEngine: + old, err := s.GetSettingWithAutoFillingRO(types.SettingNameV1DataEngine) + if err != nil { + return err + } + if old.Value != value { + dataEngineEnabled, err := strconv.ParseBool(value) + if err != nil { + return err + } + err = s.ValidateV1DataEngineEnabled(dataEngineEnabled) + if err != nil { + return err + } + } case types.SettingNameV2DataEngine: old, err := s.GetSettingWithAutoFillingRO(types.SettingNameV2DataEngine) if err != nil { return err } if old.Value != value { - v2DataEngineEnabled, err := strconv.ParseBool(value) + dataEngineEnabled, err := strconv.ParseBool(value) if err != nil { return err } - err = s.ValidateV2DataEngine(v2DataEngineEnabled) + err = s.ValidateV2DataEngineEnabled(dataEngineEnabled) if err != nil { return err } @@ -400,21 +415,35 @@ func (s *DataStore) ValidateSetting(name, value string) (err error) { return nil } -func (s *DataStore) ValidateV2DataEngine(v2DataEngineEnabled bool) error { - if !v2DataEngineEnabled { - allV2VolumesDetached, err := s.AreAllV2VolumesDetached() +func (s *DataStore) ValidateV1DataEngineEnabled(dataEngineEnabled bool) error { + if !dataEngineEnabled { + allVolumesDetached, err := s.AreAllVolumesDetached(longhorn.BackendStoreDriverTypeV1) + if err != nil { + return errors.Wrapf(err, "failed to check volume detachment for %v setting update", types.SettingNameV1DataEngine) + } + if !allVolumesDetached { + return &types.ErrorInvalidState{Reason: fmt.Sprintf("cannot apply %v setting to Longhorn workloads when there are attached v1 volumes", types.SettingNameV1DataEngine)} + } + } + + return nil +} + +func (s *DataStore) ValidateV2DataEngineEnabled(dataEngineEnabled bool) error { + if !dataEngineEnabled { + allVolumesDetached, err := s.AreAllVolumesDetached(longhorn.BackendStoreDriverTypeV2) if err != nil { return errors.Wrapf(err, "failed to check volume detachment for %v setting update", types.SettingNameV2DataEngine) } - if !allV2VolumesDetached { - return &types.ErrorInvalidState{Reason: fmt.Sprintf("cannot apply %v setting to Longhorn workloads when there are attached volumes", types.SettingNameV2DataEngine)} + if !allVolumesDetached { + return &types.ErrorInvalidState{Reason: fmt.Sprintf("cannot apply %v setting to Longhorn workloads when there are attached v2 volumes", types.SettingNameV2DataEngine)} } - allBlockTypeDisksRemoved, err := s.AreAllBlockTypeDisksRemoved() + allDisksRemoved, err := s.AreAllDisksRemovedByDiskType(longhorn.DiskTypeBlock) if err != nil { return errors.Wrapf(err, "failed to check block-type disk removal for %v setting update", types.SettingNameV2DataEngine) } - if !allBlockTypeDisksRemoved { + if !allDisksRemoved { return &types.ErrorInvalidState{Reason: fmt.Sprintf("cannot apply %v setting to Longhorn workloads when there are block-type disks", types.SettingNameV2DataEngine)} } return nil @@ -441,7 +470,7 @@ func (s *DataStore) ValidateV2DataEngine(v2DataEngineEnabled bool) error { continue } - if v2DataEngineEnabled { + if dataEngineEnabled { capacity, ok := node.Status.Capacity["hugepages-2Mi"] if !ok { return errors.Errorf("failed to get hugepages-2Mi capacity for node %v", node.Name) @@ -469,19 +498,19 @@ func (s *DataStore) ValidateV2DataEngine(v2DataEngineEnabled bool) error { return nil } -func (s *DataStore) AreAllVolumesDetached() (bool, error) { +func (s *DataStore) AreAllVolumesDetached(backendStoreDriver longhorn.BackendStoreDriverType) (bool, error) { nodes, err := s.ListNodes() if err != nil { return false, err } for node := range nodes { - engineInstanceManagers, err := s.ListInstanceManagersBySelectorRO(node, "", longhorn.InstanceManagerTypeEngine, longhorn.BackendStoreDriverTypeV1) + + engineInstanceManagers, err := s.ListInstanceManagersBySelectorRO(node, "", longhorn.InstanceManagerTypeEngine, backendStoreDriver) if err != nil && !ErrorIsNotFound(err) { return false, err } - - aioInstanceManagers, err := s.ListInstanceManagersBySelectorRO(node, "", longhorn.InstanceManagerTypeAllInOne, "") + aioInstanceManagers, err := s.ListInstanceManagersBySelectorRO(node, "", longhorn.InstanceManagerTypeAllInOne, backendStoreDriver) if err != nil && !ErrorIsNotFound(err) { return false, err } @@ -493,33 +522,10 @@ func (s *DataStore) AreAllVolumesDetached() (bool, error) { } } } - - return true, nil -} - -func (s *DataStore) AreAllV2VolumesDetached() (bool, error) { - nodes, err := s.ListNodesRO() - if err != nil { - return false, err - } - - for _, node := range nodes { - aioInstanceManagers, err := s.ListInstanceManagersBySelectorRO(node.Name, "", longhorn.InstanceManagerTypeAllInOne, longhorn.BackendStoreDriverTypeV2) - if err != nil && !ErrorIsNotFound(err) { - return false, err - } - - for _, instanceManager := range aioInstanceManagers { - if len(instanceManager.Status.InstanceEngines)+len(instanceManager.Status.Instances) > 0 { - return false, nil - } - } - } - return true, nil } -func (s *DataStore) AreAllBlockTypeDisksRemoved() (bool, error) { +func (s *DataStore) AreAllDisksRemovedByDiskType(diskType longhorn.DiskType) (bool, error) { nodes, err := s.ListNodesRO() if err != nil { return false, err @@ -527,7 +533,7 @@ func (s *DataStore) AreAllBlockTypeDisksRemoved() (bool, error) { for _, node := range nodes { for _, disk := range node.Spec.Disks { - if disk.Type == longhorn.DiskTypeBlock { + if disk.Type == diskType { return false, nil } } @@ -1747,11 +1753,9 @@ func (s *DataStore) CheckEngineImageCompatiblityByImage(image string) error { // CheckEngineImageReadiness return true if the engine IMAGE is deployed on all nodes in the NODES list func (s *DataStore) CheckEngineImageReadiness(image string, nodes ...string) (isReady bool, err error) { if len(nodes) == 0 { - logrus.Warnf("CheckEngineImageReadiness: no nodes specified") return false, nil } if len(nodes) == 1 && nodes[0] == "" { - logrus.Warnf("CheckEngineImageReadiness: only one node specified and it's empty") return false, nil } ei, err := s.GetEngineImageRO(types.GetEngineImageChecksumName(image)) diff --git a/k8s/pkg/apis/longhorn/v1beta2/volume.go b/k8s/pkg/apis/longhorn/v1beta2/volume.go index 519f66f897..5e210b0d47 100644 --- a/k8s/pkg/apis/longhorn/v1beta2/volume.go +++ b/k8s/pkg/apis/longhorn/v1beta2/volume.go @@ -163,8 +163,9 @@ const ( type BackendStoreDriverType string const ( - BackendStoreDriverTypeV1 = BackendStoreDriverType("v1") - BackendStoreDriverTypeV2 = BackendStoreDriverType("v2") + BackendStoreDriverTypeV1 = BackendStoreDriverType("v1") + BackendStoreDriverTypeV2 = BackendStoreDriverType("v2") + BackendStoreDriverTypeAll = BackendStoreDriverType("all") ) type OfflineReplicaRebuilding string diff --git a/types/setting.go b/types/setting.go index dc061f5c20..6d25ba2a3e 100644 --- a/types/setting.go +++ b/types/setting.go @@ -887,7 +887,7 @@ var ( SettingDefinitionGuaranteedInstanceManagerCPU = SettingDefinition{ DisplayName: "Guaranteed Instance Manager CPU for V1 Data Engine", - Description: "This integer value indicates how many percentage of the total allocatable CPU on each node will be reserved for each instance manager Pod for v1 data engine. For example, 10 means 10% of the total CPU on a node will be allocated to each instance manager pod on this node. This will help maintain engine and replica stability during high node workload. \n\n" + + Description: "Percentage of the total allocatable CPU resources on each node to be reserved for each instance manager pod when the V1 Data Engine is enabled. For example, 10 means 10% of the total CPU on a node will be allocated to each instance manager pod on this node. This will help maintain engine and replica stability during high node workload. \n\n" + "In order to prevent unexpected volume instance (engine/replica) crash as well as guarantee a relative acceptable IO performance, you can use the following formula to calculate a value for this setting: \n\n" + "`Guaranteed Instance Manager CPU = The estimated max Longhorn volume engine and replica count on a node * 0.1 / The total allocatable CPUs on the node * 100` \n\n" + "The result of above calculation doesn't mean that's the maximum CPU resources the Longhorn workloads require. To fully exploit the Longhorn volume I/O performance, you can allocate/guarantee more CPU resources via this setting. \n\n" + @@ -907,7 +907,7 @@ var ( SettingDefinitionKubernetesClusterAutoscalerEnabled = SettingDefinition{ DisplayName: "Kubernetes Cluster Autoscaler Enabled (Experimental)", - Description: "Enabling this setting will notify Longhorn that the cluster is using Kubernetes Cluster Autoscaler. \n\n" + + Description: "Setting that notifies Longhorn that the cluster is using the Kubernetes Cluster Autoscaler. \n\n" + "Longhorn prevents data loss by only allowing the Cluster Autoscaler to scale down a node that met all conditions: \n\n" + " - No volume attached to the node \n\n" + " - Is not the last node containing the replica of any volume. \n\n" + @@ -1149,7 +1149,7 @@ var ( SettingDefinitionV1DataEngine = SettingDefinition{ DisplayName: "V1 Data Engine", - Description: "This setting allows users to activate v1 data engine.\n\n" + + Description: "Setting that allows you to enable the V1 Data Engine. \n\n" + " - DO NOT CHANGE THIS SETTING WITH ATTACHED VOLUMES. Longhorn will block this setting update when there are attached volumes. \n\n", Category: SettingCategoryGeneral, Type: SettingTypeBool, @@ -1183,7 +1183,7 @@ var ( SettingDefinitionV2DataEngineGuaranteedInstanceManagerCPU = SettingDefinition{ DisplayName: "Guaranteed Instance Manager CPU for V2 Data Engine", - Description: "This integer value indicates how many millicpus on each node are reserved for each instance manager Pod for v2 data engine. By default, the SPDK target daemon within an instance manager Pod utilizes 1 CPU core. Ensuring a minimum CPU usage is essential for sustaining engine and replica stability, especially during periods of high node workload. \n\n" + + Description: "Number of millicpus on each node to be reserved for each instance manager pod when the V2 Data Engine is enabled. By default, the Storage Performance Development Kit (SPDK) target daemon within each instance manager pod uses 1 CPU core. Configuring a minimum CPU usage value is essential for maintaining engine and replica stability, especially during periods of high node workload. \n\n" + "WARNING: \n\n" + " - Value 0 means unsetting CPU requests for instance manager pods for v2 data engine. \n\n" + " - This integer value is range from 1000 to 8000. \n\n" + diff --git a/types/types.go b/types/types.go index 572815f4c0..9c32204120 100644 --- a/types/types.go +++ b/types/types.go @@ -432,7 +432,7 @@ func GetInstanceManagerLabels(node, imImage string, imType longhorn.InstanceMana if imImage != "" { labels[GetLonghornLabelKey(LonghornLabelInstanceManagerImage)] = GetInstanceManagerImageChecksumName(GetImageCanonicalName(imImage)) } - if backendStoreDriver != "" { + if backendStoreDriver != "" && backendStoreDriver != longhorn.BackendStoreDriverTypeAll { labels[GetLonghornLabelKey(LonghornLabelBackendStoreDriver)] = string(backendStoreDriver) } diff --git a/webhook/common/common.go b/webhook/common/common.go index b52646e8ed..6feafd5473 100644 --- a/webhook/common/common.go +++ b/webhook/common/common.go @@ -13,6 +13,7 @@ import ( "github.com/longhorn/longhorn-manager/util" longhorn "github.com/longhorn/longhorn-manager/k8s/pkg/apis/longhorn/v1beta2" + werror "github.com/longhorn/longhorn-manager/webhook/error" ) var ( @@ -89,3 +90,21 @@ func GetLonghornLabelsPatchOp(obj runtime.Object, requiredLabels, removingLabels return fmt.Sprintf(`{"op": "replace", "path": "/metadata/labels", "value": %v}`, string(bytes)), nil } + +func ValidateRequiredDataEngineEnabled(ds *datastore.DataStore, backendStoreDriver longhorn.BackendStoreDriverType) error { + dataEngineSetting := types.SettingNameV1DataEngine + if datastore.IsBackendStoreDriverV2(backendStoreDriver) { + dataEngineSetting = types.SettingNameV2DataEngine + } + + enabled, err := ds.GetSettingAsBool(dataEngineSetting) + if err != nil { + err = errors.Wrapf(err, "failed to get %v setting", dataEngineSetting) + return werror.NewInternalError(err.Error()) + } + if !enabled { + err := fmt.Errorf("%v data engine is not enabled", dataEngineSetting) + return werror.NewForbiddenError(err.Error()) + } + return nil +} diff --git a/webhook/resources/engine/validator.go b/webhook/resources/engine/validator.go index c2ac0eb16f..b5e52a5864 100644 --- a/webhook/resources/engine/validator.go +++ b/webhook/resources/engine/validator.go @@ -10,10 +10,10 @@ import ( admissionregv1 "k8s.io/api/admissionregistration/v1" "github.com/longhorn/longhorn-manager/datastore" - "github.com/longhorn/longhorn-manager/types" "github.com/longhorn/longhorn-manager/webhook/admission" longhorn "github.com/longhorn/longhorn-manager/k8s/pkg/apis/longhorn/v1beta2" + wcommon "github.com/longhorn/longhorn-manager/webhook/common" werror "github.com/longhorn/longhorn-manager/webhook/error" ) @@ -55,15 +55,9 @@ func (e *engineValidator) Create(request *admission.Request, newObj runtime.Obje return werror.NewInternalError(err.Error()) } - if datastore.IsBackendStoreDriverV2(engine.Spec.BackendStoreDriver) { - v2DataEngineEnabled, err := e.ds.GetSettingAsBool(types.SettingNameV2DataEngine) - if err != nil { - err = errors.Wrapf(err, "failed to get spdk setting") - return werror.NewInvalidError(err.Error(), "") - } - if !v2DataEngineEnabled { - return werror.NewInvalidError("v2 data engine is not enabled", "") - } + err = wcommon.ValidateRequiredDataEngineEnabled(e.ds, engine.Spec.BackendStoreDriver) + if err != nil { + return err } if err := e.ds.CheckEngineImageCompatiblityByImage(engine.Spec.Image); err != nil { diff --git a/webhook/resources/replica/validator.go b/webhook/resources/replica/validator.go index cf277981a7..caec7e961f 100644 --- a/webhook/resources/replica/validator.go +++ b/webhook/resources/replica/validator.go @@ -10,10 +10,10 @@ import ( admissionregv1 "k8s.io/api/admissionregistration/v1" "github.com/longhorn/longhorn-manager/datastore" - "github.com/longhorn/longhorn-manager/types" "github.com/longhorn/longhorn-manager/webhook/admission" longhorn "github.com/longhorn/longhorn-manager/k8s/pkg/apis/longhorn/v1beta2" + wcommon "github.com/longhorn/longhorn-manager/webhook/common" werror "github.com/longhorn/longhorn-manager/webhook/error" ) @@ -46,15 +46,10 @@ func (r *replicaValidator) Create(request *admission.Request, newObj runtime.Obj if !ok { return werror.NewInvalidError(fmt.Sprintf("%v is not a *longhorn.Replica", newObj), "") } - if datastore.IsBackendStoreDriverV2(replica.Spec.BackendStoreDriver) { - v2DataEngineEnabled, err := r.ds.GetSettingAsBool(types.SettingNameV2DataEngine) - if err != nil { - err = errors.Wrapf(err, "failed to get spdk setting") - return werror.NewInvalidError(err.Error(), "") - } - if !v2DataEngineEnabled { - return werror.NewInvalidError("v2 data engine is not enabled", "") - } + + err := wcommon.ValidateRequiredDataEngineEnabled(r.ds, replica.Spec.BackendStoreDriver) + if err != nil { + return err } if err := r.ds.CheckEngineImageCompatiblityByImage(replica.Spec.Image); err != nil { @@ -88,7 +83,10 @@ func (r *replicaValidator) Update(request *admission.Request, oldObj runtime.Obj } func (r *replicaValidator) Delete(request *admission.Request, oldObj runtime.Object) error { - replica := oldObj.(*longhorn.Replica) + replica, ok := oldObj.(*longhorn.Replica) + if !ok { + return werror.NewInvalidError(fmt.Sprintf("%v is not a *longhorn.Replica", oldObj), "") + } if err := r.validateReplicaDeletion(replica); err != nil { return werror.NewInvalidError(err.Error(), "") diff --git a/webhook/resources/systemrestore/validator.go b/webhook/resources/systemrestore/validator.go index 0148fb6623..0c47420a4e 100644 --- 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() + areAllVolumesDetached, err := v.ds.AreAllVolumesDetached(longhorn.BackendStoreDriverTypeAll) if err != nil { return werror.NewInvalidError(err.Error(), "") } diff --git a/webhook/resources/volume/validator.go b/webhook/resources/volume/validator.go index 9caa760324..2a153ec7b6 100644 --- a/webhook/resources/volume/validator.go +++ b/webhook/resources/volume/validator.go @@ -19,6 +19,7 @@ import ( "github.com/longhorn/longhorn-manager/webhook/admission" longhorn "github.com/longhorn/longhorn-manager/k8s/pkg/apis/longhorn/v1beta2" + wcommon "github.com/longhorn/longhorn-manager/webhook/common" werror "github.com/longhorn/longhorn-manager/webhook/error" ) @@ -126,19 +127,9 @@ func (v *volumeValidator) Create(request *admission.Request, newObj runtime.Obje return werror.NewInvalidError(err.Error(), "") } - if datastore.IsBackendStoreDriverV2(volume.Spec.BackendStoreDriver) { - v2DataEngineEnabled, err := v.ds.GetSettingAsBool(types.SettingNameV2DataEngine) - if err != nil { - err = errors.Wrapf(err, "failed to get spdk setting") - return werror.NewInvalidError(err.Error(), "") - } - if !v2DataEngineEnabled { - return werror.NewInvalidError("v2 data engine is not enabled", "") - } - - if volume.Spec.Frontend == longhorn.VolumeFrontendISCSI { - return werror.NewInvalidError("v2 data engine does not support iSCSI frontend", "") - } + err := wcommon.ValidateRequiredDataEngineEnabled(v.ds, volume.Spec.BackendStoreDriver) + if err != nil { + return err } // TODO: remove this check when we support the following features for SPDK volumes