Repository: docker/compose-cli
Pull request #813 (2020-10-19 15:38:31)

CLI command to manage ECS volumes
**What I did**
implement VolumeService on ECS/EFS and removed ACI-only restriction on `volume` commands 

**Related issue**
https://github.com/docker/compose-cli/issues/742

**(not mandatory) A picture of a cute animal, if possible in relation with what you did**
![image](https://user-images.githubusercontent.com/132757/96473401-e33c2800-1231-11eb-8cec-f22e8c0b0649.png)



Patch:
diff --git a/cli/cmd/volume/acivolume.go b/cli/cmd/volume/command.go
similarity index 63%
rename from cli/cmd/volume/acivolume.go
rename to cli/cmd/volume/command.go
index d1d6e5d8..81199402 100644
--- a/cli/cmd/volume/acivolume.go
+++ b/cli/cmd/volume/command.go
@@ -20,25 +20,27 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/hashicorp/go-multierror"
-	"github.com/spf13/cobra"
-
 	"github.com/docker/compose-cli/aci"
 	"github.com/docker/compose-cli/api/client"
 	"github.com/docker/compose-cli/cli/formatter"
+	"github.com/docker/compose-cli/context/store"
+	"github.com/docker/compose-cli/ecs"
 	formatter2 "github.com/docker/compose-cli/formatter"
 	"github.com/docker/compose-cli/progress"
+
+	"github.com/hashicorp/go-multierror"
+	"github.com/spf13/cobra"
 )
 
