From c70f946a81ea53fb1823429bdf4b5a1effca1865 Mon Sep 17 00:00:00 2001 From: Leonardo Cecchi Date: Thu, 28 Nov 2024 13:40:39 +0000 Subject: [PATCH] Use ownership to identify volume group snapshot members Instead of marking the member snapshot object with a label, this PR uses an ownership reference. Member objects are looked up using a new index in the informer cache. --- go.mod | 2 +- pkg/common-controller/framework_test.go | 4 +- .../groupsnapshot_controller_helper.go | 47 +++- pkg/common-controller/snapshot_controller.go | 56 ++++- .../snapshot_controller_base.go | 22 +- .../groupsnapshot_helper.go | 14 +- pkg/utils/util.go | 4 - pkg/utils/vgs.go | 114 +++++++++ pkg/utils/vgs_test.go | 216 ++++++++++++++++++ 9 files changed, 451 insertions(+), 28 deletions(-) create mode 100644 pkg/utils/vgs.go create mode 100644 pkg/utils/vgs_test.go diff --git a/go.mod b/go.mod index 62d7fbab8..b2a208a05 100644 --- a/go.mod +++ b/go.mod @@ -21,6 +21,7 @@ require ( k8s.io/component-base v0.31.0 k8s.io/component-helpers v0.31.0 k8s.io/klog/v2 v2.130.1 + k8s.io/utils v0.0.0-20240711033017-18e509b52bc8 ) require ( @@ -72,7 +73,6 @@ require ( gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 // indirect - k8s.io/utils v0.0.0-20240711033017-18e509b52bc8 // indirect sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect sigs.k8s.io/yaml v1.4.0 // indirect diff --git a/pkg/common-controller/framework_test.go b/pkg/common-controller/framework_test.go index db602399d..7084bb21e 100644 --- a/pkg/common-controller/framework_test.go +++ b/pkg/common-controller/framework_test.go @@ -1728,7 +1728,7 @@ func newVolumeError(message string) *crdv1.VolumeSnapshotError { } func testSyncSnapshot(ctrl *csiSnapshotCommonController, reactor *snapshotReactor, test controllerTest) error { - return ctrl.syncSnapshot(test.initialSnapshots[0]) + return ctrl.syncSnapshot(context.TODO(), test.initialSnapshots[0]) } func testSyncGroupSnapshot(ctrl *csiSnapshotCommonController, reactor *snapshotReactor, test controllerTest) error { @@ -1736,7 +1736,7 @@ func testSyncGroupSnapshot(ctrl *csiSnapshotCommonController, reactor *snapshotR } func testSyncSnapshotError(ctrl *csiSnapshotCommonController, reactor *snapshotReactor, test controllerTest) error { - err := ctrl.syncSnapshot(test.initialSnapshots[0]) + err := ctrl.syncSnapshot(context.TODO(), test.initialSnapshots[0]) if err != nil { return nil } diff --git a/pkg/common-controller/groupsnapshot_controller_helper.go b/pkg/common-controller/groupsnapshot_controller_helper.go index 2da41bcbc..f88fded38 100644 --- a/pkg/common-controller/groupsnapshot_controller_helper.go +++ b/pkg/common-controller/groupsnapshot_controller_helper.go @@ -27,6 +27,7 @@ import ( apierrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" ref "k8s.io/client-go/tools/reference" klog "k8s.io/klog/v2" @@ -301,7 +302,7 @@ func (ctrl *csiSnapshotCommonController) syncGroupSnapshot(ctx context.Context, // Proceed with group snapshot deletion and remove finalizers when needed if groupSnapshot.ObjectMeta.DeletionTimestamp != nil { - return ctrl.processGroupSnapshotWithDeletionTimestamp(groupSnapshot) + return ctrl.processGroupSnapshotWithDeletionTimestamp(ctx, groupSnapshot) } klog.V(5).Infof("syncGroupSnapshot[%s]: validate group snapshot to make sure source has been correctly specified", utils.GroupSnapshotKey(groupSnapshot)) @@ -599,8 +600,8 @@ func (ctrl *csiSnapshotCommonController) createSnapshotsForGroupSnapshotContent( ObjectMeta: metav1.ObjectMeta{ Name: volumeSnapshotName, Namespace: volumeSnapshotNamespace, - Labels: map[string]string{ - utils.VolumeGroupSnapshotNameLabel: groupSnapshotContent.Spec.VolumeGroupSnapshotRef.Name, + OwnerReferences: []metav1.OwnerReference{ + utils.BuildVolumeGroupSnapshotOwnerReference(groupSnapshot), }, Finalizers: []string{utils.VolumeSnapshotInGroupFinalizer}, }, @@ -1376,7 +1377,7 @@ func (ctrl *csiSnapshotCommonController) addGroupSnapshotFinalizer(groupSnapshot // with information obtained from step 1. This function name is very long but the // name suggests what it does. It determines whether to remove finalizers on group // snapshot and whether to delete group snapshot content. -func (ctrl *csiSnapshotCommonController) processGroupSnapshotWithDeletionTimestamp(groupSnapshot *crdv1alpha1.VolumeGroupSnapshot) error { +func (ctrl *csiSnapshotCommonController) processGroupSnapshotWithDeletionTimestamp(ctx context.Context, groupSnapshot *crdv1alpha1.VolumeGroupSnapshot) error { klog.V(5).Infof("processGroupSnapshotWithDeletionTimestamp VolumeGroupSnapshot[%s]: %s", utils.GroupSnapshotKey(groupSnapshot), utils.GetGroupSnapshotStatusForLogging(groupSnapshot)) driverName, err := ctrl.getGroupSnapshotDriverName(groupSnapshot) @@ -1435,11 +1436,13 @@ func (ctrl *csiSnapshotCommonController) processGroupSnapshotWithDeletionTimesta return nil } - snapshotMembers, err := ctrl.snapshotLister.List(labels.SelectorFromSet( - labels.Set{ - utils.VolumeGroupSnapshotNameLabel: groupSnapshot.Name, + // Look up for members of this volume group snapshot + snapshotMembers, err := ctrl.findGroupSnapshotMembers( + types.NamespacedName{ + Name: groupSnapshot.Name, + Namespace: groupSnapshot.Namespace, }, - )) + ) if err != nil { klog.Errorf( "processGroupSnapshotWithDeletionTimestamp[%s]: Failed to look for snapshot members: %v", @@ -1489,7 +1492,7 @@ func (ctrl *csiSnapshotCommonController) processGroupSnapshotWithDeletionTimesta // VolumeGroupSnapshotContent won't be deleted immediately due to the VolumeGroupSnapshotContentFinalizer if groupSnapshotContent != nil && deleteGroupSnapshotContent { klog.V(5).Infof("processGroupSnapshotWithDeletionTimestamp[%s]: set DeletionTimeStamp on group snapshot content [%s].", utils.GroupSnapshotKey(groupSnapshot), groupSnapshotContent.Name) - err := ctrl.clientset.GroupsnapshotV1alpha1().VolumeGroupSnapshotContents().Delete(context.TODO(), groupSnapshotContent.Name, metav1.DeleteOptions{}) + err := ctrl.clientset.GroupsnapshotV1alpha1().VolumeGroupSnapshotContents().Delete(ctx, groupSnapshotContent.Name, metav1.DeleteOptions{}) if err != nil { ctrl.eventRecorder.Event(groupSnapshot, v1.EventTypeWarning, "GroupSnapshotContentObjectDeleteError", "Failed to delete group snapshot content API object") return fmt.Errorf("failed to delete VolumeGroupSnapshotContent %s from API server: %q", groupSnapshotContent.Name, err) @@ -1561,6 +1564,32 @@ func (ctrl *csiSnapshotCommonController) setAnnVolumeGroupSnapshotBeingDeleted(g return groupSnapshotContent, nil } +// findGroupSnapshotMembers get the list of members of a group snapshot +// using the local cache and indexer +func (ctrl *csiSnapshotCommonController) findGroupSnapshotMembers(groupSnapshotName types.NamespacedName) ([]*crdv1.VolumeSnapshot, error) { + // Look up for the members of this volume group snapshot + snapshotMembers, err := ctrl.snapshotIndexer.ByIndex( + utils.VolumeSnapshotParentGroupIndex, + utils.VolumeSnapshotParentGroupKeyFuncByComponents( + groupSnapshotName, + ), + ) + if err != nil { + return nil, err + } + + result := make([]*crdv1.VolumeSnapshot, len(snapshotMembers)) + for i := range snapshotMembers { + var ok bool + result[i], ok = snapshotMembers[i].(*crdv1.VolumeSnapshot) + if !ok { + return nil, fmt.Errorf("unexpected content found in snapshot index: %v", snapshotMembers[i]) + } + } + + return result, nil +} + // removeGroupSnapshotFinalizer removes a Finalizer for VolumeGroupSnapshot. func (ctrl *csiSnapshotCommonController) removeGroupSnapshotFinalizer(groupSnapshot *crdv1alpha1.VolumeGroupSnapshot, removeBoundFinalizer bool) error { if !removeBoundFinalizer { diff --git a/pkg/common-controller/snapshot_controller.go b/pkg/common-controller/snapshot_controller.go index 119c5276e..c384d41ba 100644 --- a/pkg/common-controller/snapshot_controller.go +++ b/pkg/common-controller/snapshot_controller.go @@ -173,7 +173,7 @@ func (ctrl *csiSnapshotCommonController) syncContent(content *crdv1.VolumeSnapsh // created, updated or periodically synced. We do not differentiate between // these events. // For easier readability, it is split into syncUnreadySnapshot and syncReadySnapshot -func (ctrl *csiSnapshotCommonController) syncSnapshot(snapshot *crdv1.VolumeSnapshot) error { +func (ctrl *csiSnapshotCommonController) syncSnapshot(ctx context.Context, snapshot *crdv1.VolumeSnapshot) error { klog.V(5).Infof("synchronizing VolumeSnapshot[%s]: %s", utils.SnapshotKey(snapshot), utils.GetSnapshotStatusForLogging(snapshot)) klog.V(5).Infof("syncSnapshot [%s]: check if we should remove finalizer on snapshot PVC source and remove it if we can", utils.SnapshotKey(snapshot)) @@ -214,7 +214,7 @@ func (ctrl *csiSnapshotCommonController) syncSnapshot(snapshot *crdv1.VolumeSnap if !utils.IsSnapshotReady(snapshot) || !utils.IsBoundVolumeSnapshotContentNameSet(snapshot) { return ctrl.syncUnreadySnapshot(snapshot) } - return ctrl.syncReadySnapshot(snapshot) + return ctrl.syncReadySnapshot(ctx, snapshot) } // processSnapshotWithDeletionTimestamp processes finalizers and deletes the content when appropriate. It has the following steps: @@ -395,7 +395,7 @@ func (ctrl *csiSnapshotCommonController) checkandAddSnapshotFinalizers(snapshot // syncReadySnapshot checks the snapshot which has been bound to snapshot content successfully before. // If there is any problem with the binding (e.g., snapshot points to a non-existent snapshot content), update the snapshot status and emit event. -func (ctrl *csiSnapshotCommonController) syncReadySnapshot(snapshot *crdv1.VolumeSnapshot) error { +func (ctrl *csiSnapshotCommonController) syncReadySnapshot(ctx context.Context, snapshot *crdv1.VolumeSnapshot) error { if !utils.IsBoundVolumeSnapshotContentNameSet(snapshot) { return fmt.Errorf("snapshot %s is not bound to a content", utils.SnapshotKey(snapshot)) } @@ -415,10 +415,56 @@ func (ctrl *csiSnapshotCommonController) syncReadySnapshot(snapshot *crdv1.Volum return ctrl.updateSnapshotErrorStatusWithEvent(snapshot, true, v1.EventTypeWarning, "SnapshotMisbound", "VolumeSnapshotContent is not bound to the VolumeSnapshot correctly") } + // If this snapshot is a member of a volume group snapshot, ensure we have + // the correct ownership. This happens when the user + // statically provisioned volume group snapshot members. + if utils.NeedToAddVolumeGroupSnapshotOwnership(snapshot) { + if _, err := ctrl.addVolumeGroupSnapshotOwnership(ctx, snapshot); err != nil { + return err + } + } + // everything is verified, return return nil } +// addVolumeGroupSnapshotOwnership adds the ownership information to a statically provisioned VolumeSnapshot +// that is a member of a volume group snapshot +func (ctrl *csiSnapshotCommonController) addVolumeGroupSnapshotOwnership(ctx context.Context, snapshot *crdv1.VolumeSnapshot) (*crdv1.VolumeSnapshot, error) { + klog.V(4).Infof("addVolumeGroupSnapshotOwnership[%s]: adding ownership information", utils.SnapshotKey(snapshot)) + if snapshot.Status == nil || snapshot.Status.VolumeGroupSnapshotName == nil { + klog.V(4).Infof("addVolumeGroupSnapshotOwnership[%s]: no need to add ownership information, empty volumeGroupSnapshotName", utils.SnapshotKey(snapshot)) + return nil, nil + } + parentObjectName := *snapshot.Status.VolumeGroupSnapshotName + + parentGroup, err := ctrl.groupSnapshotLister.VolumeGroupSnapshots(snapshot.Namespace).Get(parentObjectName) + if err != nil { + klog.V(4).Infof("addVolumeGroupSnapshotOwnership[%s]: error while looking for parent group %v", utils.SnapshotKey(snapshot), err) + return nil, err + } + if parentGroup == nil { + klog.V(4).Infof("addVolumeGroupSnapshotOwnership[%s]: parent group not found %v", utils.SnapshotKey(snapshot), err) + return nil, fmt.Errorf("missing parent group for snapshot %v", utils.SnapshotKey(snapshot)) + } + + updatedSnapshot := snapshot.DeepCopy() + updatedSnapshot.ObjectMeta.OwnerReferences = append( + snapshot.ObjectMeta.OwnerReferences, + utils.BuildVolumeGroupSnapshotOwnerReference(parentGroup), + ) + + newSnapshot, err := ctrl.clientset.SnapshotV1().VolumeSnapshots(snapshot.Namespace).Update(ctx, updatedSnapshot, metav1.UpdateOptions{}) + if err != nil { + klog.V(4).Infof("addVolumeGroupSnapshotOwnership[%s]: error when updating VolumeSnapshot %v", utils.SnapshotKey(snapshot), err) + return nil, err + } + + klog.V(4).Infof("addVolumeGroupSnapshotOwnership[%s]: updated ownership", utils.SnapshotKey(snapshot)) + + return newSnapshot, nil +} + // syncUnreadySnapshot is the main controller method to decide what to do with a snapshot which is not set to ready. func (ctrl *csiSnapshotCommonController) syncUnreadySnapshot(snapshot *crdv1.VolumeSnapshot) error { uniqueSnapshotName := utils.SnapshotKey(snapshot) @@ -483,7 +529,7 @@ func (ctrl *csiSnapshotCommonController) syncUnreadySnapshot(snapshot *crdv1.Vol } // member of a dynamically provisioned volume group snapshot - if _, ok := snapshot.Labels[utils.VolumeGroupSnapshotNameLabel]; ok { + if utils.IsVolumeGroupSnapshotMember(snapshot) { if snapshot.Status == nil || snapshot.Status.BoundVolumeSnapshotContentName == nil { klog.V(5).Infof( "syncUnreadySnapshot [%s]: detected group snapshot member with no content, retrying", @@ -1422,7 +1468,7 @@ func (ctrl *csiSnapshotCommonController) SetDefaultSnapshotClass(snapshot *crdv1 return nil, snapshot, nil } - if _, ok := snapshot.Labels[utils.VolumeGroupSnapshotNameLabel]; ok { + if utils.IsVolumeGroupSnapshotMember(snapshot) { // don't return error for volume group snapshot members klog.V(5).Infof("Don't need to find SnapshotClass for volume group snapshot member [%s]", snapshot.Name) return nil, snapshot, nil diff --git a/pkg/common-controller/snapshot_controller_base.go b/pkg/common-controller/snapshot_controller_base.go index 009cb8fe6..bf0e0133b 100644 --- a/pkg/common-controller/snapshot_controller_base.go +++ b/pkg/common-controller/snapshot_controller_base.go @@ -87,7 +87,8 @@ type csiSnapshotCommonController struct { preventVolumeModeConversion bool enableVolumeGroupSnapshots bool - pvIndexer cache.Indexer + pvIndexer cache.Indexer + snapshotIndexer cache.Indexer } // NewCSISnapshotController returns a new *csiSnapshotCommonController @@ -158,8 +159,20 @@ func NewCSISnapshotCommonController( }, ctrl.resyncPeriod, ) + volumeSnapshotInformer.Informer().AddIndexers(map[string]cache.IndexFunc{ + utils.VolumeSnapshotParentGroupIndex: func(obj interface{}) ([]string, error) { + if snapshot, ok := obj.(*crdv1.VolumeSnapshot); ok { + if key := utils.VolumeSnapshotParentGroupKeyFunc(snapshot); key != "" { + return []string{key}, nil + } + } + + return nil, nil + }, + }) ctrl.snapshotLister = volumeSnapshotInformer.Lister() ctrl.snapshotListerSynced = volumeSnapshotInformer.Informer().HasSynced + ctrl.snapshotIndexer = volumeSnapshotInformer.Informer().GetIndexer() volumeSnapshotContentInformer.Informer().AddEventHandlerWithResyncPeriod( cache.ResourceEventHandlerFuncs{ @@ -323,6 +336,7 @@ func (ctrl *csiSnapshotCommonController) snapshotWorker() { // syncSnapshotByKey processes a VolumeSnapshot request. func (ctrl *csiSnapshotCommonController) syncSnapshotByKey(key string) error { + ctx := context.Background() klog.V(5).Infof("syncSnapshotByKey[%s]", key) namespace, name, err := cache.SplitMetaNamespaceKey(key) @@ -344,7 +358,7 @@ func (ctrl *csiSnapshotCommonController) syncSnapshotByKey(key string) error { klog.V(5).Infof("Snapshot %q is being deleted. SnapshotClass has already been removed", key) } klog.V(5).Infof("Updating snapshot %q", key) - return ctrl.updateSnapshot(newSnapshot) + return ctrl.updateSnapshot(ctx, newSnapshot) } return err } @@ -476,7 +490,7 @@ func (ctrl *csiSnapshotCommonController) checkAndUpdateSnapshotClass(snapshot *c // updateSnapshot runs in worker thread and handles "snapshot added", // "snapshot updated" and "periodic sync" events. -func (ctrl *csiSnapshotCommonController) updateSnapshot(snapshot *crdv1.VolumeSnapshot) error { +func (ctrl *csiSnapshotCommonController) updateSnapshot(ctx context.Context, snapshot *crdv1.VolumeSnapshot) error { // Store the new snapshot version in the cache and do not process it if this is // an old version. klog.V(5).Infof("updateSnapshot %q", utils.SnapshotKey(snapshot)) @@ -488,7 +502,7 @@ func (ctrl *csiSnapshotCommonController) updateSnapshot(snapshot *crdv1.VolumeSn return nil } - err = ctrl.syncSnapshot(snapshot) + err = ctrl.syncSnapshot(ctx, snapshot) if err != nil { if errors.IsConflict(err) { // Version conflict error happens quite often and the controller diff --git a/pkg/sidecar-controller/groupsnapshot_helper.go b/pkg/sidecar-controller/groupsnapshot_helper.go index 419bd5268..97612707d 100644 --- a/pkg/sidecar-controller/groupsnapshot_helper.go +++ b/pkg/sidecar-controller/groupsnapshot_helper.go @@ -246,10 +246,18 @@ func (ctrl *csiSnapshotSideCarController) deleteCSIGroupSnapshotOperation(groupS return fmt.Errorf("failed to get input parameters to delete group snapshot for group snapshot content %s: %q", groupSnapshotContent.Name, err) } + // Collect the snapshot ids considering both dynamic and static provisioning. + // For dynamic provisioning, they can be found in groupContent.Status.VolumeSnapshotHandlePairList + // For static provisioning, they can be found in groupContent.Spec.Source.GroupSnapshotHandles.VolumeSnapshotHandles var snapshotIDs []string - if groupSnapshotContent.Status != nil && len(groupSnapshotContent.Status.VolumeSnapshotHandlePairList) != 0 { - for _, contentRef := range groupSnapshotContent.Status.VolumeSnapshotHandlePairList { - snapshotIDs = append(snapshotIDs, contentRef.SnapshotHandle) + if groupSnapshotContent.Status != nil { + if len(groupSnapshotContent.Status.VolumeSnapshotHandlePairList) != 0 { + for _, contentRef := range groupSnapshotContent.Status.VolumeSnapshotHandlePairList { + snapshotIDs = append(snapshotIDs, contentRef.SnapshotHandle) + } + } else if groupSnapshotContent.Spec.Source.GroupSnapshotHandles != nil { + ids := groupSnapshotContent.Spec.Source.GroupSnapshotHandles.VolumeSnapshotHandles + snapshotIDs = slices.Clone(ids) } } diff --git a/pkg/utils/util.go b/pkg/utils/util.go index 8f70d3d5d..1c2a97118 100644 --- a/pkg/utils/util.go +++ b/pkg/utils/util.go @@ -146,10 +146,6 @@ const ( AnnDeletionGroupSecretRefName = "groupsnapshot.storage.kubernetes.io/deletion-secret-name" AnnDeletionGroupSecretRefNamespace = "groupsnapshot.storage.kubernetes.io/deletion-secret-namespace" - // VolumeGroupSnapshotNameLabel is applied to VolumeSnapshots that are member - // of a VolumeGroupSnapshot, and indicates the name of the latter. - VolumeGroupSnapshotNameLabel = "groupsnapshot.storage.k8s.io/volumeGroupSnapshotName" - // VolumeGroupSnapshotHandleAnnotation is applied to VolumeSnapshotContents that are member // of a VolumeGroupSnapshotContent, and indicates the handle of the latter. // diff --git a/pkg/utils/vgs.go b/pkg/utils/vgs.go new file mode 100644 index 000000000..f63254a42 --- /dev/null +++ b/pkg/utils/vgs.go @@ -0,0 +1,114 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package utils + +import ( + "fmt" + + crdv1alpha1 "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumegroupsnapshot/v1alpha1" + crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumesnapshot/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" +) + +// VolumeSnapshotParentGroupIndex is the name of the cache index hosting the relationship +// between volume snapshots and their volume group snapshot owner +const VolumeSnapshotParentGroupIndex = "ByVolumeGroupSnapshotMembership" + +// getVolumeGroupSnapshotParentObjectName returns the name of the parent group snapshot, if present. +// The second return value is true when the parent object have been found, false otherwise. +func getVolumeGroupSnapshotParentObjectName(snapshot *crdv1.VolumeSnapshot) string { + if snapshot == nil { + return "" + } + + apiVersion := fmt.Sprintf( + "%s/%s", + crdv1alpha1.SchemeGroupVersion.Group, + crdv1alpha1.SchemeGroupVersion.Version, + ) + + for _, owner := range snapshot.ObjectMeta.OwnerReferences { + if owner.Kind == "VolumeGroupSnapshot" && owner.APIVersion == apiVersion { + return owner.Name + } + } + + return "" +} + +// IsVolumeGroupSnapshotMember returns true if the passed VolumeSnapshot object +// is a member of a VolumeGroupSnapshot. +func IsVolumeGroupSnapshotMember(snapshot *crdv1.VolumeSnapshot) bool { + parentName := getVolumeGroupSnapshotParentObjectName(snapshot) + return len(parentName) > 0 +} + +// VolumeSnapshotParentGroupKeyFunc maps a member snapshot to the name +// of the parent VolumeGroupSnapshot +func VolumeSnapshotParentGroupKeyFunc(snapshot *crdv1.VolumeSnapshot) string { + parentName := getVolumeGroupSnapshotParentObjectName(snapshot) + if len(parentName) == 0 { + return "" + } + + return VolumeSnapshotParentGroupKeyFuncByComponents(types.NamespacedName{ + Namespace: snapshot.Namespace, + Name: parentName, + }) +} + +// VolumeSnapshotParentGroupKeyFuncByComponents computes the index key for a certain +// name and namespace pair +func VolumeSnapshotParentGroupKeyFuncByComponents(objectKey types.NamespacedName) string { + return fmt.Sprintf("%s^%s", objectKey.Namespace, objectKey.Name) +} + +// NeedToAddVolumeGroupSnapshotOwnership checks if the passed snapshot is a member +// of a volume group snapshot but the ownership is missing +func NeedToAddVolumeGroupSnapshotOwnership(snapshot *crdv1.VolumeSnapshot) bool { + parentObjectName := getVolumeGroupSnapshotParentObjectName(snapshot) + if len(parentObjectName) > 0 { + return false + } + + // Group snapshot ownership should be added only when + // the snapshot is marked with its group snapshot name + if snapshot == nil || + snapshot.Status == nil || + snapshot.Status.VolumeGroupSnapshotName == nil || + len(*snapshot.Status.VolumeGroupSnapshotName) == 0 { + return false + } + + return true +} + +// BuildVolumeGroupSnapshotOwnerReference creates a OwnerReference record declaring an +// object as a child of passed VolumeGroupSnapshot +func BuildVolumeGroupSnapshotOwnerReference(parentGroup *crdv1alpha1.VolumeGroupSnapshot) metav1.OwnerReference { + return metav1.OwnerReference{ + APIVersion: fmt.Sprintf( + "%s/%s", + crdv1alpha1.SchemeGroupVersion.Group, + crdv1alpha1.SchemeGroupVersion.Version, + ), + Kind: "VolumeGroupSnapshot", + Name: parentGroup.Name, + UID: parentGroup.UID, + } +} diff --git a/pkg/utils/vgs_test.go b/pkg/utils/vgs_test.go new file mode 100644 index 000000000..d38a6289e --- /dev/null +++ b/pkg/utils/vgs_test.go @@ -0,0 +1,216 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package utils + +import ( + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" + + crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumesnapshot/v1" +) + +func TestIsVolumeSnapshotGroupMember(t *testing.T) { + testCases := []struct { + name string + snapshot *crdv1.VolumeSnapshot + expected bool + expectedParentObjectName string + expectedIndexKey string + }{ + { + name: "nil snapshot", + snapshot: nil, + expected: false, + expectedParentObjectName: "", + expectedIndexKey: "", + }, + { + name: "without ownership", + snapshot: &crdv1.VolumeSnapshot{}, + expected: false, + expectedParentObjectName: "", + expectedIndexKey: "", + }, + { + name: "with a different ownership", + snapshot: &crdv1.VolumeSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "v1", + Name: "test", + Kind: "Deployment", + }, + }, + }, + }, + expected: false, + expectedParentObjectName: "", + expectedIndexKey: "", + }, + { + name: "with wrong ownership", + snapshot: &crdv1.VolumeSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "test/v1beta1", + Kind: "VolumeGroupSnapshot", + Name: "vgs", + }, + }, + }, + }, + expected: false, + expectedParentObjectName: "", + expectedIndexKey: "", + }, + { + name: "with correct ownership", + snapshot: &crdv1.VolumeSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vs", + Namespace: "default", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "groupsnapshot.storage.k8s.io/v1alpha1", + Kind: "VolumeGroupSnapshot", + Name: "vgs", + }, + }, + }, + }, + expected: true, + expectedParentObjectName: "vgs", + expectedIndexKey: "default^vgs", + }, + } + + for _, test := range testCases { + t.Run(test.name, func(t *testing.T) { + result := IsVolumeGroupSnapshotMember(test.snapshot) + if result != test.expected { + t.Errorf("IsVolumeSnapshotGroupMember(s) = %v WANT %v", result, test.expected) + } + + resultParentObject := getVolumeGroupSnapshotParentObjectName(test.snapshot) + if resultParentObject != test.expectedParentObjectName { + t.Errorf("GetVolumeGroupSnapshotParentObjectName(s) = %v WANTED %v", resultParentObject, test.expectedParentObjectName) + } + + resultKey := VolumeSnapshotParentGroupKeyFunc(test.snapshot) + if resultKey != test.expectedIndexKey { + t.Errorf("VolumeSnapshotParentGroupKeyFunc(s) = %v WANTED %v", resultKey, test.expectedIndexKey) + } + }) + } +} + +func TestNeedToAddVolumeGroupSnapshotOwnership(t *testing.T) { + testCases := []struct { + name string + snapshot *crdv1.VolumeSnapshot + expected bool + }{ + { + name: "nil snapshot", + snapshot: nil, + expected: false, + }, + { + name: "snapshot with nil status", + snapshot: &crdv1.VolumeSnapshot{ + Status: nil, + }, + expected: false, + }, + { + name: "independent snapshot", + snapshot: &crdv1.VolumeSnapshot{ + Status: &crdv1.VolumeSnapshotStatus{ + VolumeGroupSnapshotName: nil, + }, + }, + expected: false, + }, + { + name: "snapshot with empty volume group snapshot name", + snapshot: &crdv1.VolumeSnapshot{ + Status: &crdv1.VolumeSnapshotStatus{ + VolumeGroupSnapshotName: ptr.To(""), + }, + }, + expected: false, + }, + { + name: "snapshot with empty ownership metadata", + snapshot: &crdv1.VolumeSnapshot{ + Status: &crdv1.VolumeSnapshotStatus{ + VolumeGroupSnapshotName: ptr.To("vgs"), + }, + }, + expected: true, + }, + { + name: "snapshot missing the group ownership but having other ownerships", + snapshot: &crdv1.VolumeSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "v1", + Kind: "Deployment", + Name: "test", + }, + }, + }, + Status: &crdv1.VolumeSnapshotStatus{ + VolumeGroupSnapshotName: ptr.To("vgs"), + }, + }, + expected: true, + }, + { + name: "snapshot with the ownership already set", + snapshot: &crdv1.VolumeSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "groupsnapshot.storage.k8s.io/v1alpha1", + Kind: "VolumeGroupSnapshot", + Name: "vgs", + }, + }, + }, + Status: &crdv1.VolumeSnapshotStatus{ + VolumeGroupSnapshotName: ptr.To("vgs"), + }, + }, + expected: false, + }, + } + + for _, test := range testCases { + t.Run(test.name, func(t *testing.T) { + result := NeedToAddVolumeGroupSnapshotOwnership(test.snapshot) + if result != test.expected { + t.Errorf("NeedToAddVolumeGroupSnapshotOwnership(s) = %v WANT %v", result, test.expected) + } + }) + } +}