Add support for extra-create-metadata when creating snapshots

Author: francoispqt

pkg/common/parameters.go

@@ -41,11 +41,21 @@ const (
 	ParameterKeyPVCNamespace = "csi.storage.k8s.io/pvc/namespace"
 	ParameterKeyPVName       = "csi.storage.k8s.io/pv/name"
 
-	// Keys for tags to put in the provisioned disk description.
+	// Keys for tags to put in the provisioned disk description
 	tagKeyCreatedForClaimNamespace = "kubernetes.io/created-for/pvc/namespace"
 	tagKeyCreatedForClaimName      = "kubernetes.io/created-for/pvc/name"
 	tagKeyCreatedForVolumeName     = "kubernetes.io/created-for/pv/name"
 	tagKeyCreatedBy                = "storage.gke.io/created-by"
+
+	// Keys for Snapshot and SnapshotContent parameters as reported by external-snapshotter
+	ParameterKeyVolumeSnapshotName        = "csi.storage.k8s.io/volumesnapshot/name"
+	ParameterKeyVolumeSnapshotNamespace   = "csi.storage.k8s.io/volumesnapshot/namespace"
+	ParameterKeyVolumeSnapshotContentName = "csi.storage.k8s.io/volumesnapshotcontent/name"
+
+	// Keys for tags to put in the provisioned snapshot description
+	tagKeyCreatedForSnapshotName        = "kubernetes.io/created-for/volumesnapshot/name"
+	tagKeyCreatedForSnapshotNamespace   = "kubernetes.io/created-for/volumesnapshot/namespace"
+	tagKeyCreatedForSnapshotContentName = "kubernetes.io/created-for/volumesnapshotcontent/name"
 )
 
 // DiskParameters contains normalized and defaulted disk parameters
@@ -72,6 +82,7 @@ type SnapshotParameters struct {
 	StorageLocations []string
 	SnapshotType     string
 	ImageFamily      string
+	Tags             map[string]string
 }
 
 // ExtractAndDefaultParameters will take the relevant parameters from a map and
@@ -133,10 +144,11 @@ func ExtractAndDefaultParameters(parameters map[string]string, driverName string
 	return p, nil
 }
 
