Merge pull request #2317 from Nikhil-Ladha/add-pinning-support-to-svg

set pinning field in CephSubvolumeGroup CR
red-hat-storage · Dec 13, 2023 · e8d575d · e8d575d
2 parents 000607b + 2df5c6e
commit e8d575d
Show file tree

Hide file tree

Showing 495 changed files with 11,528 additions and 5,831 deletions.
diff --git a/.github/workflows/ocs-operator-ci.yaml b/.github/workflows/ocs-operator-ci.yaml
@@ -37,7 +37,7 @@ jobs:
 
       - uses: golangci/golangci-lint-action@v3
         with:
-          version: v1.51.1
+          version: v1.51.2
 
           # The weird NO_FUTURE thing is a workaround suggested here:
           # # https://github.com/golangci/golangci-lint-action/issues/119#issuecomment-981090648

diff --git a/config/crd/bases/ocs.openshift.io_storageclusters.yaml b/config/crd/bases/ocs.openshift.io_storageclusters.yaml
@@ -1365,8 +1365,7 @@ spec:
                         properties:
                           enabled:
                             description: Whether to compress the data in transit across
-                              the wire. The default is not set. Requires Ceph Quincy
-                              (v17) or newer.
+                              the wire. The default is not set.
                             type: boolean
                         type: object
                       encryption:
@@ -1430,6 +1429,10 @@ spec:
                     - multus
                     nullable: true
                     type: string
+                    x-kubernetes-validations:
+                    - message: network provider must be disabled (reverted to empty
+                        string) before a new provider is enabled
+                      rule: self == '' || self == oldSelf
                   selectors:
                     additionalProperties:
                       type: string
@@ -1456,6 +1459,11 @@ spec:
                     nullable: true
                     type: object
                 type: object
+                x-kubernetes-validations:
+                - message: at least one network selector must be specified when using
+                    multus
+                  rule: '!has(self.provider) || (self.provider != ''multus'' || (self.provider
+                    == ''multus'' && size(self.selectors) > 0))'
               nfs:
                 description: NFSSpec defines specific nfs configuration options
                 properties:

diff --git a/controllers/storagecluster/cephfilesystem.go b/controllers/storagecluster/cephfilesystem.go
@@ -164,22 +164,34 @@ func (obj *ocsCephFilesystems) ensureCreated(r *StorageClusterReconciler, instan
 }
 
 func (r *StorageClusterReconciler) createDefaultSubvolumeGroup(filesystemName, filesystemNamespace string, ownerReferences []metav1.OwnerReference) error {
-	// TODO: After fix of rook issue https://github.com/rook/rook/issues/13220 add svg name spec
 
 	existingsvg := &cephv1.CephFilesystemSubVolumeGroup{}
-	err := r.Client.Get(r.ctx, types.NamespacedName{Name: defaultSubvolumeGroupName, Namespace: filesystemNamespace}, existingsvg)
+	svgName := generateNameForCephSubvolumeGroup(filesystemName)
+	err := r.Client.Get(r.ctx, types.NamespacedName{Name: svgName, Namespace: filesystemNamespace}, existingsvg)
 	if err == nil {
 		if existingsvg.DeletionTimestamp != nil {
-			r.Log.Info("Unable to restore subvolumegroup because it is marked for deletion.", "subvolumegroup", klog.KRef(filesystemNamespace, defaultSubvolumeGroupName))
+			r.Log.Info("Unable to restore subvolumegroup because it is marked for deletion.", "subvolumegroup", klog.KRef(filesystemNamespace, existingsvg.Name))
 			return fmt.Errorf("failed to restore subvolumegroup %s because it is marked for deletion", existingsvg.Name)
 		}
 	}
 
-	objectMeta := metav1.ObjectMeta{Name: defaultSubvolumeGroupName, Namespace: filesystemNamespace, OwnerReferences: ownerReferences}
-	cephFilesystemSubVolumeGroup := &cephv1.CephFilesystemSubVolumeGroup{ObjectMeta: objectMeta}
+	cephFilesystemSubVolumeGroup := &cephv1.CephFilesystemSubVolumeGroup{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:            svgName,
+			Namespace:       filesystemNamespace,
+			OwnerReferences: ownerReferences,
+		},
+	}
+
+	// Default value of "distributed" option for pinning in the CephFilesystemSubVolumeGroup CR
+	defaultPinningValue := 1
 	mutateFn := func() error {
 		cephFilesystemSubVolumeGroup.Spec = cephv1.CephFilesystemSubVolumeGroupSpec{
+			Name:           defaultSubvolumeGroupName,
 			FilesystemName: filesystemName,
+			Pinning: cephv1.CephFilesystemSubVolumeGroupSpecPinning{
+				Distributed: &defaultPinningValue,
+			},
 		}
 		return nil
 	}
