Merge pull request #530 from jsafrane/legacy-tags

Add tags that the in-tree volume plugin uses

cmd/main.go

@@ -33,6 +33,7 @@ func main() {
 		driver.WithExtraVolumeTags(options.ControllerOptions.ExtraVolumeTags),
 		driver.WithMode(options.DriverMode),
 		driver.WithVolumeAttachLimit(options.NodeOptions.VolumeAttachLimit),
+		driver.WithKubernetesClusterID(options.ControllerOptions.KubernetesClusterID),
 	)
 	if err != nil {
 		klog.Fatalln(err)

cmd/options/controller_options.go

@@ -27,8 +27,12 @@ type ControllerOptions struct {
 	// ExtraVolumeTags is a map of tags that will be attached to each dynamically provisioned
 	// volume.
 	ExtraVolumeTags map[string]string
+	// ID of the kubernetes cluster. This is used only to create the same tags on volumes that
+	// in-tree volume volume plugin does.
+	KubernetesClusterID string
 }
 
 func (s *ControllerOptions) AddFlags(fs *flag.FlagSet) {
 	fs.Var(cliflag.NewMapStringString(&s.ExtraVolumeTags), "extra-volume-tags", "Extra volume tags to attach to each dynamically provisioned volume. It is a comma separated list of key value pairs like '<key1>=<value1>,<key2>=<value2>'")
+	fs.StringVar(&s.KubernetesClusterID, "k8s-tag-cluster-id", "", "ID of the Kubernetes cluster used for tagging provisioned EBS volumes (optional).")
 }

cmd/options/controller_options_test.go

@@ -32,6 +32,11 @@ func TestControllerOptions(t *testing.T) {
 			flag:  "extra-volume-tags",
 			found: true,
 		},
+		{
+			name:  "lookup k8s-tag-cluster-id",
+			flag:  "k8s-tag-cluster-id",
+			found: true,
+		},
 		{
 			name:  "fail for non-desired flag",
 			flag:  "some-other-flag",

docs/README.md

@@ -132,7 +132,12 @@ Make sure you follow the [Prerequisites](README.md#Prerequisites) before the exa
 * [Volume Resizing](../examples/kubernetes/resizing)
 
 ## Migrating from in-tree EBS plugin
-Starting from Kubernetes 1.14, CSI migration is supported as alpha feature. If you have persistence volumes that are created with in-tree `kubernetes.io/aws-ebs` plugin, you could migrate to use EBS CSI driver. To turn on the migration, set `CSIMigration` and `CSIMigrationAWS` feature gates to `true` for `kube-controller-manager` and `kubelet`.
+Starting from Kubernetes 1.17, CSI migration is supported as beta feature (alpha since 1.14). If you have persistence volumes that are created with in-tree `kubernetes.io/aws-ebs` plugin, you could migrate to use EBS CSI driver. To turn on the migration, set `CSIMigration` and `CSIMigrationAWS` feature gates to `true` for `kube-controller-manager` and `kubelet`.
+
+To make sure dynamically provisioned EBS volumes have all tags that the in-tree volume plugin used:
+* Run the external-provisioner sidecar with `--extra-create-metadata=true` cmdline option. External-provisioner v1.6 or newer is required.
+* Run the CSI driver with `--k8s-tag-cluster-id=<ID of the Kubernetes cluster>` command line option.
+
 
 ## Development
 Please go through [CSI Spec](https://github.com/container-storage-interface/spec/blob/master/spec.md) and [General CSI driver development guideline](https://kubernetes-csi.github.io/docs/Development.html) to get some basic understanding of CSI driver before you start.

pkg/driver/constants.go

@@ -36,6 +36,51 @@ const (
 
 	// KmsKeyId represents key for KMS encryption key
 	KmsKeyIDKey = "kmskeyid"
+
+	// PVCNameKey contains name of the PVC for which is a volume provisioned.
+	PVCNameKey = "csi.storage.k8s.io/pvc/name"
+
+	// PVCNamespaceKey contains namespace of the PVC for which is a volume provisioned.
+	PVCNamespaceKey = "csi.storage.k8s.io/pvc/namespace"
+
+	// PVNameKey contains name of the final PV that will be used for the dynamically
+	// provisioned volume
+	PVNameKey = "csi.storage.k8s.io/pv/name"
+)
+
+// constants for volume tags and their values
+const (
+	// ResourceLifecycleTagPrefix is prefix of tag for provisioned EBS volume that
+	// marks them as owned by the cluster. Used only when --cluster-id is set.
+	ResourceLifecycleTagPrefix = "kubernetes.io/cluster/"
+
+	// ResourceLifecycleOwned is the value we use when tagging resources to indicate
+	// that the resource is considered owned and managed by the cluster,
+	// and in particular that the lifecycle is tied to the lifecycle of the cluster.
+	// From k8s.io/legacy-cloud-providers/aws/tags.go.
+	ResourceLifecycleOwned = "owned"
+
+	// NameTag is tag applied to provisioned EBS volume for backward compatibility with
+	// in-tree volume plugin. Used only when --cluster-id is set.
+	NameTag = "Name"
+
+	// PVCNameTag is tag applied to provisioned EBS volume for backward compatibility
+	// with in-tree volume plugin. Value of the tag is PVC name. It is applied only when
+	// the external provisioner sidecar is started with --extra-create-metadata=true and
+	// thus provides such metadata to the CSI driver.
+	PVCNameTag = "kubernetes.io/created-for/pvc/name"
+
+	// PVCNamespaceTag is tag applied to provisioned EBS volume for backward compatibility
+	// with in-tree volume plugin. Value of the tag is PVC namespace. It is applied only when
+	// the external provisioner sidecar is started with --extra-create-metadata=true and
+	// thus provides such metadata to the CSI driver.
+	PVCNamespaceTag = "kubernetes.io/created-for/pvc/namespace"
+
+	// PVNameTag is tag applied to provisioned EBS volume for backward compatibility
+	// with in-tree volume plugin. Value of the tag is PV name. It is applied only when
+	// the external provisioner sidecar is started with --extra-create-metadata=true and
+	// thus provides such metadata to the CSI driver.
+	PVNameTag = "kubernetes.io/created-for/pv/name"
 )
 
 // constants for default command line flag values

pkg/driver/controller.go

@@ -128,6 +128,9 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol
 		iopsPerGB   int
 		isEncrypted bool
 		kmsKeyID    string
+		volumeTags  = map[string]string{
+			cloud.VolumeNameTagKey: volName,
+		}
 	)
 
 	for key, value := range req.GetParameters() {
@@ -149,6 +152,12 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol
 			}
 		case KmsKeyIDKey:
 			kmsKeyID = value
+		case PVCNameKey:
+			volumeTags[PVCNameTag] = value
+		case PVCNamespaceKey:
+			volumeTags[PVCNamespaceTag] = value
+		case PVNameKey:
+			volumeTags[PVNameTag] = value
 		default:
 			return nil, status.Errorf(codes.InvalidArgument, "Invalid parameter key %s for CreateVolume", key)
 		}
@@ -178,8 +187,11 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol
 	// create a new volume
 	zone := pickAvailabilityZone(req.GetAccessibilityRequirements())
 
-	volumeTags := map[string]string{
-		cloud.VolumeNameTagKey: volName,
+	// fill volume tags
+	if d.driverOptions.kubernetesClusterID != "" {
+		resourceLifecycleTag := ResourceLifecycleTagPrefix + d.driverOptions.kubernetesClusterID
+		volumeTags[resourceLifecycleTag] = ResourceLifecycleOwned
+		volumeTags[NameTag] = d.driverOptions.kubernetesClusterID + "-dynamic-" + volName
 	}
 	for k, v := range d.driverOptions.extraVolumeTags {
 		volumeTags[k] = v

pkg/driver/controller_test.go

@@ -1048,6 +1048,129 @@ func TestCreateVolume(t *testing.T) {
 					},
 				}
 
+				_, err := awsDriver.CreateVolume(ctx, req)
+				if err != nil {
+					srvErr, ok := status.FromError(err)
+					if !ok {
+						t.Fatalf("Could not get error status code from error: %v", srvErr)
+					}
+					t.Fatalf("Unexpected error: %v", srvErr.Code())
+				}
+			},
+		},
+		{
+			name: "success with cluster-id",
+			testFunc: func(t *testing.T) {
+				const (
+					volumeName            = "random-vol-name"
+					clusterID             = "test-cluster-id"
+					expectedOwnerTag      = "kubernetes.io/cluster/test-cluster-id"
+					expectedOwnerTagValue = "owned"
+					expectedNameTag       = "Name"
+					expectedNameTagValue  = "test-cluster-id-dynamic-random-vol-name"
+				)
+				req := &csi.CreateVolumeRequest{
+					Name:               volumeName,
+					CapacityRange:      stdCapRange,
+					VolumeCapabilities: stdVolCap,
+					Parameters:         nil,
+				}
+
+				ctx := context.Background()
+
+				mockDisk := &cloud.Disk{
+					VolumeID:         req.Name,
+					AvailabilityZone: expZone,
+					CapacityGiB:      util.BytesToGiB(stdVolSize),
+				}
+
+				diskOptions := &cloud.DiskOptions{
+					CapacityBytes: stdVolSize,
+					Tags: map[string]string{
+						cloud.VolumeNameTagKey: volumeName,
+						expectedOwnerTag:       expectedOwnerTagValue,
+						expectedNameTag:        expectedNameTagValue,
+					},
+				}
+
+				mockCtl := gomock.NewController(t)
+				defer mockCtl.Finish()
+
+				mockCloud := mocks.NewMockCloud(mockCtl)
+				mockCloud.EXPECT().GetDiskByName(gomock.Eq(ctx), gomock.Eq(req.Name), gomock.Eq(stdVolSize)).Return(nil, cloud.ErrNotFound)
+				mockCloud.EXPECT().CreateDisk(gomock.Eq(ctx), gomock.Eq(req.Name), gomock.Eq(diskOptions)).Return(mockDisk, nil)
+
+				awsDriver := controllerService{
+					cloud: mockCloud,
+					driverOptions: &DriverOptions{
+						kubernetesClusterID: clusterID,
+					},
+				}
+
+				_, err := awsDriver.CreateVolume(ctx, req)
+				if err != nil {
+					srvErr, ok := status.FromError(err)
+					if !ok {
+						t.Fatalf("Could not get error status code from error: %v", srvErr)
+					}
+					t.Fatalf("Unexpected error: %v", srvErr.Code())
+				}
+			},
+		},
+		{
+			name: "success with legacy tags",
+			testFunc: func(t *testing.T) {
+				const (
+					volumeName              = "random-vol-name"
+					clusterID               = "test-cluster-id"
+					expectedPVCNameTag      = "kubernetes.io/created-for/pvc/name"
+					expectedPVCNamespaceTag = "kubernetes.io/created-for/pvc/namespace"
+					expectedPVNameTag       = "kubernetes.io/created-for/pv/name"
+					pvcNamespace            = "default"
+					pvcName                 = "my-pvc"
+					pvName                  = volumeName
+				)
+				req := &csi.CreateVolumeRequest{
+					Name:               volumeName,
+					CapacityRange:      stdCapRange,
+					VolumeCapabilities: stdVolCap,
+					Parameters: map[string]string{
+						"csi.storage.k8s.io/pvc/name":      pvcName,
+						"csi.storage.k8s.io/pvc/namespace": pvcNamespace,
+						"csi.storage.k8s.io/pv/name":       pvName,
+					},
+				}
+
+				ctx := context.Background()
+
+				mockDisk := &cloud.Disk{
+					VolumeID:         req.Name,
+					AvailabilityZone: expZone,
+					CapacityGiB:      util.BytesToGiB(stdVolSize),
+				}
+
+				diskOptions := &cloud.DiskOptions{
+					CapacityBytes: stdVolSize,
+					Tags: map[string]string{
+						cloud.VolumeNameTagKey:  volumeName,
+						expectedPVCNameTag:      pvcName,
+						expectedPVCNamespaceTag: pvcNamespace,
+						expectedPVNameTag:       pvName,
+					},
+				}
+
+				mockCtl := gomock.NewController(t)
+				defer mockCtl.Finish()
+
+				mockCloud := mocks.NewMockCloud(mockCtl)
+				mockCloud.EXPECT().GetDiskByName(gomock.Eq(ctx), gomock.Eq(req.Name), gomock.Eq(stdVolSize)).Return(nil, cloud.ErrNotFound)
+				mockCloud.EXPECT().CreateDisk(gomock.Eq(ctx), gomock.Eq(req.Name), gomock.Eq(diskOptions)).Return(mockDisk, nil)
+
+				awsDriver := controllerService{
+					cloud:         mockCloud,
+					driverOptions: &DriverOptions{},
+				}
+
 				_, err := awsDriver.CreateVolume(ctx, req)
 				if err != nil {
 					srvErr, ok := status.FromError(err)

pkg/driver/driver.go

@@ -53,10 +53,11 @@ type Driver struct {
 }
 
 type DriverOptions struct {
-	endpoint          string
-	extraVolumeTags   map[string]string
-	mode              Mode
-	volumeAttachLimit int64
+	endpoint            string
+	extraVolumeTags     map[string]string
+	mode                Mode
+	volumeAttachLimit   int64
+	kubernetesClusterID string
 }
 
 func NewDriver(options ...func(*DriverOptions)) (*Driver, error) {
@@ -162,3 +163,9 @@ func WithVolumeAttachLimit(volumeAttachLimit int64) func(*DriverOptions) {
 		o.volumeAttachLimit = volumeAttachLimit
 	}
 }
+
+func WithKubernetesClusterID(clusterID string) func(*DriverOptions) {
+	return func(o *DriverOptions) {
+		o.kubernetesClusterID = clusterID
+	}
+}

pkg/driver/driver_test.go

@@ -56,3 +56,12 @@ func TestWithVolumeAttachLimit(t *testing.T) {
 		t.Fatalf("expected volumeAttachLimit option got set to %d but is set to %d", value, options.volumeAttachLimit)
 	}
 }
+
+func TestWithClusterID(t *testing.T) {
+	var id string = "test-cluster-id"
+	options := &DriverOptions{}
+	WithKubernetesClusterID(id)(options)
+	if options.kubernetesClusterID != id {
+		t.Fatalf("expected kubernetesClusterID option got set to %s but is set to %s", id, options.kubernetesClusterID)
+	}
+}