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

Discover EFS FileSystemId instead of specifying it in storage class #1492

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
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
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ require (
github.com/onsi/gomega v1.27.6
golang.org/x/exp v0.0.0-20230817173708-d852ddb80c63
google.golang.org/grpc v1.59.0
sigs.k8s.io/yaml v1.3.0
k8s.io/api v0.28.15
k8s.io/apimachinery v0.28.15
k8s.io/client-go v0.28.15
Expand Down Expand Up @@ -135,7 +136,6 @@ require (
sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.1.2 // indirect
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect
sigs.k8s.io/yaml v1.3.0 // indirect
)

replace (
Expand Down
3 changes: 3 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ3
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/elazarl/goproxy v0.0.0-20180725130230-947c36da3153 h1:yUdfgN0XgIJw7foRItutHYUIhlcKzcSf5vDpdhQAKTc=
github.com/elazarl/goproxy v0.0.0-20180725130230-947c36da3153/go.mod h1:/Zj4wYkgs4iZTTu3o/KG3Itv/qCCa8VVMlb3i9OVuzc=
github.com/docker/distribution v2.8.2+incompatible h1:T3de5rq0dB1j30rp0sA2rER+m322EBzniBPB6ZIzuh8=
github.com/docker/distribution v2.8.2+incompatible/go.mod h1:J2gT2udsDAN96Uj4KfcMRqY0/ypR+oyYUYmja8H+y+w=
github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY=
Expand Down Expand Up @@ -262,6 +264,7 @@ github.com/prometheus/common v0.44.0/go.mod h1:ofAIvZbQ1e/nugmZGz4/qCb9Ap1VoSTIO
github.com/prometheus/procfs v0.10.1 h1:kYK1Va/YMlutzCGazswoHKo//tZVlFpKYh+PymziUAg=
github.com/prometheus/procfs v0.10.1/go.mod h1:nwNm2aOCAYw8uTR/9bWRREkZFxAUcWzPHWJq+XBB/FM=
github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6LYCDYWNEvQ=
github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=
github.com/rogpeppe/go-internal v1.10.0 h1:TMyTOH3F/DB16zRVcYyreMH6GnZZrwQVAoYjRBZyWFQ=
github.com/rogpeppe/go-internal v1.10.0/go.mod h1:UQnix2H7Ngw/k4C5ijL5+65zddjncjaFoBhdsK/akog=
github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
Expand Down
43 changes: 40 additions & 3 deletions pkg/cloud/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,9 @@ var (
)

type FileSystem struct {
FileSystemId string
FileSystemId string
FileSystemArn string
Tags map[string]string
}

type AccessPoint struct {
Expand Down Expand Up @@ -105,7 +107,8 @@ type Cloud interface {
DescribeAccessPoint(ctx context.Context, accessPointId string) (accessPoint *AccessPoint, err error)
FindAccessPointByClientToken(ctx context.Context, clientToken, fileSystemId string) (accessPoint *AccessPoint, err error)
ListAccessPoints(ctx context.Context, fileSystemId string) (accessPoints []*AccessPoint, err error)
DescribeFileSystem(ctx context.Context, fileSystemId string) (fs *FileSystem, err error)
DescribeFileSystemById(ctx context.Context, fileSystemId string) (fs *FileSystem, err error)
DescribeFileSystemByToken(ctx context.Context, creationToken string) (fs []*FileSystem, err error)
DescribeMountTargets(ctx context.Context, fileSystemId, az string) (fs *MountTarget, err error)
}

Expand Down Expand Up @@ -336,7 +339,41 @@ func (c *cloud) ListAccessPoints(ctx context.Context, fileSystemId string) (acce
return
}

func (c *cloud) DescribeFileSystem(ctx context.Context, fileSystemId string) (fs *FileSystem, err error) {
func (c *cloud) DescribeFileSystemByToken(ctx context.Context, creationToken string) (fs []*FileSystem, err error) {
var describeFsInput *efs.DescribeFileSystemsInput
if creationToken == "" {
describeFsInput = &efs.DescribeFileSystemsInput{}
} else {
describeFsInput = &efs.DescribeFileSystemsInput{CreationToken: &creationToken}
}
klog.V(5).Infof("Calling DescribeFileSystems with input: %+v", *describeFsInput)
res, err := c.efs.DescribeFileSystems(ctx, describeFsInput)
if err != nil {
if isAccessDenied(err) {
return nil, ErrAccessDenied
}
if isFileSystemNotFound(err) {
return nil, ErrNotFound
}
return nil, fmt.Errorf("Describe File System failed: %v", err)
}

var efsList = make([]*FileSystem, 0)
for _, fileSystem := range res.FileSystems {
var tagsList = make(map[string]string, 0)
for _, tag := range fileSystem.Tags {
tagsList[*tag.Key] = *tag.Value
}
efsList = append(efsList, &FileSystem{
FileSystemId: *fileSystem.FileSystemId,
FileSystemArn: *fileSystem.FileSystemArn,
Tags: tagsList,
})
}
return efsList, nil
}

func (c *cloud) DescribeFileSystemById(ctx context.Context, fileSystemId string) (fs *FileSystem, err error) {
describeFsInput := &efs.DescribeFileSystemsInput{FileSystemId: &fileSystemId}
klog.V(5).Infof("Calling DescribeFileSystems with input: %+v", *describeFsInput)
res, err := c.efs.DescribeFileSystems(ctx, describeFsInput, func(o *efs.Options) {
Expand Down
212 changes: 210 additions & 2 deletions pkg/cloud/cloud_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -867,7 +867,7 @@ func TestDescribeFileSystem(t *testing.T) {
&types.FileSystemNotFound{
Message: aws.String("File System not found"),
})
_, err := c.DescribeFileSystem(ctx, fsId)
_, err := c.DescribeFileSystemById(ctx, fsId)
if err == nil {
t.Fatalf("DescribeFileSystem did not fail")
}
Expand All @@ -890,7 +890,7 @@ func TestDescribeFileSystem(t *testing.T) {
Code: AccessDeniedException,
Message: "Access Denied",
})
_, err := c.DescribeFileSystem(ctx, fsId)
_, err := c.DescribeFileSystemById(ctx, fsId)
if err == nil {
t.Fatalf("DescribeFileSystem did not fail")
}
Expand Down Expand Up @@ -1050,6 +1050,214 @@ func TestDescribeMountTargets(t *testing.T) {
}
}

func TestDescribeFileSystemByToken(t *testing.T) {
var (
fsId = []string{"fs-abcd1234", "fs-efgh5678"}
fsArn = []string{"arn:aws:elasticfilesystem:us-west-2:1111333322228888:file-system/fs-0123456789abcdef8", "arn:aws:elasticfilesystem:us-west-2:1111333322228888:file-system/fs-987654321abcdef0"}
creationToken = "efs-for-discovery"
az = "us-east-1a"
)

testCases := []struct {
name string
testFunc func(t *testing.T)
}{
{
name: "Success: Normal flow",
testFunc: func(t *testing.T) {
mockctl := gomock.NewController(t)
mockEfs := mocks.NewMockEfs(mockctl)
c := &cloud{efs: mockEfs}

fs := &efs.DescribeFileSystemsOutput{
FileSystems: []types.FileSystemDescription{
{
FileSystemId: aws.String(fsId[0]),
FileSystemArn: aws.String(fsArn[0]),
Encrypted: aws.Bool(true),
CreationToken: aws.String("efs-for-discovery"),
AvailabilityZoneName: aws.String(az),
Tags: []types.Tag{
{
Key: aws.String("env"),
Value: aws.String("prod"),
},
{
Key: aws.String("owner"),
Value: aws.String("[email protected]"),
},
},
},
{
FileSystemId: aws.String(fsId[1]),
FileSystemArn: aws.String(fsArn[1]),
Encrypted: aws.Bool(true),
CreationToken: aws.String("efs-not-for-discovery"),
AvailabilityZoneName: aws.String(az),
Tags: []types.Tag{
{
Key: aws.String("env"),
Value: aws.String("prod"),
},
{
Key: aws.String("owner"),
Value: aws.String("[email protected]"),
},
},
},
},
}

ctx := context.Background()
mockEfs.EXPECT().DescribeFileSystems(gomock.Eq(ctx), gomock.Any()).DoAndReturn(func(ctx context.Context, input *efs.DescribeFileSystemsInput, opts ...func(*efs.Options)) (*efs.DescribeFileSystemsOutput, error) {
res := &efs.DescribeFileSystemsOutput{}
for _, fileSystem := range fs.FileSystems {
if input.CreationToken != nil && *fileSystem.CreationToken == *input.CreationToken {
res.FileSystems = append(res.FileSystems, fileSystem)
} else if input.CreationToken == nil {
res.FileSystems = append(res.FileSystems, fileSystem)
}
}
return res, nil
})

efsList, err := c.DescribeFileSystemByToken(ctx, creationToken)
if err != nil {
t.Fatalf("DescribeFileSystem failed")
}
if len(efsList) != 1 {
t.Fatalf("Expected 1 fileSystems got %d", len(efsList))
}
mockctl.Finish()
},
},
{
name: "Success: Normal flow without creation token",
testFunc: func(t *testing.T) {
mockctl := gomock.NewController(t)
mockEfs := mocks.NewMockEfs(mockctl)
c := &cloud{efs: mockEfs}

fs := &efs.DescribeFileSystemsOutput{
FileSystems: []types.FileSystemDescription{
{
FileSystemId: aws.String(fsId[0]),
FileSystemArn: aws.String(fsArn[0]),
Encrypted: aws.Bool(true),
CreationToken: aws.String("efs-for-discovery"),
AvailabilityZoneName: aws.String(az),
Tags: []types.Tag{
{
Key: aws.String("env"),
Value: aws.String("prod"),
},
{
Key: aws.String("owner"),
Value: aws.String("[email protected]"),
},
},
},
{
FileSystemId: aws.String(fsId[1]),
FileSystemArn: aws.String(fsArn[1]),
Encrypted: aws.Bool(true),
CreationToken: aws.String("efs-not-for-discovery"),
AvailabilityZoneName: aws.String(az),
Tags: []types.Tag{
{
Key: aws.String("env"),
Value: aws.String("prod"),
},
{
Key: aws.String("owner"),
Value: aws.String("[email protected]"),
},
},
},
},
}

ctx := context.Background()
mockEfs.EXPECT().DescribeFileSystems(gomock.Eq(ctx), gomock.Any()).DoAndReturn(func(ctx context.Context, input *efs.DescribeFileSystemsInput, opts ...func(*efs.Options)) (*efs.DescribeFileSystemsOutput, error) {
res := &efs.DescribeFileSystemsOutput{}
for _, fileSystem := range fs.FileSystems {
if input.CreationToken != nil && *fileSystem.CreationToken == *input.CreationToken {
res.FileSystems = append(res.FileSystems, fileSystem)
} else if input.CreationToken == nil {
res.FileSystems = append(res.FileSystems, fileSystem)
}
}
return res, nil
})

efsList, err := c.DescribeFileSystemByToken(ctx, "")
if err != nil {
t.Fatalf("DescribeFileSystem failed")
}
if len(efsList) != len(fs.FileSystems) {
t.Fatalf("Expected 1 fileSystems got %d", len(efsList))
}
for i, fileSystem := range fs.FileSystems {
for _, v := range fileSystem.Tags {
if val, exists := efsList[i].Tags[*v.Key]; !exists || val != *v.Value {
t.Fatalf("Tags list is corrupted, expected %s for %s but got %s", *v.Value, *v.Key, val)
}
}
}
mockctl.Finish()
},
},
{
name: "Fail: Access Denied",
testFunc: func(t *testing.T) {
mockctl := gomock.NewController(t)
mockEfs := mocks.NewMockEfs(mockctl)
c := &cloud{efs: mockEfs}

ctx := context.Background()
mockEfs.EXPECT().DescribeFileSystems(gomock.Eq(ctx), gomock.Any()).Return(nil, &smithy.GenericAPIError{
Code: AccessDeniedException,
Message: "Access Denied",
})

_, err := c.DescribeFileSystemByToken(ctx, "efs-discovery")
if err == nil {
t.Fatalf("DescribeFileSystemByToken did not fail")
}
if err != ErrAccessDenied {
t.Fatalf("Failed. Expected: %v, Actual:%v", ErrAccessDenied, err)
}
mockctl.Finish()
},
},
{
name: "Fail: File System not found",
testFunc: func(t *testing.T) {
mockctl := gomock.NewController(t)
mockEfs := mocks.NewMockEfs(mockctl)
c := &cloud{efs: mockEfs}

ctx := context.Background()
mockEfs.EXPECT().DescribeFileSystems(gomock.Eq(ctx), gomock.Any()).Return(nil, &types.FileSystemNotFound{
Message: aws.String("File System not found"),
})

_, err := c.DescribeFileSystemByToken(ctx, "efs-discovery")
if err == nil {
t.Fatalf("DescribeFileSystemByToken did not fail")
}
if err != ErrNotFound {
t.Fatalf("Failed. Expected: %v, Actual:%v", ErrNotFound, err)
}
mockctl.Finish()
},
},
}
for _, tc := range testCases {
t.Run(tc.name, tc.testFunc)
}
}

func testResult(t *testing.T, funcName string, ret interface{}, err error, expectError errtyp) {
if expectError.message == "" {
if err != nil {
Expand Down
35 changes: 34 additions & 1 deletion pkg/cloud/fakes.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ type FakeCloudProvider struct {
fileSystems map[string]*FileSystem
accessPoints map[string]*AccessPoint
mountTargets map[string]*MountTarget
tags map[string]map[string]string
}

func NewFakeCloudProvider() *FakeCloudProvider {
Expand All @@ -20,6 +21,7 @@ func NewFakeCloudProvider() *FakeCloudProvider {
fileSystems: make(map[string]*FileSystem),
accessPoints: make(map[string]*AccessPoint),
mountTargets: make(map[string]*MountTarget),
tags: make(map[string]map[string]string),
}
}

Expand Down Expand Up @@ -69,7 +71,7 @@ func (c *FakeCloudProvider) DescribeAccessPoint(ctx context.Context, accessPoint

// CreateVolume calls DescribeFileSystem and then CreateAccessPoint.
// Add file system into the map here to allow CreateVolume sanity tests to succeed.
func (c *FakeCloudProvider) DescribeFileSystem(ctx context.Context, fileSystemId string) (fileSystem *FileSystem, err error) {
func (c *FakeCloudProvider) DescribeFileSystemById(ctx context.Context, fileSystemId string) (fileSystem *FileSystem, err error) {
if fs, ok := c.fileSystems[fileSystemId]; ok {
return fs, nil
}
Expand All @@ -90,6 +92,37 @@ func (c *FakeCloudProvider) DescribeFileSystem(ctx context.Context, fileSystemId
return fs, nil
}

func (c *FakeCloudProvider) DescribeFileSystemByToken(ctx context.Context, creationToken string) (fileSystem []*FileSystem, err error) {
var efsList = make([]*FileSystem, 0)
if fs, ok := c.fileSystems[creationToken]; ok {
efsList = append(efsList, fs)
return efsList, nil
}

tags := map[string]string{
"env": "prod",
"owner": "[email protected]",
}

fs := &FileSystem{
FileSystemId: creationToken,
FileSystemArn: "arn:aws:elasticfilesystem:us-west-2:xxxx:file-system/fs-xxxx",
Tags: tags,
}
c.fileSystems[creationToken] = fs

mt := &MountTarget{
AZName: "us-east-1a",
AZId: "mock-AZ-id",
MountTargetId: "fsmt-abcd1234",
IPAddress: "127.0.0.1",
}

c.mountTargets[creationToken] = mt
efsList = append(efsList, c.fileSystems[creationToken])
return efsList, nil
}

func (c *FakeCloudProvider) DescribeMountTargets(ctx context.Context, fileSystemId, az string) (mountTarget *MountTarget, err error) {
if mt, ok := c.mountTargets[fileSystemId]; ok {
return mt, nil
Expand Down
Loading