@@ -193,13 +205,14 @@ func (r *StorageClusterReconciler) createDefaultSubvolumeGroup(filesystemName, f
 
 func (r *StorageClusterReconciler) deleteDefaultSubvolumeGroup(filesystemName, filesystemNamespace string, ownerReferences []metav1.OwnerReference) error {
 	existingsvg := &cephv1.CephFilesystemSubVolumeGroup{}
-	err := r.Client.Get(r.ctx, types.NamespacedName{Name: defaultSubvolumeGroupName, Namespace: filesystemNamespace}, existingsvg)
+	svgName := generateNameForCephSubvolumeGroup(filesystemName)
+	err := r.Client.Get(r.ctx, types.NamespacedName{Name: svgName, Namespace: filesystemNamespace}, existingsvg)
 	if err != nil {
 		if errors.IsNotFound(err) {
-			r.Log.Info("Uninstall: csi subvolumegroup not found.", "Subvolumegroup", klog.KRef(filesystemNamespace, defaultSubvolumeGroupName))
+			r.Log.Info("Uninstall: csi subvolumegroup not found.", "Subvolumegroup", klog.KRef(filesystemNamespace, svgName))
 			return nil
 		}
-		r.Log.Error(err, "Uninstall: Unable to retrieve subvolumegroup.", "subvolumegroup", klog.KRef(filesystemNamespace, defaultSubvolumeGroupName))
+		r.Log.Error(err, "Uninstall: Unable to retrieve subvolumegroup.", "subvolumegroup", klog.KRef(filesystemNamespace, svgName))
 		return fmt.Errorf("uninstall: Unable to retrieve csi subvolumegroup : %v", err)
 	}
 
@@ -212,14 +225,14 @@ func (r *StorageClusterReconciler) deleteDefaultSubvolumeGroup(filesystemName, f
 		}
 	}
 
-	err = r.Client.Get(r.ctx, types.NamespacedName{Name: defaultSubvolumeGroupName, Namespace: filesystemNamespace}, existingsvg)
+	err = r.Client.Get(r.ctx, types.NamespacedName{Name: svgName, Namespace: filesystemNamespace}, existingsvg)
 	if err != nil {
 		if errors.IsNotFound(err) {
-			r.Log.Info("Uninstall: subvolumegroup is deleted.", "subvolumegroup", klog.KRef(filesystemNamespace, defaultSubvolumeGroupName))
+			r.Log.Info("Uninstall: subvolumegroup is deleted.", "subvolumegroup", klog.KRef(filesystemNamespace, existingsvg.Name))
 			return nil
 		}
 	}
-	r.Log.Error(err, "Uninstall: Waiting for subvolumegroup to be deleted.", "subvolumegroup", klog.KRef(filesystemNamespace, defaultSubvolumeGroupName))
+	r.Log.Error(err, "Uninstall: Waiting for subvolumegroup to be deleted.", "subvolumegroup", klog.KRef(filesystemNamespace, existingsvg.Name))
 	return fmt.Errorf("uninstall: Waiting for subvolumegroup %v to be deleted", existingsvg.Name)
 }
 
