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 a cmdline option to add extra volume tags #353

Merged
merged 2 commits into from
Oct 31, 2019
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
18 changes: 14 additions & 4 deletions cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,25 @@ import (
"os"

"github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/driver"
cliflag "k8s.io/component-base/cli/flag"
"k8s.io/klog"
)

func main() {
var (
endpoint = flag.String("endpoint", "unix://tmp/csi.sock", "CSI Endpoint")
version = flag.Bool("version", false, "Print the version and exit.")
version bool
endpoint string
extraVolumeTags map[string]string
)

flag.BoolVar(&version, "version", false, "Print the version and exit.")
flag.StringVar(&endpoint, "endpoint", driver.DefaultCSIEndpoint, "CSI Endpoint")
flag.Var(cliflag.NewMapStringString(&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>'")

klog.InitFlags(nil)
flag.Parse()

if *version {
if version {
info, err := driver.GetVersionJSON()
if err != nil {
klog.Fatalln(err)
Expand All @@ -42,7 +49,10 @@ func main() {
os.Exit(0)
}

drv, err := driver.NewDriver(*endpoint)
drv, err := driver.NewDriver(
driver.WithEndpoint(endpoint),
driver.WithExtraVolumeTags(extraVolumeTags),
)
if err != nil {
klog.Fatalln(err)
}
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ require (
k8s.io/api v0.0.0
k8s.io/apimachinery v0.0.0
k8s.io/client-go v0.0.0
k8s.io/component-base v0.0.0
k8s.io/klog v0.4.0
k8s.io/kubernetes v1.15.2
)
Expand Down
18 changes: 16 additions & 2 deletions pkg/cloud/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,20 @@ var (
)

// AWS provisioning limits.
// Source: http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/EBSVolumeTypes.html
// Sources:
// http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/EBSVolumeTypes.html
// https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/Using_Tags.html#tag-restrictions
const (
// MinTotalIOPS represents the minimum Input Output per second.
MinTotalIOPS = 100
// MaxTotalIOPS represents the maximum Input Output per second.
MaxTotalIOPS = 20000
// MaxNumTagsPerResource represents the maximum number of tags per AWS resource.
MaxNumTagsPerResource = 50
// MaxTagKeyLength represents the maximum key length for a tag.
MaxTagKeyLength = 128
// MaxTagValueLength represents the maximum value length for a tag.
MaxTagValueLength = 256
)

// Defaults
Expand All @@ -82,6 +90,10 @@ const (
VolumeNameTagKey = "CSIVolumeName"
// SnapshotNameTagKey is the key value that refers to the snapshot's name.
SnapshotNameTagKey = "CSIVolumeSnapshotName"
// KubernetesTagKeyPrefix is the prefix of the key value that is reserved for Kubernetes.
KubernetesTagKeyPrefix = "kubernetes.io"
// AWSTagKeyPrefix is the prefix of the key value that is reserved for AWS.
AWSTagKeyPrefix = "aws:"
)

var (
Expand Down Expand Up @@ -267,7 +279,9 @@ func (c *cloud) CreateDisk(ctx context.Context, volumeName string, diskOptions *

var tags []*ec2.Tag
for key, value := range diskOptions.Tags {
tags = append(tags, &ec2.Tag{Key: &key, Value: &value})
copiedKey := key
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this needed? Could you just inline the key as before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a bug in the old code. You cannot take a reference to the iteration variable. If you have multiple key value pairs (i.e., diskOptions.Tags), they end up pointing to the same object.

copiedValue := value
tags = append(tags, &ec2.Tag{Key: &copiedKey, Value: &copiedValue})
}
tagSpec := ec2.TagSpecification{
ResourceType: aws.String("volume"),
Expand Down
5 changes: 5 additions & 0 deletions pkg/driver/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,8 @@ const (
// KmsKeyId represents key for KMS encryption key
KmsKeyIdKey = "kmskeyid"
)

// constants for default command line flag values
const (
DefaultCSIEndpoint = "unix://tmp/csi.sock"
)
18 changes: 14 additions & 4 deletions pkg/driver/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,19 +52,21 @@ var (

// controllerService represents the controller service of CSI driver
type controllerService struct {
cloud cloud.Cloud
cloud cloud.Cloud
driverOptions *DriverOptions
}

// newControllerService creates a new controller service
// it panics if failed to create the service
func newControllerService() controllerService {
func newControllerService(driverOptions *DriverOptions) controllerService {
cloud, err := cloud.NewCloud()
if err != nil {
panic(err)
}

return controllerService{
cloud: cloud,
cloud: cloud,
driverOptions: driverOptions,
}
}

Expand Down Expand Up @@ -140,9 +142,17 @@ 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,
}
for k, v := range d.driverOptions.extraVolumeTags {
volumeTags[k] = v
}

opts := &cloud.DiskOptions{
CapacityBytes: volSizeBytes,
Tags: map[string]string{cloud.VolumeNameTagKey: volName},
Tags: volumeTags,
VolumeType: volumeType,
IOPSPerGB: iopsPerGB,
AvailabilityZone: zone,
Expand Down
Loading