Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tags that the in-tree volume plugin uses #530

Merged
merged 3 commits into from
Aug 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions cmd/options/controller_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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).")
}
5 changes: 5 additions & 0 deletions cmd/options/controller_options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
7 changes: 6 additions & 1 deletion docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
45 changes: 45 additions & 0 deletions pkg/driver/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: in the future we should consider moving CO-specific constants to their own packages/files.


// constants for default command line flag values
Expand Down
16 changes: 14 additions & 2 deletions pkg/driver/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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)
}
Expand Down Expand Up @@ -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
Expand Down
123 changes: 123 additions & 0 deletions pkg/driver/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
15 changes: 11 additions & 4 deletions pkg/driver/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
}
}
9 changes: 9 additions & 0 deletions pkg/driver/driver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}