Address review comments in controller

cmd/csi-snapshotter/main.go

@@ -49,12 +49,11 @@ const (
 var (
 	snapshotter                     = flag.String("snapshotter", "", "Name of the snapshotter. The snapshotter will only create snapshot content for snapshot that requests a VolumeSnapshotClass with a snapshotter field set equal to this name.")
 	kubeconfig                      = flag.String("kubeconfig", "", "Absolute path to the kubeconfig file. Required only when running out of cluster.")
-	resync                          = flag.Duration("resync", 10*time.Second, "Resync interval of the controller.")
 	connectionTimeout               = flag.Duration("connection-timeout", 1*time.Minute, "Timeout for waiting for CSI driver socket.")
 	csiAddress                      = flag.String("csi-address", "/run/csi/socket", "Address of the CSI driver socket.")
-	createSnapshotContentRetryCount = flag.Int("createSnapshotContentRetryCount", 5, "Number of retries when we create a snapshot data object for a snapshot.")
+	createSnapshotContentRetryCount = flag.Int("create-snapshotcontent-retrycount", 5, "Number of retries when we create a snapshot data object for a snapshot.")
-	createSnapshotContentInterval   = flag.Duration("createSnapshotContentInterval", 10*time.Second, "Interval between retries when we create a snapshot data object for a snapshot.")
+	createSnapshotContentInterval   = flag.Duration("create-snapshotcontent-interval", 10*time.Second, "Interval between retries when we create a snapshot data object for a snapshot.")
-	resyncPeriod                    = flag.Duration("resyncPeriod", 60*time.Second, "The period that should be used to re-sync the snapshot.")
+	resyncPeriod                    = flag.Duration("resync-period", 60*time.Second, "Resync interval of the controller.")
 	snapshotNamePrefix              = flag.String("snapshot-name-prefix", "snapshot", "Prefix to apply to the name of a created snapshot")
 	snapshotNameUUIDLength          = flag.Int("snapshot-name-uuid-length", -1, "Length in characters for the generated uuid of a created snapshot. Defaults behavior is to NOT truncate.")
 )
@@ -82,7 +81,7 @@ func main() {
 		os.Exit(1)
 	}
 
-	factory := informers.NewSharedInformerFactory(snapClient, *resync)
+	factory := informers.NewSharedInformerFactory(snapClient, *resyncPeriod)
 
 	// Create CRD resource
 	aeclientset, err := apiextensionsclient.NewForConfig(config)
@@ -110,7 +109,7 @@ func main() {
 	defer cancel()
 
 	// Find driver name
-	if snapshotter == nil {
+	if *snapshotter == "" {
 		*snapshotter, err = csiConn.GetDriverName(ctx)
 		if err != nil {
 			glog.Error(err.Error())
@@ -136,7 +135,12 @@ func main() {
 		os.Exit(1)
 	}
 
-	glog.V(2).Infof("Start NewCSISnapshotController with snapshotter [%s] kubeconfig [%s] resync [%+v] connectionTimeout [%+v] csiAddress [%s] createSnapshotContentRetryCount [%d] createSnapshotContentInterval [%+v] resyncPeriod [%+v] snapshotNamePrefix [%s] snapshotNameUUIDLength [%d]", *snapshotter, *kubeconfig, *resync, *connectionTimeout, *csiAddress, createSnapshotContentRetryCount, *createSnapshotContentInterval, *resyncPeriod, *snapshotNamePrefix, snapshotNameUUIDLength)
+	if len(*snapshotNamePrefix) == 0 {
+		glog.Error("Snapshot name prefix cannot be of length 0")
+		os.Exit(1)
+	}
+
+	glog.V(2).Infof("Start NewCSISnapshotController with snapshotter [%s] kubeconfig [%s] connectionTimeout [%+v] csiAddress [%s] createSnapshotContentRetryCount [%d] createSnapshotContentInterval [%+v] resyncPeriod [%+v] snapshotNamePrefix [%s] snapshotNameUUIDLength [%d]", *snapshotter, *kubeconfig, *connectionTimeout, *csiAddress, createSnapshotContentRetryCount, *createSnapshotContentInterval, *resyncPeriod, *snapshotNamePrefix, snapshotNameUUIDLength)
 
 	ctrl := controller.NewCSISnapshotController(
 		snapClient,

pkg/connection/connection.go

@@ -25,7 +25,6 @@ import (
 
 	"github.com/container-storage-interface/spec/lib/go/csi/v0"
 	"github.com/golang/glog"
-	crdv1 "github.com/kubernetes-csi/external-snapshotter/pkg/apis/volumesnapshot/v1alpha1"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/connectivity"
 	"k8s.io/api/core/v1"
@@ -47,7 +46,7 @@ type CSIConnection interface {
 	SupportsControllerListSnapshots(ctx context.Context) (bool, error)
 
 	// CreateSnapshot creates a snapshot for a volume
-	CreateSnapshot(ctx context.Context, snapshotName string, snapshot *crdv1.VolumeSnapshot, volume *v1.PersistentVolume, parameters map[string]string, snapshotterCredentials map[string]string) (driverName string, snapshotId string, timestamp int64, status *csi.SnapshotStatus, err error)
+	CreateSnapshot(ctx context.Context, snapshotName string, volume *v1.PersistentVolume, parameters map[string]string, snapshotterCredentials map[string]string) (driverName string, snapshotId string, timestamp int64, status *csi.SnapshotStatus, err error)
 
 	// DeleteSnapshot deletes a snapshot from a volume
 	DeleteSnapshot(ctx context.Context, snapshotID string, snapshotterCredentials map[string]string) (err error)
@@ -189,8 +188,8 @@ func (c *csiConnection) SupportsControllerListSnapshots(ctx context.Context) (bo
 	return false, nil
 }
 
-func (c *csiConnection) CreateSnapshot(ctx context.Context, snapshotName string, snapshot *crdv1.VolumeSnapshot, volume *v1.PersistentVolume, parameters map[string]string, snapshotterCredentials map[string]string) (string, string, int64, *csi.SnapshotStatus, error) {
+func (c *csiConnection) CreateSnapshot(ctx context.Context, snapshotName string, volume *v1.PersistentVolume, parameters map[string]string, snapshotterCredentials map[string]string) (string, string, int64, *csi.SnapshotStatus, error) {
-	glog.V(5).Infof("CSI CreateSnapshot: %s", snapshot.Name)
+	glog.V(5).Infof("CSI CreateSnapshot: %s", snapshotName)
 	if volume.Spec.CSI == nil {
 		return "", "", 0, nil, fmt.Errorf("CSIPersistentVolumeSource not defined in spec")
 	}
@@ -214,7 +213,7 @@ func (c *csiConnection) CreateSnapshot(ctx context.Context, snapshotName string,
 		return "", "", 0, nil, err
 	}
 
-	glog.V(5).Infof("CSI CreateSnapshot: %s driver name [%s] snapshot ID [%s] time stamp [%s] status [%s]", snapshot.Name, driverName, rsp.Snapshot.Id, rsp.Snapshot.CreatedAt, *rsp.Snapshot.Status)
+	glog.V(5).Infof("CSI CreateSnapshot: %s driver name [%s] snapshot ID [%s] time stamp [%s] status [%s]", snapshotName, driverName, rsp.Snapshot.Id, rsp.Snapshot.CreatedAt, *rsp.Snapshot.Status)
 	return driverName, rsp.Snapshot.Id, rsp.Snapshot.CreatedAt, rsp.Snapshot.Status, nil
 }
 
@@ -258,7 +257,6 @@ func (c *csiConnection) Close() error {
 
 func logGRPC(ctx context.Context, method string, req, reply interface{}, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error {
 	glog.V(5).Infof("GRPC call: %s", method)
-	glog.V(5).Infof("GRPC request: %+v", req)
 	err := invoker(ctx, method, req, reply, cc, opts...)
 	glog.V(5).Infof("GRPC response: %+v", reply)
 	glog.V(5).Infof("GRPC error: %v", err)

pkg/controller/csi_handler.go

@@ -66,7 +66,7 @@ func (handler *csiHandler) CreateSnapshot(snapshot *crdv1.VolumeSnapshot, volume
 	if err != nil {
 		return "", "", 0, nil, err
 	}
-	return handler.csiConnection.CreateSnapshot(ctx, snapshotName, snapshot, volume, parameters, snapshotterCredentials)
+	return handler.csiConnection.CreateSnapshot(ctx, snapshotName, volume, parameters, snapshotterCredentials)
 }
 
 func (handler *csiHandler) DeleteSnapshot(content *crdv1.VolumeSnapshotContent, snapshotterCredentials map[string]string) error {
@@ -101,9 +101,6 @@ func (handler *csiHandler) GetSnapshotStatus(content *crdv1.VolumeSnapshotConten
 func makeSnapshotName(prefix, snapshotUID string, snapshotNameUUIDLength int) (string, error) {
 	// create persistent name based on a volumeNamePrefix and volumeNameUUIDLength
 	// of PVC's UID
-	if len(prefix) == 0 {
-		return "", fmt.Errorf("Snapshot name prefix cannot be of length 0")
-	}
 	if len(snapshotUID) == 0 {
 		return "", fmt.Errorf("Corrupted snapshot object, it is missing UID")
 	}

pkg/controller/snapshot_controller.go

@@ -63,7 +63,7 @@ import (
 //
 // The dynamic snapshot creation is multi-step process: first controller triggers
 // snapshot creation though csi volume plugin which should return a snapshot after
-// it is created succesfully (however, the snapshot might not be ready to use yet if
+// it is created successfully (however, the snapshot might not be ready to use yet if
 // there is an uploading phase). The creationTimestamp will be updated according to
 // VolumeSnapshot, and then a VolumeSnapshotContent object is created to represent
 // this snapshot. After that, the controller will keep checking the snapshot status
@@ -77,22 +77,20 @@ const pvcKind = "PersistentVolumeClaim"
 
 const IsDefaultSnapshotClassAnnotation = "snapshot.storage.kubernetes.io/is-default-class"
 
-const snapshotterSecretNameKey = "csiSnapshotterSecretName"
-const snapshotterSecretNamespaceKey = "csiSnapshotterSecretNamespace"
-
 // syncContent deals with one key off the queue.  It returns false when it's time to quit.
 func (ctrl *csiSnapshotController) syncContent(content *crdv1.VolumeSnapshotContent) error {
 	glog.V(4).Infof("synchronizing VolumeSnapshotContent[%s]", content.Name)
 
-	// VolumeSnapshotContent is not bind to any VolumeSnapshot, this case rare and we just return err
+	// VolumeSnapshotContent is not bound to any VolumeSnapshot, this case rare and we just return err
 	if content.Spec.VolumeSnapshotRef == nil {
-		// content is not bind
+		// content is not bound
 		glog.V(4).Infof("synchronizing VolumeSnapshotContent[%s]: VolumeSnapshotContent is not bound to any VolumeSnapshot", content.Name)
+		ctrl.eventRecorder.Event(content, v1.EventTypeWarning, "SnapshotContentNotBound", "VolumeSnapshotContent is not bound to any VolumeSnapshot")
 		return fmt.Errorf("volumeSnapshotContent %s is not bound to any VolumeSnapshot", content.Name)
 	} else {
 		glog.V(4).Infof("synchronizing VolumeSnapshotContent[%s]: content is bound to snapshot %s", content.Name, snapshotRefKey(content.Spec.VolumeSnapshotRef))
-		// The VolumeSnapshotContent is reserved for a VolumeSNapshot;
+		// The VolumeSnapshotContent is reserved for a VolumeSnapshot;
-		// that VolumeSnapshot has not yet been bound to this VolumeSnapshotCent; the VolumeSnapshot sync will handle it.
+		// that VolumeSnapshot has not yet been bound to this VolumeSnapshotContent; the VolumeSnapshot sync will handle it.
 		if content.Spec.VolumeSnapshotRef.UID == "" {
 			glog.V(4).Infof("synchronizing VolumeSnapshotContent[%s]: VolumeSnapshotContent is pre-bound to VolumeSnapshot %s", content.Name, snapshotRefKey(content.Spec.VolumeSnapshotRef))
 			return nil
@@ -133,7 +131,7 @@ func (ctrl *csiSnapshotController) syncContent(content *crdv1.VolumeSnapshotCont
 // It's invoked by appropriate cache.Controller callbacks when a snapshot is
 // created, updated or periodically synced. We do not differentiate between
 // these events.
-// For easier readability, it is split into syncCompleteSnapshot and syncUncompleteSnapshot
+// For easier readability, it is split into syncUnboundSnapshot and syncBoundSnapshot
 func (ctrl *csiSnapshotController) syncSnapshot(snapshot *crdv1.VolumeSnapshot) error {
 	glog.V(4).Infof("synchonizing VolumeSnapshot[%s]: %s", snapshotKey(snapshot), getSnapshotStatusForLogging(snapshot))
 
@@ -148,7 +146,7 @@ func (ctrl *csiSnapshotController) syncSnapshot(snapshot *crdv1.VolumeSnapshot)
 // If there is any problem with the binding (e.g., snapshot points to a non-exist snapshot content), update the snapshot status and emit event.
 func (ctrl *csiSnapshotController) syncBoundSnapshot(snapshot *crdv1.VolumeSnapshot) error {
 	if snapshot.Spec.SnapshotContentName == "" {
-		if _, err := ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotLost", "Bound snapshot has lost reference to VolumeSnapshotContent"); err != nil {
+		if err := ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotLost", "Bound snapshot has lost reference to VolumeSnapshotContent"); err != nil {
 			return err
 		}
 		return nil
@@ -158,7 +156,7 @@ func (ctrl *csiSnapshotController) syncBoundSnapshot(snapshot *crdv1.VolumeSnaps
 		return err
 	}
 	if !found {
-		if _, err = ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotLost", "Bound snapshot has lost reference to VolumeSnapshotContent"); err != nil {
+		if err = ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotMissing", "VolumeSnapshotContent for a bound snapshot is missing"); err != nil {
 			return err
 		}
 		return nil
@@ -171,13 +169,13 @@ func (ctrl *csiSnapshotController) syncBoundSnapshot(snapshot *crdv1.VolumeSnaps
 		glog.V(4).Infof("syncCompleteSnapshot[%s]: VolumeSnapshotContent %q found", snapshotKey(snapshot), content.Name)
 		if !IsSnapshotBound(snapshot, content) {
 			// snapshot is bound but content is not bound to snapshot correctly
-			if _, err = ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotMisbound", "VolumeSnapshotContent is not bound to the VolumeSnapshot correctly"); err != nil {
+			if err = ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotMisbound", "VolumeSnapshotContent is not bound to the VolumeSnapshot correctly"); err != nil {
 				return err
 			}
 			return nil
 		}
 		// Snapshot is correctly bound.
-		if _, err = ctrl.updateSnapshotBoundWithEvent(snapshot, v1.EventTypeNormal, "SnapshotBound", "Snapshot is bound to its VolumeSnapshotContent"); err != nil {
+		if err = ctrl.updateSnapshotBoundWithEvent(snapshot, v1.EventTypeNormal, "SnapshotBound", "Snapshot is bound to its VolumeSnapshotContent"); err != nil {
 			return err
 		}
 		return nil
@@ -188,11 +186,6 @@ func (ctrl *csiSnapshotController) syncUnboundSnapshot(snapshot *crdv1.VolumeSna
 	uniqueSnapshotName := snapshotKey(snapshot)
 	glog.V(4).Infof("syncSnapshot %s", uniqueSnapshotName)
 
-	// Snsapshot has errors during its creation. Controller will not try to fix it. Nothing to do.
-	if snapshot.Status.Error != nil {
-		//return nil
-	}
-
 	if snapshot.Spec.SnapshotContentName != "" {
 		contentObj, found, err := ctrl.contentStore.GetByKey(snapshot.Spec.SnapshotContentName)
 		if err != nil {
@@ -210,7 +203,7 @@ func (ctrl *csiSnapshotController) syncUnboundSnapshot(snapshot *crdv1.VolumeSna
 
 		if err := ctrl.bindSnapshotContent(snapshot, content); err != nil {
 			// snapshot is bound but content is not bound to snapshot correctly
-			ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotBoundFailed", fmt.Sprintf("Snapshot failed to bind VolumeSnapshotContent, %v", err))
+			ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotBindFailed", fmt.Sprintf("Snapshot failed to bind VolumeSnapshotContent, %v", err))
 			return fmt.Errorf("snapshot %s is bound, but VolumeSnapshotContent %s is not bound to the VolumeSnapshot correctly, %v", uniqueSnapshotName, content.Name, err)
 		}
 
@@ -221,9 +214,9 @@ func (ctrl *csiSnapshotController) syncUnboundSnapshot(snapshot *crdv1.VolumeSna
 		}
 		return nil
 	} else { // snapshot.Spec.SnapshotContentName == nil
-		if ContentObj := ctrl.getMatchSnapshotContent(snapshot); ContentObj != nil {
+		if contentObj := ctrl.getMatchSnapshotContent(snapshot); contentObj != nil {
-			glog.V(4).Infof("Find VolumeSnapshotContent object %s for snapshot %s", ContentObj.Name, uniqueSnapshotName)
+			glog.V(4).Infof("Find VolumeSnapshotContent object %s for snapshot %s", contentObj.Name, uniqueSnapshotName)
-			newSnapshot, err := ctrl.bindandUpdateVolumeSnapshot(ContentObj, snapshot)
+			newSnapshot, err := ctrl.bindandUpdateVolumeSnapshot(contentObj, snapshot)
 			if err != nil {
 				return err
 			}
@@ -242,7 +235,7 @@ func (ctrl *csiSnapshotController) syncUnboundSnapshot(snapshot *crdv1.VolumeSna
 
 // getMatchSnapshotContent looks up VolumeSnapshotContent for a VolumeSnapshot named snapshotName
 func (ctrl *csiSnapshotController) getMatchSnapshotContent(vs *crdv1.VolumeSnapshot) *crdv1.VolumeSnapshotContent {
-	var snapshotDataObj *crdv1.VolumeSnapshotContent
+	var snapshotContentObj *crdv1.VolumeSnapshotContent
 	var found bool
 
 	objs := ctrl.contentStore.List()
@@ -253,7 +246,7 @@ func (ctrl *csiSnapshotController) getMatchSnapshotContent(vs *crdv1.VolumeSnaps
 			content.Spec.VolumeSnapshotRef.Namespace == vs.Namespace &&
 			content.Spec.VolumeSnapshotRef.UID == vs.UID {
 			found = true
-			snapshotDataObj = content
+			snapshotContentObj = content
 			break
 		}
 	}
@@ -263,7 +256,7 @@ func (ctrl *csiSnapshotController) getMatchSnapshotContent(vs *crdv1.VolumeSnaps
 		return nil
 	}
 
-	return snapshotDataObj
+	return snapshotContentObj
 }
 
 // deleteSnapshotContent starts delete action.
@@ -271,7 +264,7 @@ func (ctrl *csiSnapshotController) deleteSnapshotContent(content *crdv1.VolumeSn
 	operationName := fmt.Sprintf("delete-%s[%s]", content.Name, string(content.UID))
 	glog.V(4).Infof("Snapshotter is about to delete volume snapshot and the operation named %s", operationName)
 	ctrl.scheduleOperation(operationName, func() error {
-		return ctrl.DeleteSnapshotContentOperation(content)
+		return ctrl.deleteSnapshotContentOperation(content)
 	})
 }
 
@@ -349,13 +342,9 @@ func (ctrl *csiSnapshotController) checkandUpdateSnapshotStatus(snapshot *crdv1.
 // Parameters:
 //   snapshot - snapshot to update
 //   eventtype, reason, message - event to send, see EventRecorder.Event()
-func (ctrl *csiSnapshotController) updateSnapshotErrorStatusWithEvent(snapshot *crdv1.VolumeSnapshot, eventtype, reason, message string) (*crdv1.VolumeSnapshot, error) {
+func (ctrl *csiSnapshotController) updateSnapshotErrorStatusWithEvent(snapshot *crdv1.VolumeSnapshot, eventtype, reason, message string) error {
 	glog.V(4).Infof("updateSnapshotStatusWithEvent[%s]", snapshotKey(snapshot))
 
-	if snapshot.Status.Error != nil && snapshot.Status.Ready == false {
-		glog.V(4).Infof("updateClaimStatusWithEvent[%s]: error %v already set", snapshot.Name, snapshot.Status.Error)
-		return snapshot, nil
-	}
 	snapshotClone := snapshot.DeepCopy()
 	if snapshot.Status.Error == nil {
 		statusError := &storage.VolumeError{
@@ -370,26 +359,26 @@ func (ctrl *csiSnapshotController) updateSnapshotErrorStatusWithEvent(snapshot *
 	newSnapshot, err := ctrl.clientset.VolumesnapshotV1alpha1().VolumeSnapshots(snapshotClone.Namespace).Update(snapshotClone)
 	if err != nil {
 		glog.V(4).Infof("updating VolumeSnapshot[%s] error status failed %v", snapshotKey(snapshot), err)
-		return newSnapshot, err
+		return err
 	}
 
 	_, err = ctrl.storeSnapshotUpdate(newSnapshot)
 	if err != nil {
 		glog.V(4).Infof("updating VolumeSnapshot[%s] error status: cannot update internal cache %v", snapshotKey(snapshot), err)
-		return newSnapshot, err
+		return err
 	}
 	// Emit the event only when the status change happens
 	ctrl.eventRecorder.Event(newSnapshot, eventtype, reason, message)
 
-	return newSnapshot, nil
+	return nil
 }
 
-func (ctrl *csiSnapshotController) updateSnapshotBoundWithEvent(snapshot *crdv1.VolumeSnapshot, eventtype, reason, message string) (*crdv1.VolumeSnapshot, error) {
+func (ctrl *csiSnapshotController) updateSnapshotBoundWithEvent(snapshot *crdv1.VolumeSnapshot, eventtype, reason, message string) error {
 	glog.V(4).Infof("updateSnapshotBoundWithEvent[%s]", snapshotKey(snapshot))
 	if snapshot.Status.Ready && snapshot.Status.Error == nil {
 		// Nothing to do.
 		glog.V(4).Infof("updateSnapshotBoundWithEvent[%s]: Ready %v already set", snapshotKey(snapshot), snapshot.Status.Ready)
-		return snapshot, nil
+		return nil
 	}
 
 	snapshotClone := snapshot.DeepCopy()
@@ -398,7 +387,7 @@ func (ctrl *csiSnapshotController) updateSnapshotBoundWithEvent(snapshot *crdv1.
 	newSnapshot, err := ctrl.clientset.VolumesnapshotV1alpha1().VolumeSnapshots(snapshotClone.Namespace).Update(snapshotClone)
 	if err != nil {
 		glog.V(4).Infof("updating VolumeSnapshot[%s] error status failed %v", snapshotKey(snapshot), err)
-		return newSnapshot, err
+		return err
 	}
 	// Emit the event only when the status change happens
 	ctrl.eventRecorder.Event(snapshot, eventtype, reason, message)
@@ -406,10 +395,10 @@ func (ctrl *csiSnapshotController) updateSnapshotBoundWithEvent(snapshot *crdv1.
 	_, err = ctrl.storeSnapshotUpdate(newSnapshot)
 	if err != nil {
 		glog.V(4).Infof("updating VolumeSnapshot[%s] error status: cannot update internal cache %v", snapshotKey(snapshot), err)
-		return newSnapshot, err
+		return err
 	}
 
-	return newSnapshot, nil
+	return nil
 }
 
 // Stateless functions
@@ -483,7 +472,7 @@ func (ctrl *csiSnapshotController) createSnapshotOperation(snapshot *crdv1.Volum
 	contentName := GetSnapshotContentNameForSnapshot(snapshot)
 
 	// Resolve snapshotting secret credentials.
-	snapshotterSecretRef, err := GetSecretReference(snapshotterSecretNameKey, snapshotterSecretNamespaceKey, class.Parameters, contentName, snapshot)
+	snapshotterSecretRef, err := GetSecretReference(class.Parameters, contentName, snapshot)
 	if err != nil {
 		return nil, err
 	}
@@ -494,7 +483,7 @@ func (ctrl *csiSnapshotController) createSnapshotOperation(snapshot *crdv1.Volum
 
 	driverName, snapshotID, timestamp, csiSnapshotStatus, 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)
+		return nil, fmt.Errorf("failed to take snapshot of the volume, %s: %q", volume.Name, err)
 	}
 	glog.Infof("Create snapshot driver %s, snapshotId %s, timestamp %d, csiSnapshotStatus %v", driverName, snapshotID, timestamp, csiSnapshotStatus)
 
@@ -574,7 +563,7 @@ func (ctrl *csiSnapshotController) createSnapshotOperation(snapshot *crdv1.Volum
 // 3. Delete the SnapshotContent object
 // 4. Remove the Snapshot from vsStore
 // 5. Finish
-func (ctrl *csiSnapshotController) DeleteSnapshotContentOperation(content *crdv1.VolumeSnapshotContent) error {
+func (ctrl *csiSnapshotController) deleteSnapshotContentOperation(content *crdv1.VolumeSnapshotContent) error {
 	glog.V(4).Infof("deleteSnapshotOperation [%s] started", content.Name)
 
 	// get secrets if VolumeSnapshotClass specifies it
@@ -584,7 +573,7 @@ func (ctrl *csiSnapshotController) DeleteSnapshotContentOperation(content *crdv1
 		if snapshotClass, err := ctrl.clientset.VolumesnapshotV1alpha1().VolumeSnapshotClasses().Get(snapshotClassName, metav1.GetOptions{}); err == nil {
 			// Resolve snapshotting secret credentials.
 			// No VolumeSnapshot is provided when resolving delete secret names, since the VolumeSnapshot may or may not exist at delete time.
-			snapshotterSecretRef, err := GetSecretReference(snapshotterSecretNameKey, snapshotterSecretNamespaceKey, snapshotClass.Parameters, content.Name, nil)
+			snapshotterSecretRef, err := GetSecretReference(snapshotClass.Parameters, content.Name, nil)
 			if err != nil {
 				return err
 			}
@@ -597,11 +586,13 @@ func (ctrl *csiSnapshotController) DeleteSnapshotContentOperation(content *crdv1
 
 	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)
 	}
 
 	err = ctrl.clientset.VolumesnapshotV1alpha1().VolumeSnapshotContents().Delete(content.Name, &metav1.DeleteOptions{})
 	if err != nil {
+		ctrl.eventRecorder.Event(content, v1.EventTypeWarning, "SnapshotContentObjectDeleteError", "Failed to delete snapshot content API object")
 		return fmt.Errorf("failed to delete VolumeSnapshotContent %s from API server: %q", content.Name, err)
 	}
 
@@ -666,6 +657,8 @@ func (ctrl *csiSnapshotController) updateSnapshotStatus(snapshot *crdv1.VolumeSn
 				Message: "Failed to upload the snapshot",
 			}
 			change = true
+			ctrl.eventRecorder.Event(snapshot, v1.EventTypeWarning, "SnapshotUploadError", "Failed to upload the snapshot")
+
 		}
 	case csi.SnapshotStatus_UPLOADING:
 		if status.CreationTime == nil {
@@ -751,7 +744,7 @@ func (ctrl *csiSnapshotController) GetClassFromVolumeSnapshot(snapshot *crdv1.Vo
 	}
 }
 
-// getClaimFromVolumeSnapshot is a helper function to get PV from VolumeSnapshot.
+// getClaimFromVolumeSnapshot is a helper function to get PVC from VolumeSnapshot.
 func (ctrl *csiSnapshotController) getClaimFromVolumeSnapshot(snapshot *crdv1.VolumeSnapshot) (*v1.PersistentVolumeClaim, error) {
 	if snapshot.Spec.Source == nil || snapshot.Spec.Source.Kind != pvcKind {
 		return nil, fmt.Errorf("The snapshot source is not the right type. Expected %s, Got %v", pvcKind, snapshot.Spec.Source)

pkg/controller/snapshot_controller_base.go

@@ -28,6 +28,7 @@ import (
 	"github.com/kubernetes-csi/external-snapshotter/pkg/connection"
 	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/kubernetes"
@@ -264,12 +265,22 @@ func (ctrl *csiSnapshotController) contentWorker() {
 
 		_, name, err := cache.SplitMetaNamespaceKey(key)
 		if err != nil {
-			glog.V(4).Infof("error getting name of snapshotData %q to get snapshotData from informer: %v", key, err)
+			glog.V(4).Infof("error getting name of snapshotContent %q to get snapshotContent from informer: %v", key, err)
 			return false
 		}
 		content, err := ctrl.contentLister.Get(name)
 		if err == nil {
-			// The volume still exists in informer cache, the event must have
+			// Skip update if content is for another CSI driver
+			snapshotClassName := content.Spec.VolumeSnapshotClassName
+			if len(snapshotClassName) != 0 {
+				if snapshotClass, err := ctrl.clientset.VolumesnapshotV1alpha1().VolumeSnapshotClasses().Get(snapshotClassName, metav1.GetOptions{}); err == nil {
+					if snapshotClass.Snapshotter != ctrl.snapshotterName {
+						return false
+					}
+				}
+			}
+
+			// The content still exists in informer cache, the event must have
 			// been add/update/sync
 			ctrl.updateContent(content)
 			return false
@@ -400,11 +411,11 @@ func (ctrl *csiSnapshotController) deleteContent(content *crdv1.VolumeSnapshotCo
 		glog.V(5).Infof("deleteContent[%q]: content not bound", content.Name)
 		return
 	}
-	// sync the vs when its vs is deleted.  Explicitly sync'ing the
+	// sync the snapshot when its content is deleted.  Explicitly sync'ing the
-	// vs here in response to content deletion prevents the vs from
+	// snapshot here in response to content deletion prevents the snapshot from
 	// waiting until the next sync period for its Release.
-	glog.V(5).Infof("deleteContent[%q]: scheduling sync of vs %s", content.Name, snapshotName)
+	glog.V(5).Infof("deleteContent[%q]: scheduling sync of snapshot %s", content.Name, snapshotName)
-	ctrl.contentQueue.Add(snapshotName)
+	ctrl.snapshotQueue.Add(snapshotName)
 }
 
 // initializeCaches fills all controller caches with initial data from etcd in

pkg/controller/util.go

@@ -29,22 +29,14 @@ import (
 	"k8s.io/client-go/tools/cache"
 	"os"
 	"strconv"
-	"strings"
 )
 
 var (
 	keyFunc = cache.DeletionHandlingMetaNamespaceKeyFunc
 )
 
-// GetNameAndNameSpaceFromSnapshotName retrieves the namespace and
+const snapshotterSecretNameKey = "csiSnapshotterSecretName"
-// the short name of a snapshot from its full name
+const snapshotterSecretNamespaceKey = "csiSnapshotterSecretNamespace"
-func GetNameAndNameSpaceFromSnapshotName(name string) (string, string, error) {
-	strs := strings.Split(name, "/")
-	if len(strs) != 2 {
-		return "", "", fmt.Errorf("invalid snapshot name")
-	}
-	return strs[0], strs[1], nil
-}
 
 func snapshotKey(vs *crdv1.VolumeSnapshot) string {
 	return fmt.Sprintf("%s/%s", vs.Namespace, vs.Name)
@@ -110,10 +102,10 @@ func storeObjectUpdate(store cache.Store, obj interface{}, className string) (bo
 	return true, nil
 }
 
-// GetSnapshotContentNameForSnapshot returns SnapshotData.Name for the create VolumeSnapshotContent.
+// GetSnapshotContentNameForSnapshot returns SnapshotContent.Name for the create VolumeSnapshotContent.
 // The name must be unique.
 func GetSnapshotContentNameForSnapshot(snapshot *crdv1.VolumeSnapshot) string {
-	return "snapdata-" + string(snapshot.UID)
+	return "snapcontent-" + string(snapshot.UID)
 }
 
 // IsDefaultAnnotation returns a boolean if
@@ -145,16 +137,16 @@ func IsDefaultAnnotation(obj metav1.ObjectMeta) bool {
 // - the name or namespace parameter contains a token that cannot be resolved
 // - the resolved name is not a valid secret name
 // - the resolved namespace is not a valid namespace name
-func GetSecretReference(nameKey, namespaceKey string, snapshotClassParams map[string]string, snapContentName string, snapshot *crdv1.VolumeSnapshot) (*v1.SecretReference, error) {
+func GetSecretReference(snapshotClassParams map[string]string, snapContentName string, snapshot *crdv1.VolumeSnapshot) (*v1.SecretReference, error) {
-	nameTemplate, hasName := snapshotClassParams[nameKey]
+	nameTemplate, hasName := snapshotClassParams[snapshotterSecretNameKey]
-	namespaceTemplate, hasNamespace := snapshotClassParams[namespaceKey]
+	namespaceTemplate, hasNamespace := snapshotClassParams[snapshotterSecretNamespaceKey]
 
 	if !hasName && !hasNamespace {
 		return nil, nil
 	}
 
 	if len(nameTemplate) == 0 || len(namespaceTemplate) == 0 {
-		return nil, fmt.Errorf("%s and %s parameters must be specified together", nameKey, namespaceKey)
+		return nil, fmt.Errorf("%s and %s parameters must be specified together", snapshotterSecretNameKey, snapshotterSecretNamespaceKey)
 	}
 
 	ref := &v1.SecretReference{}
@@ -170,15 +162,15 @@ func GetSecretReference(nameKey, namespaceKey string, snapshotClassParams map[st
 
 	resolvedNamespace, err := resolveTemplate(namespaceTemplate, namespaceParams)
 	if err != nil {
-		return nil, fmt.Errorf("error resolving %s value %q: %v", namespaceKey, namespaceTemplate, err)
+		return nil, fmt.Errorf("error resolving %s value %q: %v", snapshotterSecretNamespaceKey, namespaceTemplate, err)
 	}
 	glog.V(4).Infof("GetSecretReference namespaceTemplate %s, namespaceParams: %+v, resolved %s", namespaceTemplate, namespaceParams, resolvedNamespace)
 
 	if len(validation.IsDNS1123Label(resolvedNamespace)) > 0 {
 		if namespaceTemplate != resolvedNamespace {
-			return nil, fmt.Errorf("%s parameter %q resolved to %q which is not a valid namespace name", namespaceKey, namespaceTemplate, resolvedNamespace)
+			return nil, fmt.Errorf("%s parameter %q resolved to %q which is not a valid namespace name", snapshotterSecretNamespaceKey, namespaceTemplate, resolvedNamespace)
 		}
-		return nil, fmt.Errorf("%s parameter %q is not a valid namespace name", namespaceKey, namespaceTemplate)
+		return nil, fmt.Errorf("%s parameter %q is not a valid namespace name", snapshotterSecretNamespaceKey, namespaceTemplate)
 	}
 	ref.Namespace = resolvedNamespace
 
@@ -195,13 +187,13 @@ func GetSecretReference(nameKey, namespaceKey string, snapshotClassParams map[st
 	}
 	resolvedName, err := resolveTemplate(nameTemplate, nameParams)
 	if err != nil {
-		return nil, fmt.Errorf("error resolving %s value %q: %v", nameKey, nameTemplate, err)
+		return nil, fmt.Errorf("error resolving %s value %q: %v", snapshotterSecretNameKey, nameTemplate, err)
 	}
 	if len(validation.IsDNS1123Subdomain(resolvedName)) > 0 {
 		if nameTemplate != resolvedName {
-			return nil, fmt.Errorf("%s parameter %q resolved to %q which is not a valid secret name", nameKey, nameTemplate, resolvedName)
+			return nil, fmt.Errorf("%s parameter %q resolved to %q which is not a valid secret name", snapshotterSecretNameKey, nameTemplate, resolvedName)
 		}
-		return nil, fmt.Errorf("%s parameter %q is not a valid secret name", nameKey, nameTemplate)
+		return nil, fmt.Errorf("%s parameter %q is not a valid secret name", snapshotterSecretNameKey, nameTemplate)
 	}
 	ref.Name = resolvedName