Merge branch 'master' into beta

CHANGELOG-1.3.md

@@ -0,0 +1,9 @@
+# Changelog since v1.2.0
+
+## Breaking Changes
+
+- Add a prefix "external-snapshotter-leader" and the driver name to the snapshotter leader election lock name. Rolling update will not work if leader election is enabled because the lock name is changed in v1.3.0. ([#129](https://github.com/kubernetes-csi/external-snapshotter/pull/129), [@zhucan](https://github.com/zhucan))
+
+## Other Notable Changes
+
+- Snapshotter will no longer call ListSnapshots if the CSI Driver does not support this operation. ([#138](https://github.com/kubernetes-csi/external-snapshotter/pull/138), [@ggriffiths](https://github.com/ggriffiths))

pkg/controller/framework_test.go

@@ -784,6 +784,7 @@ func newContent(name, snapshotHandle, boundToSnapshotUID, boundToSnapshotName st
 		ObjectMeta: metav1.ObjectMeta{
 			Name:            name,
 			ResourceVersion: "1",
+			Annotations:     annotations,
 		},
 		Spec: crdv1.VolumeSnapshotContentSpec{
 			RestoreSize:    size,
@@ -1263,6 +1264,28 @@ func secret() *v1.Secret {
 	}
 }
 
+func secretAnnotations() map[string]string {
+	return map[string]string{
+		AnnDeletionSecretRefName:      "secret",
+		AnnDeletionSecretRefNamespace: "default",
+	}
+}
+
+func emptyNamespaceSecretAnnotations() map[string]string {
+	return map[string]string{
+		AnnDeletionSecretRefName:      "name",
+		AnnDeletionSecretRefNamespace: "",
+	}
+}
+
+// this refers to emptySecret(), which is missing data.
+func emptyDataSecretAnnotations() map[string]string {
+	return map[string]string{
+		AnnDeletionSecretRefName:      "emptysecret",
+		AnnDeletionSecretRefNamespace: "default",
+	}
+}
+
 type listCall struct {
 	snapshotID string
 	// information to return
@@ -1344,7 +1367,7 @@ func (f *fakeSnapshotter) CreateSnapshot(ctx context.Context, snapshotName strin
 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 call")
+		return fmt.Errorf("unexpected DeleteSnapshot call")
 	}
 	call := f.deleteCalls[f.deleteCallCounter]
 	f.deleteCallCounter++

pkg/controller/snapshot_controller.go

@@ -527,7 +527,7 @@ func (ctrl *csiSnapshotController) checkandBindSnapshotContent(snapshot *crdv1.V
 	return newContent, nil
 }
 
-func (ctrl *csiSnapshotController) getCreateSnapshotInput(snapshot *crdv1.VolumeSnapshot) (*crdv1.VolumeSnapshotClass, *v1.PersistentVolume, string, map[string]string, error) {
+func (ctrl *csiSnapshotController) getCreateSnapshotInput(snapshot *crdv1.VolumeSnapshot) (*crdv1.VolumeSnapshotClass, *v1.PersistentVolume, string, *v1.SecretReference, error) {
 	className := snapshot.Spec.VolumeSnapshotClassName
 	klog.V(5).Infof("getCreateSnapshotInput [%s]: VolumeSnapshotClassName [%s]", snapshot.Name, *className)
 	var class *crdv1.VolumeSnapshotClass
@@ -557,12 +557,8 @@ func (ctrl *csiSnapshotController) getCreateSnapshotInput(snapshot *crdv1.Volume
 	if err != nil {
 		return nil, nil, "", nil, err
 	}
-	snapshotterCredentials, err := getCredentials(ctrl.client, snapshotterSecretRef)
-	if err != nil {
-		return nil, nil, "", nil, err
-	}
 
-	return class, volume, contentName, snapshotterCredentials, nil
+	return class, volume, contentName, snapshotterSecretRef, nil
 }
 
 func (ctrl *csiSnapshotController) checkandUpdateBoundSnapshotStatusOperation(snapshot *crdv1.VolumeSnapshot, content *crdv1.VolumeSnapshotContent) (*crdv1.VolumeSnapshot, error) {
@@ -582,10 +578,14 @@ func (ctrl *csiSnapshotController) checkandUpdateBoundSnapshotStatusOperation(sn
 		}
 		driverName, snapshotID = content.Spec.Driver, content.Spec.SnapshotHandle
 	} else {
-		class, volume, _, snapshotterCredentials, err := ctrl.getCreateSnapshotInput(snapshot)
+		class, volume, _, snapshotterSecretRef, err := ctrl.getCreateSnapshotInput(snapshot)
 		if err != nil {
 			return nil, fmt.Errorf("failed to get input parameters to create snapshot %s: %q", snapshot.Name, err)
 		}
+		snapshotterCredentials, err := getCredentials(ctrl.client, snapshotterSecretRef)
+		if err != nil {
+			return nil, err
+		}
 		driverName, snapshotID, creationTime, size, readyToUse, err = ctrl.handler.CreateSnapshot(snapshot, volume, class.Parameters, snapshotterCredentials)
 		if err != nil {
 			klog.Errorf("checkandUpdateBoundSnapshotStatusOperation: failed to call create snapshot to check whether the snapshot is ready to use %q", err)
@@ -629,11 +629,16 @@ func (ctrl *csiSnapshotController) createSnapshotOperation(snapshot *crdv1.Volum
 		return nil, err
 	}
 
-	class, volume, contentName, snapshotterCredentials, err := ctrl.getCreateSnapshotInput(snapshot)
+	class, volume, contentName, snapshotterSecretRef, err := ctrl.getCreateSnapshotInput(snapshot)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get input parameters to create snapshot %s: %q", snapshot.Name, err)
 	}
 
+	snapshotterCredentials, err := getCredentials(ctrl.client, snapshotterSecretRef)
+	if err != nil {
+		return nil, err
+	}
+
 	driverName, snapshotID, creationTime, size, readyToUse, err := ctrl.handler.CreateSnapshot(snapshot, volume, class.Parameters, snapshotterCredentials)
 	if err != nil {
 		return nil, fmt.Errorf("failed to take snapshot of the volume, %s: %q", volume.Name, err)
@@ -679,6 +684,16 @@ func (ctrl *csiSnapshotController) createSnapshotOperation(snapshot *crdv1.Volum
 			DeletionPolicy:    class.DeletionPolicy,
 		},
 	}
+
+	// Set AnnDeletionSecretRefName and AnnDeletionSecretRefNamespace
+	if snapshotterSecretRef != nil {
+		klog.V(5).Infof("createSnapshotOperation: set annotation [%s] on content [%s].", AnnDeletionSecretRefName, snapshotContent.Name)
+		metav1.SetMetaDataAnnotation(&snapshotContent.ObjectMeta, AnnDeletionSecretRefName, snapshotterSecretRef.Name)
+
+		klog.V(5).Infof("syncContent: set annotation [%s] on content [%s].", AnnDeletionSecretRefNamespace, snapshotContent.Name)
+		metav1.SetMetaDataAnnotation(&snapshotContent.ObjectMeta, AnnDeletionSecretRefNamespace, snapshotterSecretRef.Namespace)
+	}
+
 	klog.V(3).Infof("volume snapshot content %v", snapshotContent)
 	// Try to create the VolumeSnapshotContent object several times
 	for i := 0; i < ctrl.createSnapshotContentRetryCount; i++ {
@@ -728,7 +743,8 @@ func (ctrl *csiSnapshotController) deleteSnapshotContentOperation(content *crdv1
 
 	// get secrets if VolumeSnapshotClass specifies it
 	var snapshotterCredentials map[string]string
-	/*snapshotClassName := content.Spec.VolumeSnapshotClassName
+  /* TODO(@Xing-yang): secrete ref retrivial needs to be implemented here
+  snapshotClassName := content.Spec.VolumeSnapshotClassName
 	if snapshotClassName != nil {
 		if snapshotClass, err := ctrl.classLister.Get(*snapshotClassName); err == nil {
 			// Resolve snapshotting secret credentials.
@@ -744,7 +760,7 @@ func (ctrl *csiSnapshotController) deleteSnapshotContentOperation(content *crdv1
 		}
 	}*/
 
-	err := ctrl.handler.DeleteSnapshot(content, snapshotterCredentials)
+	err = ctrl.handler.DeleteSnapshot(content, snapshotterCredentials)
 	if err != nil {
 		ctrl.eventRecorder.Event(content, v1.EventTypeWarning, "SnapshotDeleteError", "Failed to delete snapshot")
 		return fmt.Errorf("failed to delete snapshot %#v, err: %v", content.Name, err)

pkg/controller/snapshot_controller_test.go

@@ -81,7 +81,6 @@ 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", "sid1-1", "snapuid1-1", "snap1-1", nil, nil, nil, false)
 	content.ResourceVersion = "xxx"
 	_, err := storeObjectUpdate(c, content, "content")

pkg/controller/snapshot_create_test.go

@@ -365,6 +365,18 @@ func TestCreateSnapshotSync(t *testing.T) {
 			expectedEvents: []string{"Warning CreateSnapshotContentFailed"},
 			test:           testSyncSnapshot,
 		},
+		{
+			name:              "7-10 - fail create snapshot with secret not found",
+			initialContents:   nocontents,
+			expectedContents:  nocontents,
+			initialSnapshots:  newSnapshotArray("snap7-10", validSecretClass, "", "snapuid7-10", "claim7-10", false, nil, nil, nil),
+			expectedSnapshots: newSnapshotArray("snap7-10", validSecretClass, "", "snapuid7-10", "claim7-10", false, newVolumeError("Failed to create snapshot: error getting secret secret in namespace default: cannot find secret secret"), nil, nil),
+			initialClaims:     newClaimArray("claim7-10", "pvc-uid7-10", "1Gi", "volume7-10", v1.ClaimBound, &classEmpty),
+			initialVolumes:    newVolumeArray("volume7-10", "pv-uid7-10", "pv-handle7-10", "1Gi", "pvc-uid7-10", "claim7-10", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
+			initialSecrets:    []*v1.Secret{}, // no initial secret created
+			errors:            noerrors,
+			test:              testSyncSnapshot,
+		},
 	}
 	runSyncTests(t, tests, snapshotClasses)
 }

pkg/controller/snapshot_delete_test.go

@@ -320,6 +320,18 @@ func TestDeleteSync(t *testing.T) {
 			errors:            noerrors,
 			test:              testSyncContent,
 		},
+		{
+			name:                "1-16 - continue delete with snapshot class that has nonexistent secret",
+			initialContents:     newContentArray("content1-16", validSecretClass, "sid1-16", "vuid1-16", "volume1-16", "snapuid1-16", "snap1-16", &deletePolicy, nil, nil, true, secretAnnotations()),
+			expectedContents:    nocontents,
+			initialSnapshots:    nosnapshots,
+			expectedSnapshots:   nosnapshots,
+			expectedEvents:      noevents,
+			errors:              noerrors,
+			initialSecrets:      []*v1.Secret{}, // secret does not exist
+			expectedDeleteCalls: []deleteCall{{"sid1-16", nil, nil}},
+			test:                testSyncContent,
+		},
 	}
 	runSyncTests(t, tests, snapshotClasses)
 }

pkg/controller/util.go

@@ -65,6 +65,11 @@ const (
 	// Name of finalizer on VolumeSnapshotContents that are bound by VolumeSnapshots
 	VolumeSnapshotContentFinalizer = "snapshot.storage.kubernetes.io/volumesnapshotcontent-protection"
 	VolumeSnapshotFinalizer        = "snapshot.storage.kubernetes.io/volumesnapshot-protection"
+
+	// Annotation for secret name and namespace will be added to the content
+	// and used at snapshot content deletion time.
+	AnnDeletionSecretRefName      = "snapshot.storage.kubernetes.io/deletion-secret-name"
+	AnnDeletionSecretRefNamespace = "snapshot.storage.kubernetes.io/deletion-secret-namespace"
 )
 
 var snapshotterSecretParams = deprecatedSecretParamsMap{
@@ -217,7 +222,6 @@ func verifyAndGetSecretNameAndNamespaceTemplate(secret deprecatedSecretParamsMap
 // - ${volumesnapshotcontent.name}
 // - ${volumesnapshot.namespace}
 // - ${volumesnapshot.name}
-// - ${volumesnapshot.annotations['ANNOTATION_KEY']} (e.g. ${pvc.annotations['example.com/snapshot-create-secret-name']})
 //
 // supported tokens for namespace resolution:
 // - ${volumesnapshotcontent.name}
@@ -262,16 +266,12 @@ func getSecretReference(snapshotClassParams map[string]string, snapContentName s
 	}
 	ref.Namespace = resolvedNamespace
 
-	// Secret name template can make use of the VolumeSnapshotContent name, VolumeSnapshot name or namespace,
-	// or a VolumeSnapshot annotation.
+	// Secret name template can make use of the VolumeSnapshotContent name, VolumeSnapshot name or namespace.
 	// Note that VolumeSnapshot name and annotations are under the VolumeSnapshot user's control.
 	nameParams := map[string]string{"volumesnapshotcontent.name": snapContentName}
 	if snapshot != nil {
 		nameParams["volumesnapshot.name"] = snapshot.Name
 		nameParams["volumesnapshot.namespace"] = snapshot.Namespace
-		for k, v := range snapshot.Annotations {
-			nameParams["volumesnapshot.annotations['"+k+"']"] = v
-		}
 	}
 	resolvedName, err := resolveTemplate(nameTemplate, nameParams)
 	if err != nil {

pkg/controller/util_test.go

@@ -70,7 +70,7 @@ func TestGetSecretReference(t *testing.T) {
 			expectRef: nil,
 			expectErr: true,
 		},
-		"template - valid": {
+		"template - invalid": {
 			params: map[string]string{
 				prefixedSnapshotterSecretNameKey:      "static-${volumesnapshotcontent.name}-${volumesnapshot.namespace}-${volumesnapshot.name}-${volumesnapshot.annotations['akey']}",
 				prefixedSnapshotterSecretNamespaceKey: "static-${volumesnapshotcontent.name}-${volumesnapshot.namespace}",
@@ -83,7 +83,8 @@ func TestGetSecretReference(t *testing.T) {
 					Annotations: map[string]string{"akey": "avalue"},
 				},
 			},
-			expectRef: &v1.SecretReference{Name: "static-snapcontentname-snapshotnamespace-snapshotname-avalue", Namespace: "static-snapcontentname-snapshotnamespace"},
+			expectRef: nil,
+			expectErr: true,
 		},
 		"template - invalid namespace tokens": {
 			params: map[string]string{

release-tools/build.make

@@ -63,6 +63,7 @@ endif
 build-%:
 	mkdir -p bin
 	CGO_ENABLED=0 GOOS=linux go build -a -ldflags '-X main.version=$(REV) -extldflags "-static"' -o ./bin/$* ./cmd/$*
+	CGO_ENABLED=0 GOOS=windows go build -a -ldflags '-X main.version=$(REV) -extldflags "-static"' -o ./bin/$*.exe ./cmd/$*
 
 container-%: build-%
 	docker build -t $*:latest -f $(shell if [ -e ./cmd/$*/Dockerfile ]; then echo ./cmd/$*/Dockerfile; else echo Dockerfile; fi) --label revision=$(REV) .

release-tools/prow.sh

@@ -101,7 +101,7 @@ configvar CSI_PROW_GO_VERSION_GINKGO "${CSI_PROW_GO_VERSION_BUILD}" "Go version
 # kind version to use. If the pre-installed version is different,
 # the desired version is downloaded from https://github.com/kubernetes-sigs/kind/releases/download/
 # (if available), otherwise it is built from source.
-configvar CSI_PROW_KIND_VERSION 2555d8e09d5a77ee718414cec9f6083dfa028dc5 "kind"
+configvar CSI_PROW_KIND_VERSION "v0.5.0" "kind"
 
 # ginkgo test runner version to use. If the pre-installed version is
 # different, the desired version is built from source.
@@ -126,19 +126,19 @@ configvar CSI_PROW_BUILD_JOB true "building code in repo enabled"
 # use the same settings as for "latest" Kubernetes. This works
 # as long as there are no breaking changes in Kubernetes, like
 # deprecating or changing the implementation of an alpha feature.
-configvar CSI_PROW_KUBERNETES_VERSION 1.13.3 "Kubernetes"
+configvar CSI_PROW_KUBERNETES_VERSION 1.15.3 "Kubernetes"
 
 # This is a hack to workaround the issue that each version
 # of kind currently only supports specific patch versions of
 # Kubernetes. We need to override CSI_PROW_KUBERNETES_VERSION
 # passed in by our CI/pull jobs to the versions that
-# kind v0.4.0 supports.
+# kind v0.5.0 supports.
 #
 # If the version is prefixed with "release-", then nothing
 # is overridden.
-override_k8s_version "1.13.7"
-override_k8s_version "1.14.3"
-override_k8s_version "1.15.0"
+override_k8s_version "1.13.10"
+override_k8s_version "1.14.6"
+override_k8s_version "1.15.3"
 
 # CSI_PROW_KUBERNETES_VERSION reduced to first two version numbers and
 # with underscore (1_13 instead of 1.13.3) and in uppercase (LATEST
@@ -223,6 +223,10 @@ configvar CSI_PROW_SANITY_SERVICE "hostpath-service" "Kubernetes TCP service nam
 configvar CSI_PROW_SANITY_POD "csi-hostpathplugin-0" "Kubernetes pod with CSI driver"
 configvar CSI_PROW_SANITY_CONTAINER "hostpath" "Kubernetes container with CSI driver"
 
+# The version of dep to use for 'make test-vendor'. Ignored if the project doesn't
+# use dep. Only binary releases of dep are supported (https://github.com/golang/dep/releases).
+configvar CSI_PROW_DEP_VERSION v0.5.1 "golang dep version to be used for vendor checking"
+
 # Each job can run one or more of the following tests, identified by
 # a single word:
 # - unit testing
@@ -396,6 +400,15 @@ install_ginkgo () {
     mv "$GOPATH/bin/ginkgo" "${CSI_PROW_BIN}"
 }
 
+# Ensure that we have the desired version of dep.
+install_dep () {
+    if dep version 2>/dev/null | grep -q "version:.*${CSI_PROW_DEP_VERSION}$"; then
+        return
+    fi
+    run curl --fail --location -o "${CSI_PROW_WORK}/bin/dep" "https://github.com/golang/dep/releases/download/v0.5.4/dep-linux-amd64" &&
+        chmod u+x "${CSI_PROW_WORK}/bin/dep"
+}
+
 # This checks out a repo ("https://github.com/kubernetes/kubernetes")
 # in a certain location ("$GOPATH/src/k8s.io/kubernetes") at
 # a certain revision (a hex commit hash, v1.13.1, master). It's okay
@@ -508,6 +521,7 @@ kind: Cluster
 apiVersion: kind.sigs.k8s.io/v1alpha3
 nodes:
 - role: control-plane
+- role: worker
 EOF
 
     # kubeadm has API dependencies between apiVersion and Kubernetes version
@@ -936,6 +950,10 @@ main () {
         # changes in "release-tools" in a PR (that fails the "is release-tools unmodified"
         # test).
         if tests_enabled "unit"; then
+            if [ -f Gopkg.toml ] && ! install_dep; then
+                warn "installing 'dep' failed, cannot test vendoring"
+                ret=1
+            fi
             if ! run_with_go "${CSI_PROW_GO_VERSION_BUILD}" make -k test 2>&1 | make_test_to_junit; then
                 warn "'make test' failed, proceeding anyway"
                 ret=1