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

Delete individual volume snapshots as part of the group snapshot delete API and prevent users from deleting individual volume snapshots if it is part of an existing group snapshot

Browse Source 
Also revert commit bb29899ca3.
This commit is contained in:
Raunak Pradip Shah
2023-11-07 11:26:37 +05:30
parent 5ca300ec1e
commit 060ee23d6d
13 changed files with 129 additions and 80 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
pkg/sidecar-controller/content_create_test.go
View File

@@ -49,7 +49,7 @@ func TestSyncContent(t *testing.T) {
readyToUse: true,
},
},
expectedListCalls: []listCall{{"sid1-1", map[string]string{}, true, time.Now(), 1, nil}},
expectedListCalls: []listCall{{"sid1-1", map[string]string{}, true, time.Now(), 1, nil, ""}},
expectSuccess: true,
errors: noerrors,
test: testSyncContent,
@@ -78,7 +78,7 @@ func TestSyncContent(t *testing.T) {
size: defaultSize,
},
},
expectedListCalls: []listCall{{"sid1-2", map[string]string{}, true, time.Now(), 1, nil}},
expectedListCalls: []listCall{{"sid1-2", map[string]string{}, true, time.Now(), 1, nil, ""}},
expectSuccess: true,
errors: noerrors,
test: testSyncContent,
@@ -194,7 +194,7 @@ func TestSyncContent(t *testing.T) {
readyToUse: true,
},
},
expectedListCalls: []listCall{{"sid1-6", map[string]string{}, true, time.Now(), 1, nil}},
expectedListCalls: []listCall{{"sid1-6", map[string]string{}, true, time.Now(), 1, nil, ""}},
errors: noerrors,
test: testSyncContent,
},

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

