From 42b0d4b622fc386dbab244f6c7791943c39560ae Mon Sep 17 00:00:00 2001 From: Matthew Cary Date: Wed, 29 Jul 2020 14:13:22 +0300 Subject: [PATCH] Add gce disk labels support via create volume parameters Add labels validation to match gce requirements add e2e test case for labels --- README.md | 1 + cmd/gce-pd-csi-driver/main.go | 13 +- .../disk_labels.yaml | 4 + .../kustomization.yaml | 8 + examples/kubernetes/demo-labeled-sc.yaml | 10 + pkg/common/parameters.go | 23 ++- pkg/common/parameters_test.go | 61 ++++-- pkg/common/utils.go | 59 ++++++ pkg/common/utils_test.go | 182 ++++++++++++++++++ pkg/gce-cloud-provider/compute/fake-gce.go | 1 + pkg/gce-cloud-provider/compute/gce-compute.go | 1 + pkg/gce-pd-csi-driver/controller_test.go | 25 +++ test/e2e/tests/single_zone_e2e_test.go | 40 ++++ test/e2e/utils/utils.go | 9 +- test/k8s-integration/config/sc-standard.yaml | 2 + test/run-e2e-local.sh | 1 + 16 files changed, 413 insertions(+), 27 deletions(-) create mode 100644 deploy/kubernetes/overlays/prow-gke-release-staging-rc-master/disk_labels.yaml create mode 100644 examples/kubernetes/demo-labeled-sc.yaml diff --git a/README.md b/README.md index 23ec3ba0e..ee36f24a6 100644 --- a/README.md +++ b/README.md @@ -68,6 +68,7 @@ See Github [Issues](https://github.com/kubernetes-sigs/gcp-compute-persistent-di | type | `pd-ssd` OR `pd-standard` | `pd-standard` | Type allows you to choose between standard Persistent Disks or Solid State Drive Persistent Disks | | replication-type | `none` OR `regional-pd` | `none` | Replication type allows you to choose between Zonal Persistent Disks or Regional Persistent Disks | | disk-encryption-kms-key | Fully qualified resource identifier for the key to use to encrypt new disks. | Empty string. | Encrypt disk using Customer Managed Encryption Key (CMEK). See [GKE Docs](https://cloud.google.com/kubernetes-engine/docs/how-to/using-cmek#create_a_cmek_protected_attached_disk) for details. | +| labels | `key1=value1,key2=value2` | | Labels allow you to assign custom [GCE Disk labels](https://cloud.google.com/compute/docs/labeling-resources). | ### Topology diff --git a/cmd/gce-pd-csi-driver/main.go b/cmd/gce-pd-csi-driver/main.go index 20fceaa0d..18cdce59b 100644 --- a/cmd/gce-pd-csi-driver/main.go +++ b/cmd/gce-pd-csi-driver/main.go @@ -24,7 +24,7 @@ import ( "k8s.io/klog" - cliflag "k8s.io/component-base/cli/flag" + "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/common" gce "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/gce-cloud-provider/compute" metadataservice "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/gce-cloud-provider/metadata" driver "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/gce-pd-csi-driver" @@ -39,7 +39,7 @@ var ( runNodeService = flag.Bool("run-node-service", true, "If set to false then the CSI driver does not activate its node service (default: true)") httpEndpoint = flag.String("http-endpoint", "", "The TCP network address where the prometheus metrics endpoint will listen (example: `:8080`). The default is empty string, which means metrics endpoint is disabled.") metricsPath = flag.String("metrics-path", "/metrics", "The HTTP path where prometheus metrics will be exposed. Default is `/metrics`.") - extraVolumeLabels map[string]string + extraVolumeLabelsStr = flag.String("extra-labels", "", "Extra labels to attach to each PD created. It is a comma separated list of key value pairs like '=,='. See https://cloud.google.com/compute/docs/labeling-resources for details") version string ) @@ -58,7 +58,6 @@ func init() { } func main() { - flag.Var(cliflag.NewMapStringString(&extraVolumeLabels), "extra-labels", "Extra labels to attach to each PD created. It is a comma separated list of key value pairs like '=,='") flag.Parse() rand.Seed(time.Now().UnixNano()) handle() @@ -79,6 +78,14 @@ func handle() { mm.EmitGKEComponentVersion() } + if len(*extraVolumeLabelsStr) > 0 && !*runControllerService { + klog.Fatalf("Extra volume labels provided but not running controller") + } + extraVolumeLabels, err := common.ConvertLabelsStringToMap(*extraVolumeLabelsStr) + if err != nil { + klog.Fatalf("Bad extra volume labels: %v", err) + } + gceDriver := driver.GetGCEDriver() //Initialize GCE Driver diff --git a/deploy/kubernetes/overlays/prow-gke-release-staging-rc-master/disk_labels.yaml b/deploy/kubernetes/overlays/prow-gke-release-staging-rc-master/disk_labels.yaml new file mode 100644 index 000000000..aaa4cb03d --- /dev/null +++ b/deploy/kubernetes/overlays/prow-gke-release-staging-rc-master/disk_labels.yaml @@ -0,0 +1,4 @@ +# for gce-pd-driver +- op: add + path: /spec/template/spec/containers/4/args/2 + value: "--extra-labels=csi=gce-pd-driver # Used for testing, not to be merged to stable" diff --git a/deploy/kubernetes/overlays/prow-gke-release-staging-rc-master/kustomization.yaml b/deploy/kubernetes/overlays/prow-gke-release-staging-rc-master/kustomization.yaml index a71c106e3..eb1c6f6f9 100644 --- a/deploy/kubernetes/overlays/prow-gke-release-staging-rc-master/kustomization.yaml +++ b/deploy/kubernetes/overlays/prow-gke-release-staging-rc-master/kustomization.yaml @@ -4,5 +4,13 @@ namespace: gce-pd-csi-driver resources: - ../stable-master +patchesJson6902: +# disk_labels.yaml is used for testing and should not be merged to stable. +- target: + group: apps + version: v1 + kind: Deployment + name: csi-gce-pd-controller + path: disk_labels.yaml transformers: - ../../images/prow-gke-release-staging-rc-master diff --git a/examples/kubernetes/demo-labeled-sc.yaml b/examples/kubernetes/demo-labeled-sc.yaml new file mode 100644 index 000000000..33857ebd6 --- /dev/null +++ b/examples/kubernetes/demo-labeled-sc.yaml @@ -0,0 +1,10 @@ +# Example StorageClass that adds labels to the GCP PD. +# This requires v1.2.1 or higher. +apiVersion: storage.k8s.io/v1 +kind: StorageClass +metadata: + name: csi-gce-pd +provisioner: pd.csi.storage.gke.io +parameters: + labels: key1=value1,key2=value2 +volumeBindingMode: WaitForFirstConsumer diff --git a/pkg/common/parameters.go b/pkg/common/parameters.go index b8d1f5564..d90469da1 100644 --- a/pkg/common/parameters.go +++ b/pkg/common/parameters.go @@ -25,6 +25,7 @@ const ( ParameterKeyType = "type" ParameterKeyReplicationType = "replication-type" ParameterKeyDiskEncryptionKmsKey = "disk-encryption-kms-key" + ParameterKeyLabels = "labels" replicationTypeNone = "none" @@ -33,7 +34,7 @@ const ( ParameterKeyPVCNamespace = "csi.storage.k8s.io/pvc/namespace" ParameterKeyPVName = "csi.storage.k8s.io/pv/name" - // Keys for tags to attach to the provisioned disk. + // 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" @@ -60,7 +61,9 @@ type DiskParameters struct { } // ExtractAndDefaultParameters will take the relevant parameters from a map and -// put them into a well defined struct making sure to default unspecified fields +// put them into a well defined struct making sure to default unspecified fields. +// extraVolumeLabels are added as labels; if there are also labels specified in +// parameters, any matching extraVolumeLabels will be overridden. func ExtractAndDefaultParameters(parameters map[string]string, driverName string, extraVolumeLabels map[string]string) (DiskParameters, error) { p := DiskParameters{ DiskType: "pd-standard", // Default @@ -70,6 +73,10 @@ func ExtractAndDefaultParameters(parameters map[string]string, driverName string Labels: make(map[string]string), // Default } + for k, v := range extraVolumeLabels { + p.Labels[k] = v + } + for k, v := range parameters { if k == "csiProvisionerSecretName" || k == "csiProvisionerSecretNamespace" { // These are hardcoded secrets keys required to function but not needed by GCE PD @@ -93,6 +100,15 @@ func ExtractAndDefaultParameters(parameters map[string]string, driverName string p.Tags[tagKeyCreatedForClaimNamespace] = v case ParameterKeyPVName: p.Tags[tagKeyCreatedForVolumeName] = v + case ParameterKeyLabels: + paramLabels, err := ConvertLabelsStringToMap(v) + if err != nil { + return p, fmt.Errorf("parameters contain invalid labels parameter: %w", err) + } + // Override any existing labels with those from this parameter. + for labelKey, labelValue := range paramLabels { + p.Labels[labelKey] = labelValue + } default: return p, fmt.Errorf("parameters contains invalid option %q", k) } @@ -100,8 +116,5 @@ func ExtractAndDefaultParameters(parameters map[string]string, driverName string if len(p.Tags) > 0 { p.Tags[tagKeyCreatedBy] = driverName } - for k, v := range extraVolumeLabels { - p.Labels[k] = v - } return p, nil } diff --git a/pkg/common/parameters_test.go b/pkg/common/parameters_test.go index 4692e7e18..c7f4538f0 100644 --- a/pkg/common/parameters_test.go +++ b/pkg/common/parameters_test.go @@ -37,50 +37,53 @@ func TestExtractAndDefaultParameters(t *testing.T) { DiskType: "pd-standard", ReplicationType: "none", DiskEncryptionKMSKey: "", - Tags: make(map[string]string), - Labels: make(map[string]string), + Tags: map[string]string{}, + Labels: map[string]string{}, }, }, { name: "specified empties", - parameters: map[string]string{ParameterKeyType: "", ParameterKeyReplicationType: "", ParameterKeyDiskEncryptionKmsKey: ""}, + parameters: map[string]string{ParameterKeyType: "", ParameterKeyReplicationType: "", ParameterKeyDiskEncryptionKmsKey: "", ParameterKeyLabels: ""}, labels: map[string]string{}, expectParams: DiskParameters{ DiskType: "pd-standard", ReplicationType: "none", DiskEncryptionKMSKey: "", - Tags: make(map[string]string), - Labels: make(map[string]string), + Tags: map[string]string{}, + Labels: map[string]string{}, }, }, { name: "random keys", - parameters: map[string]string{ParameterKeyType: "", "foo": "", ParameterKeyDiskEncryptionKmsKey: ""}, + parameters: map[string]string{ParameterKeyType: "", "foo": "", ParameterKeyDiskEncryptionKmsKey: "", ParameterKeyLabels: ""}, labels: map[string]string{}, expectErr: true, }, { - name: "real values", - parameters: map[string]string{ParameterKeyType: "pd-ssd", ParameterKeyReplicationType: "regional-pd", ParameterKeyDiskEncryptionKmsKey: "foo/key"}, + name: "values from parameters", + parameters: map[string]string{ParameterKeyType: "pd-ssd", ParameterKeyReplicationType: "regional-pd", ParameterKeyDiskEncryptionKmsKey: "foo/key", ParameterKeyLabels: "key1=value1,key2=value2"}, labels: map[string]string{}, expectParams: DiskParameters{ DiskType: "pd-ssd", ReplicationType: "regional-pd", DiskEncryptionKMSKey: "foo/key", - Tags: make(map[string]string), - Labels: make(map[string]string), + Tags: map[string]string{}, + Labels: map[string]string{ + "key1": "value1", + "key2": "value2", + }, }, }, { - name: "real values, checking balanced pd", + name: "values from parameters, checking balanced pd", parameters: map[string]string{ParameterKeyType: "pd-balanced", ParameterKeyReplicationType: "regional-pd", ParameterKeyDiskEncryptionKmsKey: "foo/key"}, labels: map[string]string{}, expectParams: DiskParameters{ DiskType: "pd-balanced", ReplicationType: "regional-pd", DiskEncryptionKMSKey: "foo/key", - Tags: make(map[string]string), - Labels: make(map[string]string), + Tags: map[string]string{}, + Labels: map[string]string{}, }, }, { @@ -91,8 +94,8 @@ func TestExtractAndDefaultParameters(t *testing.T) { DiskType: "pd-standard", ReplicationType: "none", DiskEncryptionKMSKey: "foo/key", - Tags: make(map[string]string), - Labels: make(map[string]string), + Tags: map[string]string{}, + Labels: map[string]string{}, }, }, { @@ -104,11 +107,11 @@ func TestExtractAndDefaultParameters(t *testing.T) { ReplicationType: "none", DiskEncryptionKMSKey: "", Tags: map[string]string{tagKeyCreatedForClaimName: "testPVCName", tagKeyCreatedForClaimNamespace: "testPVCNamespace", tagKeyCreatedForVolumeName: "testPVName", tagKeyCreatedBy: "testDriver"}, - Labels: make(map[string]string), + Labels: map[string]string{}, }, }, { - name: "labels", + name: "extra labels", parameters: map[string]string{}, labels: map[string]string{"label-1": "label-value-1", "label-2": "label-value-2"}, expectParams: DiskParameters{ @@ -119,6 +122,30 @@ func TestExtractAndDefaultParameters(t *testing.T) { Labels: map[string]string{"label-1": "label-value-1", "label-2": "label-value-2"}, }, }, + { + name: "parameter and extra labels", + parameters: map[string]string{ParameterKeyLabels: "key1=value1,key2=value2"}, + labels: map[string]string{"label-1": "label-value-1", "label-2": "label-value-2"}, + expectParams: DiskParameters{ + DiskType: "pd-standard", + ReplicationType: "none", + DiskEncryptionKMSKey: "", + Tags: map[string]string{}, + Labels: map[string]string{"key1": "value1", "key2": "value2", "label-1": "label-value-1", "label-2": "label-value-2"}, + }, + }, + { + name: "parameter and extra labels, overlapping", + parameters: map[string]string{ParameterKeyLabels: "label-1=value-a,key1=value1"}, + labels: map[string]string{"label-1": "label-value-1", "label-2": "label-value-2"}, + expectParams: DiskParameters{ + DiskType: "pd-standard", + ReplicationType: "none", + DiskEncryptionKMSKey: "", + Tags: map[string]string{}, + Labels: map[string]string{"key1": "value1", "label-1": "value-a", "label-2": "label-value-2"}, + }, + }, } for _, tc := range tests { diff --git a/pkg/common/utils.go b/pkg/common/utils.go index c48045e33..64acef3da 100644 --- a/pkg/common/utils.go +++ b/pkg/common/utils.go @@ -18,6 +18,7 @@ package common import ( "fmt" + "regexp" "strings" "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta" @@ -156,3 +157,61 @@ func CreateNodeID(project, zone, name string) string { func CreateZonalVolumeID(project, zone, name string) string { return fmt.Sprintf(volIDZonalFmt, project, zone, name) } + +// ConvertLabelsStringToMap converts the labels from string to map +// example: "key1=value1,key2=value2" gets converted into {"key1": "value1", "key2": "value2"} +// See https://cloud.google.com/compute/docs/labeling-resources#label_format for details. +func ConvertLabelsStringToMap(labels string) (map[string]string, error) { + const labelsDelimiter = "," + const labelsKeyValueDelimiter = "=" + + labelsMap := make(map[string]string) + if labels == "" { + return labelsMap, nil + } + + regexKey, _ := regexp.Compile(`^\p{Ll}[\p{Ll}0-9_-]{0,62}$`) + checkLabelKeyFn := func(key string) error { + if !regexKey.MatchString(key) { + return fmt.Errorf("label value %q is invalid (should start with lowercase letter / lowercase letter, digit, _ and - chars are allowed / 1-63 characters", key) + } + return nil + } + + regexValue, _ := regexp.Compile(`^[\p{Ll}0-9_-]{0,63}$`) + checkLabelValueFn := func(value string) error { + if !regexValue.MatchString(value) { + return fmt.Errorf("label value %q is invalid (lowercase letter, digit, _ and - chars are allowed / 0-63 characters", value) + } + + return nil + } + + keyValueStrings := strings.Split(labels, labelsDelimiter) + for _, keyValue := range keyValueStrings { + keyValue := strings.Split(keyValue, labelsKeyValueDelimiter) + + if len(keyValue) != 2 { + return nil, fmt.Errorf("labels %q are invalid, correct format: 'key1=value1,key2=value2'", labels) + } + + key := strings.TrimSpace(keyValue[0]) + if err := checkLabelKeyFn(key); err != nil { + return nil, err + } + + value := strings.TrimSpace(keyValue[1]) + if err := checkLabelValueFn(value); err != nil { + return nil, err + } + + labelsMap[key] = value + } + + const maxNumberOfLabels = 64 + if len(labelsMap) > maxNumberOfLabels { + return nil, fmt.Errorf("more than %d labels is not allowed, given: %d", maxNumberOfLabels, len(labelsMap)) + } + + return labelsMap, nil +} diff --git a/pkg/common/utils_test.go b/pkg/common/utils_test.go index 57de58c30..be1f31734 100644 --- a/pkg/common/utils_test.go +++ b/pkg/common/utils_test.go @@ -338,3 +338,185 @@ func TestKeyToVolumeID(t *testing.T) { } } + +func TestConvertLabelsStringToMap(t *testing.T) { + t.Run("parsing labels string into map", func(t *testing.T) { + testCases := []struct { + name string + labels string + expectedOutput map[string]string + expectedError bool + }{ + { + name: "should return empty map when labels string is empty", + labels: "", + expectedOutput: map[string]string{}, + expectedError: false, + }, + { + name: "single label string", + labels: "key=value", + expectedOutput: map[string]string{ + "key": "value", + }, + expectedError: false, + }, + { + name: "multiple label string", + labels: "key1=value1,key2=value2", + expectedOutput: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + expectedError: false, + }, + { + name: "multiple labels string with whitespaces gets trimmed", + labels: "key1=value1, key2=value2", + expectedOutput: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + expectedError: false, + }, + { + name: "malformed labels string (no keys and values)", + labels: ",,", + expectedOutput: nil, + expectedError: true, + }, + { + name: "malformed labels string (incorrect format)", + labels: "foo,bar", + expectedOutput: nil, + expectedError: true, + }, + { + name: "malformed labels string (missing key)", + labels: "key1=value1,=bar", + expectedOutput: nil, + expectedError: true, + }, + { + name: "malformed labels string (missing key and value)", + labels: "key1=value1,=bar,=", + expectedOutput: nil, + expectedError: true, + }, + } + + for _, tc := range testCases { + t.Logf("test case: %s", tc.name) + output, err := ConvertLabelsStringToMap(tc.labels) + if tc.expectedError && err == nil { + t.Errorf("Expected error but got none") + } + if err != nil { + if !tc.expectedError { + t.Errorf("Did not expect error but got: %v", err) + } + continue + } + + if !reflect.DeepEqual(output, tc.expectedOutput) { + t.Errorf("Got labels %v, but expected %v", output, tc.expectedOutput) + } + } + }) + + t.Run("checking google requirements", func(t *testing.T) { + testCases := []struct { + name string + labels string + expectedError bool + }{ + { + name: "64 labels at most", + labels: `k1=v,k2=v,k3=v,k4=v,k5=v,k6=v,k7=v,k8=v,k9=v,k10=v,k11=v,k12=v,k13=v,k14=v,k15=v,k16=v,k17=v,k18=v,k19=v,k20=v, + k21=v,k22=v,k23=v,k24=v,k25=v,k26=v,k27=v,k28=v,k29=v,k30=v,k31=v,k32=v,k33=v,k34=v,k35=v,k36=v,k37=v,k38=v,k39=v,k40=v, + k41=v,k42=v,k43=v,k44=v,k45=v,k46=v,k47=v,k48=v,k49=v,k50=v,k51=v,k52=v,k53=v,k54=v,k55=v,k56=v,k57=v,k58=v,k59=v,k60=v, + k61=v,k62=v,k63=v,k64=v,k65=v`, + expectedError: true, + }, + { + name: "label key must start with lowercase char (# case)", + labels: "#k=v", + expectedError: true, + }, + { + name: "label key must start with lowercase char (_ case)", + labels: "_k=v", + expectedError: true, + }, + { + name: "label key must start with lowercase char (- case)", + labels: "-k=v", + expectedError: true, + }, + { + name: "label key can only contain lowercase chars, digits, _ and -)", + labels: "k*=v", + expectedError: true, + }, + { + name: "label key may not have over 63 characters", + labels: "abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij1234=v", + expectedError: true, + }, + { + name: "label key cannot contain . and /", + labels: "kubernetes.io/created-for/pvc/namespace=v", + expectedError: true, + }, + { + name: "label value can only contain lowercase chars, digits, _ and -)", + labels: "k1=###", + expectedError: true, + }, + { + name: "label value may not have over 63 characters", + labels: "abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij1234=v", + expectedError: true, + }, + { + name: "label value cannot contain . and /", + labels: "kubernetes_io_created-for_pvc_namespace=v./", + expectedError: true, + }, + { + name: "label key can have up to 63 characters", + labels: "abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij123=v", + expectedError: false, + }, + { + name: "label value can have up to 63 characters", + labels: "abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij123=v", + expectedError: false, + }, + { + name: "label key can contain _ and -", + labels: "kubernetes_io_created-for_pvc_namespace=v", + expectedError: false, + }, + { + name: "label value can contain _ and -", + labels: "k=my_value-2", + expectedError: false, + }, + } + + for _, tc := range testCases { + t.Logf("test case: %s", tc.name) + _, err := ConvertLabelsStringToMap(tc.labels) + + if tc.expectedError && err == nil { + t.Errorf("Expected error but got none") + } + + if !tc.expectedError && err != nil { + t.Errorf("Did not expect error but got: %v", err) + } + } + }) + +} diff --git a/pkg/gce-cloud-provider/compute/fake-gce.go b/pkg/gce-cloud-provider/compute/fake-gce.go index 910d6bcf7..a4d3f9a95 100644 --- a/pkg/gce-cloud-provider/compute/fake-gce.go +++ b/pkg/gce-cloud-provider/compute/fake-gce.go @@ -267,6 +267,7 @@ func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key Type: cloud.GetDiskTypeURI(volKey, params.DiskType), SourceSnapshotId: snapshotID, Status: cloud.mockDiskStatus, + Labels: params.Labels, } if params.DiskEncryptionKMSKey != "" { computeDisk.DiskEncryptionKey = &computev1.CustomerEncryptionKey{ diff --git a/pkg/gce-cloud-provider/compute/gce-compute.go b/pkg/gce-cloud-provider/compute/gce-compute.go index 3a460e4b6..a33e2e78b 100644 --- a/pkg/gce-cloud-provider/compute/gce-compute.go +++ b/pkg/gce-cloud-provider/compute/gce-compute.go @@ -384,6 +384,7 @@ func (cloud *CloudProvider) insertRegionalDisk( SizeGb: common.BytesToGbRoundUp(capBytes), Description: description, Type: cloud.GetDiskTypeURI(volKey, params.DiskType), + Labels: params.Labels, } if snapshotID != "" { diskToCreate.SourceSnapshot = snapshotID diff --git a/pkg/gce-pd-csi-driver/controller_test.go b/pkg/gce-pd-csi-driver/controller_test.go index e712f9a1e..a3c904f42 100644 --- a/pkg/gce-pd-csi-driver/controller_test.go +++ b/pkg/gce-pd-csi-driver/controller_test.go @@ -674,6 +674,31 @@ func TestCreateVolumeArguments(t *testing.T) { AccessibleTopology: stdTopology, }, }, + { + name: "success with labels parameter", + req: &csi.CreateVolumeRequest{ + Name: name, + CapacityRange: stdCapRange, + VolumeCapabilities: stdVolCaps, + Parameters: map[string]string{"labels": "key1=value1,key2=value2"}, + }, + expVol: &csi.Volume{ + CapacityBytes: common.GbToBytes(20), + VolumeId: testVolumeID, + VolumeContext: nil, + AccessibleTopology: stdTopology, + }, + }, + { + name: "fail with malformed labels parameter", + req: &csi.CreateVolumeRequest{ + Name: name, + CapacityRange: stdCapRange, + VolumeCapabilities: stdVolCaps, + Parameters: map[string]string{"labels": "key1=value1,#=$;;"}, + }, + expErrCode: codes.InvalidArgument, + }, } // Run test cases diff --git a/test/e2e/tests/single_zone_e2e_test.go b/test/e2e/tests/single_zone_e2e_test.go index 93d179ee3..dde521757 100644 --- a/test/e2e/tests/single_zone_e2e_test.go +++ b/test/e2e/tests/single_zone_e2e_test.go @@ -292,6 +292,46 @@ var _ = Describe("GCE PD CSI Driver", func() { }() }) + It("Should create and delete disk with labels", func() { + Expect(testContexts).ToNot(BeEmpty()) + testContext := getRandomTestContext() + + p, z, _ := testContext.Instance.GetIdentity() + client := testContext.Client + + // Create Disk + volName := testNamePrefix + string(uuid.NewUUID()) + params := map[string]string{ + common.ParameterKeyLabels: "key1=value1,key2=value2", + } + volID, err := client.CreateVolume(volName, params, defaultSizeGb, nil) + 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.Labels).To(Equal(map[string]string{ + "key1": "value1", + "key2": "value2", + // The label below is added as an --extra-label driver command line argument. + testutils.DiskLabelKey: testutils.DiskLabelValue, + })) + Expect(cloudDisk.Name).To(Equal(volName)) + + defer func() { + // Delete Disk + err := client.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") + }() + }) + // Test volume already exists idempotency // Test volume with op pending diff --git a/test/e2e/utils/utils.go b/test/e2e/utils/utils.go index c901e2359..6074de512 100644 --- a/test/e2e/utils/utils.go +++ b/test/e2e/utils/utils.go @@ -34,6 +34,11 @@ import ( remote "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/test/remote" ) +const ( + DiskLabelKey = "csi" + DiskLabelValue = "e2e-test" +) + var ( boskos, _ = boskosclient.NewClient(os.Getenv("JOB_NAME"), "http://boskos", "", "") ) @@ -50,8 +55,8 @@ func GCEClientAndDriverSetup(instance *remote.InstanceInfo) (*remote.TestContext endpoint := fmt.Sprintf("tcp://localhost:%s", port) workspace := remote.NewWorkspaceDir("gce-pd-e2e-") - driverRunCmd := fmt.Sprintf("sh -c '/usr/bin/nohup %s/gce-pd-csi-driver -v=4 --endpoint=%s 2> %s/prog.out < /dev/null > /dev/null &'", - workspace, endpoint, workspace) + driverRunCmd := fmt.Sprintf("sh -c '/usr/bin/nohup %s/gce-pd-csi-driver -v=4 --endpoint=%s --extra-labels=%s=%s 2> %s/prog.out < /dev/null > /dev/null &'", + workspace, endpoint, DiskLabelKey, DiskLabelValue, workspace) config := &remote.ClientConfig{ PkgPath: pkgPath, diff --git a/test/k8s-integration/config/sc-standard.yaml b/test/k8s-integration/config/sc-standard.yaml index ab8d631f2..5a85b3fc7 100644 --- a/test/k8s-integration/config/sc-standard.yaml +++ b/test/k8s-integration/config/sc-standard.yaml @@ -5,4 +5,6 @@ metadata: provisioner: pd.csi.storage.gke.io parameters: type: pd-standard + # Add labels for testing. + labels: key1=value1,key2=value2 volumeBindingMode: WaitForFirstConsumer diff --git a/test/run-e2e-local.sh b/test/run-e2e-local.sh index be0455a1e..ae8e12ebc 100755 --- a/test/run-e2e-local.sh +++ b/test/run-e2e-local.sh @@ -6,3 +6,4 @@ set -o errexit readonly PKGDIR=sigs.k8s.io/gcp-compute-persistent-disk-csi-driver ginkgo --v "test/e2e/tests" -- --project "${PROJECT}" --service-account "${IAM_NAME}" --v=4 --logtostderr +