Merge pull request #9 from xing-yang/snapshot_controller_snapclass

Address review comments

cmd/csi-snapshotter/create_crd.go

@@ -15,7 +15,6 @@ package main
 
 import (
 	"reflect"
-	"time"
 
 	"github.com/golang/glog"
 	crdv1 "github.com/kubernetes-csi/external-snapshotter/pkg/apis/volumesnapshot/v1alpha1"
@@ -25,7 +24,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
-	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/rest"
 )
 
@@ -122,18 +120,3 @@ func CreateCRD(clientset apiextensionsclient.Interface) error {
 
 	return nil
 }
-
-// WaitForSnapshotResource waits for the snapshot resource
-func WaitForSnapshotResource(snapshotClient *rest.RESTClient) error {
-	return wait.Poll(100*time.Millisecond, 60*time.Second, func() (bool, error) {
-		_, err := snapshotClient.Get().
-			Resource(crdv1.VolumeSnapshotContentResourcePlural).DoRaw()
-		if err == nil {
-			return true, nil
-		}
-		if apierrors.IsNotFound(err) {
-			return false, nil
-		}
-		return false, err
-	})
-}

cmd/csi-snapshotter/main.go

@@ -47,7 +47,7 @@ const (
 
 // Command line flags
 var (
-	snapshotter                     = flag.String("snapshotter", "", "Name of the snapshotter. The snapshotter will only create snapshot data for snapshot that request a StorageClass with a snapshotter field set equal to this name.")
+	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.")
@@ -56,7 +56,7 @@ var (
 	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.")
 	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")
+	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.")
 )
 
 func main() {
@@ -105,10 +105,20 @@ func main() {
 		os.Exit(1)
 	}
 
-	// Find driver name.
+	// Pass a context with a timeout
 	ctx, cancel := context.WithTimeout(context.Background(), csiTimeout)
 	defer cancel()
 
+	// Find driver name
+	if snapshotter == nil {
+		*snapshotter, err = csiConn.GetDriverName(ctx)
+		if err != nil {
+			glog.Error(err.Error())
+			os.Exit(1)
+		}
+	}
+	glog.V(2).Infof("CSI driver name: %q", *snapshotter)
+
 	// Check it's ready
 	if err = waitForDriverReady(csiConn, *connectionTimeout); err != nil {
 		glog.Error(err.Error())
@@ -122,11 +132,11 @@ func main() {
 		os.Exit(1)
 	}
 	if !supportsCreateSnapshot {
-		glog.Error("CSI driver does not support ControllerCreateSnapshot")
+		glog.Errorf("CSI driver %s does not support ControllerCreateSnapshot", *snapshotter)
 		os.Exit(1)
 	}
 
-	glog.V(2).Infof("Start NewCSISnapshotController with snapshotter %s", *snapshotter)
+	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)
 
 	ctrl := controller.NewCSISnapshotController(
 		snapClient,

pkg/apis/volumesnapshot/v1alpha1/types.go

@@ -80,6 +80,10 @@ type VolumeSnapshotSpec struct {
 	// be used if it is available.
 	// +optional
 	VolumeSnapshotClassName string `json:"snapshotClassName" protobuf:"bytes,3,opt,name=snapshotClassName"`
+
+	// A description of the volume snapshot's resources and size.
+	// +optional
+	Size core_v1.ResourceList `json:"size,omitempty" protobuf:"bytes,4,rep,name=size,casttype=ResourceList,castkey=ResourceName"`
 }
 
 // VolumeSnapshotStatus is the status of the VolumeSnapshot
@@ -194,6 +198,11 @@ type VolumeSnapshotContentSpec struct {
 	// taken from. It becomes non-nil when VolumeSnapshot and VolumeSnapshotContent are bound.
 	// +optional
 	PersistentVolumeRef *core_v1.ObjectReference `json:"persistentVolumeRef" protobuf:"bytes,3,opt,name=persistentVolumeRef"`
+
+	// Name of the VolumeSnapshotClass used by the VolumeSnapshot. If not specified, a default snapshot class will
+	// be used if it is available.
+	// +optional
+	VolumeSnapshotClassName string `json:"snapshotClassName" protobuf:"bytes,4,opt,name=snapshotClassName"`
 }
 
 // VolumeSnapshotSource represents the actual location and type of the snapshot. Only one of its members may be specified.
@@ -206,19 +215,26 @@ type VolumeSnapshotSource struct {
 // Represents the source from CSI volume snapshot
 type CSIVolumeSnapshotSource struct {
 	// Driver is the name of the driver to use for this snapshot.
+	// This MUST be the same name returned by the CSI GetPluginName() call for
+	// that driver.
 	// Required.
-	Driver string `json:"driver"`
+	Driver string `json:"driver" protobuf:"bytes,1,opt,name=driver"`
 
 	// SnapshotHandle is the unique snapshot id returned by the CSI volume
 	// plugin’s CreateSnapshot to refer to the snapshot on all subsequent calls.
 	// Required.
-	SnapshotHandle string `json:"snapshotHandle"`
+	SnapshotHandle string `json:"snapshotHandle" protobuf:"bytes,2,opt,name=snapshotHandle"`
 
 	// Timestamp when the point-in-time snapshot is taken on the storage
 	// system. This timestamp will be generated by the CSI volume driver after
 	// the snapshot is cut. The format of this field should be a Unix nanoseconds
 	// time encoded as an int64. On Unix, the command `date +%s%N` returns
 	// the  current time in nanoseconds since 1970-01-01 00:00:00 UTC.
-	// This field is REQUIRED.
+	// This field is required in the CSI spec but optional here to support static binding.
+	// +optional
 	CreatedAt int64 `json:"createdAt,omitempty" protobuf:"varint,3,opt,name=createdAt"`
+
+	// A description of the volume snapshot's resources and size.
+	// +optional
+	Size core_v1.ResourceList `json:"size,omitempty" protobuf:"bytes,4,rep,name=size,casttype=ResourceList,castkey=ResourceName"`
 }

pkg/connection/connection.go

@@ -50,7 +50,7 @@ type CSIConnection interface {
 	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)
 
 	// DeleteSnapshot deletes a snapshot from a volume
-	DeleteSnapshot(ctx context.Context, snapshotID string) (err error)
+	DeleteSnapshot(ctx context.Context, snapshotID string, snapshotterCredentials map[string]string) (err error)
 
 	// GetSnapshotStatus lists snapshot from a volume
 	GetSnapshotStatus(ctx context.Context, snapshotID string) (*csi.SnapshotStatus, int64, error)
@@ -218,12 +218,12 @@ func (c *csiConnection) CreateSnapshot(ctx context.Context, snapshotName string,
 	return driverName, rsp.Snapshot.Id, rsp.Snapshot.CreatedAt, rsp.Snapshot.Status, nil
 }
 
-func (c *csiConnection) DeleteSnapshot(ctx context.Context, snapshotID string) (err error) {
+func (c *csiConnection) DeleteSnapshot(ctx context.Context, snapshotID string, snapshotterCredentials map[string]string) (err error) {
 	client := csi.NewControllerClient(c.conn)
 
 	req := csi.DeleteSnapshotRequest{
 		SnapshotId:            snapshotID,
-		DeleteSnapshotSecrets: nil,
+		DeleteSnapshotSecrets: snapshotterCredentials,
 	}
 
 	if _, err := client.DeleteSnapshot(ctx, &req); err != nil {

pkg/controller/csi_handler.go

@@ -31,7 +31,7 @@ import (
 // Handler is responsible for handling VolumeSnapshot events from informer.
 type Handler interface {
 	CreateSnapshot(snapshot *crdv1.VolumeSnapshot, volume *v1.PersistentVolume, parameters map[string]string, snapshotterCredentials map[string]string) (string, string, int64, *csi.SnapshotStatus, error)
-	DeleteSnapshot(content *crdv1.VolumeSnapshotContent) error
+	DeleteSnapshot(content *crdv1.VolumeSnapshotContent, snapshotterCredentials map[string]string) error
 	GetSnapshotStatus(content *crdv1.VolumeSnapshotContent) (*csi.SnapshotStatus, int64, error)
 }
 
@@ -69,14 +69,14 @@ func (handler *csiHandler) CreateSnapshot(snapshot *crdv1.VolumeSnapshot, volume
 	return handler.csiConnection.CreateSnapshot(ctx, snapshotName, snapshot, volume, parameters, snapshotterCredentials)
 }
 
-func (handler *csiHandler) DeleteSnapshot(content *crdv1.VolumeSnapshotContent) error {
+func (handler *csiHandler) DeleteSnapshot(content *crdv1.VolumeSnapshotContent, snapshotterCredentials map[string]string) error {
 	if content.Spec.CSI == nil {
 		return fmt.Errorf("CSISnapshot not defined in spec")
 	}
 	ctx, cancel := context.WithTimeout(context.Background(), handler.timeout)
 	defer cancel()
 
-	err := handler.csiConnection.DeleteSnapshot(ctx, content.Spec.CSI.SnapshotHandle)
+	err := handler.csiConnection.DeleteSnapshot(ctx, content.Spec.CSI.SnapshotHandle, snapshotterCredentials)
 	if err != nil {
 		return fmt.Errorf("failed to delete snapshot data %s: %q", content.Name, err)
 	}