Merge pull request kubernetes-sigs#570 from saad-ali/labelPDs

Add metadata info in tag on PD

Author: k8s-ci-robot

deploy/kubernetes/overlays/prow-gke-release-staging-head/enable-extra-create-metadata.yaml

@@ -0,0 +1,4 @@
+# Enable extra-create-metadata flag for external-provisioner
+- op: add
+  path: /spec/template/spec/containers/0/args/-
+  value: "--extra-create-metadata"

deploy/kubernetes/overlays/prow-gke-release-staging-head/kustomization.yaml

@@ -24,3 +24,9 @@ patchesJson6902:
     kind: Deployment
     name: csi-gce-pd-controller
   path: increase-sidecar-operation-timeout.yaml
+- target:
+    group: apps
+    version: v1
+    kind: Deployment
+    name: csi-gce-pd-controller
+  path: enable-extra-create-metadata.yaml

pkg/common/parameters.go

@@ -27,6 +27,17 @@ const (
 	ParameterKeyDiskEncryptionKmsKey = "disk-encryption-kms-key"
 
 	replicationTypeNone = "none"
+
+	// Keys for PV and PVC parameters as reported by external-provisioner
+	ParameterKeyPVCName      = "csi.storage.k8s.io/pvc/name"
+	ParameterKeyPVCNamespace = "csi.storage.k8s.io/pvc/namespace"
+	ParameterKeyPVName       = "csi.storage.k8s.io/pv/name"
+
+	// Keys for tags to attach to the provisioned disk.
+	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"
 )
 
 // DiskParameters contains normalized and defaulted disk parameters
@@ -40,16 +51,21 @@ type DiskParameters struct {
 	// Values: {string}
 	// Default: ""
 	DiskEncryptionKMSKey string
+	// Values: {map[string]string}
+	// Default: ""
+	Tags map[string]string
 }
 
 // ExtractAndDefaultParameters will take the relevant parameters from a map and
 // put them into a well defined struct making sure to default unspecified fields