@@ -34,7 +34,7 @@ import (
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, 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)
GetGroupSnapshotStatus(content *crdv1alpha1.VolumeGroupSnapshotContent, snapshotterListCredentials map[string]string) (bool, time.Time, error)
DeleteGroupSnapshot(content *crdv1alpha1.VolumeGroupSnapshotContent, SnapshotID []string, snapshotterCredentials map[string]string) error
@@ -113,7 +113,7 @@ func (handler *csiHandler) DeleteSnapshot(content *crdv1.VolumeSnapshotContent,
return nil
}
func (handler *csiHandler) GetSnapshotStatus(content *crdv1.VolumeSnapshotContent, snapshotterListCredentials map[string]string) (bool, time.Time, int64, error) {
func (handler *csiHandler) GetSnapshotStatus(content *crdv1.VolumeSnapshotContent, snapshotterListCredentials map[string]string) (bool, time.Time, int64, string, error) {
ctx, cancel := context.WithTimeout(context.Background(), handler.timeout)
defer cancel()
@@ -124,15 +124,15 @@ func (handler *csiHandler) GetSnapshotStatus(content *crdv1.VolumeSnapshotConten
} else if content.Spec.Source.SnapshotHandle != nil {
snapshotHandle = *content.Spec.Source.SnapshotHandle
} else {
return false, time.Time{}, 0, fmt.Errorf("failed to list snapshot for content %s: snapshotHandle is missing", content.Name)
return false, time.Time{}, 0, "", fmt.Errorf("failed to list snapshot for content %s: snapshotHandle is missing", content.Name)
}
csiSnapshotStatus, timestamp, size, err := handler.snapshotter.GetSnapshotStatus(ctx, snapshotHandle, snapshotterListCredentials)
csiSnapshotStatus, timestamp, size, groupSnapshotID, err := handler.snapshotter.GetSnapshotStatus(ctx, snapshotHandle, snapshotterListCredentials)
if err != nil {
return false, time.Time{}, 0, fmt.Errorf("failed to list snapshot for content %s: %q", content.Name, err)
return false, time.Time{}, 0, "", fmt.Errorf("failed to list snapshot for content %s: %q", content.Name, err)
}
return csiSnapshotStatus, timestamp, size, nil
return csiSnapshotStatus, timestamp, size, groupSnapshotID, nil
}
func makeSnapshotName(prefix, snapshotUID string, snapshotNameUUIDLength int) (string, error) {

17
pkg/sidecar-controller/framework_test.go
View File

@@ -882,10 +882,11 @@ type listCall struct {
snapshotID string
secrets map[string]string
// information to return
readyToUse bool
createTime time.Time
size int64
err error
readyToUse bool
createTime time.Time
size int64
err error
groupSnapshotID string
}
type deleteCall struct {
@@ -982,10 +983,10 @@ func (f *fakeSnapshotter) DeleteSnapshot(ctx context.Context, snapshotID string,
return call.err
}
func (f *fakeSnapshotter) GetSnapshotStatus(ctx context.Context, snapshotID string, snapshotterListCredentials map[string]string) (bool, time.Time, int64, error) {
func (f *fakeSnapshotter) GetSnapshotStatus(ctx context.Context, snapshotID string, snapshotterListCredentials map[string]string) (bool, time.Time, int64, string, error) {
if f.listCallCounter >= len(f.listCalls) {
f.t.Errorf("Unexpected CSI list Snapshot call: snapshotID=%s, index: %d, calls: %+v", snapshotID, f.createCallCounter, f.createCalls)
return false, time.Time{}, 0, fmt.Errorf("unexpected call")
return false, time.Time{}, 0, "", fmt.Errorf("unexpected call")
}
call := f.listCalls[f.listCallCounter]
f.listCallCounter++
@@ -1002,10 +1003,10 @@ func (f *fakeSnapshotter) GetSnapshotStatus(ctx context.Context, snapshotID stri
}
if err != nil {
return false, time.Time{}, 0, fmt.Errorf("unexpected call")
return false, time.Time{}, 0, "", fmt.Errorf("unexpected call")
}
return call.readyToUse, call.createTime, call.size, call.err
return call.readyToUse, call.createTime, call.size, call.groupSnapshotID, call.err
}
func newSnapshotError(message string) *crdv1.VolumeSnapshotError {

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

@@ -471,7 +471,6 @@ func (ctrl *csiSnapshotSideCarController) createGroupSnapshotWrapper(groupSnapsh
},
},
}
vsc, err := ctrl.clientset.SnapshotV1().VolumeSnapshotContents().Create(context.TODO(), volumeSnapshotContent, metav1.CreateOptions{})
if err != nil {
return groupSnapshotContent, err

27
pkg/sidecar-controller/snapshot_controller.go
View File

@@ -22,13 +22,14 @@ import (
"strings"
"time"
crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1"
"github.com/kubernetes-csi/external-snapshotter/v6/pkg/utils"
codes "google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
klog "k8s.io/klog/v2"
crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1"
"github.com/kubernetes-csi/external-snapshotter/v6/pkg/utils"
)
// Design:
@@ -251,7 +252,7 @@ func (ctrl *csiSnapshotSideCarController) checkandUpdateContentStatusOperation(c
var size int64
readyToUse := false
var driverName string
var snapshotID string
var snapshotID, groupSnapshotID string
var snapshotterListCredentials map[string]string
if content.Spec.Source.SnapshotHandle != nil {
@@ -278,7 +279,7 @@ func (ctrl *csiSnapshotSideCarController) checkandUpdateContentStatusOperation(c
}
}
readyToUse, creationTime, size, err = ctrl.handler.GetSnapshotStatus(content, snapshotterListCredentials)
readyToUse, creationTime, size, groupSnapshotID, err = ctrl.handler.GetSnapshotStatus(content, snapshotterListCredentials)
if err != nil {
klog.Errorf("checkandUpdateContentStatusOperation: failed to call get snapshot status to check whether snapshot is ready to use %q", err)
return content, err
@@ -286,13 +287,13 @@ func (ctrl *csiSnapshotSideCarController) checkandUpdateContentStatusOperation(c
driverName = content.Spec.Driver
snapshotID = *content.Spec.Source.SnapshotHandle
klog.V(5).Infof("checkandUpdateContentStatusOperation: driver %s, snapshotId %s, creationTime %v, size %d, readyToUse %t", driverName, snapshotID, creationTime, size, readyToUse)
klog.V(5).Infof("checkandUpdateContentStatusOperation: driver %s, snapshotId %s, creationTime %v, size %d, readyToUse %t, groupSnapshotID %s", driverName, snapshotID, creationTime, size, readyToUse, groupSnapshotID)
if creationTime.IsZero() {
creationTime = time.Now()
}
updatedContent, err := ctrl.updateSnapshotContentStatus(content, snapshotID, readyToUse, creationTime.UnixNano(), size)
updatedContent, err := ctrl.updateSnapshotContentStatus(content, snapshotID, readyToUse, creationTime.UnixNano(), size, groupSnapshotID)
if err != nil {
return content, err
}
@@ -354,7 +355,7 @@ func (ctrl *csiSnapshotSideCarController) createSnapshotWrapper(content *crdv1.V
creationTime = time.Now()
}
newContent, err := ctrl.updateSnapshotContentStatus(content, snapshotID, readyToUse, creationTime.UnixNano(), size)
newContent, err := ctrl.updateSnapshotContentStatus(content, snapshotID, readyToUse, creationTime.UnixNano(), size, "")
if err != nil {
klog.Errorf("error updating status for volume snapshot content %s: %v.", content.Name, err)
return content, fmt.Errorf("error updating status for volume snapshot content %s: %v", content.Name, err)
@@ -429,8 +430,9 @@ func (ctrl *csiSnapshotSideCarController) updateSnapshotContentStatus(
snapshotHandle string,
readyToUse bool,
createdAt int64,
size int64) (*crdv1.VolumeSnapshotContent, error) {
klog.V(5).Infof("updateSnapshotContentStatus: updating VolumeSnapshotContent [%s], snapshotHandle %s, readyToUse %v, createdAt %v, size %d", content.Name, snapshotHandle, readyToUse, createdAt, size)
size int64,
groupSnapshotID string) (*crdv1.VolumeSnapshotContent, error) {
klog.V(5).Infof("updateSnapshotContentStatus: updating VolumeSnapshotContent [%s], snapshotHandle %s, readyToUse %v, createdAt %v, size %d, groupSnapshotID %s", content.Name, snapshotHandle, readyToUse, createdAt, size, groupSnapshotID)
contentObj, err := ctrl.clientset.SnapshotV1().VolumeSnapshotContents().Get(context.TODO(), content.Name, metav1.GetOptions{})
if err != nil {
@@ -446,6 +448,9 @@ func (ctrl *csiSnapshotSideCarController) updateSnapshotContentStatus(
CreationTime: &createdAt,
RestoreSize: &size,
}
if groupSnapshotID != "" {
newStatus.VolumeGroupSnapshotHandle = &groupSnapshotID
}
updated = true
} else {
newStatus = contentObj.Status.DeepCopy()
@@ -468,6 +473,10 @@ func (ctrl *csiSnapshotSideCarController) updateSnapshotContentStatus(
newStatus.RestoreSize = &size
updated = true
}
if newStatus.VolumeGroupSnapshotHandle == nil && groupSnapshotID != "" {
newStatus.VolumeGroupSnapshotHandle = &groupSnapshotID
updated = true
}
}
if updated {

20
pkg/sidecar-controller/snapshot_delete_test.go
View File

@@ -171,7 +171,7 @@ func TestDeleteSync(t *testing.T) {
readyToUse: true,
},
},
expectedListCalls: []listCall{{"sid1-1", map[string]string{}, true, time.Now(), 1, nil}},
expectedListCalls: []listCall{{"sid1-1", map[string]string{}, true, time.Now(), 1, nil, ""}},
expectedDeleteCalls: []deleteCall{{"sid1-1", nil, nil}},
expectSuccess: true,
test: testSyncContent,
@@ -194,7 +194,7 @@ func TestDeleteSync(t *testing.T) {
readyToUse: true,
},
},
expectedListCalls: []listCall{{"sid1-2", map[string]string{}, true, time.Now(), 1, nil}},
expectedListCalls: []listCall{{"sid1-2", map[string]string{}, true, time.Now(), 1, nil, ""}},
expectedDeleteCalls: []deleteCall{{"sid1-2", nil, nil}},
expectSuccess: true,
test: testSyncContent,
@@ -218,7 +218,7 @@ func TestDeleteSync(t *testing.T) {
},
expectedDeleteCalls: []deleteCall{{"sid1-3", nil, fmt.Errorf("mock csi driver delete error")}},
expectedEvents: []string{"Warning SnapshotDeleteError"},
expectedListCalls: []listCall{{"sid1-3", map[string]string{}, true, time.Now(), 1, nil}},
expectedListCalls: []listCall{{"sid1-3", map[string]string{}, true, time.Now(), 1, nil, ""}},
test: testSyncContent,
},
{
@@ -238,7 +238,7 @@ func TestDeleteSync(t *testing.T) {
name: "1-5 - csi driver delete snapshot returns error, bound finalizer should remain",
initialContents: newContentArrayWithDeletionTimestamp("content1-5", "sid1-5", "snap1-5", "sid1-5", validSecretClass, "", "snap1-5-volumehandle", deletionPolicy, nil, &defaultSize, true, &timeNowMetav1),
expectedContents: newContentArrayWithDeletionTimestamp("content1-5", "sid1-5", "snap1-5", "sid1-5", validSecretClass, "", "snap1-5-volumehandle", deletionPolicy, nil, &defaultSize, true, &timeNowMetav1),
expectedListCalls: []listCall{{"sid1-5", map[string]string{}, true, time.Now(), 1000, nil}},
expectedListCalls: []listCall{{"sid1-5", map[string]string{}, true, time.Now(), 1000, nil, ""}},
expectedDeleteCalls: []deleteCall{{"sid1-5", nil, errors.New("mock csi driver delete error")}},
expectedEvents: []string{"Warning SnapshotDeleteError"},
errors: noerrors,
@@ -249,7 +249,7 @@ func TestDeleteSync(t *testing.T) {
name: "1-6 - content is deleted before deleting",
initialContents: newContentArray("content1-6", "sid1-6", "snap1-6", "sid1-6", classGold, "sid1-6", "", deletionPolicy, nil, nil, true),
expectedContents: nocontents,
expectedListCalls: []listCall{{"sid1-6", nil, false, time.Now(), 0, nil}},
expectedListCalls: []listCall{{"sid1-6", nil, false, time.Now(), 0, nil, ""}},
expectedDeleteCalls: []deleteCall{{"sid1-6", map[string]string{"foo": "bar"}, nil}},
expectedEvents: noevents,
errors: noerrors,
@@ -265,7 +265,7 @@ func TestDeleteSync(t *testing.T) {
initialContents: newContentArrayWithReadyToUse("content1-7", "", "snap1-7", "sid1-7", validSecretClass, "sid1-7", "", deletePolicy, nil, &defaultSize, &True, true),
expectedContents: newContentArrayWithReadyToUse("content1-7", "", "snap1-7", "sid1-7", validSecretClass, "sid1-7", "", deletePolicy, nil, &defaultSize, &True, true),
expectedEvents: noevents,
expectedListCalls: []listCall{{"sid1-7", map[string]string{}, true, time.Now(), 1000, nil}},
expectedListCalls: []listCall{{"sid1-7", map[string]string{}, true, time.Now(), 1000, nil, ""}},
expectSuccess: true,
initialSecrets: []*v1.Secret{secret()},
errors: noerrors,
@@ -276,7 +276,7 @@ func TestDeleteSync(t *testing.T) {
initialContents: newContentArrayWithReadyToUse("content1-8", "sid1-8", "none-existed-snapshot", "sid1-8", validSecretClass, "sid1-8", "", retainPolicy, nil, &defaultSize, &True, true),
expectedContents: newContentArrayWithReadyToUse("content1-8", "sid1-8", "none-existed-snapshot", "sid1-8", validSecretClass, "sid1-8", "", retainPolicy, nil, &defaultSize, &True, true),
expectedEvents: noevents,
expectedListCalls: []listCall{{"sid1-8", map[string]string{}, true, time.Now(), 0, nil}},
expectedListCalls: []listCall{{"sid1-8", map[string]string{}, true, time.Now(), 0, nil, ""}},
expectSuccess: true,
errors: noerrors,
test: testSyncContent,
@@ -286,7 +286,7 @@ func TestDeleteSync(t *testing.T) {
initialContents: newContentArrayWithDeletionTimestamp("content1-9", "sid1-9", "snap1-9", "sid1-9", emptySecretClass, "", "snap1-9-volumehandle", deletePolicy, nil, &defaultSize, true, &nonFractionalTime),
expectedContents: newContentArrayWithDeletionTimestamp("content1-9", "sid1-9", "snap1-9", "", emptySecretClass, "", "snap1-9-volumehandle", deletePolicy, nil, &defaultSize, false, &nonFractionalTime),
expectedEvents: noevents,
expectedListCalls: []listCall{{"sid1-9", map[string]string{}, true, time.Now(), 0, nil}},
expectedListCalls: []listCall{{"sid1-9", map[string]string{}, true, time.Now(), 0, nil, ""}},
expectSuccess: true,
errors: noerrors,
initialSecrets: []*v1.Secret{}, // secret does not exist
@@ -298,7 +298,7 @@ func TestDeleteSync(t *testing.T) {
initialContents: newContentArrayWithDeletionTimestamp("content1-10", "sid1-10", "snap1-10", "sid1-10", emptySecretClass, "", "snap1-10-volumehandle", retainPolicy, nil, &defaultSize, true, &nonFractionalTime),
expectedContents: newContentArrayWithDeletionTimestamp("content1-10", "sid1-10", "snap1-10", "sid1-10", emptySecretClass, "", "snap1-10-volumehandle", retainPolicy, nil, &defaultSize, false, &nonFractionalTime),
expectedEvents: noevents,
expectedListCalls: []listCall{{"sid1-10", map[string]string{}, true, time.Now(), 0, nil}},
expectedListCalls: []listCall{{"sid1-10", map[string]string{}, true, time.Now(), 0, nil, ""}},
expectSuccess: true,
errors: noerrors,
initialSecrets: []*v1.Secret{},
@@ -319,7 +319,7 @@ func TestDeleteSync(t *testing.T) {
initialContents: newContentArrayWithDeletionTimestamp("content1-12", "sid1-12", "snap1-12", "sid1-12", emptySecretClass, "sid1-12", "", retainPolicy, nil, &defaultSize, true, &nonFractionalTime),
expectedContents: newContentArrayWithDeletionTimestamp("content1-12", "sid1-12", "snap1-12", "sid1-12", emptySecretClass, "sid1-12", "", retainPolicy, nil, &defaultSize, false, &nonFractionalTime),
expectedEvents: noevents,
expectedListCalls: []listCall{{"sid1-12", map[string]string{}, true, time.Now(), 0, nil}},
expectedListCalls: []listCall{{"sid1-12", map[string]string{}, true, time.Now(), 0, nil, ""}},
expectSuccess: true,
errors: noerrors,
initialSecrets: []*v1.Secret{},