Skip to content

Commit

Permalink
Add more sidecar tests and cleanup sidecar code
Browse files Browse the repository at this point in the history 
Signed-off-by: Grant Griffiths <[email protected]>
  • Loading branch information
@ggriffiths
ggriffiths committed Mar 5, 2020
1 parent 3dd4e07 commit fe1c355
Show file tree
Hide file tree
Showing 4 changed files with 210 additions and 47 deletions.
174 changes: 155 additions & 19 deletions pkg/sidecar-controller/content_create_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,32 +17,168 @@ limitations under the License.
package sidecar_controller

import (
"errors"
"testing"
"time"

crdv1 "github.com/kubernetes-csi/external-snapshotter/v2/pkg/apis/volumesnapshot/v1beta1"
"github.com/kubernetes-csi/external-snapshotter/v2/pkg/utils"
v1 "k8s.io/api/core/v1"
)

func TestSyncContent(t *testing.T) {
var tests []controllerTest

tests = append(tests, controllerTest{
name: "Basic content update ready to use",
initialContents: newContentArrayWithReadyToUse("content1-1", "snapuid1-1", "snap1-1", "sid1-1", defaultClass, "", "volume-handle-1-1", retainPolicy, nil, &defaultSize, &False, true),
expectedContents: newContentArrayWithReadyToUse("content1-1", "snapuid1-1", "snap1-1", "sid1-1", defaultClass, "", "volume-handle-1-1", retainPolicy, nil, &defaultSize, &True, true),
expectedEvents: noevents,
expectedCreateCalls: []createCall{
{
volumeHandle: "volume-handle-1-1",
snapshotName: "snapshot-snapuid1-1",
driverName: mockDriverName,
snapshotId: "snapuid1-1",
creationTime: timeNow,
readyToUse: true,

tests := []controllerTest{
{
name: "1-1: Basic content update ready to use",
initialContents: newContentArrayWithReadyToUse("content1-1", "snapuid1-1", "snap1-1", "sid1-1", defaultClass, "", "volume-handle-1-1", retainPolicy, nil, &defaultSize, &False, true),
expectedContents: newContentArrayWithReadyToUse("content1-1", "snapuid1-1", "snap1-1", "sid1-1", defaultClass, "", "volume-handle-1-1", retainPolicy, nil, &defaultSize, &True, true),
expectedEvents: noevents,
expectedCreateCalls: []createCall{
{
volumeHandle: "volume-handle-1-1",
snapshotName: "snapshot-snapuid1-1",
driverName: mockDriverName,
snapshotId: "snapuid1-1",
creationTime: timeNow,
readyToUse: true,
},
},
expectedListCalls: []listCall{{"sid1-1", map[string]string{}, true, time.Now(), 1, nil}},
errors: noerrors,
test: testSyncContent,
},
{
name: "1-2: Basic sync content create snapshot",
initialContents: withContentStatus(newContentArray("content1-2", "snapuid1-2", "snap1-2", "sid1-2", defaultClass, "", "volume-handle-1-2", retainPolicy, nil, &defaultSize, true),
nil),
expectedContents: withContentStatus(newContentArray("content1-2", "snapuid1-2", "snap1-2", "sid1-2", defaultClass, "", "volume-handle-1-2", retainPolicy, nil, &defaultSize, true),
&crdv1.VolumeSnapshotContentStatus{SnapshotHandle: toStringPointer("snapuid1-2"), RestoreSize: &defaultSize, ReadyToUse: &True}),
expectedEvents: noevents,
expectedCreateCalls: []createCall{
{
volumeHandle: "volume-handle-1-2",
snapshotName: "snapshot-snapuid1-2",
driverName: mockDriverName,
snapshotId: "snapuid1-2",
creationTime: timeNow,
readyToUse: true,
size: defaultSize,
},
},
expectedListCalls: []listCall{{"sid1-2", map[string]string{}, true, time.Now(), 1, nil}},
errors: noerrors,
test: testSyncContent,
},
{
name: "1-3: Basic sync content create snapshot with non-existent secret",
initialContents: withContentAnnotations(withContentStatus(newContentArray("content1-3", "snapuid1-3", "snap1-3", "sid1-3", invalidSecretClass, "", "volume-handle-1-3", retainPolicy, nil, &defaultSize, true),
nil), map[string]string{
utils.AnnDeletionSecretRefName: "",
utils.AnnDeletionSecretRefNamespace: "",
}),
expectedContents: withContentAnnotations(withContentStatus(newContentArray("content1-3", "snapuid1-3", "snap1-3", "sid1-3", invalidSecretClass, "", "volume-handle-1-3", retainPolicy, nil, &defaultSize, true),
&crdv1.VolumeSnapshotContentStatus{
SnapshotHandle: nil,
RestoreSize: nil,
ReadyToUse: &False,
Error: newSnapshotError("Failed to create snapshot: failed to get input parameters to create snapshot for content content1-3: \"cannot retrieve secrets for snapshot content \\\"content1-3\\\", err: secret name or namespace not specified\""),
}), map[string]string{
utils.AnnDeletionSecretRefName: "",
utils.AnnDeletionSecretRefNamespace: "",
}), initialSecrets: []*v1.Secret{}, // no initial secret created
expectedEvents: []string{"Warning SnapshotCreationFailed"},
errors: noerrors,
test: testSyncContent,
},
{
name: "1-4: Basic sync content create snapshot with valid secret",
initialContents: withContentAnnotations(withContentStatus(newContentArray("content1-4", "snapuid1-4", "snap1-4", "sid1-4", validSecretClass, "", "volume-handle-1-4", retainPolicy, nil, &defaultSize, true),
nil), map[string]string{
utils.AnnDeletionSecretRefName: "secret",
utils.AnnDeletionSecretRefNamespace: "default",
}),
expectedContents: withContentAnnotations(withContentStatus(newContentArray("content1-4", "snapuid1-4", "snap1-4", "sid1-4", validSecretClass, "", "volume-handle-1-4", retainPolicy, nil, &defaultSize, true),
&crdv1.VolumeSnapshotContentStatus{
SnapshotHandle: toStringPointer("snapuid1-4"),
RestoreSize: &defaultSize,
ReadyToUse: &True,
Error: nil,
}), map[string]string{
utils.AnnDeletionSecretRefName: "secret",
utils.AnnDeletionSecretRefNamespace: "default",
}),
expectedCreateCalls: []createCall{
{
volumeHandle: "volume-handle-1-4",
snapshotName: "snapshot-snapuid1-4",
parameters: class5Parameters,
secrets: map[string]string{
"foo": "bar",
},
driverName: mockDriverName,
snapshotId: "snapuid1-4",
creationTime: timeNow,
readyToUse: true,
size: defaultSize,
},
},
initialSecrets: []*v1.Secret{secret()},
expectedEvents: noevents,
errors: noerrors,
test: testSyncContent,
},
{
name: "1-5: Basic sync content create snapshot with failed secret call",
initialContents: withContentAnnotations(withContentStatus(newContentArray("content1-5", "snapuid1-5", "snap1-5", "sid1-5", invalidSecretClass, "", "volume-handle-1-5", retainPolicy, nil, &defaultSize, true),
nil), map[string]string{
utils.AnnDeletionSecretRefName: "secret",
utils.AnnDeletionSecretRefNamespace: "default",
}),
expectedContents: withContentAnnotations(withContentStatus(newContentArray("content1-5", "snapuid1-5", "snap1-5", "sid1-5", invalidSecretClass, "", "volume-handle-1-5", retainPolicy, nil, &defaultSize, true),
&crdv1.VolumeSnapshotContentStatus{
SnapshotHandle: nil,
RestoreSize: nil,
ReadyToUse: &False,
Error: newSnapshotError("Failed to create snapshot: failed to get input parameters to create snapshot for content content1-5: \"cannot get credentials for snapshot content \\\"content1-5\\\"\""),
}), map[string]string{
utils.AnnDeletionSecretRefName: "secret",
utils.AnnDeletionSecretRefNamespace: "default",
}), initialSecrets: []*v1.Secret{}, // no initial secret created
expectedEvents: []string{"Warning SnapshotCreationFailed"},
errors: []reactorError{
// Inject error to the first client.VolumesnapshotV1beta1().VolumeSnapshots().Update call.
// All other calls will succeed.
{"get", "secrets", errors.New("mock secrets error")},
},
test: testSyncContent,
},
{
name: "1-6: Basic content update ready to use bad snapshot class",
initialContents: newContentArrayWithReadyToUse("content1-6", "snapuid1-6", "snap1-6", "sid1-6", "bad-class", "", "volume-handle-1-6", retainPolicy, nil, &defaultSize, &False, true),
expectedContents: withContentStatus(newContentArray("content1-6", "snapuid1-6", "snap1-6", "sid1-6", "bad-class", "", "volume-handle-1-6", retainPolicy, nil, &defaultSize, true),
&crdv1.VolumeSnapshotContentStatus{
SnapshotHandle: toStringPointer("sid1-6"),
RestoreSize: &defaultSize,
ReadyToUse: &False,
Error: newSnapshotError("Failed to check and update snapshot content: failed to get input parameters to create snapshot content1-6: \"failed to retrieve snapshot class bad-class from the informer: \\\"volumesnapshotclass.snapshot.storage.k8s.io \\\\\\\"bad-class\\\\\\\" not found\\\"\""),
}),
expectedEvents: []string{"Warning SnapshotContentCheckandUpdateFailed"},
expectedCreateCalls: []createCall{
{
volumeHandle: "volume-handle-1-6",
snapshotName: "snapshot-snapuid1-6",
driverName: mockDriverName,
snapshotId: "snapuid1-6",
creationTime: timeNow,
readyToUse: true,
},
},
expectedListCalls: []listCall{{"sid1-6", map[string]string{}, true, time.Now(), 1, nil}},
errors: noerrors,
test: testSyncContent,
},
expectedListCalls: []listCall{{"sid1-1", map[string]string{}, true, time.Now(), 1, nil}},
errors: noerrors,
test: testSyncContent,
})
}

runSyncContentTests(t, tests, snapshotClasses)
}
56 changes: 43 additions & 13 deletions pkg/sidecar-controller/framework_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ var noerrors = []reactorError{}
// the list.
type snapshotReactor struct {
secrets map[string]*v1.Secret
snapshotClasses map[string]*crdv1.VolumeSnapshotClass
contents map[string]*crdv1.VolumeSnapshotContent
changedObjects []interface{}
changedSinceLastSync int
Expand Down Expand Up @@ -283,7 +284,12 @@ func (r *snapshotReactor) checkContents(expectedContents []*crdv1.VolumeSnapshot
v := v.DeepCopy()
v.ResourceVersion = ""
v.Spec.VolumeSnapshotRef.ResourceVersion = ""
v.Status.CreationTime = nil
if v.Status != nil {
v.Status.CreationTime = nil
}
if v.Status.Error != nil {
v.Status.Error.Time = &metav1.Time{}
}
expectedMap[v.Name] = v
}
for _, v := range r.contents {
Expand All @@ -292,7 +298,13 @@ func (r *snapshotReactor) checkContents(expectedContents []*crdv1.VolumeSnapshot
v := v.DeepCopy()
v.ResourceVersion = ""
v.Spec.VolumeSnapshotRef.ResourceVersion = ""
v.Status.CreationTime = nil
if v.Status != nil {
v.Status.CreationTime = nil
if v.Status.Error != nil {
v.Status.Error.Time = &metav1.Time{}
}
}

gotMap[v.Name] = v
}
if !reflect.DeepEqual(expectedMap, gotMap) {
Expand Down Expand Up @@ -487,6 +499,7 @@ func (r *snapshotReactor) modifyContentEvent(content *crdv1.VolumeSnapshotConten
func newSnapshotReactor(kubeClient *kubefake.Clientset, client *fake.Clientset, ctrl *csiSnapshotSideCarController, fakeVolumeWatch, fakeClaimWatch *watch.FakeWatcher, errors []reactorError) *snapshotReactor {
reactor := &snapshotReactor{
secrets: make(map[string]*v1.Secret),
snapshotClasses: make(map[string]*crdv1.VolumeSnapshotClass),
contents: make(map[string]*crdv1.VolumeSnapshotContent),
ctrl: ctrl,
fakeContentWatch: fakeVolumeWatch,
Expand All @@ -497,6 +510,7 @@ func newSnapshotReactor(kubeClient *kubefake.Clientset, client *fake.Clientset,
client.AddReactor("update", "volumesnapshotcontents", reactor.React)
client.AddReactor("get", "volumesnapshotcontents", reactor.React)
client.AddReactor("delete", "volumesnapshotcontents", reactor.React)
kubeClient.AddReactor("get", "secrets", reactor.React)

return reactor
}
Expand Down Expand Up @@ -615,16 +629,6 @@ func newContentArrayWithReadyToUse(contentName, boundToSnapshotUID, boundToSnaps
}
}

func newContentWithUnmatchDriverArray(contentName, boundToSnapshotUID, boundToSnapshotName, snapshotHandle, snapshotClassName, desiredSnapshotHandle, volumeHandle string,
deletionPolicy crdv1.DeletionPolicy, size, creationTime *int64,
withFinalizer bool) []*crdv1.VolumeSnapshotContent {
content := newContent(contentName, boundToSnapshotUID, boundToSnapshotName, snapshotHandle, snapshotClassName, desiredSnapshotHandle, volumeHandle, deletionPolicy, size, creationTime, withFinalizer, nil)
content.Spec.Driver = "fake"
return []*crdv1.VolumeSnapshotContent{
content,
}
}

func newContentArrayWithDeletionTimestamp(contentName, boundToSnapshotUID, boundToSnapshotName, snapshotHandle, snapshotClassName, desiredSnapshotHandle, volumeHandle string,
deletionPolicy crdv1.DeletionPolicy, size, creationTime *int64,
withFinalizer bool, deletionTime *metav1.Time) []*crdv1.VolumeSnapshotContent {
Expand All @@ -633,6 +637,22 @@ func newContentArrayWithDeletionTimestamp(contentName, boundToSnapshotUID, bound
}
}

func withContentStatus(content []*crdv1.VolumeSnapshotContent, status *crdv1.VolumeSnapshotContentStatus) []*crdv1.VolumeSnapshotContent {
for i := range content {
content[i].Status = status
}

return content
}

func withContentAnnotations(content []*crdv1.VolumeSnapshotContent, annotations map[string]string) []*crdv1.VolumeSnapshotContent {
for i := range content {
content[i].ObjectMeta.Annotations = annotations
}

return content
}

func testSyncContent(ctrl *csiSnapshotSideCarController, reactor *snapshotReactor, test controllerTest) error {
return ctrl.syncContent(test.initialContents[0])
}
Expand Down Expand Up @@ -742,6 +762,7 @@ func runSyncContentTests(t *testing.T, tests []controllerTest, snapshotClasses [
indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{})
for _, class := range snapshotClasses {
indexer.Add(class)
reactor.snapshotClasses[class.Name] = class
}
ctrl.classLister = storagelisters.NewVolumeSnapshotClassLister(indexer)

Expand Down Expand Up @@ -875,7 +896,7 @@ func (f *fakeSnapshotter) CreateSnapshot(ctx context.Context, snapshotName strin
err = fmt.Errorf("unexpected create snapshot call")
}

if !reflect.DeepEqual(call.secrets, snapshotterCredentials) {
if !reflect.DeepEqual(call.secrets, snapshotterCredentials) && !(len(call.secrets) == 0 && len(snapshotterCredentials) == 0) {
f.t.Errorf("Wrong CSI Create Snapshot call: snapshotName=%s, volumeHandle=%s, expected secrets %+v, got %+v", snapshotName, volumeHandle, call.secrets, snapshotterCredentials)
err = fmt.Errorf("unexpected create snapshot call")
}
Expand Down Expand Up @@ -937,3 +958,12 @@ func (f *fakeSnapshotter) GetSnapshotStatus(ctx context.Context, snapshotID stri

return call.readyToUse, call.createTime, call.size, call.err
}

func newSnapshotError(message string) *crdv1.VolumeSnapshotError {
return &crdv1.VolumeSnapshotError{
Time: &metav1.Time{},
Message: &message,
}
}

func toStringPointer(str string) *string { return &str }
17 changes: 4 additions & 13 deletions pkg/sidecar-controller/snapshot_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ const controllerUpdateFailMsg = "snapshot controller failed to update"
func (ctrl *csiSnapshotSideCarController) syncContent(content *crdv1.VolumeSnapshotContent) error {
klog.V(5).Infof("synchronizing VolumeSnapshotContent[%s]", content.Name)

var err error
if ctrl.shouldDelete(content) {
klog.V(4).Infof("VolumeSnapshotContent[%s]: the policy is %s", content.Name, content.Spec.DeletionPolicy)
if content.Spec.DeletionPolicy == crdv1.VolumeSnapshotContentDelete &&
Expand All @@ -75,10 +74,7 @@ func (ctrl *csiSnapshotSideCarController) syncContent(content *crdv1.VolumeSnaps
} else {
if content.Spec.Source.VolumeHandle != nil && content.Status == nil {
klog.V(5).Infof("syncContent: Call CreateSnapshot for content %s", content.Name)
if err = ctrl.createSnapshot(content); err != nil {
ctrl.updateContentErrorStatusWithEvent(content, v1.EventTypeWarning, "SnapshotCreationFailed", fmt.Sprintf("Failed to create snapshot with error %v", err))
return err
}
ctrl.createSnapshot(content)
} else {
// Skip checkandUpdateContentStatus() if ReadyToUse is
// already true. We don't want to keep calling CreateSnapshot
Expand All @@ -87,10 +83,7 @@ func (ctrl *csiSnapshotSideCarController) syncContent(content *crdv1.VolumeSnaps
if content.Status != nil && content.Status.ReadyToUse != nil && *content.Status.ReadyToUse == true {
return nil
}
if err = ctrl.checkandUpdateContentStatus(content); err != nil {
ctrl.updateContentErrorStatusWithEvent(content, v1.EventTypeWarning, "SnapshotContentStatusUpdateFailed", fmt.Sprintf("Failed to update snapshot content status with error %v", err))
return err
}
ctrl.checkandUpdateContentStatus(content)
}
}
return nil
Expand Down Expand Up @@ -129,7 +122,7 @@ func (ctrl *csiSnapshotSideCarController) storeContentUpdate(content interface{}
}

// createSnapshot starts new asynchronous operation to create snapshot
func (ctrl *csiSnapshotSideCarController) createSnapshot(content *crdv1.VolumeSnapshotContent) error {
func (ctrl *csiSnapshotSideCarController) createSnapshot(content *crdv1.VolumeSnapshotContent) {
klog.V(5).Infof("createSnapshot for content [%s]: started", content.Name)
opName := fmt.Sprintf("create-%s", content.Name)
ctrl.scheduleOperation(opName, func() error {
Expand All @@ -147,10 +140,9 @@ func (ctrl *csiSnapshotSideCarController) createSnapshot(content *crdv1.VolumeSn
}
return nil
})
return nil
}

func (ctrl *csiSnapshotSideCarController) checkandUpdateContentStatus(content *crdv1.VolumeSnapshotContent) error {
func (ctrl *csiSnapshotSideCarController) checkandUpdateContentStatus(content *crdv1.VolumeSnapshotContent) {
klog.V(5).Infof("checkandUpdateContentStatus[%s] started", content.Name)
opName := fmt.Sprintf("check-%s", content.Name)
ctrl.scheduleOperation(opName, func() error {
Expand All @@ -168,7 +160,6 @@ func (ctrl *csiSnapshotSideCarController) checkandUpdateContentStatus(content *c

return nil
})
return nil
}

// updateContentStatusWithEvent saves new content.Status to API server and emits
Expand Down
10 changes: 8 additions & 2 deletions pkg/sidecar-controller/snapshot_delete_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@ var class6Parameters = map[string]string{
utils.PrefixedSnapshotterListSecretNamespaceKey: "default",
}

var class7Annotations = map[string]string{
utils.AnnDeletionSecretRefName: "secret-x",
utils.AnnDeletionSecretRefNamespace: "default-x",
}

var snapshotClasses = []*crdv1.VolumeSnapshotClass{
{
TypeMeta: metav1.TypeMeta{
Expand Down Expand Up @@ -107,10 +112,11 @@ var snapshotClasses = []*crdv1.VolumeSnapshotClass{
Kind: "VolumeSnapshotClass",
},
ObjectMeta: metav1.ObjectMeta{
Name: invalidSecretClass,
Name: invalidSecretClass,
Annotations: class7Annotations,
},
Driver: mockDriverName,
Parameters: class3Parameters,
Parameters: class2Parameters,
DeletionPolicy: crdv1.VolumeSnapshotContentDelete,
},
{
Expand Down

0 comments on commit fe1c355

Please sign in to comment.