-func ExtractAndDefaultParameters(parameters map[string]string) (DiskParameters, error) {
+func ExtractAndDefaultParameters(parameters map[string]string, driverName string) (DiskParameters, error) {
 	p := DiskParameters{
-		DiskType:             "pd-standard",       // Default
-		ReplicationType:      replicationTypeNone, // Default
-		DiskEncryptionKMSKey: "",                  // Default
+		DiskType:             "pd-standard",           // Default
+		ReplicationType:      replicationTypeNone,     // Default
+		DiskEncryptionKMSKey: "",                      // Default
+		Tags:                 make(map[string]string), // Default
 	}
+
 	for k, v := range parameters {
 		if k == "csiProvisionerSecretName" || k == "csiProvisionerSecretNamespace" {
 			// These are hardcoded secrets keys required to function but not needed by GCE PD
@@ -67,9 +83,18 @@ func ExtractAndDefaultParameters(parameters map[string]string) (DiskParameters,
 		case ParameterKeyDiskEncryptionKmsKey:
 			// Resource names (e.g. "keyRings", "cryptoKeys", etc.) are case sensitive, so do not change case
 			p.DiskEncryptionKMSKey = v
+		case ParameterKeyPVCName:
+			p.Tags[tagKeyCreatedForClaimName] = v
+		case ParameterKeyPVCNamespace:
+			p.Tags[tagKeyCreatedForClaimNamespace] = v
+		case ParameterKeyPVName:
+			p.Tags[tagKeyCreatedForVolumeName] = 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

@@ -35,6 +35,7 @@ func TestExtractAndDefaultParameters(t *testing.T) {
 				DiskType:             "pd-standard",
 				ReplicationType:      "none",
 				DiskEncryptionKMSKey: "",
+				Tags:                 make(map[string]string),
 			},
 		},
 		{
@@ -44,6 +45,7 @@ func TestExtractAndDefaultParameters(t *testing.T) {
 				DiskType:             "pd-standard",
 				ReplicationType:      "none",
 				DiskEncryptionKMSKey: "",
+				Tags:                 make(map[string]string),
 			},
 		},
 		{
@@ -58,6 +60,7 @@ func TestExtractAndDefaultParameters(t *testing.T) {
 				DiskType:             "pd-ssd",
 				ReplicationType:      "regional-pd",
 				DiskEncryptionKMSKey: "foo/key",
+				Tags:                 make(map[string]string),
 			},
 		},
 		{
@@ -67,13 +70,24 @@ func TestExtractAndDefaultParameters(t *testing.T) {
 				DiskType:             "pd-standard",
 				ReplicationType:      "none",
 				DiskEncryptionKMSKey: "foo/key",
+				Tags:                 make(map[string]string),
+			},
+		},
+		{
+			name:       "tags",
+			parameters: map[string]string{ParameterKeyPVCName: "testPVCName", ParameterKeyPVCNamespace: "testPVCNamespace", ParameterKeyPVName: "testPVName"},
+			expectParams: DiskParameters{
+				DiskType:             "pd-standard",
+				ReplicationType:      "none",
+				DiskEncryptionKMSKey: "",
+				Tags:                 map[string]string{tagKeyCreatedForClaimName: "testPVCName", tagKeyCreatedForClaimNamespace: "testPVCNamespace", tagKeyCreatedForVolumeName: "testPVName", tagKeyCreatedBy: "testDriver"},
 			},
 		},
 	}
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			p, err := ExtractAndDefaultParameters(tc.parameters)
+			p, err := ExtractAndDefaultParameters(tc.parameters, "testDriver")
 			if gotErr := err != nil; gotErr != tc.expectErr {
 				t.Fatalf("ExtractAndDefaultParameters(%+v) = %v; expectedErr: %v", tc.parameters, err, tc.expectErr)
 			}

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

@@ -15,6 +15,7 @@ limitations under the License.
 package gcecloudprovider
 
 import (
+	"encoding/json"
 	"fmt"
 	"strings"
 	"time"
@@ -268,21 +269,42 @@ func ValidateDiskParameters(disk *CloudDisk, params common.DiskParameters) error
 
 func (cloud *CloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key, params common.DiskParameters, capBytes int64, capacityRange *csi.CapacityRange, replicaZones []string, snapshotID string) error {
 	klog.V(5).Infof("Inserting disk %v", volKey)
+
+	description, err := encodeDiskTags(params.Tags)
+	if err != nil {
+		return err
+	}
+
 	switch volKey.Type() {
 	case meta.Zonal:
-		return cloud.insertZonalDisk(ctx, volKey, params, capBytes, capacityRange, snapshotID)
+		if description == "" {
+			description = "Disk created by GCE-PD CSI Driver"
+		}
+		return cloud.insertZonalDisk(ctx, volKey, params, capBytes, capacityRange, snapshotID, description)
 	case meta.Regional:
-		return cloud.insertRegionalDisk(ctx, volKey, params, capBytes, capacityRange, replicaZones, snapshotID)
+		if description == "" {
+			description = "Regional disk created by GCE-PD CSI Driver"
+		}
+		return cloud.insertRegionalDisk(ctx, volKey, params, capBytes, capacityRange, replicaZones, snapshotID, description)
 	default:
 		return fmt.Errorf("could not insert disk, key was neither zonal nor regional, instead got: %v", volKey.String())
 	}
 }
 
-func (cloud *CloudProvider) insertRegionalDisk(ctx context.Context, volKey *meta.Key, params common.DiskParameters, capBytes int64, capacityRange *csi.CapacityRange, replicaZones []string, snapshotID string) error {
+func (cloud *CloudProvider) insertRegionalDisk(
+	ctx context.Context,
+	volKey *meta.Key,
+	params common.DiskParameters,
+	capBytes int64,
+	capacityRange *csi.CapacityRange,
+	replicaZones []string,
+	snapshotID string,
+	description string) error {
+
 	diskToCreate := &computev1.Disk{
 		Name:        volKey.Name,
 		SizeGb:      common.BytesToGb(capBytes),
-		Description: "Regional disk created by GCE-PD CSI Driver",
+		Description: description,
 		Type:        cloud.GetDiskTypeURI(volKey, params.DiskType),
 	}
 	if snapshotID != "" {
@@ -337,11 +359,18 @@ func (cloud *CloudProvider) insertRegionalDisk(ctx context.Context, volKey *meta
 	return nil
 }
 
-func (cloud *CloudProvider) insertZonalDisk(ctx context.Context, volKey *meta.Key, params common.DiskParameters, capBytes int64, capacityRange *csi.CapacityRange, snapshotID string) error {
+func (cloud *CloudProvider) insertZonalDisk(
+	ctx context.Context,
+	volKey *meta.Key,
+	params common.DiskParameters,
+	capBytes int64,
+	capacityRange *csi.CapacityRange,
+	snapshotID string,
+	description string) error {
 	diskToCreate := &computev1.Disk{
 		Name:        volKey.Name,
 		SizeGb:      common.BytesToGb(capBytes),
-		Description: "Disk created by GCE-PD CSI Driver",
+		Description: description,
 		Type:        cloud.GetDiskTypeURI(volKey, params.DiskType),
 	}
 
@@ -788,3 +817,18 @@ 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) {
+	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 string(enc), nil
+}

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

@@ -110,7 +110,7 @@ func (gceCS *GCEControllerServer) CreateVolume(ctx context.Context, req *csi.Cre
 
 	// Apply Parameters (case-insensitive). We leave validation of
 	// the values to the cloud provider.
-	params, err := common.ExtractAndDefaultParameters(req.GetParameters())
+	params, err := common.ExtractAndDefaultParameters(req.GetParameters(), gceCS.Driver.name)
 	if err != nil {
 		return nil, status.Errorf(codes.InvalidArgument, "failed to extract parameters: %v", err)
 	}
@@ -471,7 +471,7 @@ func (gceCS *GCEControllerServer) ValidateVolumeCapabilities(ctx context.Context
 	}
 
 	// Validate the disk parameters match the disk we GET
-	params, err := common.ExtractAndDefaultParameters(req.GetParameters())
+	params, err := common.ExtractAndDefaultParameters(req.GetParameters(), gceCS.Driver.name)
 	if err != nil {
 		return nil, status.Errorf(codes.InvalidArgument, "failed to extract parameters: %v", err)
 	}

test/e2e/tests/single_zone_e2e_test.go

@@ -676,6 +676,42 @@ var _ = Describe("GCE PD CSI Driver", func() {
 		Expect(err).To(BeNil(), "Failed to go through volume lifecycle")
 	})
 
+	It("Should successfully create disk with PVC/PV tags", func() {
+		Expect(testContexts).ToNot(BeEmpty())
+		testContext := getRandomTestContext()
+
+		controllerInstance := testContext.Instance
+		controllerClient := testContext.Client
+
+		p, z, _ := controllerInstance.GetIdentity()
+
+		// Create Disk
+		volName := testNamePrefix + string(uuid.NewUUID())
+		volID, err := controllerClient.CreateVolume(volName, map[string]string{
+			common.ParameterKeyPVCName:      "test-pvc",
+			common.ParameterKeyPVCNamespace: "test-pvc-namespace",
+			common.ParameterKeyPVName:       "test-pv-name",
+		}, defaultSizeGb, nil /* topReq */)
+		Expect(err).To(BeNil(), "CreateVolume failed with error: %v", err)
+
+		// Validate Disk Created
+		cloudDisk, err := computeService.Disks.Get(p, z, volName).Do()
+		Expect(err).To(BeNil(), "Could not get disk from cloud directly")
+		Expect(cloudDisk.Type).To(ContainSubstring(standardDiskType))
+		Expect(cloudDisk.Status).To(Equal(readyState))
+		Expect(cloudDisk.SizeGb).To(Equal(defaultSizeGb))
+		Expect(cloudDisk.Name).To(Equal(volName))
+		Expect(cloudDisk.Description).To(Equal("{\"kubernetes.io/created-for/pv/name\":\"test-pv-name\",\"kubernetes.io/created-for/pvc/name\":\"test-pvc\",\"kubernetes.io/created-for/pvc/namespace\":\"test-pvc-namespace\",\"storage.gke.io/created-by\":\"pd.csi.storage.gke.io\"}"))
+		defer func() {
+			// Delete Disk
+			controllerClient.DeleteVolume(volID)
+			Expect(err).To(BeNil(), "DeleteVolume failed")
+
+			// Validate Disk Deleted
+			_, err = computeService.Disks.Get(p, z, volName).Do()
+			Expect(gce.IsGCEError(err, "notFound")).To(BeTrue(), "Expected disk to not be found")
+		}()
+	})
 })
 
 func equalWithinEpsilon(a, b, epsiolon int64) bool {