-// ACICommand manage volumes
-func ACICommand() *cobra.Command {
+// Command manage volumes
+func Command(ctype string) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "volume",
 		Short: "Manages volumes",
 	}
 
 	cmd.AddCommand(
-		createVolume(),
+		createVolume(ctype),
 		listVolume(),
 		rmVolume(),
 		inspectVolume(),
@@ -46,11 +48,25 @@ func ACICommand() *cobra.Command {
 	return cmd
 }
 
-func createVolume() *cobra.Command {
-	aciOpts := aci.VolumeCreateOptions{}
+func createVolume(ctype string) *cobra.Command {
+	var usage string
+	var short string
+	switch ctype {
+	case store.AciContextType:
+		usage = "create --storage-account ACCOUNT VOLUME"
+		short = "Creates an Azure file share to use as ACI volume."
+	case store.EcsContextType:
+		usage = "create [OPTIONS] VOLUME"
+		short = "Creates an EFS filesystem to use as AWS volume."
+	default:
+		usage = "create [OPTIONS] VOLUME"
+		short = "Creates a volume"
+	}
+
+	var opts interface{}
 	cmd := &cobra.Command{
-		Use:   "create --storage-account ACCOUNT VOLUME",
-		Short: "Creates an Azure file share to use as ACI volume.",
+		Use:   usage,
+		Short: short,
 		Args:  cobra.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			ctx := cmd.Context()
@@ -59,7 +75,7 @@ func createVolume() *cobra.Command {
 				return err
 			}
 			result, err := progress.Run(ctx, func(ctx context.Context) (string, error) {
-				volume, err := c.VolumeService().Create(ctx, args[0], aciOpts)
+				volume, err := c.VolumeService().Create(ctx, args[0], opts)
 				if err != nil {
 					return "", err
 				}
@@ -73,8 +89,20 @@ func createVolume() *cobra.Command {
 		},
 	}
 
-	cmd.Flags().StringVar(&aciOpts.Account, "storage-account", "", "Storage account name")
-	_ = cmd.MarkFlagRequired("storage-account")
+	switch ctype {
+	case store.AciContextType:
+		aciOpts := aci.VolumeCreateOptions{}
+		cmd.Flags().StringVar(&aciOpts.Account, "storage-account", "", "Storage account name")
+		_ = cmd.MarkFlagRequired("storage-account")
+		opts = aciOpts
+	case store.EcsContextType:
+		ecsOpts := ecs.VolumeCreateOptions{}
+		cmd.Flags().StringVar(&ecsOpts.KmsKeyID, "kms-key", "", "ID of the AWS KMS CMK to be used to protect the encrypted file system")
+		cmd.Flags().StringVar(&ecsOpts.PerformanceMode, "performance-mode", "", "performance mode of the file system. (generalPurpose|maxIO)")
+		cmd.Flags().Float64Var(&ecsOpts.ProvisionedThroughputInMibps, "provisioned-throughput", 0, "throughput in MiB/s (1-1024)")
+		cmd.Flags().StringVar(&ecsOpts.ThroughputMode, "throughput-mode", "", "throughput mode (bursting|provisioned)")
+		opts = ecsOpts
+	}
 	return cmd
 }
 
diff --git a/cli/main.go b/cli/main.go
index 7b70d887..6a717ed5 100644
--- a/cli/main.go
+++ b/cli/main.go
@@ -182,13 +182,9 @@ func main() {
 	root.AddCommand(
 		run.Command(ctype),
 		compose.Command(ctype),
+		volume.Command(ctype),
 	)
 
-	if ctype == store.AciContextType {
-		// we can also pass ctype as a parameter to the volume command and customize subcommands, flags, etc. when we have other backend implementations
-		root.AddCommand(volume.ACICommand())
-	}
-
 	ctx = apicontext.WithCurrentContext(ctx, currentContext)
 	ctx = store.WithContextStore(ctx, s)
 
diff --git a/ecs/aws.go b/ecs/aws.go
index 915c1902..b030c7aa 100644
--- a/ecs/aws.go
+++ b/ecs/aws.go
@@ -73,7 +73,7 @@ type API interface {
 	DeleteCapacityProvider(ctx context.Context, arn string) error
 	DeleteAutoscalingGroup(ctx context.Context, arn string) error
 	ResolveFileSystem(ctx context.Context, id string) (awsResource, error)
-	FindFileSystem(ctx context.Context, tags map[string]string) (awsResource, error)
-	CreateFileSystem(ctx context.Context, tags map[string]string) (string, error)
+	ListFileSystems(ctx context.Context, tags map[string]string) ([]awsResource, error)
+	CreateFileSystem(ctx context.Context, tags map[string]string, options VolumeCreateOptions) (awsResource, error)
 	DeleteFileSystem(ctx context.Context, id string) error
 }
diff --git a/ecs/awsResources.go b/ecs/awsResources.go
index 27c1374d..16924608 100644
--- a/ecs/awsResources.go
+++ b/ecs/awsResources.go
@@ -253,12 +253,16 @@ func (b *ecsAPIService) parseExternalVolumes(ctx context.Context, project *types
 			compose.ProjectTag: project.Name,
 			compose.VolumeTag:  name,
 		}
-		fileSystem, err := b.aws.FindFileSystem(ctx, tags)
+		previous, err := b.aws.ListFileSystems(ctx, tags)
 		if err != nil {
 			return nil, err
 		}
-		if fileSystem != nil {
-			filesystems[name] = fileSystem
+
+		if len(previous) > 1 {
+			return nil, fmt.Errorf("multiple filesystems are tags as project=%q, volume=%q", project.Name, name)
+		}
+		if len(previous) == 1 {
+			filesystems[name] = previous[0]
 		}
 	}
 	return filesystems, nil
diff --git a/ecs/aws_mock.go b/ecs/aws_mock.go
index 5c669cb5..bfdaed14 100644
--- a/ecs/aws_mock.go
+++ b/ecs/aws_mock.go
@@ -6,13 +6,12 @@ package ecs
 
 import (
 	context "context"
-	reflect "reflect"
-
 	cloudformation "github.com/aws/aws-sdk-go/service/cloudformation"
 	ecs "github.com/aws/aws-sdk-go/service/ecs"
 	compose "github.com/docker/compose-cli/api/compose"
 	secrets "github.com/docker/compose-cli/api/secrets"
 	gomock "github.com/golang/mock/gomock"
+	reflect "reflect"
 )
 
 // MockAPI is a mock of API interface
@@ -97,18 +96,18 @@ func (mr *MockAPIMockRecorder) CreateCluster(arg0, arg1 interface{}) *gomock.Cal
 }
 
 // CreateFileSystem mocks base method
-func (m *MockAPI) CreateFileSystem(arg0 context.Context, arg1 map[string]string) (string, error) {
+func (m *MockAPI) CreateFileSystem(arg0 context.Context, arg1 map[string]string, arg2 VolumeCreateOptions) (awsResource, error) {
 	m.ctrl.T.Helper()
-	ret := m.ctrl.Call(m, "CreateFileSystem", arg0, arg1)
-	ret0, _ := ret[0].(string)
+	ret := m.ctrl.Call(m, "CreateFileSystem", arg0, arg1, arg2)
+	ret0, _ := ret[0].(awsResource)
 	ret1, _ := ret[1].(error)
 	return ret0, ret1
 }
 
 // CreateFileSystem indicates an expected call of CreateFileSystem
-func (mr *MockAPIMockRecorder) CreateFileSystem(arg0, arg1 interface{}) *gomock.Call {
+func (mr *MockAPIMockRecorder) CreateFileSystem(arg0, arg1, arg2 interface{}) *gomock.Call {
 	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateFileSystem", reflect.TypeOf((*MockAPI)(nil).CreateFileSystem), arg0, arg1)
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateFileSystem", reflect.TypeOf((*MockAPI)(nil).CreateFileSystem), arg0, arg1, arg2)
 }
 
 // CreateSecret mocks base method
@@ -240,21 +239,6 @@ func (mr *MockAPIMockRecorder) DescribeStackEvents(arg0, arg1 interface{}) *gomo
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DescribeStackEvents", reflect.TypeOf((*MockAPI)(nil).DescribeStackEvents), arg0, arg1)
 }
 
-// FindFileSystem mocks base method
-func (m *MockAPI) FindFileSystem(arg0 context.Context, arg1 map[string]string) (awsResource, error) {
-	m.ctrl.T.Helper()
-	ret := m.ctrl.Call(m, "FindFileSystem", arg0, arg1)
-	ret0, _ := ret[0].(awsResource)
-	ret1, _ := ret[1].(error)
-	return ret0, ret1
-}
-
-// FindFileSystem indicates an expected call of FindFileSystem
-func (mr *MockAPIMockRecorder) FindFileSystem(arg0, arg1 interface{}) *gomock.Call {
-	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FindFileSystem", reflect.TypeOf((*MockAPI)(nil).FindFileSystem), arg0, arg1)
-}
-
 // GetDefaultVPC mocks base method
 func (m *MockAPI) GetDefaultVPC(arg0 context.Context) (string, error) {
 	m.ctrl.T.Helper()
@@ -454,6 +438,21 @@ func (mr *MockAPIMockRecorder) InspectSecret(arg0, arg1 interface{}) *gomock.Cal
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "InspectSecret", reflect.TypeOf((*MockAPI)(nil).InspectSecret), arg0, arg1)
 }
 
+// ListFileSystems mocks base method
+func (m *MockAPI) ListFileSystems(arg0 context.Context, arg1 map[string]string) ([]awsResource, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "ListFileSystems", arg0, arg1)
+	ret0, _ := ret[0].([]awsResource)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
+}
+
+// ListFileSystems indicates an expected call of ListFileSystems
+func (mr *MockAPIMockRecorder) ListFileSystems(arg0, arg1 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListFileSystems", reflect.TypeOf((*MockAPI)(nil).ListFileSystems), arg0, arg1)
+}
+
 // ListSecrets mocks base method
 func (m *MockAPI) ListSecrets(arg0 context.Context) ([]secrets.Secret, error) {
 	m.ctrl.T.Helper()
diff --git a/ecs/backend.go b/ecs/backend.go
index 70155729..5d18bbc8 100644
--- a/ecs/backend.go
+++ b/ecs/backend.go
@@ -98,7 +98,7 @@ func (b *ecsAPIService) SecretsService() secrets.Service {
 }
 
 func (b *ecsAPIService) VolumeService() volumes.Service {
-	return nil
+	return ecsVolumeService{backend: b}
 }
 
 func (b *ecsAPIService) ResourceService() resources.Service {
diff --git a/ecs/sdk.go b/ecs/sdk.go
index a0e67ed7..f8cc7093 100644
--- a/ecs/sdk.go
+++ b/ecs/sdk.go
@@ -904,7 +904,8 @@ func (s sdk) ResolveFileSystem(ctx context.Context, id string) (awsResource, err
 	}, nil
 }
 
-func (s sdk) FindFileSystem(ctx context.Context, tags map[string]string) (awsResource, error) {
+func (s sdk) ListFileSystems(ctx context.Context, tags map[string]string) ([]awsResource, error) {
+	var results []awsResource
 	var token *string
 	for {
 		desc, err := s.EFS.DescribeFileSystemsWithContext(ctx, &efs.DescribeFileSystemsInput{
@@ -915,14 +916,14 @@ func (s sdk) FindFileSystem(ctx context.Context, tags map[string]string) (awsRes
 		}
 		for _, filesystem := range desc.FileSystems {
 			if containsAll(filesystem.Tags, tags) {
-				return existingAWSResource{
+				results = append(results, existingAWSResource{
 					arn: aws.StringValue(filesystem.FileSystemArn),
 					id:  aws.StringValue(filesystem.FileSystemId),
-				}, nil
+				})
 			}
 		}
 		if desc.NextMarker == token {
-			return nil, nil
+			return results, nil
 		}
 		token = desc.NextMarker
 	}
@@ -941,7 +942,7 @@ TAGS:
 	return true
 }
 
-func (s sdk) CreateFileSystem(ctx context.Context, tags map[string]string) (string, error) {
+func (s sdk) CreateFileSystem(ctx context.Context, tags map[string]string, options VolumeCreateOptions) (awsResource, error) {
 	var efsTags []*efs.Tag
 	for k, v := range tags {
 		efsTags = append(efsTags, &efs.Tag{
@@ -949,16 +950,39 @@ func (s sdk) CreateFileSystem(ctx context.Context, tags map[string]string) (stri
 			Value: aws.String(v),
 		})
 	}
+	var (
+		k *string
+		p *string
+		f *float64
+		t *string
+	)
+	if options.ProvisionedThroughputInMibps > 1 {
+		f = aws.Float64(options.ProvisionedThroughputInMibps)
+	}
+	if options.KmsKeyID != "" {
+		k = aws.String(options.KmsKeyID)
+	}
+	if options.PerformanceMode != "" {
+		p = aws.String(options.PerformanceMode)
+	}
+	if options.ThroughputMode != "" {
+		t = aws.String(options.ThroughputMode)
+	}
 	res, err := s.EFS.CreateFileSystemWithContext(ctx, &efs.CreateFileSystemInput{
-		Encrypted: aws.Bool(true),
-		Tags:      efsTags,
+		Encrypted:                    aws.Bool(true),
+		KmsKeyId:                     k,
+		PerformanceMode:              p,
+		ProvisionedThroughputInMibps: f,
+		ThroughputMode:               t,
+		Tags:                         efsTags,
 	})
 	if err != nil {
-		return "", err
+		return nil, err
 	}
-	id := aws.StringValue(res.FileSystemId)
-	logrus.Debugf("Created file system %q", id)
-	return id, nil
+	return existingAWSResource{
+		id:  aws.StringValue(res.FileSystemId),
+		arn: aws.StringValue(res.FileSystemArn),
+	}, nil
 }
 
 func (s sdk) DeleteFileSystem(ctx context.Context, id string) error {
diff --git a/ecs/volumes.go b/ecs/volumes.go
index 939ea9f7..49295e45 100644
--- a/ecs/volumes.go
+++ b/ecs/volumes.go
@@ -17,13 +17,17 @@
 package ecs
 
 import (
+	"context"
 	"fmt"
 
 	"github.com/docker/compose-cli/api/compose"
+	"github.com/docker/compose-cli/api/volumes"
+	"github.com/docker/compose-cli/errdefs"
 
 	"github.com/awslabs/goformation/v4/cloudformation"
 	"github.com/awslabs/goformation/v4/cloudformation/efs"
 	"github.com/compose-spec/compose-go/types"
+	"github.com/pkg/errors"
 )
 
 func (b *ecsAPIService) createNFSMountTarget(project *types.Project, resources awsResources, template *cloudformation.Template) {
@@ -97,3 +101,56 @@ func (b *ecsAPIService) createAccessPoints(project *types.Project, r awsResource
 		template.Resources[n] = &ap
 	}
 }
+
+// VolumeCreateOptions hold EFS filesystem creation options
+type VolumeCreateOptions struct {
+	KmsKeyID                     string
+	PerformanceMode              string
+	ProvisionedThroughputInMibps float64
+	ThroughputMode               string
+}
+
+type ecsVolumeService struct {
+	backend *ecsAPIService
+}
+
+func (e ecsVolumeService) List(ctx context.Context) ([]volumes.Volume, error) {
+	filesystems, err := e.backend.aws.ListFileSystems(ctx, nil)
+	if err != nil {
+		return nil, err
+	}
+	var vol []volumes.Volume
+	for _, fs := range filesystems {
+		vol = append(vol, volumes.Volume{
+			ID:          fs.ID(),
+			Description: fs.ARN(),
+		})
+	}
+	return vol, nil
+}
+
+func (e ecsVolumeService) Create(ctx context.Context, name string, options interface{}) (volumes.Volume, error) {
+	fs, err := e.backend.aws.CreateFileSystem(ctx, map[string]string{
+		"Name": name,
+	}, options.(VolumeCreateOptions))
+	return volumes.Volume{
+		ID:          fs.ID(),
+		Description: fs.ARN(),
+	}, err
+
+}
+
+func (e ecsVolumeService) Delete(ctx context.Context, volumeID string, options interface{}) error {
+	return e.backend.aws.DeleteFileSystem(ctx, volumeID)
+}
+
+func (e ecsVolumeService) Inspect(ctx context.Context, volumeID string) (volumes.Volume, error) {
+	ok, err := e.backend.aws.ResolveFileSystem(ctx, volumeID)
+	if ok == nil {
+		err = errors.Wrapf(errdefs.ErrNotFound, "filesystem %q does not exists", volumeID)
+	}
+	return volumes.Volume{
+		ID:          volumeID,
+		Description: ok.ARN(),
+	}, err
+}


Test patch:
diff --git a/ecs/cloudformation_test.go b/ecs/cloudformation_test.go
index b4584a04..920f7fcf 100644
--- a/ecs/cloudformation_test.go
+++ b/ecs/cloudformation_test.go
@@ -390,7 +390,7 @@ volumes:
         throughput_mode: provisioned
         provisioned_throughput: 1024
 `, useDefaultVPC, func(m *MockAPIMockRecorder) {
-		m.FindFileSystem(gomock.Any(), map[string]string{
+		m.ListFileSystems(gomock.Any(), map[string]string{
 			compose.ProjectTag: t.Name(),
 			compose.VolumeTag:  "db-data",
 		}).Return(nil, nil)
@@ -420,7 +420,7 @@ volumes:
       uid: 1002
       gid: 1002
 `, useDefaultVPC, func(m *MockAPIMockRecorder) {
-		m.FindFileSystem(gomock.Any(), gomock.Any()).Return(nil, nil)
+		m.ListFileSystems(gomock.Any(), gomock.Any()).Return(nil, nil)
 	})
 	a := template.Resources["DbdataAccessPoint"].(*efs.AccessPoint)
 	assert.Check(t, a != nil)
@@ -436,10 +436,14 @@ services:
 volumes:
   db-data: {}
 `, useDefaultVPC, func(m *MockAPIMockRecorder) {
-		m.FindFileSystem(gomock.Any(), map[string]string{
+		m.ListFileSystems(gomock.Any(), map[string]string{
 			compose.ProjectTag: t.Name(),
 			compose.VolumeTag:  "db-data",
-		}).Return(existingAWSResource{id: "fs-123abc"}, nil)
+		}).Return([]awsResource{
+			existingAWSResource{
+				id: "fs-123abc",
+			},
+		}, nil)
 	})
 	s := template.Resources["DbdataNFSMountTargetOnSubnet1"].(*efs.MountTarget)
 	assert.Check(t, s != nil)