Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

set pinning field in CephSubvolumeGroup CR #2317

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
12 changes: 10 additions & 2 deletions config/crd/bases/ocs.openshift.io_storageclusters.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand Down
38 changes: 26 additions & 12 deletions controllers/storagecluster/cephfilesystem.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Nikhil-Ladha marked this conversation as resolved.
Show resolved Hide resolved
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))
Nikhil-Ladha marked this conversation as resolved.
Show resolved Hide resolved
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
Nikhil-Ladha marked this conversation as resolved.
Show resolved Hide resolved
Nikhil-Ladha marked this conversation as resolved.
Show resolved Hide resolved
mutateFn := func() error {
cephFilesystemSubVolumeGroup.Spec = cephv1.CephFilesystemSubVolumeGroupSpec{
Name: defaultSubvolumeGroupName,
FilesystemName: filesystemName,
Pinning: cephv1.CephFilesystemSubVolumeGroupSpecPinning{
Distributed: &defaultPinningValue,
},
}
return nil
}
Expand All @@ -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)
}

Expand All @@ -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)
}

Expand All @@ -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
}
}
Expand Down
6 changes: 4 additions & 2 deletions controllers/storagecluster/cephfilesystem_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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
}

Expand Down
5 changes: 5 additions & 0 deletions controllers/storagecluster/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Nikhil-Ladha marked this conversation as resolved.
Show resolved Hide resolved
return fmt.Sprintf("%s-%s", filesystemName, defaultSubvolumeGroupName)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ spec:
properties:
blockPoolName:
type: string
name:
type: string
required:
- blockPoolName
type: object
Expand Down
27 changes: 27 additions & 0 deletions deploy/csv-templates/crds/rook/ceph.rook.io_cephclusters.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -156,6 +163,9 @@ spec:
dataDirHostPath:
pattern: ^/(\S+)
type: string
x-kubernetes-validations:
- message: DataDirHostPath is immutable
rule: self == oldSelf
disruptionManagement:
nullable: true
properties:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading