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.

Sign up for GitHub

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

Jump to bottom

Tags configurations dynamically provisioning #1351

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Tags configurations dynamically provisioning #1351

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4a821f1
Add support for pvctags in dynamic provisioning
omerap12 May 25, 2024
49f2f1a
Merge branch 'master' into tags_configurations_dynamically_provisioning
omerap12 Aug 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions examples/kubernetes/dynamic_provisioning/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ This example requires Kubernetes 1.17 or later and a driver version of 1.2.0 or
any dynamically provisioned path, as in the above example. This can be turned off but this requires you as the
administrator to ensure that your storage classes are set up correctly. Otherwise, it's possible that 2 pods could
end up writing to the same directory by accident. **Please think very carefully before setting this to false!**
* `pvcTags` (Optional) - A comma-separated list of tags to be applied to the dynamically provisioned PersistentVolumeClaim (PVC).

4. Deploy the storage class.

Expand Down
3 changes: 2 additions & 1 deletion examples/kubernetes/dynamic_provisioning/specs/storageclass.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ parameters:
basePath: "/dynamic_provisioning" # optional
subPathPattern: "${.PVC.namespace}/${.PVC.name}" # optional
ensureUniqueDirectory: "true" # optional
reuseAccessPoint: "false" # optional
reuseAccessPoint: "false" # optional
pvcTags: "service_name=my_service,team=infra,environment=development" # optional
22 changes: 22 additions & 0 deletions pkg/driver/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ const (
DefaultGidMax = DefaultGidMin + cloud.AccessPointPerFsLimit
DefaultTagKey = "efs.csi.aws.com/cluster"
DefaultTagValue = "true"
PvcTags = "pvcTags"
DirectoryPerms = "directoryPerms"
EnsureUniqueDirectory = "ensureUniqueDirectory"
FsId = "fileSystemId"
Expand Down Expand Up @@ -144,6 +145,27 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
return nil, status.Errorf(codes.InvalidArgument, "Missing %v parameter", ProvisioningMode)
}

// Create tags
tags := map[string]string{
DefaultTagKey: DefaultTagValue,
}

// Append input tags to default tag
if len(d.tags) != 0 {
for k, v := range d.tags {
tags[k] = v
}
}

// Adding from tags parameter
if value, ok := volumeParams[PvcTags]; ok {
klog.Infof("Adding tags from PVC tags parameter ", value)
tagsFromStr := parseTagsFromStr(value, ",", "=")
for k, v := range tagsFromStr {
tags[k] = v
}
}

accessPointsOptions := &cloud.AccessPointOptions{
CapacityGiB: volSize,
}
Expand Down
98 changes: 83 additions & 15 deletions pkg/driver/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ package driver

import (
"context"
"encoding/json"
"errors"
"fmt"
"reflect"
"regexp"
"strconv"
"testing"
Expand Down Expand Up @@ -542,7 +544,7 @@ func TestCreateVolume(t *testing.T) {
endpoint: endpoint,
cloud: mockCloud,
gidAllocator: NewGidAllocator(),
tags: parseTagsFromStr(""),
tags: parseTagsFromStr("", " ", ":"),
}

req := &csi.CreateVolumeRequest{
Expand Down Expand Up @@ -658,7 +660,7 @@ func TestCreateVolume(t *testing.T) {
endpoint: endpoint,
cloud: mockCloud,
gidAllocator: NewGidAllocator(),
tags: parseTagsFromStr("cluster:efs"),
tags: parseTagsFromStr("cluster:efs", " ", ":"),
}

req := &csi.CreateVolumeRequest{
Expand Down Expand Up @@ -717,7 +719,7 @@ func TestCreateVolume(t *testing.T) {
endpoint: endpoint,
cloud: mockCloud,
gidAllocator: NewGidAllocator(),
tags: parseTagsFromStr("cluster-efs"),
tags: parseTagsFromStr("cluster-efs", " ", ":"),
}

req := &csi.CreateVolumeRequest{
Expand Down Expand Up @@ -766,6 +768,72 @@ func TestCreateVolume(t *testing.T) {
mockCtl.Finish()
},
},
{
name: "Success: Normal flow with pvcTags",
testFunc: func(t *testing.T) {
mockCtl := gomock.NewController(t)
mockCloud := mocks.NewMockCloud(mockCtl)

driver := &Driver{
endpoint: endpoint,
cloud: mockCloud,
}

expectedTags := map[string]string{"service_name": "my_service", "team": "infra", "environment": "development", "efs.csi.aws.com/cluster": "true"}
req := &csi.CreateVolumeRequest{
Name: volumeName,
VolumeCapabilities: []*csi.VolumeCapability{
stdVolCap,
},
CapacityRange: &csi.CapacityRange{
RequiredBytes: capacityRange,
},
Parameters: map[string]string{
ProvisioningMode: "efs-ap",
FsId: fsId,
GidMin: "1000",
GidMax: "2000",
DirectoryPerms: "777",
PvcTags: "service_name=my_service,team=infra,environment=development",
},
}
ctx := context.Background()
accessPoint := &cloud.AccessPoint{
AccessPointId: apId,
FileSystemId: fsId,
}
accessPoints := []*cloud.AccessPoint{accessPoint}
mockCloud.EXPECT().ListAccessPoints(gomock.Eq(ctx), gomock.Any()).Return(accessPoints, nil)
mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any(), gomock.Any()).Return(accessPoint, nil).Do(
func(ctx context.Context, volumeName string, accessPointsOptions *cloud.AccessPointOptions, reuseAccessPointName bool) {
if !reflect.DeepEqual(expectedTags, accessPointsOptions.Tags) {
t.Fatalf("Tags mismatched. Expected: %v, Actual: %v", expectedTags, accessPointsOptions.Tags)
tagsMarshal, err := json.Marshal(accessPointsOptions.Tags)
if err != nil {
t.Fatalf("Failed to marshal access point tags: %v", err)
}
expectedTagsMarshal, err := json.Marshal(expectedTags)
if err != nil {
t.Fatalf("Tags mismatched. Expected: %v, Actual: %v", string(expectedTagsMarshal), string(tagsMarshal))
}
}
},
)
res, err := driver.CreateVolume(ctx, req)
if err != nil {
t.Fatalf("CreateVolume failed: %v", err)
}

if res.Volume == nil {
t.Fatal("Volume is nil")
}

if res.Volume.VolumeId != volumeId {
t.Fatalf("Volume Id mismatched. Expected: %v, Actual: %v", volumeId, res.Volume.VolumeId)
}
mockCtl.Finish()
},
},
{
name: "Success: reuseAccessPointName is true",
testFunc: func(t *testing.T) {
Expand All @@ -776,7 +844,7 @@ func TestCreateVolume(t *testing.T) {
endpoint: endpoint,
cloud: mockCloud,
gidAllocator: NewGidAllocator(),
tags: parseTagsFromStr(""),
tags: parseTagsFromStr("", " ", ":"),
}
pvcNameVal := "test-pvc"

Expand Down Expand Up @@ -839,7 +907,7 @@ func TestCreateVolume(t *testing.T) {
endpoint: endpoint,
cloud: mockCloud,
gidAllocator: NewGidAllocator(),
tags: parseTagsFromStr(""),
tags: parseTagsFromStr("", " ", ":"),
}

pvName := "foo"
Expand Down Expand Up @@ -909,7 +977,7 @@ func TestCreateVolume(t *testing.T) {
endpoint: endpoint,
cloud: mockCloud,
gidAllocator: NewGidAllocator(),
tags: parseTagsFromStr(""),
tags: parseTagsFromStr("", " ", ":"),
}

pvcName := "foo"
Expand Down Expand Up @@ -977,7 +1045,7 @@ func TestCreateVolume(t *testing.T) {
endpoint: endpoint,
cloud: mockCloud,
gidAllocator: NewGidAllocator(),
tags: parseTagsFromStr(""),
tags: parseTagsFromStr("", " ", ":"),
}

pvcName := "foo"
Expand Down Expand Up @@ -1048,7 +1116,7 @@ func TestCreateVolume(t *testing.T) {
endpoint: endpoint,
cloud: mockCloud,
gidAllocator: NewGidAllocator(),
tags: parseTagsFromStr(""),
tags: parseTagsFromStr("", " ", ":"),
}

pvcName := "foo"
Expand Down Expand Up @@ -1120,7 +1188,7 @@ func TestCreateVolume(t *testing.T) {
endpoint: endpoint,
cloud: mockCloud,
gidAllocator: NewGidAllocator(),
tags: parseTagsFromStr(""),
tags: parseTagsFromStr("", " ", ":"),
}

pvcName := "foo"
Expand Down Expand Up @@ -1189,7 +1257,7 @@ func TestCreateVolume(t *testing.T) {
endpoint: endpoint,
cloud: mockCloud,
gidAllocator: NewGidAllocator(),
tags: parseTagsFromStr(""),
tags: parseTagsFromStr("", " ", ":"),
}

req := &csi.CreateVolumeRequest{
Expand Down Expand Up @@ -1254,7 +1322,7 @@ func TestCreateVolume(t *testing.T) {
endpoint: endpoint,
cloud: mockCloud,
gidAllocator: NewGidAllocator(),
tags: parseTagsFromStr(""),
tags: parseTagsFromStr("", " ", ":"),
}

req := &csi.CreateVolumeRequest{
Expand Down Expand Up @@ -1320,7 +1388,7 @@ func TestCreateVolume(t *testing.T) {
endpoint: endpoint,
cloud: mockCloud,
gidAllocator: NewGidAllocator(),
tags: parseTagsFromStr(""),
tags: parseTagsFromStr("", " ", ":"),
}

pvcName := "foo"
Expand Down Expand Up @@ -1519,7 +1587,7 @@ func TestCreateVolume(t *testing.T) {
endpoint: endpoint,
cloud: mockCloud,
gidAllocator: NewGidAllocator(),
tags: parseTagsFromStr(""),
tags: parseTagsFromStr("", " ", ":"),
}

req := &csi.CreateVolumeRequest{
Expand Down Expand Up @@ -2480,7 +2548,7 @@ func TestCreateVolume(t *testing.T) {
endpoint: endpoint,
cloud: mockCloud,
gidAllocator: NewGidAllocator(),
tags: parseTagsFromStr(""),
tags: parseTagsFromStr("", " ", ":"),
}

secrets := map[string]string{}
Expand Down Expand Up @@ -3036,7 +3104,7 @@ func TestDeleteVolume(t *testing.T) {
endpoint: endpoint,
cloud: mockCloud,
gidAllocator: NewGidAllocator(),
tags: parseTagsFromStr(""),
tags: parseTagsFromStr("", " ", ":"),
}

secrets := map[string]string{}
Expand Down
8 changes: 4 additions & 4 deletions pkg/driver/driver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func NewDriver(endpoint, efsUtilsCfgPath, efsUtilsStaticFilesPath, tags string,
volMetricsFsRateLimit: volMetricsFsRateLimit,
gidAllocator: NewGidAllocator(),
deleteAccessPointRootDir: deleteAccessPointRootDir,
tags: parseTagsFromStr(strings.TrimSpace(tags)),
tags: parseTagsFromStr(strings.TrimSpace(tags), " ", ":"),
}
}

Expand Down Expand Up @@ -138,7 +138,7 @@ func (d *Driver) Run() error {
return d.srv.Serve(listener)
}

func parseTagsFromStr(tagStr string) map[string]string {
func parseTagsFromStr(tagStr string, firstDel string, secondDel string) map[string]string {
defer func() {
if r := recover(); r != nil {
klog.Errorf("Failed to parse input tag string: %v", tagStr)
Expand All @@ -150,9 +150,9 @@ func parseTagsFromStr(tagStr string) map[string]string {
klog.Infof("Did not find any input tags.")
return m
}
tagsSplit := strings.Split(tagStr, " ")
tagsSplit := strings.Split(tagStr, firstDel)
for _, pair := range tagsSplit {
p := strings.Split(pair, ":")
p := strings.Split(pair, secondDel)
m[p[0]] = p[1]
}
return m
Expand Down