@@ -245,9 +258,10 @@ func (obj *ocsCephFilesystems) ensureDeleted(r *StorageClusterReconciler, sc *oc
 		// delete csi subvolume group for particular filesystem
 		// skip for the ocs provider mode
 		if !sc.Spec.AllowRemoteStorageConsumers {
+			cephSVGName := generateNameForCephSubvolumeGroup(cephFilesystem.Name)
 			err = r.deleteDefaultSubvolumeGroup(cephFilesystem.Name, cephFilesystem.Namespace, cephFilesystem.ObjectMeta.OwnerReferences)
 			if err != nil {
-				r.Log.Error(err, "Uninstall: unable to delete subvolumegroup", "subvolumegroup", klog.KRef(cephFilesystem.Namespace, defaultSubvolumeGroupName))
+				r.Log.Error(err, "Uninstall: unable to delete subvolumegroup", "subvolumegroup", klog.KRef(cephFilesystem.Namespace, cephSVGName))
 				return reconcile.Result{}, err
 			}
 		}

diff --git a/controllers/storagecluster/cephfilesystem_test.go b/controllers/storagecluster/cephfilesystem_test.go
@@ -71,7 +71,8 @@ func TestCreateDefaultSubvolumeGroup(t *testing.T) {
 	assert.NoError(t, err)
 
 	svg := &cephv1.CephFilesystemSubVolumeGroup{}
-	err = reconciler.Client.Get(context.TODO(), types.NamespacedName{Name: defaultSubvolumeGroupName, Namespace: filesystem[0].Namespace}, svg)
+	expectedsvgName := generateNameForCephSubvolumeGroup(filesystem[0].Name)
+	err = reconciler.Client.Get(context.TODO(), types.NamespacedName{Name: expectedsvgName, Namespace: filesystem[0].Namespace}, svg)
 	assert.NoError(t, err) // no error
 }
 
@@ -86,7 +87,8 @@ func TestDeleteDefaultSubvolumeGroup(t *testing.T) {
 	assert.NoError(t, err)
 
 	svg := &cephv1.CephFilesystemSubVolumeGroup{}
-	err = reconciler.Client.Get(context.TODO(), types.NamespacedName{Name: defaultSubvolumeGroupName, Namespace: filesystem[0].Namespace}, svg)
+	expectedsvgName := generateNameForCephSubvolumeGroup(filesystem[0].Name)
+	err = reconciler.Client.Get(context.TODO(), types.NamespacedName{Name: expectedsvgName, Namespace: filesystem[0].Namespace}, svg)
 	assert.Error(t, err) // error as csi svg is deleted
 }
 

diff --git a/controllers/storagecluster/generate.go b/controllers/storagecluster/generate.go
@@ -171,3 +171,8 @@ func generateCephReplicatedSpec(initData *ocsv1.StorageCluster, poolType string)
 func generateStorageQuotaName(storageClassName, quotaName string) string {
 	return fmt.Sprintf("%s-%s", storageClassName, quotaName)
 }
+
+// generateNameForCephSubvolumeGroup function generates a name for CephFilesystemSubVolumeGroup
+func generateNameForCephSubvolumeGroup(filesystemName string) string {
+	return fmt.Sprintf("%s-%s", filesystemName, defaultSubvolumeGroupName)
+}
diff --git a/deploy/csv-templates/crds/ocs/ocs.openshift.io_storageclusters.yaml b/deploy/csv-templates/crds/ocs/ocs.openshift.io_storageclusters.yaml
@@ -1365,8 +1365,7 @@ spec:
                         properties:
                           enabled:
                             description: Whether to compress the data in transit across
-                              the wire. The default is not set. Requires Ceph Quincy
-                              (v17) or newer.
+                              the wire. The default is not set.
                             type: boolean
                         type: object
                       encryption:
@@ -1430,6 +1429,10 @@ spec:
                     - multus
                     nullable: true
                     type: string
+                    x-kubernetes-validations:
+                    - message: network provider must be disabled (reverted to empty
+                        string) before a new provider is enabled
+                      rule: self == '' || self == oldSelf
                   selectors:
                     additionalProperties:
                       type: string
@@ -1456,6 +1459,11 @@ spec:
                     nullable: true
                     type: object
                 type: object
+                x-kubernetes-validations:
+                - message: at least one network selector must be specified when using
+                    multus
+                  rule: '!has(self.provider) || (self.provider != ''multus'' || (self.provider
+                    == ''multus'' && size(self.selectors) > 0))'
               nfs:
                 description: NFSSpec defines specific nfs configuration options
                 properties:

