diff --git a/cmd/csi-snapshotter/main.go b/cmd/csi-snapshotter/main.go index 1b7d9406..29928f59 100644 --- a/cmd/csi-snapshotter/main.go +++ b/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.") - createSnapshotContentInterval = flag.Duration("createSnapshotContentInterval", 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.") + createSnapshotContentRetryCount = flag.Int("create-snapshotcontent-retrycount", 5, "Number of 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("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, diff --git a/pkg/connection/connection.go b/pkg/connection/connection.go index dc63c3a4..c2f9960f 100644 --- a/pkg/connection/connection.go +++ b/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) { - glog.V(5).Infof("CSI CreateSnapshot: %s", snapshot.Name) +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", 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) diff --git a/pkg/controller/csi_handler.go b/pkg/controller/csi_handler.go index dd93b186..3358feac 100644 --- a/pkg/controller/csi_handler.go +++ b/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") } diff --git a/pkg/controller/snapshot_controller.go b/pkg/controller/snapshot_controller.go index 645113c5..4c3d6965 100644 --- a/pkg/controller/snapshot_controller.go +++ b/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; - // that VolumeSnapshot has not yet been bound to this VolumeSnapshotCent; the VolumeSnapshot sync will handle it. + // The VolumeSnapshotContent is reserved for a VolumeSnapshot; + // 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 { - glog.V(4).Infof("Find VolumeSnapshotContent object %s for snapshot %s", ContentObj.Name, uniqueSnapshotName) - newSnapshot, err := ctrl.bindandUpdateVolumeSnapshot(ContentObj, snapshot) + if contentObj := ctrl.getMatchSnapshotContent(snapshot); contentObj != nil { + glog.V(4).Infof("Find VolumeSnapshotContent object %s for snapshot %s", contentObj.Name, uniqueSnapshotName) + 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) diff --git a/pkg/controller/snapshot_controller_base.go b/pkg/controller/snapshot_controller_base.go index 2f222f66..8f4c1acb 100644 --- a/pkg/controller/snapshot_controller_base.go +++ b/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 - // vs here in response to content deletion prevents the vs from + // sync the snapshot when its content is deleted. Explicitly sync'ing the + // 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) - ctrl.contentQueue.Add(snapshotName) + glog.V(5).Infof("deleteContent[%q]: scheduling sync of snapshot %s", content.Name, snapshotName) + ctrl.snapshotQueue.Add(snapshotName) } // initializeCaches fills all controller caches with initial data from etcd in diff --git a/pkg/controller/util.go b/pkg/controller/util.go index 35fabb76..ec0a5ad3 100644 --- a/pkg/controller/util.go +++ b/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 -// the short name of a snapshot from its full name -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 -} +const snapshotterSecretNameKey = "csiSnapshotterSecretName" +const snapshotterSecretNamespaceKey = "csiSnapshotterSecretNamespace" 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) { - nameTemplate, hasName := snapshotClassParams[nameKey] - namespaceTemplate, hasNamespace := snapshotClassParams[namespaceKey] +func GetSecretReference(snapshotClassParams map[string]string, snapContentName string, snapshot *crdv1.VolumeSnapshot) (*v1.SecretReference, error) { + nameTemplate, hasName := snapshotClassParams[snapshotterSecretNameKey] + 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