-func ExtractAndDefaultSnapshotParameters(parameters map[string]string) (SnapshotParameters, error) {
+func ExtractAndDefaultSnapshotParameters(parameters map[string]string, driverName string) (SnapshotParameters, error) {
 	p := SnapshotParameters{
 		StorageLocations: []string{},
 		SnapshotType:     DiskSnapshotType,
+		Tags:             make(map[string]string), // Default
 	}
 	for k, v := range parameters {
 		switch strings.ToLower(k) {
@@ -154,9 +166,18 @@ func ExtractAndDefaultSnapshotParameters(parameters map[string]string) (Snapshot
 			p.SnapshotType = v
 		case ParameterKeyImageFamily:
 			p.ImageFamily = v
+		case ParameterKeyVolumeSnapshotName:
+			p.Tags[tagKeyCreatedForSnapshotName] = v
+		case ParameterKeyVolumeSnapshotNamespace:
+			p.Tags[tagKeyCreatedForSnapshotNamespace] = v
+		case ParameterKeyVolumeSnapshotContentName:
+			p.Tags[tagKeyCreatedForSnapshotContentName] = v
 		default:
 			return p, fmt.Errorf("parameters contains invalid option %q", k)
 		}
 	}
+	if len(p.Tags) > 0 {
+		p.Tags[tagKeyCreatedBy] = driverName
+	}
 	return p, nil
 }

pkg/common/parameters_test.go

@@ -175,12 +175,25 @@ func TestSnapshotParameters(t *testing.T) {
 		expectError             bool
 	}{
 		{
-			desc:       "valid parameter",
-			parameters: map[string]string{ParameterKeyStorageLocations: "ASIA ", ParameterKeySnapshotType: "images", ParameterKeyImageFamily: "test-family"},
+			desc: "valid parameter",
+			parameters: map[string]string{
+				ParameterKeyStorageLocations:          "ASIA ",
+				ParameterKeySnapshotType:              "images",
+				ParameterKeyImageFamily:               "test-family",
+				ParameterKeyVolumeSnapshotName:        "snapshot-name",
+				ParameterKeyVolumeSnapshotContentName: "snapshot-content-name",
+				ParameterKeyVolumeSnapshotNamespace:   "snapshot-namespace",
+			},
 			expectedSnapshotParames: SnapshotParameters{
 				StorageLocations: []string{"asia"},
 				SnapshotType:     DiskImageType,
 				ImageFamily:      "test-family",
+				Tags: map[string]string{
+					tagKeyCreatedForSnapshotName:        "snapshot-name",
+					tagKeyCreatedForSnapshotContentName: "snapshot-content-name",
+					tagKeyCreatedForSnapshotNamespace:   "snapshot-namespace",
+					tagKeyCreatedBy:                     "test-driver",
+				},
 			},
 			expectError: false,
 		},
@@ -190,6 +203,7 @@ func TestSnapshotParameters(t *testing.T) {
 			expectedSnapshotParames: SnapshotParameters{
 				StorageLocations: []string{},
 				SnapshotType:     DiskSnapshotType,
+				Tags:             make(map[string]string),
 			},
 			expectError: false,
 		},
@@ -201,7 +215,7 @@ func TestSnapshotParameters(t *testing.T) {
 	}
 	for _, tc := range tests {
 		t.Run(tc.desc, func(t *testing.T) {
-			p, err := ExtractAndDefaultSnapshotParameters(tc.parameters)
+			p, err := ExtractAndDefaultSnapshotParameters(tc.parameters, "test-driver")
 			if err != nil && !tc.expectError {
 				t.Errorf("Got error %v; expect no error", err)
 			}

pkg/gce-cloud-provider/compute/gce-compute.go

@@ -324,7 +324,7 @@ func ValidateDiskParameters(disk *CloudDisk, params common.DiskParameters) error
 func (cloud *CloudProvider) InsertDisk(ctx context.Context, project string, volKey *meta.Key, params common.DiskParameters, capBytes int64, capacityRange *csi.CapacityRange, replicaZones []string, snapshotID string, volumeContentSourceVolumeID string, multiWriter bool) error {
 	klog.V(5).Infof("Inserting disk %v", volKey)
 
-	description, err := encodeDiskTags(params.Tags)
+	description, err := encodeTags(params.Tags)
 	if err != nil {
 		return err
 	}
@@ -848,18 +848,40 @@ func (cloud *CloudProvider) DeleteSnapshot(ctx context.Context, project, snapsho
 
 func (cloud *CloudProvider) CreateSnapshot(ctx context.Context, project string, volKey *meta.Key, snapshotName string, snapshotParams common.SnapshotParameters) (*computev1.Snapshot, error) {
 	klog.V(5).Infof("Creating snapshot %s for volume %v", snapshotName, volKey)
+
+	description, err := encodeTags(snapshotParams.Tags)
+	if err != nil {
+		return nil, err
+	}
+
 	switch volKey.Type() {
 	case meta.Zonal:
-		return cloud.createZonalDiskSnapshot(ctx, project, volKey, snapshotName, snapshotParams)
+		if description == "" {
+			description = "Snapshot created by GCE-PD CSI Driver"
+		}
+		return cloud.createZonalDiskSnapshot(ctx, project, volKey, snapshotName, snapshotParams, description)
 	case meta.Regional:
-		return cloud.createRegionalDiskSnapshot(ctx, project, volKey, snapshotName, snapshotParams)
+		if description == "" {
+			description = "Regional Snapshot created by GCE-PD CSI Driver"
+		}
+		return cloud.createRegionalDiskSnapshot(ctx, project, volKey, snapshotName, snapshotParams, description)
 	default:
 		return nil, fmt.Errorf("could not create snapshot, key was neither zonal nor regional, instead got: %v", volKey.String())
 	}
 }
 
 func (cloud *CloudProvider) CreateImage(ctx context.Context, project string, volKey *meta.Key, imageName string, snapshotParams common.SnapshotParameters) (*computev1.Image, error) {
 	klog.V(5).Infof("Creating image %s for source %v", imageName, volKey)
+
+	description, err := encodeTags(snapshotParams.Tags)
+	if err != nil {
+		return nil, err
+	}
+
+	if description == "" {
+		description = "Image created by GCE-PD CSI Driver"
+	}
+
 	diskID, err := common.KeyToVolumeID(volKey, project)
 	if err != nil {
 		return nil, err
@@ -869,6 +891,7 @@ func (cloud *CloudProvider) CreateImage(ctx context.Context, project string, vol
 		Family:           snapshotParams.ImageFamily,
 		Name:             imageName,
 		StorageLocations: snapshotParams.StorageLocations,
+		Description:      description,
 	}
 
 	_, err = cloud.service.Images.Insert(project, image).Context(ctx).Do()
@@ -1013,10 +1036,11 @@ func (cloud *CloudProvider) resizeRegionalDisk(ctx context.Context, project stri
 	return requestGb, nil
 }
 
-func (cloud *CloudProvider) createZonalDiskSnapshot(ctx context.Context, project string, volKey *meta.Key, snapshotName string, snapshotParams common.SnapshotParameters) (*computev1.Snapshot, error) {
+func (cloud *CloudProvider) createZonalDiskSnapshot(ctx context.Context, project string, volKey *meta.Key, snapshotName string, snapshotParams common.SnapshotParameters, description string) (*computev1.Snapshot, error) {
 	snapshotToCreate := &computev1.Snapshot{
 		Name:             snapshotName,
 		StorageLocations: snapshotParams.StorageLocations,
+		Description:      description,
 	}
 
 	_, err := cloud.service.Disks.CreateSnapshot(project, volKey.Zone, volKey.Name, snapshotToCreate).Context(ctx).Do()
@@ -1028,10 +1052,11 @@ func (cloud *CloudProvider) createZonalDiskSnapshot(ctx context.Context, project
 	return cloud.waitForSnapshotCreation(ctx, project, snapshotName)
 }
 
-func (cloud *CloudProvider) createRegionalDiskSnapshot(ctx context.Context, project string, volKey *meta.Key, snapshotName string, snapshotParams common.SnapshotParameters) (*computev1.Snapshot, error) {
+func (cloud *CloudProvider) createRegionalDiskSnapshot(ctx context.Context, project string, volKey *meta.Key, snapshotName string, snapshotParams common.SnapshotParameters, description string) (*computev1.Snapshot, error) {
 	snapshotToCreate := &computev1.Snapshot{
 		Name:             snapshotName,
 		StorageLocations: snapshotParams.StorageLocations,
+		Description:      description,
 	}
 
 	_, err := cloud.service.RegionDisks.CreateSnapshot(project, volKey.Region, volKey.Name, snapshotToCreate).Context(ctx).Do()
@@ -1088,17 +1113,17 @@ func removeCryptoKeyVersion(kmsKey string) string {
 	return kmsKey
 }
 
-// encodeDiskTags encodes requested volume tags into JSON string, as GCE does
-// not support tags on GCE PDs and we use Description field as fallback.
-func encodeDiskTags(tags map[string]string) (string, error) {
+// encodeTags encodes requested volume or snapshot tags into JSON string, suitable for putting into
+// the PD or Snapshot Description field.
+func encodeTags(tags map[string]string) (string, error) {
 	if len(tags) == 0 {
 		// No tags -> empty JSON
 		return "", nil
 	}
 
 	enc, err := json.Marshal(tags)
 	if err != nil {
-		return "", fmt.Errorf("failed to encodeDiskTags %v: %v", tags, err)
+		return "", fmt.Errorf("failed to encodeTags %v: %v", tags, err)
 	}
 	return string(enc), nil
 }

pkg/gce-pd-csi-driver/controller.go

@@ -811,7 +811,7 @@ func (gceCS *GCEControllerServer) CreateSnapshot(ctx context.Context, req *csi.C
 		return nil, status.Error(codes.Internal, fmt.Sprintf("CreateSnapshot unknown get disk error: %v", err))
 	}
 
-	snapshotParams, err := common.ExtractAndDefaultSnapshotParameters(req.GetParameters())
+	snapshotParams, err := common.ExtractAndDefaultSnapshotParameters(req.GetParameters(), gceCS.Driver.name)
 	if err != nil {
 		return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("Invalid snapshot parameters: %v", err))
 	}

pkg/gce-pd-csi-driver/controller_test.go

@@ -940,7 +940,7 @@ func TestCreateVolumeWithVolumeSourceFromSnapshot(t *testing.T) {
 		// Setup new driver each time so no interference
 		gceDriver := initGCEDriver(t, nil)
 
-		snapshotParams, err := common.ExtractAndDefaultSnapshotParameters(nil)
+		snapshotParams, err := common.ExtractAndDefaultSnapshotParameters(nil, gceDriver.name)
 		if err != nil {
 			t.Errorf("Got error extracting snapshot parameters: %v", err)
 		}

test/e2e/tests/single_zone_e2e_test.go

@@ -839,14 +839,20 @@ var _ = Describe("GCE PD CSI Driver", func() {
 		// This is safe because we hardcode the zones.
 		snapshotLocation := z[:len(z)-2]
 
-		snapshotParams := map[string]string{common.ParameterKeyStorageLocations: snapshotLocation}
+		snapshotParams := map[string]string{
+			common.ParameterKeyStorageLocations:          snapshotLocation,
+			common.ParameterKeyVolumeSnapshotName:        "test-volumesnapshot-name",
+			common.ParameterKeyVolumeSnapshotNamespace:   "test-volumesnapshot-namespace",
+			common.ParameterKeyVolumeSnapshotContentName: "test-volumesnapshotcontent-name",
+		}
 		snapshotID, err := client.CreateSnapshot(snapshotName, volID, snapshotParams)
 		Expect(err).To(BeNil(), "CreateSnapshot failed with error: %v", err)
 
 		// Validate Snapshot Created
 		snapshot, err := computeService.Snapshots.Get(p, snapshotName).Do()
 		Expect(err).To(BeNil(), "Could not get snapshot from cloud directly")
 		Expect(snapshot.Name).To(Equal(snapshotName))
+		Expect(snapshot.Description).To(Equal("{\"kubernetes.io/created-for/volumesnapshot/name\":\"test-volumesnapshot-name\",\"kubernetes.io/created-for/volumesnapshot/namespace\":\"test-volumesnapshot-namespace\",\"kubernetes.io/created-for/volumesnapshotcontent/name\":\"test-volumesnapshotcontent-name\",\"storage.gke.io/created-by\":\"pd.csi.storage.gke.io\"}"))
 
 		err = wait.Poll(10*time.Second, 3*time.Minute, func() (bool, error) {
 			snapshot, err := computeService.Snapshots.Get(p, snapshotName).Do()