diff --git a/deploy/csv-templates/crds/rook/ceph.rook.io_cephblockpoolradosnamespaces.yaml b/deploy/csv-templates/crds/rook/ceph.rook.io_cephblockpoolradosnamespaces.yaml
@@ -28,6 +28,8 @@ spec:
             properties:
               blockPoolName:
                 type: string
+              name:
+                type: string
             required:
             - blockPoolName
             type: object

diff --git a/deploy/csv-templates/crds/rook/ceph.rook.io_cephclusters.yaml b/deploy/csv-templates/crds/rook/ceph.rook.io_cephclusters.yaml
@@ -64,6 +64,13 @@ spec:
                 nullable: true
                 type: object
                 x-kubernetes-preserve-unknown-fields: true
+              cephConfig:
+                additionalProperties:
+                  additionalProperties:
+                    type: string
+                  type: object
+                nullable: true
+                type: object
               cephVersion:
                 nullable: true
                 properties:
@@ -156,6 +163,9 @@ spec:
               dataDirHostPath:
                 pattern: ^/(\S+)
                 type: string
+                x-kubernetes-validations:
+                - message: DataDirHostPath is immutable
+                  rule: self == oldSelf
               disruptionManagement:
                 nullable: true
                 properties:
@@ -992,6 +1002,14 @@ spec:
                       type: object
                     type: array
                 type: object
+                x-kubernetes-validations:
+                - message: zones must be less than or equal to count
+                  rule: '!has(self.zones) || (has(self.zones) && (size(self.zones)
+                    <= self.count))'
+                - message: stretchCluster zones must be equal to 3
+                  rule: '!has(self.stretchCluster) || (has(self.stretchCluster) &&
+                    (size(self.stretchCluster.zones) > 0) && (size(self.stretchCluster.zones)
+                    == 3))'
               monitoring:
                 nullable: true
                 properties:
@@ -1102,13 +1120,22 @@ spec:
                     - multus
                     nullable: true
                     type: string
+                    x-kubernetes-validations:
+                    - message: network provider must be disabled (reverted to empty
+                        string) before a new provider is enabled
+                      rule: self == '' || self == oldSelf
                   selectors:
                     additionalProperties:
                       type: string
                     nullable: true
                     type: object
                 type: object
                 x-kubernetes-preserve-unknown-fields: true
+                x-kubernetes-validations:
+                - message: at least one network selector must be specified when using
+                    multus
+                  rule: '!has(self.provider) || (self.provider != ''multus'' || (self.provider
+                    == ''multus'' && size(self.selectors) > 0))'
               placement:
                 additionalProperties:
                   properties:

diff --git a/deploy/csv-templates/crds/rook/ceph.rook.io_cephfilesystemsubvolumegroups.yaml b/deploy/csv-templates/crds/rook/ceph.rook.io_cephfilesystemsubvolumegroups.yaml
@@ -34,6 +34,30 @@ spec:
                 type: string
               name:
                 type: string
+              pinning:
+                properties:
+                  distributed:
+                    maximum: 1
+                    minimum: 0
+                    nullable: true
+                    type: integer
+                  export:
+                    maximum: 256
+                    minimum: -1
+                    nullable: true
+                    type: integer
+                  random:
+                    maximum: 1
+                    minimum: 0
+                    nullable: true
+                    type: number
+                type: object
+                x-kubernetes-validations:
+                - message: only one pinning type should be set
+                  rule: (has(self.export) && !has(self.distributed) && !has(self.random))
+                    || (!has(self.export) && has(self.distributed) && !has(self.random))
+                    || (!has(self.export) && !has(self.distributed) && has(self.random))
+                    || (!has(self.export) && !has(self.distributed) && !has(self.random))
             required:
             - filesystemName
             type: object