From 2ac906d13fbe13fb95607af7f90faceb864b6600 Mon Sep 17 00:00:00 2001 From: Grant Griffiths Date: Thu, 7 Nov 2019 10:35:48 -0800 Subject: [PATCH 1/2] Add sidecar-controller unit tests Signed-off-by: Grant Griffiths --- pkg/sidecar-controller/content_create_test.go | 47 + pkg/sidecar-controller/framework_test.go | 1025 +++++++++++++++++ .../snapshot_controller_test.go | 91 ++ .../snapshot_delete_test.go | 440 +++++++ .../snapshot_finalizer_test.go | 34 + 5 files changed, 1637 insertions(+) create mode 100644 pkg/sidecar-controller/content_create_test.go create mode 100644 pkg/sidecar-controller/framework_test.go create mode 100644 pkg/sidecar-controller/snapshot_controller_test.go create mode 100644 pkg/sidecar-controller/snapshot_delete_test.go create mode 100644 pkg/sidecar-controller/snapshot_finalizer_test.go diff --git a/pkg/sidecar-controller/content_create_test.go b/pkg/sidecar-controller/content_create_test.go new file mode 100644 index 000000000..435fa44fc --- /dev/null +++ b/pkg/sidecar-controller/content_create_test.go @@ -0,0 +1,47 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package sidecar_controller + +import ( + "testing" + "time" +) + +func TestSyncContent(t *testing.T) { + var tests []controllerTest + + tests = append(tests, controllerTest{ + name: "Basic content create ready to use", + initialContents: newContentArrayWithReadyToUse("content1-1", "snapuid1-1", "snap1-1", "sid1-1", defaultClass, "", "", retainPolicy, nil, &defaultSize, &False, true), + expectedContents: newContentArrayWithReadyToUse("content1-1", "snapuid1-1", "snap1-1", "sid1-1", defaultClass, "", "", retainPolicy, nil, &defaultSize, &True, true), + expectedEvents: noevents, + expectedCreateCalls: []createCall{ + { + snapshotName: "snapshot-snapuid1-1", + driverName: mockDriverName, + snapshotId: "snapuid1-1", + creationTime: timeNow, + readyToUse: true, + }, + }, + expectedListCalls: []listCall{{"sid1-1", true, time.Now(), 1, nil}}, + errors: noerrors, + test: testSyncContent, + }) + + runSyncContentTests(t, tests, snapshotClasses) +} diff --git a/pkg/sidecar-controller/framework_test.go b/pkg/sidecar-controller/framework_test.go new file mode 100644 index 000000000..49ec971ea --- /dev/null +++ b/pkg/sidecar-controller/framework_test.go @@ -0,0 +1,1025 @@ +/* +Copyright 2019 The Kubernetes Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package sidecar_controller + +import ( + "context" + "errors" + "fmt" + "reflect" + "strconv" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + crdv1 "github.com/kubernetes-csi/external-snapshotter/pkg/apis/volumesnapshot/v1beta1" + clientset "github.com/kubernetes-csi/external-snapshotter/pkg/client/clientset/versioned" + "github.com/kubernetes-csi/external-snapshotter/pkg/client/clientset/versioned/fake" + snapshotscheme "github.com/kubernetes-csi/external-snapshotter/pkg/client/clientset/versioned/scheme" + informers "github.com/kubernetes-csi/external-snapshotter/pkg/client/informers/externalversions" + storagelisters "github.com/kubernetes-csi/external-snapshotter/pkg/client/listers/volumesnapshot/v1beta1" + "github.com/kubernetes-csi/external-snapshotter/pkg/utils" + v1 "k8s.io/api/core/v1" + storagev1 "k8s.io/api/storage/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/diff" + "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/apimachinery/pkg/watch" + "k8s.io/client-go/kubernetes" + kubefake "k8s.io/client-go/kubernetes/fake" + "k8s.io/client-go/kubernetes/scheme" + core "k8s.io/client-go/testing" + "k8s.io/client-go/tools/cache" + "k8s.io/client-go/tools/record" + "k8s.io/klog" +) + +// This is a unit test framework for snapshot controller. +// It fills the controller with test snapshots/contents and can simulate these +// scenarios: +// 1) Call syncSnapshot/syncContent once. +// 2) Call syncSnapshot/syncContent several times (both simulating "snapshot/content +// modified" events and periodic sync), until the controller settles down and +// does not modify anything. +// 3) Simulate almost real API server/etcd and call add/update/delete +// content/snapshot. +// In all these scenarios, when the test finishes, the framework can compare +// resulting snapshots/contents with list of expected snapshots/contents and report +// differences. + +// controllerTest contains a single controller test input. +// Each test has initial set of contents and snapshots that are filled into the +// controller before the test starts. The test then contains a reference to +// function to call as the actual test. Available functions are: +// - testSyncSnapshot - calls syncSnapshot on the first snapshot in initialSnapshots. +// - testSyncSnapshotError - calls syncSnapshot on the first snapshot in initialSnapshots +// and expects an error to be returned. +// - testSyncContent - calls syncContent on the first content in initialContents. +// - any custom function for specialized tests. +// The test then contains list of contents/snapshots that are expected at the end +// of the test and list of generated events. +type controllerTest struct { + // Name of the test, for logging + name string + // Initial content of controller content cache. + initialContents []*crdv1.VolumeSnapshotContent + // Expected content of controller content cache at the end of the test. + expectedContents []*crdv1.VolumeSnapshotContent + // Initial content of controller Secret cache. + initialSecrets []*v1.Secret + // Expected events - any event with prefix will pass, we don't check full + // event message. + expectedEvents []string + // Errors to produce on matching action + errors []reactorError + // List of expected CSI Create snapshot calls + expectedCreateCalls []createCall + // List of expected CSI Delete snapshot calls + expectedDeleteCalls []deleteCall + // List of expected CSI list snapshot calls + expectedListCalls []listCall + // Function to call as the test. + test testCall + expectSuccess bool +} + +type testCall func(ctrl *csiSnapshotSideCarController, reactor *snapshotReactor, test controllerTest) error + +const testNamespace = "default" +const mockDriverName = "csi-mock-plugin" + +var errVersionConflict = errors.New("VersionError") +var nocontents []*crdv1.VolumeSnapshotContent +var nosnapshots []*crdv1.VolumeSnapshot +var noevents = []string{} +var noerrors = []reactorError{} + +// snapshotReactor is a core.Reactor that simulates etcd and API server. It +// stores: +// - Latest version of snapshots contents saved by the controller. +// - Queue of all saves (to simulate "content/snapshot updated" events). This queue +// contains all intermediate state of an object - e.g. a snapshot.VolumeName +// is updated first and snapshot.Phase second. This queue will then contain both +// updates as separate entries. +// - Number of changes since the last call to snapshotReactor.syncAll(). +// - Optionally, content and snapshot fake watchers which should be the same ones +// used by the controller. Any time an event function like deleteContentEvent +// is called to simulate an event, the reactor's stores are updated and the +// controller is sent the event via the fake watcher. +// - Optionally, list of error that should be returned by reactor, simulating +// etcd / API server failures. These errors are evaluated in order and every +// error is returned only once. I.e. when the reactor finds matching +// reactorError, it return appropriate error and removes the reactorError from +// the list. +type snapshotReactor struct { + secrets map[string]*v1.Secret + storageClasses map[string]*storagev1.StorageClass + volumes map[string]*v1.PersistentVolume + claims map[string]*v1.PersistentVolumeClaim + contents map[string]*crdv1.VolumeSnapshotContent + snapshots map[string]*crdv1.VolumeSnapshot + changedObjects []interface{} + changedSinceLastSync int + ctrl *csiSnapshotSideCarController + fakeContentWatch *watch.FakeWatcher + fakeSnapshotWatch *watch.FakeWatcher + lock sync.Mutex + errors []reactorError +} + +// reactorError is an error that is returned by test reactor (=simulated +// etcd+/API server) when an action performed by the reactor matches given verb +// ("get", "update", "create", "delete" or "*"") on given resource +// ("volumesnapshotcontents", "volumesnapshots" or "*"). +type reactorError struct { + verb string + resource string + error error +} + +func withContentFinalizer(content *crdv1.VolumeSnapshotContent) *crdv1.VolumeSnapshotContent { + content.ObjectMeta.Finalizers = append(content.ObjectMeta.Finalizers, utils.VolumeSnapshotContentFinalizer) + return content +} + +// React is a callback called by fake kubeClient from the controller. +// In other words, every snapshot/content change performed by the controller ends +// here. +// This callback checks versions of the updated objects and refuse those that +// are too old (simulating real etcd). +// All updated objects are stored locally to keep track of object versions and +// to evaluate test results. +// All updated objects are also inserted into changedObjects queue and +// optionally sent back to the controller via its watchers. +func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.Object, err error) { + r.lock.Lock() + defer r.lock.Unlock() + + klog.V(4).Infof("reactor got operation %q on %q", action.GetVerb(), action.GetResource()) + + // Inject error when requested + err = r.injectReactError(action) + if err != nil { + return true, nil, err + } + + // Test did not request to inject an error, continue simulating API server. + switch { + case action.Matches("create", "volumesnapshotcontents"): + obj := action.(core.UpdateAction).GetObject() + content := obj.(*crdv1.VolumeSnapshotContent) + + // check the content does not exist + _, found := r.contents[content.Name] + if found { + return true, nil, fmt.Errorf("cannot create content %s: content already exists", content.Name) + } + + // Store the updated object to appropriate places. + r.contents[content.Name] = content + r.changedObjects = append(r.changedObjects, content) + r.changedSinceLastSync++ + klog.V(5).Infof("created content %s", content.Name) + return true, content, nil + + case action.Matches("update", "volumesnapshotcontents"): + obj := action.(core.UpdateAction).GetObject() + content := obj.(*crdv1.VolumeSnapshotContent) + + // Check and bump object version + storedVolume, found := r.contents[content.Name] + if found { + storedVer, _ := strconv.Atoi(storedVolume.ResourceVersion) + requestedVer, _ := strconv.Atoi(content.ResourceVersion) + if storedVer != requestedVer { + return true, obj, errVersionConflict + } + // Don't modify the existing object + content = content.DeepCopy() + content.ResourceVersion = strconv.Itoa(storedVer + 1) + } else { + return true, nil, fmt.Errorf("cannot update content %s: content not found", content.Name) + } + + // Store the updated object to appropriate places. + r.contents[content.Name] = content + r.changedObjects = append(r.changedObjects, content) + r.changedSinceLastSync++ + klog.V(4).Infof("saved updated content %s", content.Name) + return true, content, nil + + case action.Matches("get", "volumesnapshotcontents"): + name := action.(core.GetAction).GetName() + content, found := r.contents[name] + if found { + klog.V(4).Infof("GetVolume: found %s", content.Name) + return true, content, nil + } + klog.V(4).Infof("GetVolume: content %s not found", name) + return true, nil, fmt.Errorf("cannot find content %s", name) + + case action.Matches("delete", "volumesnapshotcontents"): + name := action.(core.DeleteAction).GetName() + klog.V(4).Infof("deleted content %s", name) + _, found := r.contents[name] + if found { + delete(r.contents, name) + r.changedSinceLastSync++ + return true, nil, nil + } + return true, nil, fmt.Errorf("cannot delete content %s: not found", name) + + case action.Matches("get", "secrets"): + name := action.(core.GetAction).GetName() + secret, found := r.secrets[name] + if found { + klog.V(4).Infof("GetSecret: found %s", secret.Name) + return true, secret, nil + } + klog.V(4).Infof("GetSecret: secret %s not found", name) + return true, nil, fmt.Errorf("cannot find secret %s", name) + + } + + return false, nil, nil +} + +// injectReactError returns an error when the test requested given action to +// fail. nil is returned otherwise. +func (r *snapshotReactor) injectReactError(action core.Action) error { + if len(r.errors) == 0 { + // No more errors to inject, everything should succeed. + return nil + } + + for i, expected := range r.errors { + klog.V(4).Infof("trying to match %q %q with %q %q", expected.verb, expected.resource, action.GetVerb(), action.GetResource()) + if action.Matches(expected.verb, expected.resource) { + // That's the action we're waiting for, remove it from injectedErrors + r.errors = append(r.errors[:i], r.errors[i+1:]...) + klog.V(4).Infof("reactor found matching error at index %d: %q %q, returning %v", i, expected.verb, expected.resource, expected.error) + return expected.error + } + } + return nil +} + +// checkContents compares all expectedContents with set of contents at the end of +// the test and reports differences. +func (r *snapshotReactor) checkContents(expectedContents []*crdv1.VolumeSnapshotContent) error { + r.lock.Lock() + defer r.lock.Unlock() + + expectedMap := make(map[string]*crdv1.VolumeSnapshotContent) + gotMap := make(map[string]*crdv1.VolumeSnapshotContent) + // Clear any ResourceVersion from both sets + for _, v := range expectedContents { + // Don't modify the existing object + v := v.DeepCopy() + v.ResourceVersion = "" + v.Spec.VolumeSnapshotRef.ResourceVersion = "" + v.Status.CreationTime = nil + expectedMap[v.Name] = v + } + for _, v := range r.contents { + // We must clone the content because of golang race check - it was + // written by the controller without any locks on it. + v := v.DeepCopy() + v.ResourceVersion = "" + v.Spec.VolumeSnapshotRef.ResourceVersion = "" + v.Status.CreationTime = nil + gotMap[v.Name] = v + } + if !reflect.DeepEqual(expectedMap, gotMap) { + // Print ugly but useful diff of expected and received objects for + // easier debugging. + return fmt.Errorf("content check failed [A-expected, B-got]: %s", diff.ObjectDiff(expectedMap, gotMap)) + } + return nil +} + +// checkSnapshots compares all expectedSnapshots with set of snapshots at the end of the +// test and reports differences. +func (r *snapshotReactor) checkSnapshots(expectedSnapshots []*crdv1.VolumeSnapshot) error { + r.lock.Lock() + defer r.lock.Unlock() + + expectedMap := make(map[string]*crdv1.VolumeSnapshot) + gotMap := make(map[string]*crdv1.VolumeSnapshot) + for _, c := range expectedSnapshots { + // Don't modify the existing object + c = c.DeepCopy() + c.ResourceVersion = "" + if c.Status.Error != nil { + c.Status.Error.Time = &metav1.Time{} + } + expectedMap[c.Name] = c + } + for _, c := range r.snapshots { + // We must clone the snapshot because of golang race check - it was + // written by the controller without any locks on it. + c = c.DeepCopy() + c.ResourceVersion = "" + if c.Status.Error != nil { + c.Status.Error.Time = &metav1.Time{} + } + gotMap[c.Name] = c + } + if !reflect.DeepEqual(expectedMap, gotMap) { + // Print ugly but useful diff of expected and received objects for + // easier debugging. + return fmt.Errorf("snapshot check failed [A-expected, B-got result]: %s", diff.ObjectDiff(expectedMap, gotMap)) + } + return nil +} + +// checkEvents compares all expectedEvents with events generated during the test +// and reports differences. +func checkEvents(t *testing.T, expectedEvents []string, ctrl *csiSnapshotSideCarController) error { + var err error + + // Read recorded events - wait up to 1 minute to get all the expected ones + // (just in case some goroutines are slower with writing) + timer := time.NewTimer(time.Minute) + defer timer.Stop() + + fakeRecorder := ctrl.eventRecorder.(*record.FakeRecorder) + gotEvents := []string{} + finished := false + for len(gotEvents) < len(expectedEvents) && !finished { + select { + case event, ok := <-fakeRecorder.Events: + if ok { + klog.V(5).Infof("event recorder got event %s", event) + gotEvents = append(gotEvents, event) + } else { + klog.V(5).Infof("event recorder finished") + finished = true + } + case _, _ = <-timer.C: + klog.V(5).Infof("event recorder timeout") + finished = true + } + } + + // Evaluate the events + for i, expected := range expectedEvents { + if len(gotEvents) <= i { + t.Errorf("Event %q not emitted", expected) + err = fmt.Errorf("Events do not match") + continue + } + received := gotEvents[i] + if !strings.HasPrefix(received, expected) { + t.Errorf("Unexpected event received, expected %q, got %q", expected, received) + err = fmt.Errorf("Events do not match") + } + } + for i := len(expectedEvents); i < len(gotEvents); i++ { + t.Errorf("Unexpected event received: %q", gotEvents[i]) + err = fmt.Errorf("Events do not match") + } + return err +} + +// popChange returns one recorded updated object, either *crdv1.VolumeSnapshotContent +// or *crdv1.VolumeSnapshot. Returns nil when there are no changes. +func (r *snapshotReactor) popChange() interface{} { + r.lock.Lock() + defer r.lock.Unlock() + + if len(r.changedObjects) == 0 { + return nil + } + + // For debugging purposes, print the queue + for _, obj := range r.changedObjects { + switch obj.(type) { + case *crdv1.VolumeSnapshotContent: + vol, _ := obj.(*crdv1.VolumeSnapshotContent) + klog.V(4).Infof("reactor queue: %s", vol.Name) + case *crdv1.VolumeSnapshot: + snapshot, _ := obj.(*crdv1.VolumeSnapshot) + klog.V(4).Infof("reactor queue: %s", snapshot.Name) + } + } + + // Pop the first item from the queue and return it + obj := r.changedObjects[0] + r.changedObjects = r.changedObjects[1:] + return obj +} + +// syncAll simulates the controller periodic sync of contents and snapshot. It +// simply adds all these objects to the internal queue of updates. This method +// should be used when the test manually calls syncSnapshot/syncContent. Test that +// use real controller loop (ctrl.Run()) will get periodic sync automatically. +func (r *snapshotReactor) syncAll() { + r.lock.Lock() + defer r.lock.Unlock() + + for _, c := range r.snapshots { + r.changedObjects = append(r.changedObjects, c) + } + for _, v := range r.contents { + r.changedObjects = append(r.changedObjects, v) + } + for _, pvc := range r.claims { + r.changedObjects = append(r.changedObjects, pvc) + } + r.changedSinceLastSync = 0 +} + +func (r *snapshotReactor) getChangeCount() int { + r.lock.Lock() + defer r.lock.Unlock() + return r.changedSinceLastSync +} + +// waitForIdle waits until all tests, controllers and other goroutines do their +// job and no new actions are registered for 10 milliseconds. +func (r *snapshotReactor) waitForIdle() { + r.ctrl.runningOperations.WaitForCompletion() + // Check every 10ms if the controller does something and stop if it's + // idle. + oldChanges := -1 + for { + time.Sleep(10 * time.Millisecond) + changes := r.getChangeCount() + if changes == oldChanges { + // No changes for last 10ms -> controller must be idle. + break + } + oldChanges = changes + } +} + +// waitTest waits until all tests, controllers and other goroutines do their +// job and list of current contents/snapshots is equal to list of expected +// contents/snapshots (with ~10 second timeout). +func (r *snapshotReactor) waitTest(test controllerTest) error { + // start with 10 ms, multiply by 2 each step, 10 steps = 10.23 seconds + backoff := wait.Backoff{ + Duration: 10 * time.Millisecond, + Jitter: 0, + Factor: 2, + Steps: 10, + } + err := wait.ExponentialBackoff(backoff, func() (done bool, err error) { + // Finish all operations that are in progress + r.ctrl.runningOperations.WaitForCompletion() + + // Return 'true' if the reactor reached the expected state + err1 := r.checkContents(test.expectedContents) + if err1 == nil { + return true, nil + } + return false, nil + }) + return err +} + +// deleteContentEvent simulates that a content has been deleted in etcd and +// the controller receives 'content deleted' event. +func (r *snapshotReactor) deleteContentEvent(content *crdv1.VolumeSnapshotContent) { + r.lock.Lock() + defer r.lock.Unlock() + + // Remove the content from list of resulting contents. + delete(r.contents, content.Name) + + // Generate deletion event. Cloned content is needed to prevent races (and we + // would get a clone from etcd too). + if r.fakeContentWatch != nil { + r.fakeContentWatch.Delete(content.DeepCopy()) + } +} + +// deleteSnapshotEvent simulates that a snapshot has been deleted in etcd and the +// controller receives 'snapshot deleted' event. +func (r *snapshotReactor) deleteSnapshotEvent(snapshot *crdv1.VolumeSnapshot) { + r.lock.Lock() + defer r.lock.Unlock() + + // Remove the snapshot from list of resulting snapshots. + delete(r.snapshots, snapshot.Name) + + // Generate deletion event. Cloned content is needed to prevent races (and we + // would get a clone from etcd too). + if r.fakeSnapshotWatch != nil { + r.fakeSnapshotWatch.Delete(snapshot.DeepCopy()) + } +} + +// addContentEvent simulates that a content has been added in etcd and the +// controller receives 'content added' event. +func (r *snapshotReactor) addContentEvent(content *crdv1.VolumeSnapshotContent) { + r.lock.Lock() + defer r.lock.Unlock() + + r.contents[content.Name] = content + // Generate event. No cloning is needed, this snapshot is not stored in the + // controller cache yet. + if r.fakeContentWatch != nil { + r.fakeContentWatch.Add(content) + } +} + +// modifyContentEvent simulates that a content has been modified in etcd and the +// controller receives 'content modified' event. +func (r *snapshotReactor) modifyContentEvent(content *crdv1.VolumeSnapshotContent) { + r.lock.Lock() + defer r.lock.Unlock() + + r.contents[content.Name] = content + // Generate deletion event. Cloned content is needed to prevent races (and we + // would get a clone from etcd too). + if r.fakeContentWatch != nil { + r.fakeContentWatch.Modify(content.DeepCopy()) + } +} + +// addSnapshotEvent simulates that a snapshot has been deleted in etcd and the +// controller receives 'snapshot added' event. +func (r *snapshotReactor) addSnapshotEvent(snapshot *crdv1.VolumeSnapshot) { + r.lock.Lock() + defer r.lock.Unlock() + + r.snapshots[snapshot.Name] = snapshot + // Generate event. No cloning is needed, this snapshot is not stored in the + // controller cache yet. + if r.fakeSnapshotWatch != nil { + r.fakeSnapshotWatch.Add(snapshot) + } +} + +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), + contents: make(map[string]*crdv1.VolumeSnapshotContent), + ctrl: ctrl, + fakeContentWatch: fakeVolumeWatch, + errors: errors, + } + + client.AddReactor("create", "volumesnapshotcontents", reactor.React) + client.AddReactor("update", "volumesnapshotcontents", reactor.React) + client.AddReactor("update", "volumesnapshots", reactor.React) + client.AddReactor("get", "volumesnapshotcontents", reactor.React) + client.AddReactor("get", "volumesnapshots", reactor.React) + client.AddReactor("delete", "volumesnapshotcontents", reactor.React) + client.AddReactor("delete", "volumesnapshots", reactor.React) + + return reactor +} + +func alwaysReady() bool { return true } + +func newTestController(kubeClient kubernetes.Interface, clientset clientset.Interface, + informerFactory informers.SharedInformerFactory, t *testing.T, test controllerTest) (*csiSnapshotSideCarController, error) { + if informerFactory == nil { + informerFactory = informers.NewSharedInformerFactory(clientset, utils.NoResyncPeriodFunc()) + } + + // Construct controller + fakeSnapshot := &fakeSnapshotter{ + t: t, + listCalls: test.expectedListCalls, + createCalls: test.expectedCreateCalls, + deleteCalls: test.expectedDeleteCalls, + } + + ctrl := NewCSISnapshotSideCarController( + clientset, + kubeClient, + mockDriverName, + informerFactory.Snapshot().V1beta1().VolumeSnapshotContents(), + informerFactory.Snapshot().V1beta1().VolumeSnapshotClasses(), + fakeSnapshot, + 5*time.Millisecond, + 60*time.Second, + "snapshot", + -1, + ) + + ctrl.eventRecorder = record.NewFakeRecorder(1000) + + ctrl.contentListerSynced = alwaysReady + ctrl.classListerSynced = alwaysReady + + return ctrl, nil +} + +func newContent(contentName, boundToSnapshotUID, boundToSnapshotName, snapshotHandle, snapshotClassName, desiredSnapshotHandle, volumeHandle string, + deletionPolicy crdv1.DeletionPolicy, creationTime, size *int64, + withFinalizer bool, deletionTime *metav1.Time) *crdv1.VolumeSnapshotContent { + var annotations map[string]string + + content := crdv1.VolumeSnapshotContent{ + ObjectMeta: metav1.ObjectMeta{ + Name: contentName, + ResourceVersion: "1", + DeletionTimestamp: deletionTime, + Annotations: annotations, + }, + Spec: crdv1.VolumeSnapshotContentSpec{ + Driver: mockDriverName, + DeletionPolicy: deletionPolicy, + Source: crdv1.VolumeSnapshotContentSource{ + SnapshotHandle: &snapshotHandle, + VolumeHandle: &volumeHandle, + }, + }, + Status: &crdv1.VolumeSnapshotContentStatus{ + CreationTime: creationTime, + RestoreSize: size, + }, + } + if deletionTime != nil { + metav1.SetMetaDataAnnotation(&content.ObjectMeta, utils.AnnVolumeSnapshotBeingDeleted, "yes") + } + + if snapshotHandle != "" { + content.Status.SnapshotHandle = &snapshotHandle + } + + if snapshotClassName != "" { + content.Spec.VolumeSnapshotClassName = &snapshotClassName + } + + if volumeHandle != "" { + content.Spec.Source = crdv1.VolumeSnapshotContentSource{ + VolumeHandle: &volumeHandle, + } + } else if desiredSnapshotHandle != "" { + content.Spec.Source = crdv1.VolumeSnapshotContentSource{ + SnapshotHandle: &desiredSnapshotHandle, + } + } + + if boundToSnapshotName != "" { + content.Spec.VolumeSnapshotRef = v1.ObjectReference{ + Kind: "VolumeSnapshot", + APIVersion: "snapshot.storage.k8s.io/v1beta1", + UID: types.UID(boundToSnapshotUID), + Namespace: testNamespace, + Name: boundToSnapshotName, + } + } + + if withFinalizer { + return withContentFinalizer(&content) + } + return &content +} + +func newContentArray(contentName, boundToSnapshotUID, boundToSnapshotName, snapshotHandle, snapshotClassName, desiredSnapshotHandle, volumeHandle string, + deletionPolicy crdv1.DeletionPolicy, size, creationTime *int64, + withFinalizer bool) []*crdv1.VolumeSnapshotContent { + return []*crdv1.VolumeSnapshotContent{ + newContent(contentName, boundToSnapshotUID, boundToSnapshotName, snapshotHandle, snapshotClassName, desiredSnapshotHandle, volumeHandle, deletionPolicy, creationTime, size, withFinalizer, nil), + } +} + +func newContentArrayWithReadyToUse(contentName, boundToSnapshotUID, boundToSnapshotName, snapshotHandle, snapshotClassName, desiredSnapshotHandle, volumeHandle string, + deletionPolicy crdv1.DeletionPolicy, creationTime, size *int64, readyToUse *bool, + withFinalizer bool) []*crdv1.VolumeSnapshotContent { + content := newContent(contentName, boundToSnapshotUID, boundToSnapshotName, snapshotHandle, snapshotClassName, desiredSnapshotHandle, volumeHandle, deletionPolicy, creationTime, size, withFinalizer, nil) + content.Status.ReadyToUse = readyToUse + return []*crdv1.VolumeSnapshotContent{ + content, + } +} + +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 { + return []*crdv1.VolumeSnapshotContent{ + newContent(contentName, boundToSnapshotUID, boundToSnapshotName, snapshotHandle, snapshotClassName, desiredSnapshotHandle, volumeHandle, deletionPolicy, creationTime, size, withFinalizer, deletionTime), + } +} + +func testSyncContent(ctrl *csiSnapshotSideCarController, reactor *snapshotReactor, test controllerTest) error { + return ctrl.syncContent(test.initialContents[0]) +} +func testSyncContentError(ctrl *csiSnapshotSideCarController, reactor *snapshotReactor, test controllerTest) error { + err := ctrl.syncContent(test.initialContents[0]) + if err != nil { + return nil + } + return fmt.Errorf("syncSnapshotContent succeeded when failure was expected") +} + +var ( + classEmpty string + classGold = "gold" + classSilver = "silver" + classNonExisting = "non-existing" + defaultClass = "default-class" + emptySecretClass = "empty-secret-class" + invalidSecretClass = "invalid-secret-class" + validSecretClass = "valid-secret-class" + sameDriver = "sameDriver" + diffDriver = "diffDriver" + noClaim = "" + noBoundUID = "" + noVolume = "" +) + +// wrapTestWithInjectedOperation returns a testCall that: +// - starts the controller and lets it run original testCall until +// scheduleOperation() call. It blocks the controller there and calls the +// injected function to simulate that something is happening when the +// controller waits for the operation lock. Controller is then resumed and we +// check how it behaves. +func wrapTestWithInjectedOperation(toWrap testCall, injectBeforeOperation func(ctrl *csiSnapshotSideCarController, reactor *snapshotReactor)) testCall { + + return func(ctrl *csiSnapshotSideCarController, reactor *snapshotReactor, test controllerTest) error { + // Inject a hook before async operation starts + klog.V(4).Infof("reactor:injecting call") + injectBeforeOperation(ctrl, reactor) + + // Run the tested function (typically syncSnapshot/syncContent) in a + // separate goroutine. + var testError error + var testFinished int32 + + go func() { + testError = toWrap(ctrl, reactor, test) + // Let the "main" test function know that syncContent has finished. + atomic.StoreInt32(&testFinished, 1) + }() + + // Wait for the controller to finish the test function. + for atomic.LoadInt32(&testFinished) == 0 { + time.Sleep(time.Millisecond * 10) + } + + return testError + } +} + +func evaluateTestResults(ctrl *csiSnapshotSideCarController, reactor *snapshotReactor, test controllerTest, t *testing.T) { + // Evaluate results + if test.expectedContents != nil { + if err := reactor.checkContents(test.expectedContents); err != nil { + t.Errorf("Test %q: %v", test.name, err) + } + } + + if err := checkEvents(t, test.expectedEvents, ctrl); err != nil { + t.Errorf("Test %q: %v", test.name, err) + } +} + +// Test single call to syncSnapshot and syncContent methods. +// For all tests: +// 1. Fill in the controller with initial data +// 2. Call the tested function (syncContent) via +// controllerTest.testCall *once*. +// 3. Compare resulting contents and snapshots with expected contents and snapshots. +func runSyncContentTests(t *testing.T, tests []controllerTest, snapshotClasses []*crdv1.VolumeSnapshotClass) { + snapshotscheme.AddToScheme(scheme.Scheme) + for _, test := range tests { + klog.V(4).Infof("starting test %q", test.name) + + // Initialize the controller + kubeClient := &kubefake.Clientset{} + client := &fake.Clientset{} + + ctrl, err := newTestController(kubeClient, client, nil, t, test) + if err != nil { + t.Fatalf("Test %q construct persistent content failed: %v", test.name, err) + } + + reactor := newSnapshotReactor(kubeClient, client, ctrl, nil, nil, test.errors) + for _, content := range test.initialContents { + if ctrl.isDriverMatch(test.initialContents[0]) { + ctrl.contentStore.Add(content) + reactor.contents[content.Name] = content + } + } + + for _, secret := range test.initialSecrets { + reactor.secrets[secret.Name] = secret + } + + // Inject classes into controller via a custom lister. + indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) + for _, class := range snapshotClasses { + indexer.Add(class) + } + ctrl.classLister = storagelisters.NewVolumeSnapshotClassLister(indexer) + + // Run the tested functions + err = test.test(ctrl, reactor, test) + if err != nil { + t.Errorf("Test %q failed: %v", test.name, err) + } + + // Wait for the target state + err = reactor.waitTest(test) + if err != nil { + t.Errorf("Test %q failed: %v", test.name, err) + } + + evaluateTestResults(ctrl, reactor, test, t) + } +} + +func getSize(size int64) *resource.Quantity { + return resource.NewQuantity(size, resource.BinarySI) +} + +func emptySecret() *v1.Secret { + return &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "emptysecret", + Namespace: "default", + }, + } +} + +func secret() *v1.Secret { + return &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "secret", + Namespace: "default", + }, + Data: map[string][]byte{ + "foo": []byte("bar"), + }, + } +} + +func secretAnnotations() map[string]string { + return map[string]string{ + utils.AnnDeletionSecretRefName: "secret", + utils.AnnDeletionSecretRefNamespace: "default", + } +} + +func emptyNamespaceSecretAnnotations() map[string]string { + return map[string]string{ + utils.AnnDeletionSecretRefName: "name", + utils.AnnDeletionSecretRefNamespace: "", + } +} + +// this refers to emptySecret(), which is missing data. +func emptyDataSecretAnnotations() map[string]string { + return map[string]string{ + utils.AnnDeletionSecretRefName: "emptysecret", + utils.AnnDeletionSecretRefNamespace: "default", + } +} + +type listCall struct { + snapshotID string + // information to return + readyToUse bool + createTime time.Time + size int64 + err error +} + +type deleteCall struct { + snapshotID string + secrets map[string]string + err error +} + +type createCall struct { + // expected request parameter + snapshotName string + volumeHandle string + parameters map[string]string + secrets map[string]string + // information to return + driverName string + snapshotId string + creationTime time.Time + size int64 + readyToUse bool + err error +} + +// Fake SnapShotter implementation that check that Attach/Detach is called +// with the right parameters and it returns proper error code and metadata. +type fakeSnapshotter struct { + createCalls []createCall + createCallCounter int + deleteCalls []deleteCall + deleteCallCounter int + listCalls []listCall + listCallCounter int + t *testing.T +} + +func (f *fakeSnapshotter) CreateSnapshot(ctx context.Context, snapshotName string, volumeHandle string, parameters map[string]string, snapshotterCredentials map[string]string) (string, string, time.Time, int64, bool, error) { + if f.createCallCounter >= len(f.createCalls) { + f.t.Errorf("Unexpected CSI Create Snapshot call: snapshotName=%s, volumeHandle=%v, index: %d, calls: %+v", snapshotName, volumeHandle, f.createCallCounter, f.createCalls) + return "", "", time.Time{}, 0, false, fmt.Errorf("unexpected call") + } + call := f.createCalls[f.createCallCounter] + f.createCallCounter++ + + var err error + if call.snapshotName != snapshotName { + f.t.Errorf("Wrong CSI Create Snapshot call: snapshotName=%s, volumeHandle=%s, expected snapshotName: %s", snapshotName, volumeHandle, call.snapshotName) + err = fmt.Errorf("unexpected create snapshot call") + } + + if call.volumeHandle != volumeHandle { + f.t.Errorf("Wrong CSI Create Snapshot call: snapshotName=%s, volumeHandle=%s, expected volumeHandle: %s", snapshotName, volumeHandle, call.volumeHandle) + err = fmt.Errorf("unexpected create snapshot call") + } + + if !reflect.DeepEqual(call.parameters, parameters) && !(len(call.parameters) == 0 && len(parameters) == 0) { + f.t.Errorf("Wrong CSI Create Snapshot call: snapshotName=%s, volumeHandle=%s, expected parameters %+v, got %+v", snapshotName, volumeHandle, call.parameters, parameters) + err = fmt.Errorf("unexpected create snapshot call") + } + + if !reflect.DeepEqual(call.secrets, snapshotterCredentials) { + 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") + } + + if err != nil { + return "", "", time.Time{}, 0, false, fmt.Errorf("unexpected call") + } + return call.driverName, call.snapshotId, call.creationTime, call.size, call.readyToUse, call.err +} + +func (f *fakeSnapshotter) DeleteSnapshot(ctx context.Context, snapshotID string, snapshotterCredentials map[string]string) error { + if f.deleteCallCounter >= len(f.deleteCalls) { + f.t.Errorf("Unexpected CSI Delete Snapshot call: snapshotID=%s, index: %d, calls: %+v", snapshotID, f.createCallCounter, f.createCalls) + return fmt.Errorf("unexpected DeleteSnapshot call") + } + call := f.deleteCalls[f.deleteCallCounter] + f.deleteCallCounter++ + + var err error + if call.snapshotID != snapshotID { + f.t.Errorf("Wrong CSI Create Snapshot call: snapshotID=%s, expected snapshotID: %s", snapshotID, call.snapshotID) + err = fmt.Errorf("unexpected Delete snapshot call") + } + + if !reflect.DeepEqual(call.secrets, snapshotterCredentials) { + f.t.Errorf("Wrong CSI Delete Snapshot call: snapshotID=%s, expected secrets %+v, got %+v", snapshotID, call.secrets, snapshotterCredentials) + err = fmt.Errorf("unexpected Delete Snapshot call") + } + + if err != nil { + return fmt.Errorf("unexpected call") + } + + return call.err +} + +func (f *fakeSnapshotter) GetSnapshotStatus(ctx context.Context, snapshotID string) (bool, time.Time, int64, 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") + } + call := f.listCalls[f.listCallCounter] + f.listCallCounter++ + + var err error + if call.snapshotID != snapshotID { + f.t.Errorf("Wrong CSI List Snapshot call: snapshotID=%s, expected snapshotID: %s", snapshotID, call.snapshotID) + err = fmt.Errorf("unexpected List snapshot call") + } + + if err != nil { + return false, time.Time{}, 0, fmt.Errorf("unexpected call") + } + + return call.readyToUse, call.createTime, call.size, call.err +} diff --git a/pkg/sidecar-controller/snapshot_controller_test.go b/pkg/sidecar-controller/snapshot_controller_test.go new file mode 100644 index 000000000..c4d598e97 --- /dev/null +++ b/pkg/sidecar-controller/snapshot_controller_test.go @@ -0,0 +1,91 @@ +/* +Copyright 2019 The Kubernetes Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package sidecar_controller + +import ( + "testing" + + crdv1 "github.com/kubernetes-csi/external-snapshotter/pkg/apis/volumesnapshot/v1beta1" + "github.com/kubernetes-csi/external-snapshotter/pkg/utils" + "k8s.io/client-go/tools/cache" +) + +var deletionPolicy = crdv1.VolumeSnapshotContentDelete + +func storeVersion(t *testing.T, prefix string, c cache.Store, version string, expectedReturn bool) { + content := newContent("contentName", "snapuid1-1", "snap1-1", "sid1-1", classGold, "", "pv-handle-1-1", deletionPolicy, nil, nil, false, nil) + content.ResourceVersion = version + ret, err := utils.StoreObjectUpdate(c, content, "content") + if err != nil { + t.Errorf("%s: expected storeObjectUpdate to succeed, got: %v", prefix, err) + } + if expectedReturn != ret { + t.Errorf("%s: expected storeObjectUpdate to return %v, got: %v", prefix, expectedReturn, ret) + } + + // find the stored version + + contentObj, found, err := c.GetByKey("contentName") + if err != nil { + t.Errorf("expected content 'contentName' in the cache, got error instead: %v", err) + } + if !found { + t.Errorf("expected content 'contentName' in the cache but it was not found") + } + content, ok := contentObj.(*crdv1.VolumeSnapshotContent) + if !ok { + t.Errorf("expected content in the cache, got different object instead: %#v", contentObj) + } + + if ret { + if content.ResourceVersion != version { + t.Errorf("expected content with version %s in the cache, got %s instead", version, content.ResourceVersion) + } + } else { + if content.ResourceVersion == version { + t.Errorf("expected content with version other than %s in the cache, got %s instead", version, content.ResourceVersion) + } + } +} + +// TestControllerCache tests func storeObjectUpdate() +func TestControllerCache(t *testing.T) { + // Cache under test + c := cache.NewStore(cache.DeletionHandlingMetaNamespaceKeyFunc) + + // Store new PV + storeVersion(t, "Step1", c, "1", true) + // Store the same PV + storeVersion(t, "Step2", c, "1", true) + // Store newer PV + storeVersion(t, "Step3", c, "2", true) + // Store older PV - simulating old "PV updated" event or periodic sync with + // old data + storeVersion(t, "Step4", c, "1", false) + // Store newer PV - test integer parsing ("2" > "10" as string, + // while 2 < 10 as integers) + storeVersion(t, "Step5", c, "10", true) +} + +func TestControllerCacheParsingError(t *testing.T) { + c := cache.NewStore(cache.DeletionHandlingMetaNamespaceKeyFunc) + // There must be something in the cache to compare with + storeVersion(t, "Step1", c, "1", true) + content := newContent("contentName", "snapuid1-1", "snap1-1", "sid1-1", classGold, "", "pv-handle-1-1", deletionPolicy, nil, nil, false, nil) + content.ResourceVersion = "xxx" + _, err := utils.StoreObjectUpdate(c, content, "content") + if err == nil { + t.Errorf("Expected parsing error, got nil instead") + } +} diff --git a/pkg/sidecar-controller/snapshot_delete_test.go b/pkg/sidecar-controller/snapshot_delete_test.go new file mode 100644 index 000000000..593b71c2a --- /dev/null +++ b/pkg/sidecar-controller/snapshot_delete_test.go @@ -0,0 +1,440 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package sidecar_controller + +import ( + "fmt" + "testing" + "time" + + "errors" + + crdv1 "github.com/kubernetes-csi/external-snapshotter/pkg/apis/volumesnapshot/v1beta1" + "github.com/kubernetes-csi/external-snapshotter/pkg/utils" + "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +var defaultSize int64 = 1000 +var emptySize int64 = 0 +var deletePolicy = crdv1.VolumeSnapshotContentDelete +var retainPolicy = crdv1.VolumeSnapshotContentRetain +var timeNow = time.Now() +var timeNowMetav1 = metav1.Now() +var False = false +var True = true + +var class1Parameters = map[string]string{ + "param1": "value1", +} + +var class2Parameters = map[string]string{ + "param2": "value2", +} + +var class3Parameters = map[string]string{ + "param3": "value3", + utils.AnnDeletionSecretRefName: "name", +} + +var class4Parameters = map[string]string{ + utils.AnnDeletionSecretRefName: "emptysecret", + utils.AnnDeletionSecretRefNamespace: "default", +} + +var class5Parameters = map[string]string{ + utils.AnnDeletionSecretRefName: "secret", + utils.AnnDeletionSecretRefNamespace: "default", +} + +var snapshotClasses = []*crdv1.VolumeSnapshotClass{ + { + TypeMeta: metav1.TypeMeta{ + Kind: "VolumeSnapshotClass", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: classGold, + }, + Driver: mockDriverName, + Parameters: class1Parameters, + DeletionPolicy: crdv1.VolumeSnapshotContentDelete, + }, + { + TypeMeta: metav1.TypeMeta{ + Kind: "VolumeSnapshotClass", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: classSilver, + }, + Driver: mockDriverName, + Parameters: class2Parameters, + DeletionPolicy: crdv1.VolumeSnapshotContentDelete, + }, + { + TypeMeta: metav1.TypeMeta{ + Kind: "VolumeSnapshotClass", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: emptySecretClass, + }, + Driver: mockDriverName, + Parameters: class4Parameters, + DeletionPolicy: crdv1.VolumeSnapshotContentDelete, + }, + { + TypeMeta: metav1.TypeMeta{ + Kind: "VolumeSnapshotClass", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: invalidSecretClass, + }, + Driver: mockDriverName, + Parameters: class3Parameters, + DeletionPolicy: crdv1.VolumeSnapshotContentDelete, + }, + { + TypeMeta: metav1.TypeMeta{ + Kind: "VolumeSnapshotClass", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: validSecretClass, + }, + Driver: mockDriverName, + Parameters: class5Parameters, + DeletionPolicy: crdv1.VolumeSnapshotContentDelete, + }, + { + TypeMeta: metav1.TypeMeta{ + Kind: "VolumeSnapshotClass", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: defaultClass, + Annotations: map[string]string{utils.IsDefaultSnapshotClassAnnotation: "true"}, + }, + Driver: mockDriverName, + DeletionPolicy: crdv1.VolumeSnapshotContentDelete, + }, +} + +// Test single call to syncContent, expecting deleting to happen. +// 1. Fill in the controller with initial data +// 2. Call the syncContent *once*. +// 3. Compare resulting contents with expected contents. +func TestDeleteSync(t *testing.T) { + + tests := []controllerTest{ + { + name: "1-1 - content non-nil DeletionTimestamp with delete policy will delete snapshot", + initialContents: newContentArrayWithDeletionTimestamp("content1-1", "snapuid1-1", "snap1-1", "sid1-1", classGold, "", "snap1-1-volumehandle", deletionPolicy, nil, nil, true, &timeNowMetav1), + expectedContents: newContentArrayWithDeletionTimestamp("content1-1", "snapuid1-1", "snap1-1", "sid1-1", classGold, "", "snap1-1-volumehandle", deletionPolicy, nil, nil, false, &timeNowMetav1), + expectedEvents: noevents, + errors: noerrors, + initialSecrets: []*v1.Secret{secret()}, + expectedCreateCalls: []createCall{ + { + snapshotName: "snapshot-snapuid1-1", + volumeHandle: "snap1-1-volumehandle", + parameters: map[string]string{"param1": "value1"}, + driverName: mockDriverName, + size: defaultSize, + snapshotId: "snapuid1-1-deleted", + creationTime: timeNow, + readyToUse: true, + }, + }, + expectedListCalls: []listCall{{"sid1-1", true, time.Now(), 1, nil}}, + expectedDeleteCalls: []deleteCall{{"sid1-1", nil, nil}}, + test: testSyncContent, + }, + { + name: "1-2 - content non-nil DeletionTimestamp with retain policy will not delete snapshot", + initialContents: newContentArrayWithDeletionTimestamp("content1-2", "snapuid1-2", "snap1-2", "sid1-2", classGold, "", "snap1-2-volumehandle", retainPolicy, nil, nil, true, &timeNowMetav1), + expectedContents: newContentArrayWithDeletionTimestamp("content1-2", "snapuid1-2", "snap1-2", "sid1-2", classGold, "", "snap1-2-volumehandle", retainPolicy, nil, nil, false, &timeNowMetav1), + expectedEvents: noevents, + errors: noerrors, + expectedCreateCalls: []createCall{ + { + snapshotName: "snapshot-snapuid1-2", + volumeHandle: "snap1-2-volumehandle", + parameters: map[string]string{"param1": "value1"}, + driverName: mockDriverName, + size: defaultSize, + snapshotId: "snapuid1-2-deleted", + creationTime: timeNow, + readyToUse: true, + }, + }, + expectedListCalls: []listCall{{"sid1-2", true, time.Now(), 1, nil}}, + expectedDeleteCalls: []deleteCall{{"sid1-2", nil, nil}}, + test: testSyncContent, + }, + { + name: "1-3 - delete snapshot error should result in an event", + initialContents: newContentArrayWithDeletionTimestamp("content1-3", "snapuid1-3", "snap1-3", "sid1-3", validSecretClass, "", "snap1-3-volumehandle", deletePolicy, nil, nil, true, &timeNowMetav1), + expectedContents: newContentArrayWithDeletionTimestamp("content1-3", "snapuid1-3", "snap1-3", "sid1-3", validSecretClass, "", "snap1-3-volumehandle", deletePolicy, nil, nil, false, &timeNowMetav1), + errors: noerrors, + expectedCreateCalls: []createCall{ + { + snapshotName: "snapshot-snapuid1-3", + volumeHandle: "snap1-3-volumehandle", + parameters: map[string]string{"foo": "bar"}, + driverName: mockDriverName, + size: defaultSize, + snapshotId: "snapuid1-3-deleted", + creationTime: timeNow, + readyToUse: true, + }, + }, + expectedDeleteCalls: []deleteCall{{"sid1-3", nil, fmt.Errorf("mock csi driver delete error")}}, + expectedEvents: []string{"Warning SnapshotDeleteError"}, + expectedListCalls: []listCall{{"sid1-3", true, time.Now(), 1, nil}}, + test: testSyncContent, + }, + /*{ + name: "1-4 - create snapshot error should result in an event", + initialContents: newContentArrayWithDeletionTimestamp("content1-4", "snapuid1-4", "snap1-4", "sid1-4", classGold, "", "snap1-4-volumehandle", deletePolicy, nil, nil, true, nil), + //expectedContents: newContentArrayWithDeletionTimestamp("content1-4", "snapuid1-4", "snap1-4", "sid1-4", classGold, "", "snap1-4-volumehandle", deletePolicy, nil, nil, true, nil), + errors: []reactorError{}, + expectedCreateCalls: []createCall{ + { + snapshotName: "snapshot-snapuid1-4", + volumeHandle: "snap1-4-volumehandle", + parameters: map[string]string{"param1": "value1"}, + err: fmt.Errorf("Create failed"), + }, + }, + //expectedDeleteCalls: []deleteCall{{"sid1-4", nil, nil}}, + expectedListCalls: []listCall{{"sid1-4", true, time.Now(), 1, nil}}, + expectedEvents: []string{"Warning SnapshotContentCheckandUpdateFailed Failed to check and update snapshot content: Create failed"}, + test: testSyncContent, + },*/ + /*{ + name: "2-1 - content with empty snapshot class will not be deleted if it is bound to a non-exist snapshot but it does not have a snapshot uid specified", + initialContents: newContentArray("content2-1", "", "snap2-1", "sid2-1", "", "", "", deletionPolicy, nil, nil, true), + expectedContents: newContentArray("content2-1", "", "snap2-1", "sid2-1", "", "", "", deletionPolicy, nil, nil, true), + expectedEvents: noevents, + errors: noerrors, + expectedDeleteCalls: []deleteCall{{"sid2-1", nil, nil}}, + test: testSyncContent, + },*/ + /*{ + name: "1-2 - successful delete with snapshot class that has empty secret parameter", + initialContents: newContentArray("content1-2", "sid1-2", "snap1-2", "sid1-2", emptySecretClass, "", "volumeHandle", deletionPolicy, nil, nil, true), + expectedContents: nocontents, + initialSnapshots: nosnapshots, + expectedSnapshots: nosnapshots, + initialSecrets: []*v1.Secret{emptySecret()}, + expectedEvents: noevents, + errors: noerrors, + expectedDeleteCalls: []deleteCall{{"sid1-2", map[string]string{}, nil}}, + test: testSyncContent, + },*/ + /*{ + name: "1-3 - content non-nil DeletionTimestamp with delete policy will delete snapshot", + initialContents: newContentArrayWithDeletionTimestamp("content1-3", "snapuid1-1", "snap1-1", "sid1-3", classGold, "", "snap1-3-volumehandle", deletionPolicy, nil, nil, true, &timeNowMetav1), + expectedContents: newContentArrayWithDeletionTimestamp("content1-3", "snapuid1-1", "snap1-1", "sid1-3", classGold, "", "snap1-3-volumehandle", deletionPolicy, nil, nil, false, &timeNowMetav1), + initialSnapshots: newSnapshotArray("snap1-3", "snapuid1-3", "claim1-3", "", validSecretClass, "", &False, nil, nil, nil), + expectedSnapshots: newSnapshotArray("snap1-3", "snapuid1-3", "claim1-3", "", validSecretClass, "", &False, nil, nil, nil), + expectedEvents: noevents, + errors: noerrors, + initialSecrets: []*v1.Secret{secret()}, + expectedCreateCalls: []createCall{ + { + snapshotName: "snapshot-snapuid1-1", + volumeHandle: "snap1-1-volumehandle", + parameters: map[string]string{"param1": "value1"}, + driverName: mockDriverName, + size: defaultSize, + snapshotId: "snapuid1-1-deleted", + creationTime: timeNow, + readyToUse: true, + }, + }, + expectedListCalls: []listCall{{"sid1-1", true, time.Now(), 1, nil}}, + expectedDeleteCalls: []deleteCall{{"sid1-1", nil, nil}}, + test: testSyncContent, + },*/ + + /* name: "1-3 - successful delete with snapshot class that has valid secret parameter", + initialContents: newContentArray("content1-3", "sid1-3", "snap1-3", "sid1-3", validSecretClass, "", "snap1-3-volumehandle", deletionPolicy, nil, nil, true), + expectedContents: newContentArray("content1-3", "sid1-3", "snap1-3", "sid1-3", validSecretClass, "", "snap1-3-volumehandle", deletionPolicy, nil, nil, false), + initialSnapshots: newSnapshotArray("snapshot-snapuid1-3", "snapuid1-3", "claim1-3", "", validSecretClass, "", &False, nil, nil, nil), + expectedSnapshots: newSnapshotArray("snapshot-snapuid1-3", "snapuid1-3", "claim1-3", "", validSecretClass, "", &False, nil, nil, nil), + expectedEvents: noevents, + errors: noerrors, + initialSecrets: []*v1.Secret{secret()}, + expectedCreateCalls: []createCall{ + { + snapshotName: "snap1-3", + volumeHandle: "snap1-3-volumehandle", + parameters: map[string]string{"param1": "value1"}, + driverName: mockDriverName, + size: defaultSize, + snapshotId: "sid1-3", + creationTime: timeNow, + }, + }, + expectedListCalls: []listCall{{"sid1-3", true, time.Now(), 1, nil}}, + expectedDeleteCalls: []deleteCall{{"sid1-3", map[string]string{"param1": "value1"}, nil}}, + test: testSyncContent, + }, + */ + { + name: "1-4 - fail delete with snapshot class that has invalid secret parameter", + initialContents: newContentArrayWithDeletionTimestamp("content1-1", "snapuid1-1", "snap1-1", "sid1-1", "invalid", "", "snap1-1-volumehandle", deletionPolicy, nil, nil, true, &timeNowMetav1), + expectedContents: newContentArrayWithDeletionTimestamp("content1-1", "snapuid1-1", "snap1-1", "sid1-1", "invalid", "", "snap1-1-volumehandle", deletionPolicy, nil, nil, false, &timeNowMetav1), + expectedEvents: noevents, + errors: noerrors, + test: testSyncContent, + }, + { + name: "1-5 - csi driver delete snapshot returns error", + initialContents: newContentArrayWithDeletionTimestamp("content1-5", "sid1-5", "snap1-5", "sid1-5", validSecretClass, "", "", deletionPolicy, nil, &defaultSize, true, &timeNowMetav1), + expectedContents: newContentArrayWithDeletionTimestamp("content1-5", "sid1-5", "snap1-5", "sid1-5", validSecretClass, "", "", deletionPolicy, nil, &defaultSize, false, &timeNowMetav1), + expectedListCalls: []listCall{{"sid1-5", true, time.Now(), 1000, nil}}, + expectedDeleteCalls: []deleteCall{{"sid1-5", nil, errors.New("mock csi driver delete error")}}, + expectedEvents: []string{"Warning SnapshotDeleteError"}, + errors: noerrors, + test: testSyncContent, + }, + /*{ + name: "1-6 - api server delete content returns error", + initialContents: newContentArray("content1-6", "sid1-6", "snap1-6", "sid1-6", classGold, "", "", deletionPolicy, nil, nil, true), + //expectedContents: newContentArray("content1-6", "sid1-6", "snap1-6", "sid1-6", classGold, "", "", deletionPolicy, nil, nil, true), + expectedDeleteCalls: []deleteCall{{"sid1-6", map[string]string{"foo": "bar"}, nil}}, + expectedListCalls: []listCall{{"sid1-6", false, time.Now(), 0, nil}}, + expectedEvents: []string{"Warning SnapshotContentObjectDeleteError"}, + errors: []reactorError{ + // Inject error to the first client.VolumesnapshotV1beta1().VolumeSnapshotContents().Delete call. + // All other calls will succeed. + {"delete", "volumesnapshotcontents", errors.New("mock delete error")}, + }, + test: testSyncContent, + },*/ + /* + { + // delete success - snapshot that the content was pointing to was deleted, and another + // with the same name created. + name: "1-7 - prebound content is deleted while the snapshot exists", + initialContents: newContentArray("content1-7", "sid1-7", "snap1-7", "sid1-7", emptySecretClass, "", "", deletionPolicy, nil, nil, true), + expectedContents: nocontents, + initialSecrets: []*v1.Secret{secret()}, + expectedDeleteCalls: []deleteCall{{"sid1-7", map[string]string{"foo": "bar"}, nil}}, + expectedEvents: noevents, + errors: noerrors, + test: testSyncContent, + },*/ + { + // delete success(?) - content is deleted before doDelete() starts + name: "1-8 - content is deleted before deleting", + initialContents: newContentArray("content1-8", "sid1-8", "snap1-8", "sid1-8", classGold, "", "", deletionPolicy, nil, nil, true), + expectedContents: nocontents, + expectedListCalls: []listCall{{"sid1-8", false, time.Now(), 0, nil}}, + expectedDeleteCalls: []deleteCall{{"sid1-8", map[string]string{"foo": "bar"}, nil}}, + expectedEvents: noevents, + errors: noerrors, + test: wrapTestWithInjectedOperation(testSyncContent, func(ctrl *csiSnapshotSideCarController, reactor *snapshotReactor) { + // Delete the volume before delete operation starts + reactor.lock.Lock() + delete(reactor.contents, "content1-8") + reactor.lock.Unlock() + }), + }, + /*{ + name: "1-9 - content will not be deleted if it is bound to a snapshot correctly, snapshot uid is specified", + expectedContents: newContentArrayWithDeletionTimestamp("content1-9", "snapuid1-9", "snap1-9", "sid1-9", classGold, "", "snap1-9-volumehandle", retainPolicy, nil, nil, false, &timeNowMetav1), + initialContents: newContentArrayWithDeletionTimestamp("content1-9", "snapuid1-9", "snap1-9", "sid1-9", classGold, "", "snap1-9-volumehandle", retainPolicy, nil, nil, false, &timeNowMetav1), + expectedEvents: noevents, + expectedListCalls: []listCall{{"sid1-9", true, time.Now(), 0, nil}}, + initialSecrets: []*v1.Secret{secret()}, + errors: noerrors, + test: testSyncContent, + },*/ + /* + { + name: "1-10 - will not delete content with retain policy set which is bound to a snapshot incorrectly", + initialContents: newContentArray("content1-10", "snapuid1-10-x", "snap1-10", "sid1-10", validSecretClass, "", "", retainPolicy, nil, nil, true), + expectedContents: newContentArray("content1-10", "snapuid1-10-x", "snap1-10", "sid1-10", validSecretClass, "", "", retainPolicy, nil, nil, true), + initialSnapshots: newSnapshotArray("snap1-10", "snapuid1-10", "claim1-10", "", validSecretClass, "content1-10", &False, nil, nil, nil), + expectedSnapshots: newSnapshotArray("snap1-10", "snapuid1-10", "claim1-10", "", validSecretClass, "content1-10", &False, nil, nil, nil), + expectedEvents: noevents, + initialSecrets: []*v1.Secret{secret()}, + errors: noerrors, + test: testSyncContent, + },*/ + { + name: "1-11 - content will not be deleted if it is bound to a snapshot correctly, snapsht uid is not specified", + initialContents: newContentArrayWithReadyToUse("content1-11", "", "snap1-11", "sid1-11", validSecretClass, "", "", deletePolicy, nil, &defaultSize, &True, true), + expectedContents: newContentArrayWithReadyToUse("content1-11", "", "snap1-11", "sid1-11", validSecretClass, "", "", deletePolicy, nil, &defaultSize, &True, true), + expectedEvents: noevents, + expectedCreateCalls: []createCall{ + { + snapshotName: "snap1-11", + volumeHandle: "snap1-11", + parameters: map[string]string{"param1": "value1"}, + driverName: mockDriverName, + size: defaultSize, + snapshotId: "snapuid1-1-deleted", + creationTime: timeNow, + readyToUse: true, + }, + }, + expectedListCalls: []listCall{{"sid1-11", true, time.Now(), 1000, nil}}, + initialSecrets: []*v1.Secret{secret()}, + errors: noerrors, + test: testSyncContent, + }, + { + name: "1-12 - content with retain policy will not be deleted if it is bound to a non-exist snapshot and also has a snapshot uid specified", + initialContents: newContentArrayWithReadyToUse("content1-12", "sid1-12", "snap1-11", "sid1-11", validSecretClass, "", "", retainPolicy, nil, &defaultSize, &True, true), + expectedContents: newContentArrayWithReadyToUse("content1-12", "sid1-12", "snap1-11", "sid1-11", validSecretClass, "", "", retainPolicy, nil, &defaultSize, &True, true), + expectedEvents: noevents, + expectedListCalls: []listCall{{"sid1-11", true, time.Now(), 0, nil}}, + errors: noerrors, + test: testSyncContent, + }, + /*{ + name: "1-13 - content with empty snapshot class is not deleted when Deletion policy is not set even if it is bound to a non-exist snapshot and also has a snapshot uid specified", + initialContents: newContentArray("content1-13", "sid1-13", "snap1-13", "sid1-13", validSecretClass, "", "", retainPolicy, nil, nil, true), + expectedContents: newContentArray("content1-13", "sid1-13", "snap1-13", "sid1-13", validSecretClass, "", "", retainPolicy, nil, nil, true), + expectedEvents: noevents, + errors: noerrors, + test: testSyncContent, + }, + { + name: "1-14 - content will not be deleted if it is bound to a snapshot correctly, snapshot uid is specified", + initialContents: newContentArray("content1-14", "snapuid1-14", "snap1-14", "sid1-14", validSecretClass, "", "", retainPolicy, nil, nil, true), + expectedContents: newContentArray("content1-14", "snapuid1-14", "snap1-14", "sid1-14", validSecretClass, "", "", retainPolicy, nil, nil, true), + expectedEvents: noevents, + initialSecrets: []*v1.Secret{secret()}, + errors: noerrors, + test: testSyncContent, + },*/ + { + name: "1-16 - continue delete with snapshot class that has nonexistent secret", + initialContents: newContentArrayWithDeletionTimestamp("content1-16", "sid1-16", "snap1-16", "sid1-16", emptySecretClass, "", "", deletePolicy, nil, &defaultSize, true, &timeNowMetav1), + expectedContents: newContentArrayWithDeletionTimestamp("content1-16", "sid1-16", "snap1-16", "sid1-16", emptySecretClass, "", "", deletePolicy, nil, &defaultSize, false, &timeNowMetav1), + expectedEvents: noevents, + expectedListCalls: []listCall{{"sid1-16", true, time.Now(), 0, nil}}, + errors: noerrors, + initialSecrets: []*v1.Secret{}, // secret does not exist + expectedDeleteCalls: []deleteCall{{"sid1-16", nil, nil}}, + test: testSyncContent, + }, + } + runSyncContentTests(t, tests, snapshotClasses) +} diff --git a/pkg/sidecar-controller/snapshot_finalizer_test.go b/pkg/sidecar-controller/snapshot_finalizer_test.go new file mode 100644 index 000000000..a11ea2d4c --- /dev/null +++ b/pkg/sidecar-controller/snapshot_finalizer_test.go @@ -0,0 +1,34 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package sidecar_controller + +import ( + "testing" +) + +// Test single call to ensureSnapshotSourceFinalizer and checkandRemoveSnapshotSourceFinalizer, +// expecting PVCFinalizer to be added or removed +func TestContentFinalizer(t *testing.T) { + + // GG TODO - add content finalizer tests + /* + tests := []controllerTest{ + {}, + } + runPVCFinalizerTests(t, tests, snapshotClasses) + */ +} From e4e2f3b44b9be3d1de9dca3f5bc1f26d036ec194 Mon Sep 17 00:00:00 2001 From: Grant Griffiths Date: Fri, 8 Nov 2019 15:55:46 -0800 Subject: [PATCH 2/2] Cleanup framework_test comments and old code Signed-off-by: Grant Griffiths --- pkg/sidecar-controller/framework_test.go | 124 ++++------------------- 1 file changed, 18 insertions(+), 106 deletions(-) diff --git a/pkg/sidecar-controller/framework_test.go b/pkg/sidecar-controller/framework_test.go index 49ec971ea..73b8ea357 100644 --- a/pkg/sidecar-controller/framework_test.go +++ b/pkg/sidecar-controller/framework_test.go @@ -33,7 +33,6 @@ import ( storagelisters "github.com/kubernetes-csi/external-snapshotter/pkg/client/listers/volumesnapshot/v1beta1" "github.com/kubernetes-csi/external-snapshotter/pkg/utils" v1 "k8s.io/api/core/v1" - storagev1 "k8s.io/api/storage/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -50,29 +49,26 @@ import ( "k8s.io/klog" ) -// This is a unit test framework for snapshot controller. -// It fills the controller with test snapshots/contents and can simulate these +// This is a unit test framework for snapshot sidecar controller. +// It fills the controller with test contents and can simulate these // scenarios: -// 1) Call syncSnapshot/syncContent once. -// 2) Call syncSnapshot/syncContent several times (both simulating "snapshot/content +// 1) Call syncContent once. +// 2) Call syncContent several times (both simulating "content // modified" events and periodic sync), until the controller settles down and // does not modify anything. // 3) Simulate almost real API server/etcd and call add/update/delete -// content/snapshot. +// content. // In all these scenarios, when the test finishes, the framework can compare -// resulting snapshots/contents with list of expected snapshots/contents and report +// resulting contents with list of expected contents and report // differences. // controllerTest contains a single controller test input. -// Each test has initial set of contents and snapshots that are filled into the +// Each test has initial set of contents that are filled into the // controller before the test starts. The test then contains a reference to // function to call as the actual test. Available functions are: -// - testSyncSnapshot - calls syncSnapshot on the first snapshot in initialSnapshots. -// - testSyncSnapshotError - calls syncSnapshot on the first snapshot in initialSnapshots -// and expects an error to be returned. // - testSyncContent - calls syncContent on the first content in initialContents. // - any custom function for specialized tests. -// The test then contains list of contents/snapshots that are expected at the end +// The test then contains list of contents that are expected at the end // of the test and list of generated events. type controllerTest struct { // Name of the test, for logging @@ -106,19 +102,17 @@ const mockDriverName = "csi-mock-plugin" var errVersionConflict = errors.New("VersionError") var nocontents []*crdv1.VolumeSnapshotContent -var nosnapshots []*crdv1.VolumeSnapshot var noevents = []string{} var noerrors = []reactorError{} // snapshotReactor is a core.Reactor that simulates etcd and API server. It // stores: // - Latest version of snapshots contents saved by the controller. -// - Queue of all saves (to simulate "content/snapshot updated" events). This queue -// contains all intermediate state of an object - e.g. a snapshot.VolumeName -// is updated first and snapshot.Phase second. This queue will then contain both +// - Queue of all saves (to simulate "content updated" events). This queue +// contains all intermediate state of an object. This queue will then contain both // updates as separate entries. // - Number of changes since the last call to snapshotReactor.syncAll(). -// - Optionally, content and snapshot fake watchers which should be the same ones +// - Optionally, content watcher which should be the same ones // used by the controller. Any time an event function like deleteContentEvent // is called to simulate an event, the reactor's stores are updated and the // controller is sent the event via the fake watcher. @@ -129,16 +123,11 @@ var noerrors = []reactorError{} // the list. type snapshotReactor struct { secrets map[string]*v1.Secret - storageClasses map[string]*storagev1.StorageClass - volumes map[string]*v1.PersistentVolume - claims map[string]*v1.PersistentVolumeClaim contents map[string]*crdv1.VolumeSnapshotContent - snapshots map[string]*crdv1.VolumeSnapshot changedObjects []interface{} changedSinceLastSync int ctrl *csiSnapshotSideCarController fakeContentWatch *watch.FakeWatcher - fakeSnapshotWatch *watch.FakeWatcher lock sync.Mutex errors []reactorError } @@ -146,7 +135,7 @@ type snapshotReactor struct { // reactorError is an error that is returned by test reactor (=simulated // etcd+/API server) when an action performed by the reactor matches given verb // ("get", "update", "create", "delete" or "*"") on given resource -// ("volumesnapshotcontents", "volumesnapshots" or "*"). +// ("volumesnapshotcontents" or "*"). type reactorError struct { verb string resource string @@ -203,9 +192,9 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O content := obj.(*crdv1.VolumeSnapshotContent) // Check and bump object version - storedVolume, found := r.contents[content.Name] + storedContent, found := r.contents[content.Name] if found { - storedVer, _ := strconv.Atoi(storedVolume.ResourceVersion) + storedVer, _ := strconv.Atoi(storedContent.ResourceVersion) requestedVer, _ := strconv.Atoi(content.ResourceVersion) if storedVer != requestedVer { return true, obj, errVersionConflict @@ -314,41 +303,6 @@ func (r *snapshotReactor) checkContents(expectedContents []*crdv1.VolumeSnapshot return nil } -// checkSnapshots compares all expectedSnapshots with set of snapshots at the end of the -// test and reports differences. -func (r *snapshotReactor) checkSnapshots(expectedSnapshots []*crdv1.VolumeSnapshot) error { - r.lock.Lock() - defer r.lock.Unlock() - - expectedMap := make(map[string]*crdv1.VolumeSnapshot) - gotMap := make(map[string]*crdv1.VolumeSnapshot) - for _, c := range expectedSnapshots { - // Don't modify the existing object - c = c.DeepCopy() - c.ResourceVersion = "" - if c.Status.Error != nil { - c.Status.Error.Time = &metav1.Time{} - } - expectedMap[c.Name] = c - } - for _, c := range r.snapshots { - // We must clone the snapshot because of golang race check - it was - // written by the controller without any locks on it. - c = c.DeepCopy() - c.ResourceVersion = "" - if c.Status.Error != nil { - c.Status.Error.Time = &metav1.Time{} - } - gotMap[c.Name] = c - } - if !reflect.DeepEqual(expectedMap, gotMap) { - // Print ugly but useful diff of expected and received objects for - // easier debugging. - return fmt.Errorf("snapshot check failed [A-expected, B-got result]: %s", diff.ObjectDiff(expectedMap, gotMap)) - } - return nil -} - // checkEvents compares all expectedEvents with events generated during the test // and reports differences. func checkEvents(t *testing.T, expectedEvents []string, ctrl *csiSnapshotSideCarController) error { @@ -414,9 +368,6 @@ func (r *snapshotReactor) popChange() interface{} { case *crdv1.VolumeSnapshotContent: vol, _ := obj.(*crdv1.VolumeSnapshotContent) klog.V(4).Infof("reactor queue: %s", vol.Name) - case *crdv1.VolumeSnapshot: - snapshot, _ := obj.(*crdv1.VolumeSnapshot) - klog.V(4).Infof("reactor queue: %s", snapshot.Name) } } @@ -426,23 +377,17 @@ func (r *snapshotReactor) popChange() interface{} { return obj } -// syncAll simulates the controller periodic sync of contents and snapshot. It +// syncAll simulates the controller periodic sync of contents. It // simply adds all these objects to the internal queue of updates. This method -// should be used when the test manually calls syncSnapshot/syncContent. Test that +// should be used when the test manually calls syncContent. Test that // use real controller loop (ctrl.Run()) will get periodic sync automatically. func (r *snapshotReactor) syncAll() { r.lock.Lock() defer r.lock.Unlock() - for _, c := range r.snapshots { - r.changedObjects = append(r.changedObjects, c) - } for _, v := range r.contents { r.changedObjects = append(r.changedObjects, v) } - for _, pvc := range r.claims { - r.changedObjects = append(r.changedObjects, pvc) - } r.changedSinceLastSync = 0 } @@ -511,22 +456,6 @@ func (r *snapshotReactor) deleteContentEvent(content *crdv1.VolumeSnapshotConten } } -// deleteSnapshotEvent simulates that a snapshot has been deleted in etcd and the -// controller receives 'snapshot deleted' event. -func (r *snapshotReactor) deleteSnapshotEvent(snapshot *crdv1.VolumeSnapshot) { - r.lock.Lock() - defer r.lock.Unlock() - - // Remove the snapshot from list of resulting snapshots. - delete(r.snapshots, snapshot.Name) - - // Generate deletion event. Cloned content is needed to prevent races (and we - // would get a clone from etcd too). - if r.fakeSnapshotWatch != nil { - r.fakeSnapshotWatch.Delete(snapshot.DeepCopy()) - } -} - // addContentEvent simulates that a content has been added in etcd and the // controller receives 'content added' event. func (r *snapshotReactor) addContentEvent(content *crdv1.VolumeSnapshotContent) { @@ -555,20 +484,6 @@ func (r *snapshotReactor) modifyContentEvent(content *crdv1.VolumeSnapshotConten } } -// addSnapshotEvent simulates that a snapshot has been deleted in etcd and the -// controller receives 'snapshot added' event. -func (r *snapshotReactor) addSnapshotEvent(snapshot *crdv1.VolumeSnapshot) { - r.lock.Lock() - defer r.lock.Unlock() - - r.snapshots[snapshot.Name] = snapshot - // Generate event. No cloning is needed, this snapshot is not stored in the - // controller cache yet. - if r.fakeSnapshotWatch != nil { - r.fakeSnapshotWatch.Add(snapshot) - } -} - 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), @@ -580,11 +495,8 @@ func newSnapshotReactor(kubeClient *kubefake.Clientset, client *fake.Clientset, client.AddReactor("create", "volumesnapshotcontents", reactor.React) client.AddReactor("update", "volumesnapshotcontents", reactor.React) - client.AddReactor("update", "volumesnapshots", reactor.React) client.AddReactor("get", "volumesnapshotcontents", reactor.React) - client.AddReactor("get", "volumesnapshots", reactor.React) client.AddReactor("delete", "volumesnapshotcontents", reactor.React) - client.AddReactor("delete", "volumesnapshots", reactor.React) return reactor } @@ -765,7 +677,7 @@ func wrapTestWithInjectedOperation(toWrap testCall, injectBeforeOperation func(c klog.V(4).Infof("reactor:injecting call") injectBeforeOperation(ctrl, reactor) - // Run the tested function (typically syncSnapshot/syncContent) in a + // Run the tested function (typically syncContent) in a // separate goroutine. var testError error var testFinished int32 @@ -798,7 +710,7 @@ func evaluateTestResults(ctrl *csiSnapshotSideCarController, reactor *snapshotRe } } -// Test single call to syncSnapshot and syncContent methods. +// Test single call to syncContent methods. // For all tests: // 1. Fill in the controller with initial data // 2. Call the tested function (syncContent) via