alexlebens/external-snapshotter
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

update API names in group snapshot content to be consistent with volume snapshots

Browse Source
This commit is contained in:
Raunak Pradip Shah
2024-01-05 11:29:03 -08:00
parent bffa5ae25f
commit 0bd355cf29
14 changed files with 169 additions and 126 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
pkg/common-controller/groupsnapshot_controller_helper.go
View File

@@ -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

16
pkg/sidecar-controller/csi_handler.go
View File

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

48
pkg/sidecar-controller/groupsnapshot_helper.go
View File

@@ -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)

2
pkg/sidecar-controller/snapshot_controller_base.go
View File

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

8
pkg/validation-webhook/groupsnapshot.go
View File

@@ -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

14
pkg/validation-webhook/groupsnapshot_test.go
View File

@@ -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",