From 0bd355cf294e8a3938b3f58a94f7dff23a226f35 Mon Sep 17 00:00:00 2001 From: Raunak Pradip Shah Date: Fri, 5 Jan 2024 11:29:03 -0800 Subject: [PATCH] update API names in group snapshot content to be consistent with volume snapshots --- .../volumegroupsnapshot/v1alpha1/types.go | 10 ++-- .../v1alpha1/zz_generated.deepcopy.go | 54 +++++++++---------- .../v1/zz_generated.deepcopy.go | 2 +- ...ge.k8s.io_volumegroupsnapshotcontents.yaml | 16 +++--- client/hack/README.md | 45 +++++++++++----- .../groupsnapshot_controller_helper.go | 18 +++---- pkg/sidecar-controller/csi_handler.go | 16 +++--- .../groupsnapshot_helper.go | 48 ++++++----------- .../snapshot_controller_base.go | 2 +- pkg/validation-webhook/groupsnapshot.go | 8 +-- pkg/validation-webhook/groupsnapshot_test.go | 14 +++-- .../volumegroupsnapshot/v1alpha1/types.go | 25 +++++++-- .../v1alpha1/zz_generated.deepcopy.go | 35 +++++++++--- .../v1/zz_generated.deepcopy.go | 2 +- 14 files changed, 169 insertions(+), 126 deletions(-) diff --git a/client/apis/volumegroupsnapshot/v1alpha1/types.go b/client/apis/volumegroupsnapshot/v1alpha1/types.go index 9efe144b..7e27b793 100644 --- a/client/apis/volumegroupsnapshot/v1alpha1/types.go +++ b/client/apis/volumegroupsnapshot/v1alpha1/types.go @@ -348,22 +348,22 @@ type VolumeGroupSnapshotContentStatus struct { // Exactly one of its members must be set. // Members in VolumeGroupSnapshotContentSource are immutable. type VolumeGroupSnapshotContentSource struct { - // PersistentVolumeNames is a list of names of PersistentVolumes to be snapshotted + // VolumeHandles is a list of volume handles on the backend to be snapshotted // together. It is specified for dynamic provisioning of the VolumeGroupSnapshot. // This field is immutable. // +optional - PersistentVolumeNames []string `json:"persistentVolumeNames,omitempty" protobuf:"bytes,1,opt,name=persistentVolumeNames"` + VolumeHandles []string `json:"volumeHandles,omitempty" protobuf:"bytes,1,opt,name=volumeHandles"` - // GroupSnapshotHandleSource specifies the CSI "group_snapshot_id" of a pre-existing + // GroupSnapshotHandles specifies the CSI "group_snapshot_id" of a pre-existing // group snapshot and a list of CSI "snapshot_id" of pre-existing snapshots // on the underlying storage system for which a Kubernetes object // representation was (or should be) created. // This field is immutable. // +optional - GroupSnapshotHandleSource *VolumeGroupSnapshotHandleSource `json:"groupSnapshotHandleSource,omitempty" protobuf:"bytes,2,opt,name=groupSnapshotHandleSource"` + GroupSnapshotHandles *GroupSnapshotHandles `json:"groupSnapshotHandles,omitempty" protobuf:"bytes,2,opt,name=groupSnapshotHandles"` } -type VolumeGroupSnapshotHandleSource struct { +type GroupSnapshotHandles struct { // VolumeGroupSnapshotHandle specifies the CSI "group_snapshot_id" of a pre-existing // group snapshot on the underlying storage system for which a Kubernetes object // representation was (or should be) created. diff --git a/client/apis/volumegroupsnapshot/v1alpha1/zz_generated.deepcopy.go b/client/apis/volumegroupsnapshot/v1alpha1/zz_generated.deepcopy.go index 9d779a8f..5dc16cda 100644 --- a/client/apis/volumegroupsnapshot/v1alpha1/zz_generated.deepcopy.go +++ b/client/apis/volumegroupsnapshot/v1alpha1/zz_generated.deepcopy.go @@ -2,7 +2,7 @@ // +build !ignore_autogenerated /* -Copyright 2023 The Kubernetes Authors. +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. @@ -27,6 +27,27 @@ import ( runtime "k8s.io/apimachinery/pkg/runtime" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *GroupSnapshotHandles) DeepCopyInto(out *GroupSnapshotHandles) { + *out = *in + if in.VolumeSnapshotHandles != nil { + in, out := &in.VolumeSnapshotHandles, &out.VolumeSnapshotHandles + *out = make([]string, len(*in)) + copy(*out, *in) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GroupSnapshotHandles. +func (in *GroupSnapshotHandles) DeepCopy() *GroupSnapshotHandles { + if in == nil { + return nil + } + out := new(GroupSnapshotHandles) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *VolumeGroupSnapshot) DeepCopyInto(out *VolumeGroupSnapshot) { *out = *in @@ -193,14 +214,14 @@ func (in *VolumeGroupSnapshotContentList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *VolumeGroupSnapshotContentSource) DeepCopyInto(out *VolumeGroupSnapshotContentSource) { *out = *in - if in.PersistentVolumeNames != nil { - in, out := &in.PersistentVolumeNames, &out.PersistentVolumeNames + if in.VolumeHandles != nil { + in, out := &in.VolumeHandles, &out.VolumeHandles *out = make([]string, len(*in)) copy(*out, *in) } - if in.GroupSnapshotHandleSource != nil { - in, out := &in.GroupSnapshotHandleSource, &out.GroupSnapshotHandleSource - *out = new(VolumeGroupSnapshotHandleSource) + if in.GroupSnapshotHandles != nil { + in, out := &in.GroupSnapshotHandles, &out.GroupSnapshotHandles + *out = new(GroupSnapshotHandles) (*in).DeepCopyInto(*out) } return @@ -280,27 +301,6 @@ func (in *VolumeGroupSnapshotContentStatus) DeepCopy() *VolumeGroupSnapshotConte return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *VolumeGroupSnapshotHandleSource) DeepCopyInto(out *VolumeGroupSnapshotHandleSource) { - *out = *in - if in.VolumeSnapshotHandles != nil { - in, out := &in.VolumeSnapshotHandles, &out.VolumeSnapshotHandles - *out = make([]string, len(*in)) - copy(*out, *in) - } - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VolumeGroupSnapshotHandleSource. -func (in *VolumeGroupSnapshotHandleSource) DeepCopy() *VolumeGroupSnapshotHandleSource { - if in == nil { - return nil - } - out := new(VolumeGroupSnapshotHandleSource) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *VolumeGroupSnapshotList) DeepCopyInto(out *VolumeGroupSnapshotList) { *out = *in diff --git a/client/apis/volumesnapshot/v1/zz_generated.deepcopy.go b/client/apis/volumesnapshot/v1/zz_generated.deepcopy.go index f1b7f908..a590aef0 100644 --- a/client/apis/volumesnapshot/v1/zz_generated.deepcopy.go +++ b/client/apis/volumesnapshot/v1/zz_generated.deepcopy.go @@ -2,7 +2,7 @@ // +build !ignore_autogenerated /* -Copyright 2023 The Kubernetes Authors. +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. diff --git a/client/config/crd/groupsnapshot.storage.k8s.io_volumegroupsnapshotcontents.yaml b/client/config/crd/groupsnapshot.storage.k8s.io_volumegroupsnapshotcontents.yaml index 65ef1b6f..c150705a 100644 --- a/client/config/crd/groupsnapshot.storage.k8s.io_volumegroupsnapshotcontents.yaml +++ b/client/config/crd/groupsnapshot.storage.k8s.io_volumegroupsnapshotcontents.yaml @@ -104,8 +104,8 @@ spec: dynamically provisioned or already exists, and just requires a Kubernetes object representation. This field is immutable after creation. Required. properties: - groupSnapshotHandleSource: - description: GroupSnapshotHandleSource specifies the CSI "group_snapshot_id" + groupSnapshotHandles: + description: GroupSnapshotHandles specifies the CSI "group_snapshot_id" of a pre-existing group snapshot and a list of CSI "snapshot_id" of pre-existing snapshots on the underlying storage system for which a Kubernetes object representation was (or should be) @@ -129,17 +129,17 @@ spec: - volumeGroupSnapshotHandle - volumeSnapshotHandles type: object - persistentVolumeNames: - description: PersistentVolumeNames is a list of names of PersistentVolumes - to be snapshotted together. It is specified for dynamic provisioning - of the VolumeGroupSnapshot. This field is immutable. + volumeHandles: + description: VolumeHandles is a list of volume handles on the + backend to be snapshotted together. It is specified for dynamic + provisioning of the VolumeGroupSnapshot. This field is immutable. items: type: string type: array type: object oneOf: - - required: ["persistentVolumeNames"] - - required: ["groupSnapshotHandleSource"] + - required: ["volumeHandles"] + - required: ["groupSnapshotHandles"] volumeGroupSnapshotClassName: description: VolumeGroupSnapshotClassName is the name of the VolumeGroupSnapshotClass from which this group snapshot was (or will be) created. Note that diff --git a/client/hack/README.md b/client/hack/README.md index 20c0220f..91bb64e0 100644 --- a/client/hack/README.md +++ b/client/hack/README.md @@ -144,7 +144,7 @@ Update the restoreSize property to use type string only: * Add the VolumeSnapshot namespace to the `additionalPrinterColumns` section. Refer https://github.com/kubernetes-csi/external-snapshotter/pull/535 for more details. -* In `client/config/crd/groupsnapshot.storage.k8s.io_volumegroupsnapshotcontents.yaml `, we need to add the `oneOf` constraint to make sure only one of `persistentVolumeNames` and `volumeGroupSnapshotHandle` is specified in the `source` field of the `spec` of `VolumeGroupSnapshotContent`. +* In `client/config/crd/groupsnapshot.storage.k8s.io_volumegroupsnapshotcontents.yaml `, we need to add the `oneOf` constraint to make sure only one of `volumeHandles` and `groupSnapshotHandles` is specified in the `source` field of the `spec` of `VolumeGroupSnapshotContent`. ```bash source: @@ -152,22 +152,41 @@ Update the restoreSize property to use type string only: dynamically provisioned or already exists, and just requires a Kubernetes object representation. This field is immutable after creation. Required. properties: - persistentVolumeNames: - description: PersistentVolumeNames is a list of names of PersistentVolumes - to be snapshotted together. It is specified for dynamic provisioning - of the VolumeGroupSnapshot. This field is immutable. + groupSnapshotHandles: + description: GroupSnapshotHandles specifies the CSI "group_snapshot_id" + of a pre-existing group snapshot and a list of CSI "snapshot_id" + of pre-existing snapshots on the underlying storage system for + which a Kubernetes object representation was (or should be) + created. This field is immutable. + properties: + volumeGroupSnapshotHandle: + description: VolumeGroupSnapshotHandle specifies the CSI "group_snapshot_id" + of a pre-existing group snapshot on the underlying storage + system for which a Kubernetes object representation was + (or should be) created. This field is immutable. Required. + type: string + volumeSnapshotHandles: + description: VolumeSnapshotHandles is a list of CSI "snapshot_id" + of pre-existing snapshots on the underlying storage system + for which Kubernetes objects representation were (or should + be) created. This field is immutable. Required. + items: + type: string + type: array + required: + - volumeGroupSnapshotHandle + - volumeSnapshotHandles + type: object + volumeHandles: + description: VolumeHandles is a list of volume handles on the + backend to be snapshotted together. It is specified for dynamic + provisioning of the VolumeGroupSnapshot. This field is immutable. items: type: string type: array - volumeGroupSnapshotHandle: - description: VolumeGroupSnapshotHandle specifies the CSI "group_snapshot_id" - of a pre-existing group snapshot on the underlying storage system - for which a Kubernetes object representation was (or should - be) created. This field is immutable. - type: string type: object oneOf: - - required: ["persistentVolumeNames"] - - required: ["volumeGroupSnapshotHandle"] + - required: ["volumeHandles"] + - required: ["groupSnapshotHandles"] volumeGroupSnapshotClassName: ``` diff --git a/pkg/common-controller/groupsnapshot_controller_helper.go b/pkg/common-controller/groupsnapshot_controller_helper.go index 525ba363..26cc44d6 100644 --- a/pkg/common-controller/groupsnapshot_controller_helper.go +++ b/pkg/common-controller/groupsnapshot_controller_helper.go @@ -426,7 +426,7 @@ func (ctrl *csiSnapshotCommonController) syncUnreadyGroupSnapshot(groupSnapshot if contentObj != nil { klog.V(5).Infof("Found VolumeGroupSnapshotContent object %s for group snapshot %s", contentObj.Name, uniqueGroupSnapshotName) - if contentObj.Spec.Source.VolumeGroupSnapshotHandle != nil { + if contentObj.Spec.Source.GroupSnapshotHandles != nil { ctrl.updateGroupSnapshotErrorStatusWithEvent(groupSnapshot, true, v1.EventTypeWarning, "GroupSnapshotHandleSet", fmt.Sprintf("GroupSnapshot handle should not be set in group snapshot content %s for dynamic provisioning", uniqueGroupSnapshotName)) return fmt.Errorf("VolumeGroupSnapshotHandle should not be set in the group snapshot content for dynamic provisioning for group snapshot %s", uniqueGroupSnapshotName) } @@ -483,7 +483,7 @@ func (ctrl *csiSnapshotCommonController) getPreprovisionedGroupSnapshotContentFr return nil, nil } // check whether the content is a pre-provisioned VolumeGroupSnapshotContent - if groupSnapshotContent.Spec.Source.VolumeGroupSnapshotHandle == nil { + if groupSnapshotContent.Spec.Source.GroupSnapshotHandles == nil { // found a group snapshot content which represents a dynamically provisioned group snapshot // update the group snapshot and return an error ctrl.updateGroupSnapshotErrorStatusWithEvent(groupSnapshot, true, v1.EventTypeWarning, "GroupSnapshotContentMismatch", "VolumeGroupSnapshotContent is dynamically provisioned while expecting a pre-provisioned one") @@ -682,7 +682,7 @@ func (ctrl *csiSnapshotCommonController) getDynamicallyProvisionedGroupContentFr return nil, nil } // check whether the group snapshot content represents a dynamically provisioned snapshot - if groupSnapshotContent.Spec.Source.VolumeGroupSnapshotHandle != nil { + if groupSnapshotContent.Spec.Source.GroupSnapshotHandles != nil { ctrl.updateGroupSnapshotErrorStatusWithEvent(groupSnapshot, true, v1.EventTypeWarning, "GroupSnapshotContentMismatch", "VolumeGroupSnapshotContent "+contentName+" is pre-provisioned while expecting a dynamically provisioned one") klog.V(4).Infof("sync group snapshot[%s]: group snapshot content %s is pre-provisioned while expecting a dynamically provisioned one", utils.GroupSnapshotKey(groupSnapshot), contentName) return nil, fmt.Errorf("group snapshot %s expects a dynamically provisioned VolumeGroupSnapshotContent %s but gets a pre-provisioned one", utils.GroupSnapshotKey(groupSnapshot), contentName) @@ -752,9 +752,9 @@ func (ctrl *csiSnapshotCommonController) createGroupSnapshotContent(groupSnapsho if err != nil { return nil, err } - var pvNames []string + var volumeHandles []string for _, pv := range volumes { - pvNames = append(pvNames, pv.Name) + volumeHandles = append(volumeHandles, pv.Spec.CSI.VolumeHandle) } groupSnapshotContent := &crdv1alpha1.VolumeGroupSnapshotContent{ @@ -764,7 +764,7 @@ func (ctrl *csiSnapshotCommonController) createGroupSnapshotContent(groupSnapsho Spec: crdv1alpha1.VolumeGroupSnapshotContentSpec{ VolumeGroupSnapshotRef: *snapshotRef, Source: crdv1alpha1.VolumeGroupSnapshotContentSource{ - PersistentVolumeNames: pvNames, + VolumeHandles: volumeHandles, }, VolumeGroupSnapshotClassName: &(groupSnapshotClass.Name), DeletionPolicy: groupSnapshotClass.DeletionPolicy, @@ -846,9 +846,9 @@ func (ctrl *csiSnapshotCommonController) syncGroupSnapshotContent(groupSnapshotC klog.V(5).Infof("syncGroupSnapshotContent[%s]: check if we should add invalid label on group snapshot content", groupSnapshotContent.Name) // Keep this check in the controller since the validation webhook may not have been deployed. - if (groupSnapshotContent.Spec.Source.VolumeGroupSnapshotHandle == nil && len(groupSnapshotContent.Spec.Source.PersistentVolumeNames) == 0) || - (groupSnapshotContent.Spec.Source.VolumeGroupSnapshotHandle != nil && len(groupSnapshotContent.Spec.Source.PersistentVolumeNames) > 0) { - err := fmt.Errorf("Exactly one of VolumeGroupSnapshotHandle and PersistentVolumeNames should be specified") + if (groupSnapshotContent.Spec.Source.GroupSnapshotHandles == nil && len(groupSnapshotContent.Spec.Source.VolumeHandles) == 0) || + (groupSnapshotContent.Spec.Source.GroupSnapshotHandles != nil && len(groupSnapshotContent.Spec.Source.VolumeHandles) > 0) { + err := fmt.Errorf("Exactly one of GroupSnapshotHandles and VolumeHandles should be specified") klog.Errorf("syncGroupSnapshotContent[%s]: validation error, %s", groupSnapshotContent.Name, err.Error()) ctrl.eventRecorder.Event(groupSnapshotContent, v1.EventTypeWarning, "GroupContentValidationError", err.Error()) return err diff --git a/pkg/sidecar-controller/csi_handler.go b/pkg/sidecar-controller/csi_handler.go index c0f89a77..c17261d0 100644 --- a/pkg/sidecar-controller/csi_handler.go +++ b/pkg/sidecar-controller/csi_handler.go @@ -35,7 +35,7 @@ type Handler interface { CreateSnapshot(content *crdv1.VolumeSnapshotContent, parameters map[string]string, snapshotterCredentials map[string]string) (string, string, time.Time, int64, bool, error) DeleteSnapshot(content *crdv1.VolumeSnapshotContent, snapshotterCredentials map[string]string) error GetSnapshotStatus(content *crdv1.VolumeSnapshotContent, snapshotterListCredentials map[string]string) (bool, time.Time, int64, string, error) - CreateGroupSnapshot(content *crdv1alpha1.VolumeGroupSnapshotContent, volumeIDs []string, parameters map[string]string, snapshotterCredentials map[string]string) (string, string, []*csi.Snapshot, time.Time, bool, error) + CreateGroupSnapshot(content *crdv1alpha1.VolumeGroupSnapshotContent, parameters map[string]string, snapshotterCredentials map[string]string) (string, string, []*csi.Snapshot, time.Time, bool, error) GetGroupSnapshotStatus(content *crdv1alpha1.VolumeGroupSnapshotContent, snapshotterListCredentials map[string]string) (bool, time.Time, error) DeleteGroupSnapshot(content *crdv1alpha1.VolumeGroupSnapshotContent, SnapshotID []string, snapshotterCredentials map[string]string) error } @@ -148,7 +148,7 @@ func makeSnapshotName(prefix, snapshotUID string, snapshotNameUUIDLength int) (s return fmt.Sprintf("%s-%s", prefix, strings.Replace(snapshotUID, "-", "", -1)[0:snapshotNameUUIDLength]), nil } -func (handler *csiHandler) CreateGroupSnapshot(content *crdv1alpha1.VolumeGroupSnapshotContent, volumeIDs []string, parameters map[string]string, snapshotterCredentials map[string]string) (string, string, []*csi.Snapshot, time.Time, bool, error) { +func (handler *csiHandler) CreateGroupSnapshot(content *crdv1alpha1.VolumeGroupSnapshotContent, parameters map[string]string, snapshotterCredentials map[string]string) (string, string, []*csi.Snapshot, time.Time, bool, error) { ctx, cancel := context.WithTimeout(context.Background(), handler.timeout) defer cancel() @@ -156,7 +156,7 @@ func (handler *csiHandler) CreateGroupSnapshot(content *crdv1alpha1.VolumeGroupS return "", "", nil, time.Time{}, false, fmt.Errorf("cannot create group snapshot. Group snapshot content %s not bound to a group snapshot", content.Name) } - if len(volumeIDs) == 0 { + if len(content.Spec.Source.VolumeHandles) == 0 { return "", "", nil, time.Time{}, false, fmt.Errorf("cannot create group snapshot. PVCs to be snapshotted not found in group snapshot content %s", content.Name) } @@ -164,7 +164,7 @@ func (handler *csiHandler) CreateGroupSnapshot(content *crdv1alpha1.VolumeGroupS if err != nil { return "", "", nil, time.Time{}, false, err } - return handler.groupSnapshotter.CreateGroupSnapshot(ctx, groupSnapshotName, volumeIDs, parameters, snapshotterCredentials) + return handler.groupSnapshotter.CreateGroupSnapshot(ctx, groupSnapshotName, content.Spec.Source.VolumeHandles, parameters, snapshotterCredentials) } func (handler *csiHandler) DeleteGroupSnapshot(content *crdv1alpha1.VolumeGroupSnapshotContent, snapshotIDs []string, snapshotterCredentials map[string]string) error { @@ -179,8 +179,8 @@ func (handler *csiHandler) DeleteGroupSnapshot(content *crdv1alpha1.VolumeGroupS if content.Status != nil && content.Status.VolumeGroupSnapshotHandle != nil { groupSnapshotHandle = *content.Status.VolumeGroupSnapshotHandle - } else if content.Spec.Source.VolumeGroupSnapshotHandle != nil { - groupSnapshotHandle = *content.Spec.Source.VolumeGroupSnapshotHandle + } else if content.Spec.Source.GroupSnapshotHandles != nil { + groupSnapshotHandle = content.Spec.Source.GroupSnapshotHandles.VolumeGroupSnapshotHandle } else { return fmt.Errorf("failed to delete group snapshot content %s: groupsnapshotHandle is missing", content.Name) } @@ -196,8 +196,8 @@ func (handler *csiHandler) GetGroupSnapshotStatus(groupSnapshotContent *crdv1alp var err error if groupSnapshotContent.Status != nil && groupSnapshotContent.Status.VolumeGroupSnapshotHandle != nil { groupSnapshotHandle = *groupSnapshotContent.Status.VolumeGroupSnapshotHandle - } else if groupSnapshotContent.Spec.Source.VolumeGroupSnapshotHandle != nil { - groupSnapshotHandle = *groupSnapshotContent.Spec.Source.VolumeGroupSnapshotHandle + } else if groupSnapshotContent.Spec.Source.GroupSnapshotHandles != nil { + groupSnapshotHandle = groupSnapshotContent.Spec.Source.GroupSnapshotHandles.VolumeGroupSnapshotHandle } else { return false, time.Time{}, fmt.Errorf("failed to list group snapshot for group snapshot content %s: groupSnapshotHandle is missing", groupSnapshotContent.Name) } diff --git a/pkg/sidecar-controller/groupsnapshot_helper.go b/pkg/sidecar-controller/groupsnapshot_helper.go index 286eb96a..bbe6d04b 100644 --- a/pkg/sidecar-controller/groupsnapshot_helper.go +++ b/pkg/sidecar-controller/groupsnapshot_helper.go @@ -182,7 +182,7 @@ func (ctrl *csiSnapshotSideCarController) syncGroupSnapshotContent(groupSnapshot return ctrl.removeGroupSnapshotContentFinalizer(groupSnapshotContent) } - if len(groupSnapshotContent.Spec.Source.PersistentVolumeNames) != 0 && groupSnapshotContent.Status == nil { + if len(groupSnapshotContent.Spec.Source.VolumeHandles) != 0 && groupSnapshotContent.Status == nil { klog.V(5).Infof("syncGroupSnapshotContent: Call CreateGroupSnapshot for group snapshot content %s", groupSnapshotContent.Name) return ctrl.createGroupSnapshot(groupSnapshotContent) } @@ -331,7 +331,7 @@ func (ctrl *csiSnapshotSideCarController) shouldDeleteGroupSnapshotContent(group } // 1) shouldDeleteGroupSnapshot returns true if a content is not bound // (VolumeGroupSnapshotRef == "") for pre-provisioned snapshot - if groupSnapshotContent.Spec.Source.VolumeGroupSnapshotHandle != nil && groupSnapshotContent.Spec.VolumeGroupSnapshotRef.UID == "" { + if groupSnapshotContent.Spec.Source.GroupSnapshotHandles != nil && groupSnapshotContent.Spec.VolumeGroupSnapshotRef.UID == "" { return true } @@ -401,8 +401,7 @@ func (ctrl *csiSnapshotSideCarController) createGroupSnapshotWrapper(groupSnapsh parameters[utils.PrefixedVolumeGroupSnapshotContentNameKey] = groupSnapshotContent.Name } - volumeIDs, uuidMap, err := ctrl.getGroupSnapshotVolumeIDs(groupSnapshotContent) - driverName, groupSnapshotID, snapshots, creationTime, readyToUse, err := ctrl.handler.CreateGroupSnapshot(groupSnapshotContent, volumeIDs, parameters, snapshotterCredentials) + driverName, groupSnapshotID, snapshots, creationTime, readyToUse, err := ctrl.handler.CreateGroupSnapshot(groupSnapshotContent, parameters, snapshotterCredentials) if err != nil { // NOTE(xyang): handle create timeout // If it is a final error, remove annotation to indicate @@ -415,7 +414,7 @@ func (ctrl *csiSnapshotSideCarController) createGroupSnapshotWrapper(groupSnapsh } } - return groupSnapshotContent, fmt.Errorf("failed to take group snapshot of the volumes %s: %q", groupSnapshotContent.Spec.Source.PersistentVolumeNames, err) + return groupSnapshotContent, fmt.Errorf("failed to take group snapshot of the volumes %s: %q", groupSnapshotContent.Spec.Source.VolumeHandles, err) } klog.V(5).Infof("Created group snapshot: driver %s, groupSnapshotId %s, creationTime %v, readyToUse %t", driverName, groupSnapshotID, creationTime, readyToUse) @@ -427,12 +426,14 @@ func (ctrl *csiSnapshotSideCarController) createGroupSnapshotWrapper(groupSnapsh // Create individual snapshots and snapshot contents var snapshotContentNames []string for _, snapshot := range snapshots { - uuid, ok := uuidMap[snapshot.SourceVolumeId] - if !ok { - continue - } - volumeSnapshotContentName := GetSnapshotContentNameForVolumeGroupSnapshotContent(string(groupSnapshotContent.UID), uuid) - volumeSnapshotName := GetSnapshotNameForVolumeGroupSnapshotContent(string(groupSnapshotContent.UID), uuid) + /* + uuid, ok := uuidMap[snapshot.SourceVolumeId] + if !ok { + continue + } + */ + volumeSnapshotContentName := GetSnapshotContentNameForVolumeGroupSnapshotContent(string(groupSnapshotContent.UID), snapshot.SourceVolumeId) + volumeSnapshotName := GetSnapshotNameForVolumeGroupSnapshotContent(string(groupSnapshotContent.UID), snapshot.SourceVolumeId) volumeSnapshotNamespace := groupSnapshotContent.Spec.VolumeGroupSnapshotRef.Namespace volumeSnapshotContent := &crdv1.VolumeSnapshotContent{ ObjectMeta: metav1.ObjectMeta{ @@ -514,7 +515,7 @@ func (ctrl *csiSnapshotSideCarController) getCSIGroupSnapshotInput(groupSnapshot } } else { // If dynamic provisioning, return failure if no group snapshot class - if len(groupSnapshotContent.Spec.Source.PersistentVolumeNames) != 0 { + if len(groupSnapshotContent.Spec.Source.VolumeHandles) != 0 { klog.Errorf("failed to getCSISnapshotInput %s without a group snapshot class", groupSnapshotContent.Name) return nil, nil, fmt.Errorf("failed to take group snapshot %s without a group snapshot class", groupSnapshotContent.Name) } @@ -582,25 +583,6 @@ func (ctrl *csiSnapshotSideCarController) setAnnVolumeGroupSnapshotBeingCreated( return groupSnapshotContent, nil } -func (ctrl *csiSnapshotSideCarController) getGroupSnapshotVolumeIDs(groupSnapshotContent *crdv1alpha1.VolumeGroupSnapshotContent) ([]string, map[string]string, error) { - // TODO: Get add PV lister - var volumeIDs []string - uuidMap := make(map[string]string) - for _, pvName := range groupSnapshotContent.Spec.Source.PersistentVolumeNames { - pv, err := ctrl.client.CoreV1().PersistentVolumes().Get(context.TODO(), pvName, metav1.GetOptions{}) - if err != nil { - return nil, nil, err - } - if pv.Spec.CSI != nil && pv.Spec.CSI.VolumeHandle != "" { - volumeIDs = append(volumeIDs, pv.Spec.CSI.VolumeHandle) - if pv.Spec.ClaimRef != nil { - uuidMap[pv.Spec.CSI.VolumeHandle] = string(pv.Spec.ClaimRef.UID) - } - } - } - return volumeIDs, uuidMap, nil -} - // removeAnnVolumeGroupSnapshotBeingCreated removes the VolumeGroupSnapshotBeingCreated // annotation from a groupSnapshotContent if there exists one. func (ctrl csiSnapshotSideCarController) removeAnnVolumeGroupSnapshotBeingCreated(groupSnapshotContent *crdv1alpha1.VolumeGroupSnapshotContent) (*crdv1alpha1.VolumeGroupSnapshotContent, error) { @@ -802,7 +784,7 @@ func (ctrl *csiSnapshotSideCarController) checkandUpdateGroupSnapshotContentStat var groupSnapshotID string var snapshotterListCredentials map[string]string - if groupSnapshotContent.Spec.Source.VolumeGroupSnapshotHandle != nil { + if groupSnapshotContent.Spec.Source.GroupSnapshotHandles != nil { klog.V(5).Infof("checkandUpdateGroupSnapshotContentStatusOperation: call GetGroupSnapshotStatus for group snapshot which is pre-bound to group snapshot content [%s]", groupSnapshotContent.Name) if groupSnapshotContent.Spec.VolumeGroupSnapshotClassName != nil { @@ -832,7 +814,7 @@ func (ctrl *csiSnapshotSideCarController) checkandUpdateGroupSnapshotContentStat return groupSnapshotContent, err } driverName = groupSnapshotContent.Spec.Driver - groupSnapshotID = *groupSnapshotContent.Spec.Source.VolumeGroupSnapshotHandle + groupSnapshotID = groupSnapshotContent.Spec.Source.GroupSnapshotHandles.VolumeGroupSnapshotHandle klog.V(5).Infof("checkandUpdateGroupSnapshotContentStatusOperation: driver %s, groupSnapshotId %s, creationTime %v, readyToUse %t", driverName, groupSnapshotID, creationTime, readyToUse) diff --git a/pkg/sidecar-controller/snapshot_controller_base.go b/pkg/sidecar-controller/snapshot_controller_base.go index 44cb8fe2..569049da 100644 --- a/pkg/sidecar-controller/snapshot_controller_base.go +++ b/pkg/sidecar-controller/snapshot_controller_base.go @@ -316,7 +316,7 @@ func (ctrl *csiSnapshotSideCarController) isDriverMatch(object interface{}) bool return true } if content, ok := object.(*crdv1alpha1.VolumeGroupSnapshotContent); ok { - if content.Spec.Source.VolumeGroupSnapshotHandle == nil && len(content.Spec.Source.PersistentVolumeNames) == 0 { + if content.Spec.Source.GroupSnapshotHandles == nil && len(content.Spec.Source.VolumeHandles) == 0 { // Skip this group snapshot content if it does not have a valid source return false } diff --git a/pkg/validation-webhook/groupsnapshot.go b/pkg/validation-webhook/groupsnapshot.go index 05f3344b..15ad917f 100644 --- a/pkg/validation-webhook/groupsnapshot.go +++ b/pkg/validation-webhook/groupsnapshot.go @@ -233,11 +233,11 @@ func checkGroupSnapshotContentImmutableFieldsV1Alpha1(groupSnapcontent, oldGroup source := groupSnapcontent.Spec.Source oldSource := oldGroupSnapcontent.Spec.Source - if !reflect.DeepEqual(source.VolumeGroupSnapshotHandle, oldSource.VolumeGroupSnapshotHandle) { - return fmt.Errorf("Spec.Source.VolumeGroupSnapshotHandle is immutable but was changed from %s to %s", strPtrDereference(oldSource.VolumeGroupSnapshotHandle), strPtrDereference(source.VolumeGroupSnapshotHandle)) + if !reflect.DeepEqual(source.GroupSnapshotHandles, oldSource.GroupSnapshotHandles) { + return fmt.Errorf("Spec.Source.GroupSnapshotHandles is immutable but was changed from %s to %s", oldSource.GroupSnapshotHandles, source.GroupSnapshotHandles) } - if !reflect.DeepEqual(source.PersistentVolumeNames, oldSource.PersistentVolumeNames) { - return fmt.Errorf("Spec.Source.PersistentVolumeNames is immutable but was changed from %v to %v", oldSource.PersistentVolumeNames, source.PersistentVolumeNames) + if !reflect.DeepEqual(source.VolumeHandles, oldSource.VolumeHandles) { + return fmt.Errorf("Spec.Source.VolumeHandles is immutable but was changed from %v to %v", oldSource.VolumeHandles, source.VolumeHandles) } ref := groupSnapcontent.Spec.VolumeGroupSnapshotRef diff --git a/pkg/validation-webhook/groupsnapshot_test.go b/pkg/validation-webhook/groupsnapshot_test.go index 40701c8d..4dfc214c 100644 --- a/pkg/validation-webhook/groupsnapshot_test.go +++ b/pkg/validation-webhook/groupsnapshot_test.go @@ -250,11 +250,15 @@ func TestAdmitVolumeGroupSnapshotContentV1Alpha1(t *testing.T) { volumeHandle := "volumeHandle1" modifiedField := "modified-field" groupSnapshotHandle := "groupsnapshotHandle1" + groupSnapshotHandles := &volumegroupsnapshotv1alpha1.GroupSnapshotHandles{ + VolumeGroupSnapshotHandle: "volumeGroupSnapshotHandle1", + VolumeSnapshotHandles: []string{"volumeSnapshotHandle1", "volumeSnapshotHandle2"}, + } volumeGroupSnapshotClassName := "volume-snapshot-class-1" validContent := &volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContent{ Spec: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContentSpec{ Source: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContentSource{ - VolumeGroupSnapshotHandle: &groupSnapshotHandle, + GroupSnapshotHandles: groupSnapshotHandles, }, VolumeGroupSnapshotRef: core_v1.ObjectReference{ Name: "group-snapshot-ref", @@ -266,8 +270,8 @@ func TestAdmitVolumeGroupSnapshotContentV1Alpha1(t *testing.T) { invalidContent := &volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContent{ Spec: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContentSpec{ Source: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContentSource{ - VolumeGroupSnapshotHandle: &groupSnapshotHandle, - PersistentVolumeNames: []string{volumeHandle}, + GroupSnapshotHandles: groupSnapshotHandles, + VolumeHandles: []string{volumeHandle}, }, VolumeGroupSnapshotRef: core_v1.ObjectReference{ Name: "", @@ -303,7 +307,7 @@ func TestAdmitVolumeGroupSnapshotContentV1Alpha1(t *testing.T) { oldGroupSnapContent: validContent, shouldAdmit: false, operation: v1.Update, - msg: fmt.Sprintf("Spec.Source.PersistentVolumeNames is immutable but was changed from %s to %s", []string{}, []string{volumeHandle}), + msg: fmt.Sprintf("Spec.Source.VolumeHandles is immutable but was changed from %s to %s", []string{}, []string{volumeHandle}), }, { name: "Update: old is valid and new is valid", @@ -355,7 +359,7 @@ func TestAdmitVolumeGroupSnapshotContentV1Alpha1(t *testing.T) { oldGroupSnapContent: invalidContent, shouldAdmit: false, operation: v1.Update, - msg: fmt.Sprintf("Spec.Source.PersistentVolumeNames is immutable but was changed from %s to %s", []string{volumeHandle}, []string{}), + msg: fmt.Sprintf("Spec.Source.VolumeHandles is immutable but was changed from %s to %s", []string{volumeHandle}, []string{}), }, { name: "Update: old is invalid and new is invalid", diff --git a/vendor/github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumegroupsnapshot/v1alpha1/types.go b/vendor/github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumegroupsnapshot/v1alpha1/types.go index 5663b84d..7e27b793 100644 --- a/vendor/github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumegroupsnapshot/v1alpha1/types.go +++ b/vendor/github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumegroupsnapshot/v1alpha1/types.go @@ -348,16 +348,33 @@ type VolumeGroupSnapshotContentStatus struct { // Exactly one of its members must be set. // Members in VolumeGroupSnapshotContentSource are immutable. type VolumeGroupSnapshotContentSource struct { - // PersistentVolumeNames is a list of names of PersistentVolumes to be snapshotted + // VolumeHandles is a list of volume handles on the backend to be snapshotted // together. It is specified for dynamic provisioning of the VolumeGroupSnapshot. // This field is immutable. // +optional - PersistentVolumeNames []string `json:"persistentVolumeNames,omitempty" protobuf:"bytes,1,opt,name=persistentVolumeNames"` + VolumeHandles []string `json:"volumeHandles,omitempty" protobuf:"bytes,1,opt,name=volumeHandles"` + // GroupSnapshotHandles specifies the CSI "group_snapshot_id" of a pre-existing + // group snapshot and a list of CSI "snapshot_id" of pre-existing snapshots + // on the underlying storage system for which a Kubernetes object + // representation was (or should be) created. + // This field is immutable. + // +optional + GroupSnapshotHandles *GroupSnapshotHandles `json:"groupSnapshotHandles,omitempty" protobuf:"bytes,2,opt,name=groupSnapshotHandles"` +} + +type GroupSnapshotHandles struct { // VolumeGroupSnapshotHandle specifies the CSI "group_snapshot_id" of a pre-existing // group snapshot on the underlying storage system for which a Kubernetes object // representation was (or should be) created. // This field is immutable. - // +optional - VolumeGroupSnapshotHandle *string `json:"volumeGroupSnapshotHandle,omitempty" protobuf:"bytes,2,opt,name=volumeGroupSnapshotHandle"` + // Required. + VolumeGroupSnapshotHandle string `json:"volumeGroupSnapshotHandle" protobuf:"bytes,1,opt,name=volumeGroupSnapshotHandle"` + + // VolumeSnapshotHandles is a list of CSI "snapshot_id" of pre-existing + // snapshots on the underlying storage system for which Kubernetes objects + // representation were (or should be) created. + // This field is immutable. + // Required. + VolumeSnapshotHandles []string `json:"volumeSnapshotHandles" protobuf:"bytes,2,opt,name=volumeSnapshotHandles"` } diff --git a/vendor/github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumegroupsnapshot/v1alpha1/zz_generated.deepcopy.go b/vendor/github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumegroupsnapshot/v1alpha1/zz_generated.deepcopy.go index b15906fe..5dc16cda 100644 --- a/vendor/github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumegroupsnapshot/v1alpha1/zz_generated.deepcopy.go +++ b/vendor/github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumegroupsnapshot/v1alpha1/zz_generated.deepcopy.go @@ -2,7 +2,7 @@ // +build !ignore_autogenerated /* -Copyright 2023 The Kubernetes Authors. +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. @@ -27,6 +27,27 @@ import ( runtime "k8s.io/apimachinery/pkg/runtime" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *GroupSnapshotHandles) DeepCopyInto(out *GroupSnapshotHandles) { + *out = *in + if in.VolumeSnapshotHandles != nil { + in, out := &in.VolumeSnapshotHandles, &out.VolumeSnapshotHandles + *out = make([]string, len(*in)) + copy(*out, *in) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GroupSnapshotHandles. +func (in *GroupSnapshotHandles) DeepCopy() *GroupSnapshotHandles { + if in == nil { + return nil + } + out := new(GroupSnapshotHandles) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *VolumeGroupSnapshot) DeepCopyInto(out *VolumeGroupSnapshot) { *out = *in @@ -193,15 +214,15 @@ func (in *VolumeGroupSnapshotContentList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *VolumeGroupSnapshotContentSource) DeepCopyInto(out *VolumeGroupSnapshotContentSource) { *out = *in - if in.PersistentVolumeNames != nil { - in, out := &in.PersistentVolumeNames, &out.PersistentVolumeNames + if in.VolumeHandles != nil { + in, out := &in.VolumeHandles, &out.VolumeHandles *out = make([]string, len(*in)) copy(*out, *in) } - if in.VolumeGroupSnapshotHandle != nil { - in, out := &in.VolumeGroupSnapshotHandle, &out.VolumeGroupSnapshotHandle - *out = new(string) - **out = **in + if in.GroupSnapshotHandles != nil { + in, out := &in.GroupSnapshotHandles, &out.GroupSnapshotHandles + *out = new(GroupSnapshotHandles) + (*in).DeepCopyInto(*out) } return } diff --git a/vendor/github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1/zz_generated.deepcopy.go b/vendor/github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1/zz_generated.deepcopy.go index f1b7f908..a590aef0 100644 --- a/vendor/github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1/zz_generated.deepcopy.go +++ b/vendor/github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1/zz_generated.deepcopy.go @@ -2,7 +2,7 @@ // +build !ignore_autogenerated /* -Copyright 2023 The Kubernetes Authors. +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.