From 45179a5bff31ad615652dd1b431a63c34cfd6861 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Mon, 7 Sep 2020 14:13:24 +0200 Subject: [PATCH 01/13] Add first volume create in ACI Signed-off-by: Guillaume Tardif --- aci/login/client.go | 14 +++++++++ aci/login/storage_helper.go | 60 +++++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+) diff --git a/aci/login/client.go b/aci/login/client.go index b9dc7c89..95e8160c 100644 --- a/aci/login/client.go +++ b/aci/login/client.go @@ -67,6 +67,20 @@ func NewStorageAccountsClient(subscriptionID string) (storage.AccountsClient, er return containerGroupsClient, nil } +// NewStorageAccountsClient get client to manipulate storage accounts +func NewFileShareClient(subscriptionID string) (storage.FileSharesClient, error) { + containerGroupsClient := storage.NewFileSharesClient(subscriptionID) + err := setupClient(&containerGroupsClient.Client) + if err != nil { + return storage.FileSharesClient{}, err + } + containerGroupsClient.PollingDelay = 5 * time.Second + containerGroupsClient.RetryAttempts = 30 + containerGroupsClient.RetryDuration = 1 * time.Second + return containerGroupsClient, nil +} + + // NewSubscriptionsClient get subscription client func NewSubscriptionsClient() (subscription.SubscriptionsClient, error) { subc := subscription.NewSubscriptionsClient() diff --git a/aci/login/storage_helper.go b/aci/login/storage_helper.go index 0da3bd2a..8e16b1f4 100644 --- a/aci/login/storage_helper.go +++ b/aci/login/storage_helper.go @@ -19,6 +19,7 @@ package login import ( "context" "fmt" + "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2019-06-01/storage" "github.com/pkg/errors" @@ -48,3 +49,62 @@ func (helper StorageAccountHelper) GetAzureStorageAccountKey(ctx context.Context key := (*result.Keys)[0] return *key.Value, nil } + +func (helper StorageAccountHelper) ListFileShare(ctx context.Context) ([]string, error) { + aciContext := helper.AciContext + accountClient, err := NewStorageAccountsClient(aciContext.SubscriptionID) + if err != nil { + return nil, err + } + result, err := accountClient.ListByResourceGroup(ctx, aciContext.ResourceGroup) + accounts := result.Value + fileShareClient, err := NewFileShareClient(aciContext.SubscriptionID) + fileShares := []string{} + for _, account := range *accounts { + fileSharePage, err := fileShareClient.List(ctx, aciContext.ResourceGroup, *account.Name, "", "", "") + if err != nil { + return nil, err + } + for ; fileSharePage.NotDone() ; fileSharePage.NextWithContext(ctx) { + values := fileSharePage.Values() + for _, fileShare := range values { + fileShares = append(fileShares, *fileShare.Name) + } + } + } + return fileShares, nil +} + +func (helper StorageAccountHelper) CreateFileShare(ctx context.Context, accountName string, fileShareName string) (storage.FileShare, error) { + aciContext := helper.AciContext + accountClient, err := NewStorageAccountsClient(aciContext.SubscriptionID) + if err != nil { + return storage.FileShare{}, err + } + account, err := accountClient.GetProperties(ctx, aciContext.ResourceGroup, accountName, "") + if err != nil { + //TODO check err not found + parameters := storage.AccountCreateParameters{ + Location: &aciContext.Location, + Sku:&storage.Sku{ + Name: storage.StandardLRS, + Tier: storage.Standard, + }, + } + // TODO progress account creation + future, err := accountClient.Create(ctx, aciContext.ResourceGroup, accountName, parameters) + if err != nil { + return storage.FileShare{}, err + } + account, err = future.Result(accountClient) + } + fileShareClient, err := NewFileShareClient(aciContext.SubscriptionID) + fileShare, err := fileShareClient.Get(ctx, aciContext.ResourceGroup, *account.Name, fileShareName, "") + if err != nil { + // TODO check err not found + fileShare, err = fileShareClient.Create(ctx, aciContext.ResourceGroup, *account.Name, fileShareName, storage.FileShare{}) + } + + return fileShare, nil +} + From 9ed06ece5b4b82a10c3f68577a5afa4269a92117 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Mon, 7 Sep 2020 14:39:19 +0200 Subject: [PATCH 02/13] Adding volume API & initial CLI command Signed-off-by: Guillaume Tardif --- aci/backend.go | 18 +++++++++++ api/client/client.go | 9 ++++++ api/client/volume.go | 36 ++++++++++++++++++++++ api/volumes/api.go | 37 ++++++++++++++++++++++ backend/backend.go | 4 ++- cli/cmd/volume/create.go | 66 ++++++++++++++++++++++++++++++++++++++++ cli/main.go | 2 ++ ecs/backend.go | 5 +++ ecs/local/backend.go | 5 +++ example/backend.go | 5 +++ local/backend.go | 5 +++ 11 files changed, 191 insertions(+), 1 deletion(-) create mode 100644 api/client/volume.go create mode 100644 api/volumes/api.go create mode 100644 cli/cmd/volume/create.go diff --git a/aci/backend.go b/aci/backend.go index ceabd7cf..5e374d49 100644 --- a/aci/backend.go +++ b/aci/backend.go @@ -35,6 +35,7 @@ import ( "github.com/docker/compose-cli/aci/login" "github.com/docker/compose-cli/api/compose" "github.com/docker/compose-cli/api/containers" + "github.com/docker/compose-cli/api/volumes" "github.com/docker/compose-cli/api/secrets" "github.com/docker/compose-cli/backend" apicontext "github.com/docker/compose-cli/context" @@ -115,6 +116,7 @@ func getAciAPIService(aciCtx store.AciContext) *aciAPIService { type aciAPIService struct { *aciContainerService *aciComposeService + *aciVolumeService } func (a *aciAPIService) ContainerService() containers.Service { @@ -129,6 +131,10 @@ func (a *aciAPIService) SecretsService() secrets.Service { return nil } +func (a *aciAPIService) VolumeService() volumes.Service { + return a.aciVolumeService +} + type aciContainerService struct { ctx store.AciContext } @@ -496,6 +502,18 @@ func (cs *aciComposeService) Convert(ctx context.Context, project *types.Project return nil, errdefs.ErrNotImplemented } +type aciVolumeService struct { + ctx store.AciContext +} + +func (cs *aciVolumeService) List(ctx context.Context) ([]volumes.Volume, error) { + return nil, nil +} + +func (cs *aciVolumeService) Create(ctx context.Context, options interface{}) (volumes.Volume, error) { + return volumes.Volume{}, nil +} + type aciCloudService struct { loginService login.AzureLoginServiceAPI } diff --git a/api/client/client.go b/api/client/client.go index a6e19bf2..937aad14 100644 --- a/api/client/client.go +++ b/api/client/client.go @@ -18,6 +18,7 @@ package client import ( "context" + "github.com/docker/compose-cli/api/volumes" "github.com/docker/compose-cli/api/compose" "github.com/docker/compose-cli/api/containers" @@ -86,3 +87,11 @@ func (c *Client) SecretsService() secrets.Service { return &secretsService{} } +// VolumeService returns the backend service for the current context +func (c *Client) VolumeService() volumes.Service { + if vs := c.bs.VolumeService(); vs != nil { + return vs + } + + return &volumeService{} +} diff --git a/api/client/volume.go b/api/client/volume.go new file mode 100644 index 00000000..37ebfac3 --- /dev/null +++ b/api/client/volume.go @@ -0,0 +1,36 @@ +/* + Copyright 2020 Docker, Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package client + +import ( + "context" + "github.com/docker/compose-cli/api/volumes" + "github.com/docker/compose-cli/errdefs" +) + +type volumeService struct { +} + +// List list volumes +func (c *volumeService) List(ctx context.Context) ([]volumes.Volume, error) { + return nil, errdefs.ErrNotImplemented +} + +// Create creates a volume +func (c *volumeService) Create(ctx context.Context, options interface {}) (volumes.Volume, error) { + return volumes.Volume{}, errdefs.ErrNotImplemented +} diff --git a/api/volumes/api.go b/api/volumes/api.go new file mode 100644 index 00000000..ce6efe04 --- /dev/null +++ b/api/volumes/api.go @@ -0,0 +1,37 @@ +/* + Copyright 2020 Docker, Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package volumes + +import ( + "context" +) + +type Volume struct { + Name string +} + +type VolumeCreateOptions struct { + account string + fileshare string +} + +// Service interacts with the underlying container backend +type Service interface { + // List returns all available volumes + List(ctx context.Context) ([]Volume, error) + Create(ctx context.Context, options interface{}) (Volume, error) +} diff --git a/backend/backend.go b/backend/backend.go index 8185f873..dc756261 100644 --- a/backend/backend.go +++ b/backend/backend.go @@ -26,6 +26,7 @@ import ( "github.com/docker/compose-cli/api/compose" "github.com/docker/compose-cli/api/containers" "github.com/docker/compose-cli/api/secrets" + "github.com/docker/compose-cli/api/volumes" "github.com/docker/compose-cli/context/cloud" "github.com/docker/compose-cli/errdefs" ) @@ -53,8 +54,9 @@ var backends = struct { // Service aggregates the service interfaces type Service interface { ContainerService() containers.Service - SecretsService() secrets.Service ComposeService() compose.Service + SecretsService() secrets.Service + VolumeService() volumes.Service } // Register adds a typed backend to the registry diff --git a/cli/cmd/volume/create.go b/cli/cmd/volume/create.go new file mode 100644 index 00000000..3a3a0af3 --- /dev/null +++ b/cli/cmd/volume/create.go @@ -0,0 +1,66 @@ +package volume + +/* + Copyright 2020 Docker, Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +import ( + "fmt" + "github.com/docker/compose-cli/api/client" + "github.com/spf13/cobra" +) + +type createVolumeOptions struct { + Account string + Fileshare string +} + +// SecretCommand manage secrets +func VolumeCommand() *cobra.Command { + cmd := &cobra.Command{ + Use: "volume", + Short: "Manages volumes", + } + + cmd.AddCommand( + createVolume(), + ) + return cmd +} + +func createVolume() *cobra.Command { + opts := createVolumeOptions{} + cmd := &cobra.Command{ + Use: "create", + Short: "Creates an Azure file share to use as ACI volume.", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + c, err := client.New(cmd.Context()) + if err != nil { + return err + } + id, err := c.VolumeService().Create(cmd.Context(), opts) + if err != nil { + return err + } + fmt.Println(id) + return nil + }, + } + + cmd.Flags().StringVar(&opts.Account, "storage-account", "", "Storage account name") + cmd.Flags().StringVar(&opts.Fileshare, "fileshare", "", "Fileshare name") + return cmd +} diff --git a/cli/main.go b/cli/main.go index 69f9b0ba..79167ac4 100644 --- a/cli/main.go +++ b/cli/main.go @@ -19,6 +19,7 @@ package main import ( "context" "fmt" + volume "github.com/docker/compose-cli/cli/cmd/volume" "math/rand" "os" "os/signal" @@ -133,6 +134,7 @@ func main() { // Place holders cmd.EcsCommand(), + volume.VolumeCommand(), ) helpFunc := root.HelpFunc() diff --git a/ecs/backend.go b/ecs/backend.go index ec18fe02..8252ad28 100644 --- a/ecs/backend.go +++ b/ecs/backend.go @@ -18,6 +18,7 @@ package ecs import ( "context" + "github.com/docker/compose-cli/api/volumes" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/session" @@ -97,6 +98,10 @@ func (a *ecsAPIService) SecretsService() secrets.Service { return a } +func (a *ecsAPIService) VolumeService() volumes.Service { + return nil +} + func getCloudService() (cloud.Service, error) { return ecsCloudService{}, nil } diff --git a/ecs/local/backend.go b/ecs/local/backend.go index 1310cfec..f0c68ab8 100644 --- a/ecs/local/backend.go +++ b/ecs/local/backend.go @@ -18,6 +18,7 @@ package local import ( "context" + "github.com/docker/compose-cli/api/volumes" "github.com/docker/compose-cli/api/compose" "github.com/docker/compose-cli/api/containers" @@ -58,6 +59,10 @@ func (e ecsLocalSimulation) ContainerService() containers.Service { return nil } +func (e ecsLocalSimulation) VolumeService() volumes.Service { + return nil +} + func (e ecsLocalSimulation) SecretsService() secrets.Service { return nil } diff --git a/example/backend.go b/example/backend.go index 2f9b420c..23775841 100644 --- a/example/backend.go +++ b/example/backend.go @@ -29,6 +29,7 @@ import ( "github.com/docker/compose-cli/api/compose" "github.com/docker/compose-cli/api/containers" "github.com/docker/compose-cli/api/secrets" + "github.com/docker/compose-cli/api/volumes" "github.com/docker/compose-cli/backend" "github.com/docker/compose-cli/context/cloud" "github.com/docker/compose-cli/errdefs" @@ -51,6 +52,10 @@ func (a *apiService) SecretsService() secrets.Service { return nil } +func (a *apiService) VolumeService() volumes.Service { + return nil +} + func init() { backend.Register("example", "example", service, cloud.NotImplementedCloudService) } diff --git a/local/backend.go b/local/backend.go index 63acaae6..99e2afc5 100644 --- a/local/backend.go +++ b/local/backend.go @@ -38,6 +38,7 @@ import ( "github.com/docker/compose-cli/api/compose" "github.com/docker/compose-cli/api/containers" + "github.com/docker/compose-cli/api/volumes" "github.com/docker/compose-cli/api/secrets" "github.com/docker/compose-cli/backend" "github.com/docker/compose-cli/context/cloud" @@ -75,6 +76,10 @@ func (ms *local) SecretsService() secrets.Service { return nil } +func (ms *local) VolumeService() volumes.Service { + return nil +} + func (ms *local) Inspect(ctx context.Context, id string) (containers.Container, error) { c, err := ms.apiClient.ContainerInspect(ctx, id) if err != nil { From 08562b403e113bb9312c976bddfbe7ad84bdfbf4 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Mon, 7 Sep 2020 18:23:28 +0200 Subject: [PATCH 03/13] Connecting it all Signed-off-by: Guillaume Tardif --- aci/backend.go | 27 +++++++--- aci/convert/convert.go | 7 +-- aci/login/client.go | 3 +- aci/login/storage_helper.go | 81 ++++++++++++++++++++++------- api/client/client.go | 2 + api/client/volume.go | 3 +- api/volumes/api.go | 10 ++-- cli/cmd/volume/create.go | 63 ++++++++++++++++++---- cli/main.go | 5 +- ecs/backend.go | 1 + ecs/local/backend.go | 4 +- local/backend.go | 2 +- tests/ecs-local-e2e/context_test.go | 3 +- 13 files changed, 154 insertions(+), 57 deletions(-) diff --git a/aci/backend.go b/aci/backend.go index 5e374d49..19e38664 100644 --- a/aci/backend.go +++ b/aci/backend.go @@ -35,8 +35,8 @@ import ( "github.com/docker/compose-cli/aci/login" "github.com/docker/compose-cli/api/compose" "github.com/docker/compose-cli/api/containers" - "github.com/docker/compose-cli/api/volumes" "github.com/docker/compose-cli/api/secrets" + "github.com/docker/compose-cli/api/volumes" "github.com/docker/compose-cli/backend" apicontext "github.com/docker/compose-cli/context" "github.com/docker/compose-cli/context/cloud" @@ -110,6 +110,9 @@ func getAciAPIService(aciCtx store.AciContext) *aciAPIService { aciComposeService: &aciComposeService{ ctx: aciCtx, }, + aciVolumeService: &aciVolumeService{ + ctx: aciCtx, + }, } } @@ -169,20 +172,20 @@ func getContainerGroups(ctx context.Context, subscriptionID string, resourceGrou var containerGroups []containerinstance.ContainerGroup result, err := groupsClient.ListByResourceGroup(ctx, resourceGroup) if err != nil { - return []containerinstance.ContainerGroup{}, err + return nil, err } for result.NotDone() { containerGroups = append(containerGroups, result.Values()...) if err := result.NextWithContext(ctx); err != nil { - return []containerinstance.ContainerGroup{}, err + return nil, err } } var groups []containerinstance.ContainerGroup for _, group := range containerGroups { group, err := groupsClient.Get(ctx, resourceGroup, *group.Name) if err != nil { - return []containerinstance.ContainerGroup{}, err + return nil, err } groups = append(groups, group) } @@ -507,11 +510,23 @@ type aciVolumeService struct { } func (cs *aciVolumeService) List(ctx context.Context) ([]volumes.Volume, error) { - return nil, nil + storageHelper := login.StorageAccountHelper{AciContext: cs.ctx} + return storageHelper.ListFileShare(ctx) +} + +//VolumeCreateOptions options to create a new ACI volume +type VolumeCreateOptions struct { + Account string + Fileshare string } func (cs *aciVolumeService) Create(ctx context.Context, options interface{}) (volumes.Volume, error) { - return volumes.Volume{}, nil + opts, ok := options.(VolumeCreateOptions) + if !ok { + return volumes.Volume{}, errors.New("Could not read azure LoginParams struct from generic parameter") + } + storageHelper := login.StorageAccountHelper{AciContext: cs.ctx} + return storageHelper.CreateFileShare(ctx, opts.Account, opts.Fileshare) } type aciCloudService struct { diff --git a/aci/convert/convert.go b/aci/convert/convert.go index eb505b4c..65ce5683 100644 --- a/aci/convert/convert.go +++ b/aci/convert/convert.go @@ -56,13 +56,8 @@ const ( func ToContainerGroup(ctx context.Context, aciContext store.AciContext, p types.Project) (containerinstance.ContainerGroup, error) { project := projectAciHelper(p) containerGroupName := strings.ToLower(project.Name) - loginService, err := login.NewAzureLoginService() - if err != nil { - return containerinstance.ContainerGroup{}, err - } storageHelper := login.StorageAccountHelper{ - LoginService: *loginService, - AciContext: aciContext, + AciContext: aciContext, } volumesCache, volumesSlice, err := project.getAciFileVolumes(ctx, storageHelper) if err != nil { diff --git a/aci/login/client.go b/aci/login/client.go index 95e8160c..7de5d327 100644 --- a/aci/login/client.go +++ b/aci/login/client.go @@ -67,7 +67,7 @@ func NewStorageAccountsClient(subscriptionID string) (storage.AccountsClient, er return containerGroupsClient, nil } -// NewStorageAccountsClient get client to manipulate storage accounts +// NewFileShareClient get client to manipulate file shares func NewFileShareClient(subscriptionID string) (storage.FileSharesClient, error) { containerGroupsClient := storage.NewFileSharesClient(subscriptionID) err := setupClient(&containerGroupsClient.Client) @@ -80,7 +80,6 @@ func NewFileShareClient(subscriptionID string) (storage.FileSharesClient, error) return containerGroupsClient, nil } - // NewSubscriptionsClient get subscription client func NewSubscriptionsClient() (subscription.SubscriptionsClient, error) { subc := subscription.NewSubscriptionsClient() diff --git a/aci/login/storage_helper.go b/aci/login/storage_helper.go index 8e16b1f4..de81c5b3 100644 --- a/aci/login/storage_helper.go +++ b/aci/login/storage_helper.go @@ -19,8 +19,12 @@ package login import ( "context" "fmt" + "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2019-06-01/storage" + "github.com/docker/compose-cli/api/volumes" + "github.com/docker/compose-cli/errdefs" + "github.com/pkg/errors" "github.com/docker/compose-cli/context/store" @@ -28,8 +32,7 @@ import ( // StorageAccountHelper helper for Azure Storage Account type StorageAccountHelper struct { - LoginService AzureLoginService - AciContext store.AciContext + AciContext store.AciContext } // GetAzureStorageAccountKey retrieves the storage account ket from the current azure login @@ -50,61 +53,99 @@ func (helper StorageAccountHelper) GetAzureStorageAccountKey(ctx context.Context return *key.Value, nil } -func (helper StorageAccountHelper) ListFileShare(ctx context.Context) ([]string, error) { +// ListFileShare list file shares in all visible storage accounts +func (helper StorageAccountHelper) ListFileShare(ctx context.Context) ([]volumes.Volume, error) { aciContext := helper.AciContext accountClient, err := NewStorageAccountsClient(aciContext.SubscriptionID) if err != nil { return nil, err } result, err := accountClient.ListByResourceGroup(ctx, aciContext.ResourceGroup) + if err != nil { + return nil, err + } accounts := result.Value fileShareClient, err := NewFileShareClient(aciContext.SubscriptionID) - fileShares := []string{} + if err != nil { + return nil, err + } + fileShares := []volumes.Volume{} for _, account := range *accounts { fileSharePage, err := fileShareClient.List(ctx, aciContext.ResourceGroup, *account.Name, "", "", "") if err != nil { return nil, err } - for ; fileSharePage.NotDone() ; fileSharePage.NextWithContext(ctx) { + + for fileSharePage.NotDone() { values := fileSharePage.Values() for _, fileShare := range values { - fileShares = append(fileShares, *fileShare.Name) + fileShares = append(fileShares, toVolume(account, *fileShare.Name)) + } + if err := fileSharePage.NextWithContext(ctx); err != nil { + return nil, err } } } return fileShares, nil } -func (helper StorageAccountHelper) CreateFileShare(ctx context.Context, accountName string, fileShareName string) (storage.FileShare, error) { +func toVolume(account storage.Account, fileShareName string) volumes.Volume { + return volumes.Volume{ + ID: fmt.Sprintf("%s@%s", *account.Name, fileShareName), + Name: fileShareName, + Description: fmt.Sprintf("Fileshare %s in %s storage account", fileShareName, *account.Name), + } +} + +// CreateFileShare create a new fileshare +func (helper StorageAccountHelper) CreateFileShare(ctx context.Context, accountName string, fileShareName string) (volumes.Volume, error) { aciContext := helper.AciContext accountClient, err := NewStorageAccountsClient(aciContext.SubscriptionID) if err != nil { - return storage.FileShare{}, err + return volumes.Volume{}, err } account, err := accountClient.GetProperties(ctx, aciContext.ResourceGroup, accountName, "") if err != nil { - //TODO check err not found - parameters := storage.AccountCreateParameters{ - Location: &aciContext.Location, - Sku:&storage.Sku{ - Name: storage.StandardLRS, - Tier: storage.Standard, - }, + if account.StatusCode != 404 { + return volumes.Volume{}, err } + //TODO confirm storage account creation + parameters := defaultStorageAccountParams(aciContext) // TODO progress account creation future, err := accountClient.Create(ctx, aciContext.ResourceGroup, accountName, parameters) if err != nil { - return storage.FileShare{}, err + return volumes.Volume{}, err } account, err = future.Result(accountClient) + if err != nil { + return volumes.Volume{}, err + } } fileShareClient, err := NewFileShareClient(aciContext.SubscriptionID) - fileShare, err := fileShareClient.Get(ctx, aciContext.ResourceGroup, *account.Name, fileShareName, "") if err != nil { - // TODO check err not found - fileShare, err = fileShareClient.Create(ctx, aciContext.ResourceGroup, *account.Name, fileShareName, storage.FileShare{}) + return volumes.Volume{}, err } - return fileShare, nil + fileShare, err := fileShareClient.Get(ctx, aciContext.ResourceGroup, *account.Name, fileShareName, "") + if err == nil { + return volumes.Volume{}, errors.Wrapf(errdefs.ErrAlreadyExists, "Azure fileshare %q already exists", fileShareName) + } + if fileShare.StatusCode != 404 { + return volumes.Volume{}, err + } + fileShare, err = fileShareClient.Create(ctx, aciContext.ResourceGroup, *account.Name, fileShareName, storage.FileShare{}) + if err != nil { + return volumes.Volume{}, err + } + return toVolume(account, *fileShare.Name), nil } +func defaultStorageAccountParams(aciContext store.AciContext) storage.AccountCreateParameters { + return storage.AccountCreateParameters{ + Location: &aciContext.Location, + Sku: &storage.Sku{ + Name: storage.StandardLRS, + Tier: storage.Standard, + }, + } +} diff --git a/api/client/client.go b/api/client/client.go index 937aad14..632dc325 100644 --- a/api/client/client.go +++ b/api/client/client.go @@ -18,6 +18,7 @@ package client import ( "context" + "github.com/docker/compose-cli/api/volumes" "github.com/docker/compose-cli/api/compose" @@ -87,6 +88,7 @@ func (c *Client) SecretsService() secrets.Service { return &secretsService{} } + // VolumeService returns the backend service for the current context func (c *Client) VolumeService() volumes.Service { if vs := c.bs.VolumeService(); vs != nil { diff --git a/api/client/volume.go b/api/client/volume.go index 37ebfac3..34adfae9 100644 --- a/api/client/volume.go +++ b/api/client/volume.go @@ -18,6 +18,7 @@ package client import ( "context" + "github.com/docker/compose-cli/api/volumes" "github.com/docker/compose-cli/errdefs" ) @@ -31,6 +32,6 @@ func (c *volumeService) List(ctx context.Context) ([]volumes.Volume, error) { } // Create creates a volume -func (c *volumeService) Create(ctx context.Context, options interface {}) (volumes.Volume, error) { +func (c *volumeService) Create(ctx context.Context, options interface{}) (volumes.Volume, error) { return volumes.Volume{}, errdefs.ErrNotImplemented } diff --git a/api/volumes/api.go b/api/volumes/api.go index ce6efe04..59a8b1d7 100644 --- a/api/volumes/api.go +++ b/api/volumes/api.go @@ -20,13 +20,11 @@ import ( "context" ) +// Volume volume info type Volume struct { - Name string -} - -type VolumeCreateOptions struct { - account string - fileshare string + ID string + Name string + Description string } // Service interacts with the underlying container backend diff --git a/cli/cmd/volume/create.go b/cli/cmd/volume/create.go index 3a3a0af3..07b2ebcf 100644 --- a/cli/cmd/volume/create.go +++ b/cli/cmd/volume/create.go @@ -18,17 +18,21 @@ package volume import ( "fmt" - "github.com/docker/compose-cli/api/client" + "io" + "os" + "strings" + "text/tabwriter" + + "github.com/docker/compose-cli/aci" + "github.com/spf13/cobra" + + "github.com/docker/compose-cli/api/client" + "github.com/docker/compose-cli/api/volumes" ) -type createVolumeOptions struct { - Account string - Fileshare string -} - -// SecretCommand manage secrets -func VolumeCommand() *cobra.Command { +// Command manage volumes +func Command() *cobra.Command { cmd := &cobra.Command{ Use: "volume", Short: "Manages volumes", @@ -36,16 +40,17 @@ func VolumeCommand() *cobra.Command { cmd.AddCommand( createVolume(), + listVolume(), ) return cmd } func createVolume() *cobra.Command { - opts := createVolumeOptions{} + opts := aci.VolumeCreateOptions{} cmd := &cobra.Command{ Use: "create", Short: "Creates an Azure file share to use as ACI volume.", - Args: cobra.ExactArgs(1), + Args: cobra.ExactArgs(0), RunE: func(cmd *cobra.Command, args []string) error { c, err := client.New(cmd.Context()) if err != nil { @@ -60,7 +65,43 @@ func createVolume() *cobra.Command { }, } - cmd.Flags().StringVar(&opts.Account, "storage-account", "", "Storage account name") + cmd.Flags().StringVar(&opts.Account, "storage-account", "", "Storage account name") cmd.Flags().StringVar(&opts.Fileshare, "fileshare", "", "Fileshare name") return cmd } + +func listVolume() *cobra.Command { + cmd := &cobra.Command{ + Use: "ls", + Short: "list Azure file shares usable as ACI volumes.", + Args: cobra.ExactArgs(0), + RunE: func(cmd *cobra.Command, args []string) error { + c, err := client.New(cmd.Context()) + if err != nil { + return err + } + vols, err := c.VolumeService().List(cmd.Context()) + if err != nil { + return err + } + printList(os.Stdout, vols) + return nil + }, + } + return cmd +} + +func printList(out io.Writer, volumes []volumes.Volume) { + printSection(out, func(w io.Writer) { + for _, vol := range volumes { + fmt.Fprintf(w, "%s\t%s\t%s\n", vol.ID, vol.Name, vol.Description) // nolint:errcheck + } + }, "ID", "NAME", "DESCRIPTION") +} + +func printSection(out io.Writer, printer func(io.Writer), headers ...string) { + w := tabwriter.NewWriter(out, 20, 1, 3, ' ', 0) + fmt.Fprintln(w, strings.Join(headers, "\t")) // nolint:errcheck + printer(w) + w.Flush() // nolint:errcheck +} diff --git a/cli/main.go b/cli/main.go index 79167ac4..69d84324 100644 --- a/cli/main.go +++ b/cli/main.go @@ -19,7 +19,6 @@ package main import ( "context" "fmt" - volume "github.com/docker/compose-cli/cli/cmd/volume" "math/rand" "os" "os/signal" @@ -28,6 +27,8 @@ import ( "syscall" "time" + volume "github.com/docker/compose-cli/cli/cmd/volume" + "github.com/docker/compose-cli/cli/cmd/compose" "github.com/docker/compose-cli/cli/cmd/logout" @@ -134,7 +135,7 @@ func main() { // Place holders cmd.EcsCommand(), - volume.VolumeCommand(), + volume.Command(), ) helpFunc := root.HelpFunc() diff --git a/ecs/backend.go b/ecs/backend.go index 8252ad28..a615cbeb 100644 --- a/ecs/backend.go +++ b/ecs/backend.go @@ -18,6 +18,7 @@ package ecs import ( "context" + "github.com/docker/compose-cli/api/volumes" "github.com/aws/aws-sdk-go/aws" diff --git a/ecs/local/backend.go b/ecs/local/backend.go index f0c68ab8..cec7c104 100644 --- a/ecs/local/backend.go +++ b/ecs/local/backend.go @@ -18,12 +18,14 @@ package local import ( "context" + "github.com/docker/compose-cli/api/volumes" + "github.com/docker/docker/client" + "github.com/docker/compose-cli/api/compose" "github.com/docker/compose-cli/api/containers" "github.com/docker/compose-cli/api/secrets" - "github.com/docker/docker/client" "github.com/docker/compose-cli/backend" "github.com/docker/compose-cli/context/cloud" diff --git a/local/backend.go b/local/backend.go index 99e2afc5..dfd8f6d4 100644 --- a/local/backend.go +++ b/local/backend.go @@ -38,8 +38,8 @@ import ( "github.com/docker/compose-cli/api/compose" "github.com/docker/compose-cli/api/containers" - "github.com/docker/compose-cli/api/volumes" "github.com/docker/compose-cli/api/secrets" + "github.com/docker/compose-cli/api/volumes" "github.com/docker/compose-cli/backend" "github.com/docker/compose-cli/context/cloud" "github.com/docker/compose-cli/errdefs" diff --git a/tests/ecs-local-e2e/context_test.go b/tests/ecs-local-e2e/context_test.go index 6bf26bb7..4d0003ae 100644 --- a/tests/ecs-local-e2e/context_test.go +++ b/tests/ecs-local-e2e/context_test.go @@ -21,8 +21,9 @@ import ( "os" "testing" - . "github.com/docker/compose-cli/tests/framework" "gotest.tools/v3/icmd" + + . "github.com/docker/compose-cli/tests/framework" ) const ( From dcbc1c3fb298184c0d768c664b2e93c3ecb8062c Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Tue, 8 Sep 2020 09:44:26 +0200 Subject: [PATCH 04/13] Volume cli minor refactor Signed-off-by: Guillaume Tardif --- cli/cmd/secrets_test.go | 2 +- cli/cmd/volume/{create.go => list.go} | 43 --------------- cli/cmd/volume/list_test.go | 22 ++++++++ cli/cmd/volume/testdata/volumes-out.golden | 2 + cli/cmd/volume/volume.go | 63 ++++++++++++++++++++++ 5 files changed, 88 insertions(+), 44 deletions(-) rename cli/cmd/volume/{create.go => list.go} (65%) create mode 100644 cli/cmd/volume/list_test.go create mode 100644 cli/cmd/volume/testdata/volumes-out.golden create mode 100644 cli/cmd/volume/volume.go diff --git a/cli/cmd/secrets_test.go b/cli/cmd/secrets_test.go index 70968136..37bb25c3 100644 --- a/cli/cmd/secrets_test.go +++ b/cli/cmd/secrets_test.go @@ -35,5 +35,5 @@ func TestPrintList(t *testing.T) { } out := &bytes.Buffer{} printList(out, secrets) - golden.Assert(t, out.String(), "secrets-out.golden") + golden.Assert(t, out.String(), "volumes-out.golden") } diff --git a/cli/cmd/volume/create.go b/cli/cmd/volume/list.go similarity index 65% rename from cli/cmd/volume/create.go rename to cli/cmd/volume/list.go index 07b2ebcf..7e8c4e33 100644 --- a/cli/cmd/volume/create.go +++ b/cli/cmd/volume/list.go @@ -22,54 +22,11 @@ import ( "os" "strings" "text/tabwriter" - - "github.com/docker/compose-cli/aci" - "github.com/spf13/cobra" - "github.com/docker/compose-cli/api/client" "github.com/docker/compose-cli/api/volumes" ) -// Command manage volumes -func Command() *cobra.Command { - cmd := &cobra.Command{ - Use: "volume", - Short: "Manages volumes", - } - - cmd.AddCommand( - createVolume(), - listVolume(), - ) - return cmd -} - -func createVolume() *cobra.Command { - opts := aci.VolumeCreateOptions{} - cmd := &cobra.Command{ - Use: "create", - Short: "Creates an Azure file share to use as ACI volume.", - Args: cobra.ExactArgs(0), - RunE: func(cmd *cobra.Command, args []string) error { - c, err := client.New(cmd.Context()) - if err != nil { - return err - } - id, err := c.VolumeService().Create(cmd.Context(), opts) - if err != nil { - return err - } - fmt.Println(id) - return nil - }, - } - - cmd.Flags().StringVar(&opts.Account, "storage-account", "", "Storage account name") - cmd.Flags().StringVar(&opts.Fileshare, "fileshare", "", "Fileshare name") - return cmd -} - func listVolume() *cobra.Command { cmd := &cobra.Command{ Use: "ls", diff --git a/cli/cmd/volume/list_test.go b/cli/cmd/volume/list_test.go new file mode 100644 index 00000000..ab21c75d --- /dev/null +++ b/cli/cmd/volume/list_test.go @@ -0,0 +1,22 @@ +package volume + +import ( + "bytes" + "github.com/docker/compose-cli/api/volumes" + "gotest.tools/v3/golden" + "testing" +) + +func TestPrintList(t *testing.T) { + secrets := []volumes.Volume{ + { + ID: "volume@123", + Name: "123", + Description: "volume 123", + }, + } + out := &bytes.Buffer{} + printList(out, secrets) + golden.Assert(t, out.String(), "volumes-out.golden") +} + diff --git a/cli/cmd/volume/testdata/volumes-out.golden b/cli/cmd/volume/testdata/volumes-out.golden new file mode 100644 index 00000000..ed27e449 --- /dev/null +++ b/cli/cmd/volume/testdata/volumes-out.golden @@ -0,0 +1,2 @@ +ID NAME DESCRIPTION +volume@123 123 volume 123 diff --git a/cli/cmd/volume/volume.go b/cli/cmd/volume/volume.go new file mode 100644 index 00000000..77362467 --- /dev/null +++ b/cli/cmd/volume/volume.go @@ -0,0 +1,63 @@ +package volume + +/* + Copyright 2020 Docker, Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +import ( + "fmt" + "github.com/spf13/cobra" + "github.com/docker/compose-cli/aci" + "github.com/docker/compose-cli/api/client" +) + +// Command manage volumes +func Command() *cobra.Command { + cmd := &cobra.Command{ + Use: "volume", + Short: "Manages volumes", + } + + cmd.AddCommand( + createVolume(), + listVolume(), + ) + return cmd +} + +func createVolume() *cobra.Command { + opts := aci.VolumeCreateOptions{} + cmd := &cobra.Command{ + Use: "create", + Short: "Creates an Azure file share to use as ACI volume.", + Args: cobra.ExactArgs(0), + RunE: func(cmd *cobra.Command, args []string) error { + c, err := client.New(cmd.Context()) + if err != nil { + return err + } + id, err := c.VolumeService().Create(cmd.Context(), opts) + if err != nil { + return err + } + fmt.Println(id) + return nil + }, + } + + cmd.Flags().StringVar(&opts.Account, "storage-account", "", "Storage account name") + cmd.Flags().StringVar(&opts.Fileshare, "fileshare", "", "Fileshare name") + return cmd +} \ No newline at end of file From 15addf5c22899e6767a9b957063f7cb57d5d650d Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Tue, 8 Sep 2020 10:11:02 +0200 Subject: [PATCH 05/13] Break out aci backend.go into several files for each service Signed-off-by: Guillaume Tardif --- aci/aci.go | 28 ++ aci/backend.go | 394 +------------------- aci/cloud.go | 48 +++ aci/compose.go | 128 +++++++ aci/containers.go | 254 +++++++++++++ aci/convert/convert.go | 4 +- aci/login/storagelogin.go | 49 +++ aci/{login/storage_helper.go => volumes.go} | 80 ++-- 8 files changed, 544 insertions(+), 441 deletions(-) create mode 100644 aci/cloud.go create mode 100644 aci/compose.go create mode 100644 aci/containers.go create mode 100644 aci/login/storagelogin.go rename aci/{login/storage_helper.go => volumes.go} (53%) diff --git a/aci/aci.go b/aci/aci.go index 4a251d3f..795a8f49 100644 --- a/aci/aci.go +++ b/aci/aci.go @@ -129,6 +129,34 @@ func getACIContainerGroup(ctx context.Context, aciContext store.AciContext, cont return containerGroupsClient.Get(ctx, aciContext.ResourceGroup, containerGroupName) } +func getACIContainerGroups(ctx context.Context, subscriptionID string, resourceGroup string) ([]containerinstance.ContainerGroup, error) { + groupsClient, err := login.NewContainerGroupsClient(subscriptionID) + if err != nil { + return nil, err + } + var containerGroups []containerinstance.ContainerGroup + result, err := groupsClient.ListByResourceGroup(ctx, resourceGroup) + if err != nil { + return nil, err + } + + for result.NotDone() { + containerGroups = append(containerGroups, result.Values()...) + if err := result.NextWithContext(ctx); err != nil { + return nil, err + } + } + var groups []containerinstance.ContainerGroup + for _, group := range containerGroups { + group, err := groupsClient.Get(ctx, resourceGroup, *group.Name) + if err != nil { + return nil, err + } + groups = append(groups, group) + } + return groups, nil +} + func deleteACIContainerGroup(ctx context.Context, aciContext store.AciContext, containerGroupName string) (containerinstance.ContainerGroup, error) { containerGroupsClient, err := login.NewContainerGroupsClient(aciContext.SubscriptionID) if err != nil { diff --git a/aci/backend.go b/aci/backend.go index 19e38664..955c487a 100644 --- a/aci/backend.go +++ b/aci/backend.go @@ -18,18 +18,11 @@ package aci import ( "context" - "fmt" - "io" - "net/http" - "strconv" "strings" "github.com/Azure/azure-sdk-for-go/services/containerinstance/mgmt/2018-10-01/containerinstance" - "github.com/Azure/go-autorest/autorest" "github.com/Azure/go-autorest/autorest/to" - "github.com/compose-spec/compose-go/types" "github.com/pkg/errors" - "github.com/sirupsen/logrus" "github.com/docker/compose-cli/aci/convert" "github.com/docker/compose-cli/aci/login" @@ -41,7 +34,6 @@ import ( apicontext "github.com/docker/compose-cli/context" "github.com/docker/compose-cli/context/cloud" "github.com/docker/compose-cli/context/store" - "github.com/docker/compose-cli/errdefs" ) const ( @@ -111,7 +103,7 @@ func getAciAPIService(aciCtx store.AciContext) *aciAPIService { ctx: aciCtx, }, aciVolumeService: &aciVolumeService{ - ctx: aciCtx, + aciContext: aciCtx, }, } } @@ -138,60 +130,6 @@ func (a *aciAPIService) VolumeService() volumes.Service { return a.aciVolumeService } -type aciContainerService struct { - ctx store.AciContext -} - -func (cs *aciContainerService) List(ctx context.Context, all bool) ([]containers.Container, error) { - containerGroups, err := getContainerGroups(ctx, cs.ctx.SubscriptionID, cs.ctx.ResourceGroup) - if err != nil { - return []containers.Container{}, err - } - var res []containers.Container - for _, group := range containerGroups { - if group.Containers == nil || len(*group.Containers) < 1 { - return []containers.Container{}, fmt.Errorf("no containers found in ACI container group %s", *group.Name) - } - - for _, container := range *group.Containers { - if isContainerVisible(container, group, all) { - continue - } - c := convert.ContainerGroupToContainer(getContainerID(group, container), group, container) - res = append(res, c) - } - } - return res, nil -} - -func getContainerGroups(ctx context.Context, subscriptionID string, resourceGroup string) ([]containerinstance.ContainerGroup, error) { - groupsClient, err := login.NewContainerGroupsClient(subscriptionID) - if err != nil { - return nil, err - } - var containerGroups []containerinstance.ContainerGroup - result, err := groupsClient.ListByResourceGroup(ctx, resourceGroup) - if err != nil { - return nil, err - } - - for result.NotDone() { - containerGroups = append(containerGroups, result.Values()...) - if err := result.NextWithContext(ctx); err != nil { - return nil, err - } - } - var groups []containerinstance.ContainerGroup - for _, group := range containerGroups { - group, err := groupsClient.Get(ctx, resourceGroup, *group.Name) - if err != nil { - return nil, err - } - groups = append(groups, group) - } - return groups, nil -} - func getContainerID(group containerinstance.ContainerGroup, container containerinstance.Container) string { containerID := *group.Name + composeContainerSeparator + *container.Name if _, ok := group.Tags[singleContainerTag]; ok { @@ -204,26 +142,6 @@ func isContainerVisible(container containerinstance.Container, group containerin return *container.Name == convert.ComposeDNSSidecarName || (!showAll && convert.GetStatus(container, group) != convert.StatusRunning) } -func (cs *aciContainerService) Run(ctx context.Context, r containers.ContainerConfig) error { - if strings.Contains(r.ID, composeContainerSeparator) { - return errors.New(fmt.Sprintf("invalid container name. ACI container name cannot include %q", composeContainerSeparator)) - } - - project, err := convert.ContainerToComposeProject(r) - if err != nil { - return err - } - - logrus.Debugf("Running container %q with name %q\n", r.Image, r.ID) - groupDefinition, err := convert.ToContainerGroup(ctx, cs.ctx, project) - if err != nil { - return err - } - addTag(&groupDefinition, singleContainerTag) - - return createACIContainers(ctx, cs.ctx, groupDefinition) -} - func addTag(groupDefinition *containerinstance.ContainerGroup, tagName string) { if groupDefinition.Tags == nil { groupDefinition.Tags = make(map[string]*string, 1) @@ -231,52 +149,6 @@ func addTag(groupDefinition *containerinstance.ContainerGroup, tagName string) { groupDefinition.Tags[tagName] = to.StringPtr(tagName) } -func (cs *aciContainerService) Start(ctx context.Context, containerID string) error { - groupName, containerName := getGroupAndContainerName(containerID) - if groupName != containerID { - msg := "cannot start specified service %q from compose application %q, you can update and restart the entire compose app with docker compose up --project-name %s" - return errors.New(fmt.Sprintf(msg, containerName, groupName, groupName)) - } - - containerGroupsClient, err := login.NewContainerGroupsClient(cs.ctx.SubscriptionID) - if err != nil { - return err - } - - future, err := containerGroupsClient.Start(ctx, cs.ctx.ResourceGroup, containerName) - if err != nil { - var aerr autorest.DetailedError - if ok := errors.As(err, &aerr); ok { - if aerr.StatusCode == http.StatusNotFound { - return errdefs.ErrNotFound - } - } - return err - } - - return future.WaitForCompletionRef(ctx, containerGroupsClient.Client) -} - -func (cs *aciContainerService) Stop(ctx context.Context, containerID string, timeout *uint32) error { - if timeout != nil && *timeout != uint32(0) { - return errors.Errorf("ACI integration does not support setting a timeout to stop a container before killing it.") - } - groupName, containerName := getGroupAndContainerName(containerID) - if groupName != containerID { - msg := "cannot stop service %q from compose application %q, you can stop the entire compose app with docker stop %s" - return errors.New(fmt.Sprintf(msg, containerName, groupName, groupName)) - } - return stopACIContainerGroup(ctx, cs.ctx, groupName) -} - -func (cs *aciContainerService) Kill(ctx context.Context, containerID string, _ string) error { - groupName, containerName := getGroupAndContainerName(containerID) - if groupName != containerID { - msg := "cannot kill service %q from compose application %q, you can kill the entire compose app with docker kill %s" - return errors.New(fmt.Sprintf(msg, containerName, groupName, groupName)) - } - return stopACIContainerGroup(ctx, cs.ctx, groupName) // As ACI doesn't have a kill command, we are using the stop implementation instead -} func getGroupAndContainerName(containerID string) (string, string) { tokens := strings.Split(containerID, composeContainerSeparator) @@ -289,267 +161,3 @@ func getGroupAndContainerName(containerID string) (string, string) { return groupName, containerName } -func (cs *aciContainerService) Exec(ctx context.Context, name string, request containers.ExecRequest) error { - err := verifyExecCommand(request.Command) - if err != nil { - return err - } - groupName, containerAciName := getGroupAndContainerName(name) - containerExecResponse, err := execACIContainer(ctx, cs.ctx, request.Command, groupName, containerAciName) - if err != nil { - return err - } - - return exec( - context.Background(), - *containerExecResponse.WebSocketURI, - *containerExecResponse.Password, - request, - ) -} - -func verifyExecCommand(command string) error { - tokens := strings.Split(command, " ") - if len(tokens) > 1 { - return errors.New("ACI exec command does not accept arguments to the command. " + - "Only the binary should be specified") - } - return nil -} - -func (cs *aciContainerService) Logs(ctx context.Context, containerName string, req containers.LogsRequest) error { - groupName, containerAciName := getGroupAndContainerName(containerName) - var tail *int32 - - if req.Follow { - return streamLogs(ctx, cs.ctx, groupName, containerAciName, req) - } - - if req.Tail != "all" { - reqTail, err := strconv.Atoi(req.Tail) - if err != nil { - return err - } - i32 := int32(reqTail) - tail = &i32 - } - - logs, err := getACIContainerLogs(ctx, cs.ctx, groupName, containerAciName, tail) - if err != nil { - return err - } - - _, err = fmt.Fprint(req.Writer, logs) - return err -} - -func (cs *aciContainerService) Delete(ctx context.Context, containerID string, request containers.DeleteRequest) error { - groupName, containerName := getGroupAndContainerName(containerID) - if groupName != containerID { - msg := "cannot delete service %q from compose application %q, you can delete the entire compose app with docker compose down --project-name %s" - return errors.New(fmt.Sprintf(msg, containerName, groupName, groupName)) - } - - if !request.Force { - containerGroupsClient, err := login.NewContainerGroupsClient(cs.ctx.SubscriptionID) - if err != nil { - return err - } - - cg, err := containerGroupsClient.Get(ctx, cs.ctx.ResourceGroup, groupName) - if err != nil { - if cg.StatusCode == http.StatusNotFound { - return errdefs.ErrNotFound - } - return err - } - - for _, container := range *cg.Containers { - status := convert.GetStatus(container, cg) - - if status == convert.StatusRunning { - return errdefs.ErrForbidden - } - } - } - - cg, err := deleteACIContainerGroup(ctx, cs.ctx, groupName) - // Delete returns `StatusNoContent` if the group is not found - if cg.StatusCode == http.StatusNoContent { - return errdefs.ErrNotFound - } - if err != nil { - return err - } - - return err -} - -func (cs *aciContainerService) Inspect(ctx context.Context, containerID string) (containers.Container, error) { - groupName, containerName := getGroupAndContainerName(containerID) - - cg, err := getACIContainerGroup(ctx, cs.ctx, groupName) - if err != nil { - return containers.Container{}, err - } - if cg.StatusCode == http.StatusNoContent { - return containers.Container{}, errdefs.ErrNotFound - } - - var cc containerinstance.Container - var found = false - for _, c := range *cg.Containers { - if to.String(c.Name) == containerName { - cc = c - found = true - break - } - } - if !found { - return containers.Container{}, errdefs.ErrNotFound - } - - return convert.ContainerGroupToContainer(containerID, cg, cc), nil -} - -type aciComposeService struct { - ctx store.AciContext -} - -func (cs *aciComposeService) Up(ctx context.Context, project *types.Project) error { - logrus.Debugf("Up on project with name %q\n", project.Name) - groupDefinition, err := convert.ToContainerGroup(ctx, cs.ctx, *project) - addTag(&groupDefinition, composeContainerTag) - - if err != nil { - return err - } - return createOrUpdateACIContainers(ctx, cs.ctx, groupDefinition) -} - -func (cs *aciComposeService) Down(ctx context.Context, project string) error { - logrus.Debugf("Down on project with name %q\n", project) - - cg, err := deleteACIContainerGroup(ctx, cs.ctx, project) - if err != nil { - return err - } - if cg.StatusCode == http.StatusNoContent { - return errdefs.ErrNotFound - } - - return err -} - -func (cs *aciComposeService) Ps(ctx context.Context, project string) ([]compose.ServiceStatus, error) { - groupsClient, err := login.NewContainerGroupsClient(cs.ctx.SubscriptionID) - if err != nil { - return nil, err - } - - group, err := groupsClient.Get(ctx, cs.ctx.ResourceGroup, project) - if err != nil { - return []compose.ServiceStatus{}, err - } - - if group.Containers == nil || len(*group.Containers) < 1 { - return []compose.ServiceStatus{}, fmt.Errorf("no containers found in ACI container group %s", project) - } - - res := []compose.ServiceStatus{} - for _, container := range *group.Containers { - if isContainerVisible(container, group, false) { - continue - } - res = append(res, convert.ContainerGroupToServiceStatus(getContainerID(group, container), group, container)) - } - return res, nil -} - -func (cs *aciComposeService) List(ctx context.Context, project string) ([]compose.Stack, error) { - containerGroups, err := getContainerGroups(ctx, cs.ctx.SubscriptionID, cs.ctx.ResourceGroup) - if err != nil { - return []compose.Stack{}, err - } - - stacks := []compose.Stack{} - for _, group := range containerGroups { - if _, found := group.Tags[composeContainerTag]; !found { - continue - } - if project != "" && *group.Name != project { - continue - } - state := compose.RUNNING - for _, container := range *group.ContainerGroupProperties.Containers { - containerState := convert.GetStatus(container, group) - if containerState != compose.RUNNING { - state = containerState - break - } - } - stacks = append(stacks, compose.Stack{ - ID: *group.ID, - Name: *group.Name, - Status: state, - }) - } - return stacks, nil -} - -func (cs *aciComposeService) Logs(ctx context.Context, project string, w io.Writer) error { - return errdefs.ErrNotImplemented -} - -func (cs *aciComposeService) Convert(ctx context.Context, project *types.Project) ([]byte, error) { - return nil, errdefs.ErrNotImplemented -} - -type aciVolumeService struct { - ctx store.AciContext -} - -func (cs *aciVolumeService) List(ctx context.Context) ([]volumes.Volume, error) { - storageHelper := login.StorageAccountHelper{AciContext: cs.ctx} - return storageHelper.ListFileShare(ctx) -} - -//VolumeCreateOptions options to create a new ACI volume -type VolumeCreateOptions struct { - Account string - Fileshare string -} - -func (cs *aciVolumeService) Create(ctx context.Context, options interface{}) (volumes.Volume, error) { - opts, ok := options.(VolumeCreateOptions) - if !ok { - return volumes.Volume{}, errors.New("Could not read azure LoginParams struct from generic parameter") - } - storageHelper := login.StorageAccountHelper{AciContext: cs.ctx} - return storageHelper.CreateFileShare(ctx, opts.Account, opts.Fileshare) -} - -type aciCloudService struct { - loginService login.AzureLoginServiceAPI -} - -func (cs *aciCloudService) Login(ctx context.Context, params interface{}) error { - opts, ok := params.(LoginParams) - if !ok { - return errors.New("Could not read azure LoginParams struct from generic parameter") - } - if opts.ClientID != "" { - return cs.loginService.LoginServicePrincipal(opts.ClientID, opts.ClientSecret, opts.TenantID) - } - return cs.loginService.Login(ctx, opts.TenantID) -} - -func (cs *aciCloudService) Logout(ctx context.Context) error { - return cs.loginService.Logout(ctx) -} - -func (cs *aciCloudService) CreateContextData(ctx context.Context, params interface{}) (interface{}, string, error) { - contextHelper := newContextCreateHelper() - createOpts := params.(ContextParams) - return contextHelper.createContextData(ctx, createOpts) -} diff --git a/aci/cloud.go b/aci/cloud.go new file mode 100644 index 00000000..2d2e9757 --- /dev/null +++ b/aci/cloud.go @@ -0,0 +1,48 @@ +/* + Copyright 2020 Docker, Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package aci + +import ( + "context" + "github.com/pkg/errors" + "github.com/docker/compose-cli/aci/login" +) + +type aciCloudService struct { + loginService login.AzureLoginServiceAPI +} + +func (cs *aciCloudService) Login(ctx context.Context, params interface{}) error { + opts, ok := params.(LoginParams) + if !ok { + return errors.New("Could not read azure LoginParams struct from generic parameter") + } + if opts.ClientID != "" { + return cs.loginService.LoginServicePrincipal(opts.ClientID, opts.ClientSecret, opts.TenantID) + } + return cs.loginService.Login(ctx, opts.TenantID) +} + +func (cs *aciCloudService) Logout(ctx context.Context) error { + return cs.loginService.Logout(ctx) +} + +func (cs *aciCloudService) CreateContextData(ctx context.Context, params interface{}) (interface{}, string, error) { + contextHelper := newContextCreateHelper() + createOpts := params.(ContextParams) + return contextHelper.createContextData(ctx, createOpts) +} diff --git a/aci/compose.go b/aci/compose.go new file mode 100644 index 00000000..07759a73 --- /dev/null +++ b/aci/compose.go @@ -0,0 +1,128 @@ +/* + Copyright 2020 Docker, Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package aci + +import ( + "context" + "fmt" + "io" + "net/http" + + "github.com/compose-spec/compose-go/types" + "github.com/sirupsen/logrus" + + "github.com/docker/compose-cli/aci/convert" + "github.com/docker/compose-cli/aci/login" + "github.com/docker/compose-cli/api/compose" + "github.com/docker/compose-cli/context/store" + "github.com/docker/compose-cli/errdefs" +) + +type aciComposeService struct { + ctx store.AciContext +} + +func (cs *aciComposeService) Up(ctx context.Context, project *types.Project) error { + logrus.Debugf("Up on project with name %q\n", project.Name) + groupDefinition, err := convert.ToContainerGroup(ctx, cs.ctx, *project) + addTag(&groupDefinition, composeContainerTag) + + if err != nil { + return err + } + return createOrUpdateACIContainers(ctx, cs.ctx, groupDefinition) +} + +func (cs *aciComposeService) Down(ctx context.Context, project string) error { + logrus.Debugf("Down on project with name %q\n", project) + + cg, err := deleteACIContainerGroup(ctx, cs.ctx, project) + if err != nil { + return err + } + if cg.StatusCode == http.StatusNoContent { + return errdefs.ErrNotFound + } + + return err +} + +func (cs *aciComposeService) Ps(ctx context.Context, project string) ([]compose.ServiceStatus, error) { + groupsClient, err := login.NewContainerGroupsClient(cs.ctx.SubscriptionID) + if err != nil { + return nil, err + } + + group, err := groupsClient.Get(ctx, cs.ctx.ResourceGroup, project) + if err != nil { + return []compose.ServiceStatus{}, err + } + + if group.Containers == nil || len(*group.Containers) < 1 { + return []compose.ServiceStatus{}, fmt.Errorf("no containers found in ACI container group %s", project) + } + + res := []compose.ServiceStatus{} + for _, container := range *group.Containers { + if isContainerVisible(container, group, false) { + continue + } + res = append(res, convert.ContainerGroupToServiceStatus(getContainerID(group, container), group, container)) + } + return res, nil +} + +func (cs *aciComposeService) List(ctx context.Context, project string) ([]compose.Stack, error) { + containerGroups, err := getACIContainerGroups(ctx, cs.ctx.SubscriptionID, cs.ctx.ResourceGroup) + if err != nil { + return []compose.Stack{}, err + } + + stacks := []compose.Stack{} + for _, group := range containerGroups { + if _, found := group.Tags[composeContainerTag]; !found { + continue + } + if project != "" && *group.Name != project { + continue + } + state := compose.RUNNING + for _, container := range *group.ContainerGroupProperties.Containers { + containerState := convert.GetStatus(container, group) + if containerState != compose.RUNNING { + state = containerState + break + } + } + stacks = append(stacks, compose.Stack{ + ID: *group.ID, + Name: *group.Name, + Status: state, + }) + } + return stacks, nil +} + +func (cs *aciComposeService) Logs(ctx context.Context, project string, w io.Writer) error { + return errdefs.ErrNotImplemented +} + +func (cs *aciComposeService) Convert(ctx context.Context, project *types.Project) ([]byte, error) { + return nil, errdefs.ErrNotImplemented +} + + diff --git a/aci/containers.go b/aci/containers.go new file mode 100644 index 00000000..08cacbc3 --- /dev/null +++ b/aci/containers.go @@ -0,0 +1,254 @@ +/* + Copyright 2020 Docker, Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package aci + +import ( + "context" + "fmt" + "net/http" + "strconv" + "strings" + + "github.com/Azure/azure-sdk-for-go/services/containerinstance/mgmt/2018-10-01/containerinstance" + "github.com/Azure/go-autorest/autorest" + "github.com/Azure/go-autorest/autorest/to" + "github.com/pkg/errors" + "github.com/sirupsen/logrus" + + "github.com/docker/compose-cli/aci/convert" + "github.com/docker/compose-cli/aci/login" + "github.com/docker/compose-cli/api/containers" + "github.com/docker/compose-cli/context/store" + "github.com/docker/compose-cli/errdefs" +) + +type aciContainerService struct { + ctx store.AciContext +} + +func (cs *aciContainerService) List(ctx context.Context, all bool) ([]containers.Container, error) { + containerGroups, err := getACIContainerGroups(ctx, cs.ctx.SubscriptionID, cs.ctx.ResourceGroup) + if err != nil { + return []containers.Container{}, err + } + var res []containers.Container + for _, group := range containerGroups { + if group.Containers == nil || len(*group.Containers) < 1 { + return []containers.Container{}, fmt.Errorf("no containers found in ACI container group %s", *group.Name) + } + + for _, container := range *group.Containers { + if isContainerVisible(container, group, all) { + continue + } + c := convert.ContainerGroupToContainer(getContainerID(group, container), group, container) + res = append(res, c) + } + } + return res, nil +} + + +func (cs *aciContainerService) Run(ctx context.Context, r containers.ContainerConfig) error { + if strings.Contains(r.ID, composeContainerSeparator) { + return errors.New(fmt.Sprintf("invalid container name. ACI container name cannot include %q", composeContainerSeparator)) + } + + project, err := convert.ContainerToComposeProject(r) + if err != nil { + return err + } + + logrus.Debugf("Running container %q with name %q\n", r.Image, r.ID) + groupDefinition, err := convert.ToContainerGroup(ctx, cs.ctx, project) + if err != nil { + return err + } + addTag(&groupDefinition, singleContainerTag) + + return createACIContainers(ctx, cs.ctx, groupDefinition) +} + +func (cs *aciContainerService) Start(ctx context.Context, containerID string) error { + groupName, containerName := getGroupAndContainerName(containerID) + if groupName != containerID { + msg := "cannot start specified service %q from compose application %q, you can update and restart the entire compose app with docker compose up --project-name %s" + return errors.New(fmt.Sprintf(msg, containerName, groupName, groupName)) + } + + containerGroupsClient, err := login.NewContainerGroupsClient(cs.ctx.SubscriptionID) + if err != nil { + return err + } + + future, err := containerGroupsClient.Start(ctx, cs.ctx.ResourceGroup, containerName) + if err != nil { + var aerr autorest.DetailedError + if ok := errors.As(err, &aerr); ok { + if aerr.StatusCode == http.StatusNotFound { + return errdefs.ErrNotFound + } + } + return err + } + + return future.WaitForCompletionRef(ctx, containerGroupsClient.Client) +} + +func (cs *aciContainerService) Stop(ctx context.Context, containerID string, timeout *uint32) error { + if timeout != nil && *timeout != uint32(0) { + return errors.Errorf("ACI integration does not support setting a timeout to stop a container before killing it.") + } + groupName, containerName := getGroupAndContainerName(containerID) + if groupName != containerID { + msg := "cannot stop service %q from compose application %q, you can stop the entire compose app with docker stop %s" + return errors.New(fmt.Sprintf(msg, containerName, groupName, groupName)) + } + return stopACIContainerGroup(ctx, cs.ctx, groupName) +} + +func (cs *aciContainerService) Kill(ctx context.Context, containerID string, _ string) error { + groupName, containerName := getGroupAndContainerName(containerID) + if groupName != containerID { + msg := "cannot kill service %q from compose application %q, you can kill the entire compose app with docker kill %s" + return errors.New(fmt.Sprintf(msg, containerName, groupName, groupName)) + } + return stopACIContainerGroup(ctx, cs.ctx, groupName) // As ACI doesn't have a kill command, we are using the stop implementation instead +} + +func (cs *aciContainerService) Exec(ctx context.Context, name string, request containers.ExecRequest) error { + err := verifyExecCommand(request.Command) + if err != nil { + return err + } + groupName, containerAciName := getGroupAndContainerName(name) + containerExecResponse, err := execACIContainer(ctx, cs.ctx, request.Command, groupName, containerAciName) + if err != nil { + return err + } + + return exec( + context.Background(), + *containerExecResponse.WebSocketURI, + *containerExecResponse.Password, + request, + ) +} + +func verifyExecCommand(command string) error { + tokens := strings.Split(command, " ") + if len(tokens) > 1 { + return errors.New("ACI exec command does not accept arguments to the command. " + + "Only the binary should be specified") + } + return nil +} + +func (cs *aciContainerService) Logs(ctx context.Context, containerName string, req containers.LogsRequest) error { + groupName, containerAciName := getGroupAndContainerName(containerName) + var tail *int32 + + if req.Follow { + return streamLogs(ctx, cs.ctx, groupName, containerAciName, req) + } + + if req.Tail != "all" { + reqTail, err := strconv.Atoi(req.Tail) + if err != nil { + return err + } + i32 := int32(reqTail) + tail = &i32 + } + + logs, err := getACIContainerLogs(ctx, cs.ctx, groupName, containerAciName, tail) + if err != nil { + return err + } + + _, err = fmt.Fprint(req.Writer, logs) + return err +} + +func (cs *aciContainerService) Delete(ctx context.Context, containerID string, request containers.DeleteRequest) error { + groupName, containerName := getGroupAndContainerName(containerID) + if groupName != containerID { + msg := "cannot delete service %q from compose application %q, you can delete the entire compose app with docker compose down --project-name %s" + return errors.New(fmt.Sprintf(msg, containerName, groupName, groupName)) + } + + if !request.Force { + containerGroupsClient, err := login.NewContainerGroupsClient(cs.ctx.SubscriptionID) + if err != nil { + return err + } + + cg, err := containerGroupsClient.Get(ctx, cs.ctx.ResourceGroup, groupName) + if err != nil { + if cg.StatusCode == http.StatusNotFound { + return errdefs.ErrNotFound + } + return err + } + + for _, container := range *cg.Containers { + status := convert.GetStatus(container, cg) + + if status == convert.StatusRunning { + return errdefs.ErrForbidden + } + } + } + + cg, err := deleteACIContainerGroup(ctx, cs.ctx, groupName) + // Delete returns `StatusNoContent` if the group is not found + if cg.StatusCode == http.StatusNoContent { + return errdefs.ErrNotFound + } + if err != nil { + return err + } + + return err +} + +func (cs *aciContainerService) Inspect(ctx context.Context, containerID string) (containers.Container, error) { + groupName, containerName := getGroupAndContainerName(containerID) + + cg, err := getACIContainerGroup(ctx, cs.ctx, groupName) + if err != nil { + return containers.Container{}, err + } + if cg.StatusCode == http.StatusNoContent { + return containers.Container{}, errdefs.ErrNotFound + } + + var cc containerinstance.Container + var found = false + for _, c := range *cg.Containers { + if to.String(c.Name) == containerName { + cc = c + found = true + break + } + } + if !found { + return containers.Container{}, errdefs.ErrNotFound + } + + return convert.ContainerGroupToContainer(containerID, cg, cc), nil +} diff --git a/aci/convert/convert.go b/aci/convert/convert.go index 65ce5683..d35b410b 100644 --- a/aci/convert/convert.go +++ b/aci/convert/convert.go @@ -56,7 +56,7 @@ const ( func ToContainerGroup(ctx context.Context, aciContext store.AciContext, p types.Project) (containerinstance.ContainerGroup, error) { project := projectAciHelper(p) containerGroupName := strings.ToLower(project.Name) - storageHelper := login.StorageAccountHelper{ + storageHelper := login.StorageLogin{ AciContext: aciContext, } volumesCache, volumesSlice, err := project.getAciFileVolumes(ctx, storageHelper) @@ -200,7 +200,7 @@ func (p projectAciHelper) getAciSecretVolumes() ([]containerinstance.Volume, err return secretVolumes, nil } -func (p projectAciHelper) getAciFileVolumes(ctx context.Context, helper login.StorageAccountHelper) (map[string]bool, []containerinstance.Volume, error) { +func (p projectAciHelper) getAciFileVolumes(ctx context.Context, helper login.StorageLogin) (map[string]bool, []containerinstance.Volume, error) { azureFileVolumesMap := make(map[string]bool, len(p.Volumes)) var azureFileVolumesSlice []containerinstance.Volume for name, v := range p.Volumes { diff --git a/aci/login/storagelogin.go b/aci/login/storagelogin.go new file mode 100644 index 00000000..446eb56d --- /dev/null +++ b/aci/login/storagelogin.go @@ -0,0 +1,49 @@ +/* + Copyright 2020 Docker, Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package login + +import ( + "context" + "fmt" + + "github.com/pkg/errors" + + "github.com/docker/compose-cli/context/store" +) + +// StorageLogin helper for Azure Storage Login +type StorageLogin struct { + AciContext store.AciContext +} + +// GetAzureStorageAccountKey retrieves the storage account ket from the current azure login +func (helper StorageLogin) GetAzureStorageAccountKey(ctx context.Context, accountName string) (string, error) { + client, err := NewStorageAccountsClient(helper.AciContext.SubscriptionID) + if err != nil { + return "", err + } + result, err := client.ListKeys(ctx, helper.AciContext.ResourceGroup, accountName, "") + if err != nil { + return "", errors.Wrap(err, fmt.Sprintf("could not access storage account acountKeys for %s, using the azure login", accountName)) + } + if result.Keys != nil && len((*result.Keys)) < 1 { + return "", fmt.Errorf("no key could be obtained for storage account %s from your azure login", accountName) + } + + key := (*result.Keys)[0] + return *key.Value, nil +} \ No newline at end of file diff --git a/aci/login/storage_helper.go b/aci/volumes.go similarity index 53% rename from aci/login/storage_helper.go rename to aci/volumes.go index de81c5b3..3ce505bc 100644 --- a/aci/login/storage_helper.go +++ b/aci/volumes.go @@ -14,11 +14,12 @@ limitations under the License. */ -package login +package aci import ( "context" "fmt" + "github.com/docker/compose-cli/aci/login" "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2019-06-01/storage" @@ -30,48 +31,27 @@ import ( "github.com/docker/compose-cli/context/store" ) -// StorageAccountHelper helper for Azure Storage Account -type StorageAccountHelper struct { - AciContext store.AciContext +type aciVolumeService struct { + aciContext store.AciContext } -// GetAzureStorageAccountKey retrieves the storage account ket from the current azure login -func (helper StorageAccountHelper) GetAzureStorageAccountKey(ctx context.Context, accountName string) (string, error) { - client, err := NewStorageAccountsClient(helper.AciContext.SubscriptionID) - if err != nil { - return "", err - } - result, err := client.ListKeys(ctx, helper.AciContext.ResourceGroup, accountName, "") - if err != nil { - return "", errors.Wrap(err, fmt.Sprintf("could not access storage account acountKeys for %s, using the azure login", accountName)) - } - if result.Keys != nil && len((*result.Keys)) < 1 { - return "", fmt.Errorf("no key could be obtained for storage account %s from your azure login", accountName) - } - - key := (*result.Keys)[0] - return *key.Value, nil -} - -// ListFileShare list file shares in all visible storage accounts -func (helper StorageAccountHelper) ListFileShare(ctx context.Context) ([]volumes.Volume, error) { - aciContext := helper.AciContext - accountClient, err := NewStorageAccountsClient(aciContext.SubscriptionID) +func (cs *aciVolumeService) List(ctx context.Context) ([]volumes.Volume, error) { + accountClient, err := login.NewStorageAccountsClient(cs.aciContext.SubscriptionID) if err != nil { return nil, err } - result, err := accountClient.ListByResourceGroup(ctx, aciContext.ResourceGroup) + result, err := accountClient.ListByResourceGroup(ctx, cs.aciContext.ResourceGroup) if err != nil { return nil, err } accounts := result.Value - fileShareClient, err := NewFileShareClient(aciContext.SubscriptionID) + fileShareClient, err := login.NewFileShareClient(cs.aciContext.SubscriptionID) if err != nil { return nil, err } fileShares := []volumes.Volume{} for _, account := range *accounts { - fileSharePage, err := fileShareClient.List(ctx, aciContext.ResourceGroup, *account.Name, "", "", "") + fileSharePage, err := fileShareClient.List(ctx, cs.aciContext.ResourceGroup, *account.Name, "", "", "") if err != nil { return nil, err } @@ -89,30 +69,30 @@ func (helper StorageAccountHelper) ListFileShare(ctx context.Context) ([]volumes return fileShares, nil } -func toVolume(account storage.Account, fileShareName string) volumes.Volume { - return volumes.Volume{ - ID: fmt.Sprintf("%s@%s", *account.Name, fileShareName), - Name: fileShareName, - Description: fmt.Sprintf("Fileshare %s in %s storage account", fileShareName, *account.Name), - } +//VolumeCreateOptions options to create a new ACI volume +type VolumeCreateOptions struct { + Account string + Fileshare string } -// CreateFileShare create a new fileshare -func (helper StorageAccountHelper) CreateFileShare(ctx context.Context, accountName string, fileShareName string) (volumes.Volume, error) { - aciContext := helper.AciContext - accountClient, err := NewStorageAccountsClient(aciContext.SubscriptionID) +func (cs *aciVolumeService) Create(ctx context.Context, options interface{}) (volumes.Volume, error) { + opts, ok := options.(VolumeCreateOptions) + if !ok { + return volumes.Volume{}, errors.New("Could not read azure LoginParams struct from generic parameter") + } + accountClient, err := login.NewStorageAccountsClient(cs.aciContext.SubscriptionID) if err != nil { return volumes.Volume{}, err } - account, err := accountClient.GetProperties(ctx, aciContext.ResourceGroup, accountName, "") + account, err := accountClient.GetProperties(ctx, cs.aciContext.ResourceGroup, opts.Account, "") if err != nil { if account.StatusCode != 404 { return volumes.Volume{}, err } //TODO confirm storage account creation - parameters := defaultStorageAccountParams(aciContext) + parameters := defaultStorageAccountParams(cs.aciContext) // TODO progress account creation - future, err := accountClient.Create(ctx, aciContext.ResourceGroup, accountName, parameters) + future, err := accountClient.Create(ctx, cs.aciContext.ResourceGroup, opts.Account, parameters) if err != nil { return volumes.Volume{}, err } @@ -121,25 +101,33 @@ func (helper StorageAccountHelper) CreateFileShare(ctx context.Context, accountN return volumes.Volume{}, err } } - fileShareClient, err := NewFileShareClient(aciContext.SubscriptionID) + fileShareClient, err := login.NewFileShareClient(cs.aciContext.SubscriptionID) if err != nil { return volumes.Volume{}, err } - fileShare, err := fileShareClient.Get(ctx, aciContext.ResourceGroup, *account.Name, fileShareName, "") + fileShare, err := fileShareClient.Get(ctx, cs.aciContext.ResourceGroup, *account.Name, opts.Fileshare, "") if err == nil { - return volumes.Volume{}, errors.Wrapf(errdefs.ErrAlreadyExists, "Azure fileshare %q already exists", fileShareName) + return volumes.Volume{}, errors.Wrapf(errdefs.ErrAlreadyExists, "Azure fileshare %q already exists", opts.Fileshare) } if fileShare.StatusCode != 404 { return volumes.Volume{}, err } - fileShare, err = fileShareClient.Create(ctx, aciContext.ResourceGroup, *account.Name, fileShareName, storage.FileShare{}) + fileShare, err = fileShareClient.Create(ctx, cs.aciContext.ResourceGroup, *account.Name, opts.Fileshare, storage.FileShare{}) if err != nil { return volumes.Volume{}, err } return toVolume(account, *fileShare.Name), nil } +func toVolume(account storage.Account, fileShareName string) volumes.Volume { + return volumes.Volume{ + ID: fmt.Sprintf("%s@%s", *account.Name, fileShareName), + Name: fileShareName, + Description: fmt.Sprintf("Fileshare %s in %s storage account", fileShareName, *account.Name), + } +} + func defaultStorageAccountParams(aciContext store.AciContext) storage.AccountCreateParameters { return storage.AccountCreateParameters{ Location: &aciContext.Location, From 2f672f6c4c8f85a0368967659d2064335840f0b2 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Tue, 8 Sep 2020 14:20:01 +0200 Subject: [PATCH 06/13] Volume e2e test Signed-off-by: Guillaume Tardif --- aci/volumes.go | 10 +++- tests/aci-e2e/e2e-aci_test.go | 93 ++++++++++++++++---------------- tests/aci-e2e/storage/storage.go | 65 ++-------------------- 3 files changed, 60 insertions(+), 108 deletions(-) diff --git a/aci/volumes.go b/aci/volumes.go index 3ce505bc..0b992a52 100644 --- a/aci/volumes.go +++ b/aci/volumes.go @@ -19,6 +19,7 @@ package aci import ( "context" "fmt" + "github.com/Azure/go-autorest/autorest/to" "github.com/docker/compose-cli/aci/login" "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2019-06-01/storage" @@ -96,6 +97,10 @@ func (cs *aciVolumeService) Create(ctx context.Context, options interface{}) (vo if err != nil { return volumes.Volume{}, err } + err = future.WaitForCompletionRef(ctx, accountClient.Client) + if err != nil { + return volumes.Volume{}, err + } account, err = future.Result(accountClient) if err != nil { return volumes.Volume{}, err @@ -130,10 +135,11 @@ func toVolume(account storage.Account, fileShareName string) volumes.Volume { func defaultStorageAccountParams(aciContext store.AciContext) storage.AccountCreateParameters { return storage.AccountCreateParameters{ - Location: &aciContext.Location, + Location: to.StringPtr(aciContext.Location), Sku: &storage.Sku{ Name: storage.StandardLRS, - Tier: storage.Standard, }, + Kind:storage.StorageV2, + AccountPropertiesCreateParameters: &storage.AccountPropertiesCreateParameters{}, } } diff --git a/tests/aci-e2e/e2e-aci_test.go b/tests/aci-e2e/e2e-aci_test.go index a31335d4..2a360f6e 100644 --- a/tests/aci-e2e/e2e-aci_test.go +++ b/tests/aci-e2e/e2e-aci_test.go @@ -39,7 +39,6 @@ import ( "gotest.tools/v3/poll" "github.com/Azure/azure-sdk-for-go/profiles/2019-03-01/resources/mgmt/resources" - azure_storage "github.com/Azure/azure-sdk-for-go/profiles/2019-03-01/storage/mgmt/storage" "github.com/Azure/azure-storage-file-go/azfile" "github.com/Azure/go-autorest/autorest/to" @@ -131,7 +130,7 @@ func TestContainerRunVolume(t *testing.T) { sID, rg := setupTestResourceGroup(t, c) const ( - testShareName = "dockertestshare" + fileshareName = "dockertestshare" testFileContent = "Volume mounted successfully!" testFileName = "index.html" ) @@ -142,31 +141,56 @@ func TestContainerRunVolume(t *testing.T) { Location: location, ResourceGroup: rg, } - saName := "e2e" + strconv.Itoa(int(time.Now().UnixNano())) - _, cleanupSa := createStorageAccount(t, aciContext, saName) - t.Cleanup(func() { - if err := cleanupSa(); err != nil { - t.Error(err) - } - }) - keys := getStorageKeys(t, aciContext, saName) - assert.Assert(t, len(keys) > 0) - k := *keys[0].Value - cred, u := createFileShare(t, k, testShareName, saName) - uploadFile(t, *cred, u.String(), testFileName, testFileContent) // Used in subtests var ( - container string - hostIP string - endpoint string + container string + hostIP string + endpoint string + volumeID string + accountName = "e2e" + strconv.Itoa(int(time.Now().UnixNano())) ) + t.Run("Create volumes", func(t *testing.T) { + c.RunDockerCmd("volume", "create", "--storage-account", accountName, "--fileshare", fileshareName) + }) + t.Cleanup(func() { deleteStorageAccount(t, aciContext, accountName) }) + + t.Run("Create second fileshare", func(t *testing.T) { + c.RunDockerCmd("volume", "create", "--storage-account", accountName, "--fileshare", "dockertestshare2") + }) + + t.Run("list volumes", func(t *testing.T) { + res := c.RunDockerCmd("volume", "ls") + out := strings.Split(strings.TrimSpace(res.Stdout()), "\n") + firstAccount := out[1] + fields := strings.Fields(firstAccount) + volumeID = accountName + "@" + fileshareName + assert.Equal(t, fields[0], volumeID) + assert.Equal(t, fields[1], fileshareName) + secondAccount := out[2] + fields = strings.Fields(secondAccount) + assert.Equal(t, fields[0], accountName + "@dockertestshare2") + assert.Equal(t, fields[1], "dockertestshare2") + //assert.Assert(t, fields[2], strings.Contains(firstAccount, fmt.Sprintf("Fileshare %s in %s storage account", fileshareName, accountName))) + }) + + t.Run("upload file", func(t *testing.T) { + storageLogin := login.StorageLogin{AciContext: aciContext} + + key, err := storageLogin.GetAzureStorageAccountKey(context.TODO(), accountName) + assert.NilError(t, err) + cred, err := azfile.NewSharedKeyCredential(accountName, key) + assert.NilError(t, err) + u, _ := url.Parse(fmt.Sprintf("https://%s.file.core.windows.net/%s", accountName, fileshareName)) + uploadFile(t, *cred, u.String(), testFileName, testFileContent) + }) + t.Run("run", func(t *testing.T) { mountTarget := "/usr/share/nginx/html" res := c.RunDockerCmd( "run", "-d", - "-v", fmt.Sprintf("%s@%s:%s", saName, testShareName, mountTarget), + "-v", fmt.Sprintf("%s:%s", volumeID, mountTarget), "-p", "80:80", "nginx", ) @@ -637,35 +661,12 @@ func createAciContextAndUseIt(t *testing.T, c *E2eCLI, sID, rgName string) { res.Assert(t, icmd.Expected{Out: contextName + " *"}) } -func createStorageAccount(t *testing.T, aciContext store.AciContext, name string) (azure_storage.Account, func() error) { - account, err := storage.CreateStorageAccount(context.TODO(), aciContext, name) - assert.Check(t, is.Nil(err)) - assert.Check(t, is.Equal(*(account.Name), name)) - return account, func() error { return deleteStorageAccount(aciContext, name) } -} - -func deleteStorageAccount(aciContext store.AciContext, name string) error { +func deleteStorageAccount(t *testing.T, aciContext store.AciContext, name string) { + fmt.Printf(" [%s] deleting storage account %s\n", t.Name(), name) _, err := storage.DeleteStorageAccount(context.TODO(), aciContext, name) - return err -} - -func getStorageKeys(t *testing.T, aciContext store.AciContext, saName string) []azure_storage.AccountKey { - l, err := storage.ListKeys(context.TODO(), aciContext, saName) - assert.NilError(t, err) - assert.Assert(t, l.Keys != nil) - return *l.Keys -} - -func createFileShare(t *testing.T, key, share, storageAccount string) (*azfile.SharedKeyCredential, *url.URL) { - // Create a ShareURL object that wraps a soon-to-be-created share's URL and a default pipeline. - u, _ := url.Parse(fmt.Sprintf("https://%s.file.core.windows.net/%s", storageAccount, share)) - cred, err := azfile.NewSharedKeyCredential(storageAccount, key) - assert.NilError(t, err) - - shareURL := azfile.NewShareURL(*u, azfile.NewPipeline(cred, azfile.PipelineOptions{})) - _, err = shareURL.Create(context.TODO(), azfile.Metadata{}, 0) - assert.NilError(t, err) - return cred, u + if err != nil { + t.Error(err) + } } func uploadFile(t *testing.T, cred azfile.SharedKeyCredential, baseURL, fileName, content string) { diff --git a/tests/aci-e2e/storage/storage.go b/tests/aci-e2e/storage/storage.go index 77eff6f0..9e83d1f3 100644 --- a/tests/aci-e2e/storage/storage.go +++ b/tests/aci-e2e/storage/storage.go @@ -18,76 +18,21 @@ package storage import ( "context" - "errors" - - "github.com/Azure/azure-sdk-for-go/profiles/2019-03-01/storage/mgmt/storage" "github.com/Azure/go-autorest/autorest" - "github.com/Azure/go-autorest/autorest/to" - "github.com/docker/compose-cli/aci/login" "github.com/docker/compose-cli/context/store" ) -// CreateStorageAccount creates a new storage account. -func CreateStorageAccount(ctx context.Context, aciContext store.AciContext, accountName string) (storage.Account, error) { - storageAccountsClient := getStorageAccountsClient(aciContext) - result, err := storageAccountsClient.CheckNameAvailability( - ctx, - storage.AccountCheckNameAvailabilityParameters{ - Name: to.StringPtr(accountName), - Type: to.StringPtr("Microsoft.Storage/storageAccounts"), - }) - - if err != nil { - return storage.Account{}, err - } - if !*result.NameAvailable { - return storage.Account{}, errors.New("storage account name already exists" + accountName) - } - - future, err := storageAccountsClient.Create( - ctx, - aciContext.ResourceGroup, - accountName, - storage.AccountCreateParameters{ - Sku: &storage.Sku{ - Name: storage.StandardLRS, - }, - Location: to.StringPtr(aciContext.Location), - AccountPropertiesCreateParameters: &storage.AccountPropertiesCreateParameters{}}) - if err != nil { - return storage.Account{}, err - } - err = future.WaitForCompletionRef(ctx, storageAccountsClient.Client) - if err != nil { - return storage.Account{}, err - } - return future.Result(storageAccountsClient) -} // DeleteStorageAccount deletes a given storage account func DeleteStorageAccount(ctx context.Context, aciContext store.AciContext, accountName string) (autorest.Response, error) { - storageAccountsClient := getStorageAccountsClient(aciContext) + storageAccountsClient, err := login.NewStorageAccountsClient(aciContext.SubscriptionID) + if err != nil { + return autorest.Response{}, err + } response, err := storageAccountsClient.Delete(ctx, aciContext.ResourceGroup, accountName) if err != nil { return autorest.Response{}, err } return response, err -} - -// ListKeys lists the storage account keys -func ListKeys(ctx context.Context, aciContext store.AciContext, accountName string) (storage.AccountListKeysResult, error) { - storageAccountsClient := getStorageAccountsClient(aciContext) - keys, err := storageAccountsClient.ListKeys(ctx, aciContext.ResourceGroup, accountName) - if err != nil { - return storage.AccountListKeysResult{}, err - } - return keys, nil -} - -func getStorageAccountsClient(aciContext store.AciContext) storage.AccountsClient { - storageAccountsClient := storage.NewAccountsClient(aciContext.SubscriptionID) - autho, _ := login.NewAuthorizerFromLogin() - storageAccountsClient.Authorizer = autho - return storageAccountsClient -} +} \ No newline at end of file From 96d785a5bd8d9b5f7c137deaf8da743acd15ba9f Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Tue, 8 Sep 2020 15:27:56 +0200 Subject: [PATCH 07/13] Added volume delete, can delete juste a file share or the storage account if confirmed Signed-off-by: Guillaume Tardif --- aci/backend.go | 3 +- aci/cloud.go | 2 + aci/compose.go | 2 - aci/containers.go | 1 - aci/login/storagelogin.go | 2 +- aci/volumes.go | 49 ++++++++++++++++++++- api/client/volume.go | 6 ++- api/volumes/api.go | 3 ++ cli/cmd/secrets_test.go | 2 +- cli/cmd/volume/list.go | 6 ++- cli/cmd/volume/list_test.go | 23 ++++++++-- cli/cmd/volume/volume.go | 30 +++++++++++-- tests/aci-e2e/e2e-aci_test.go | 73 ++++++++++++++++++++------------ tests/aci-e2e/storage/storage.go | 38 ----------------- 14 files changed, 156 insertions(+), 84 deletions(-) delete mode 100644 tests/aci-e2e/storage/storage.go diff --git a/aci/backend.go b/aci/backend.go index 955c487a..a5e08425 100644 --- a/aci/backend.go +++ b/aci/backend.go @@ -40,6 +40,7 @@ const ( backendType = store.AciContextType singleContainerTag = "docker-single-container" composeContainerTag = "docker-compose-application" + dockerVolumeTag = "docker-volume" composeContainerSeparator = "_" ) @@ -149,7 +150,6 @@ func addTag(groupDefinition *containerinstance.ContainerGroup, tagName string) { groupDefinition.Tags[tagName] = to.StringPtr(tagName) } - func getGroupAndContainerName(containerID string) (string, string) { tokens := strings.Split(containerID, composeContainerSeparator) groupName := tokens[0] @@ -160,4 +160,3 @@ func getGroupAndContainerName(containerID string) (string, string) { } return groupName, containerName } - diff --git a/aci/cloud.go b/aci/cloud.go index 2d2e9757..0dee380f 100644 --- a/aci/cloud.go +++ b/aci/cloud.go @@ -18,7 +18,9 @@ package aci import ( "context" + "github.com/pkg/errors" + "github.com/docker/compose-cli/aci/login" ) diff --git a/aci/compose.go b/aci/compose.go index 07759a73..d7fd4e32 100644 --- a/aci/compose.go +++ b/aci/compose.go @@ -124,5 +124,3 @@ func (cs *aciComposeService) Logs(ctx context.Context, project string, w io.Writ func (cs *aciComposeService) Convert(ctx context.Context, project *types.Project) ([]byte, error) { return nil, errdefs.ErrNotImplemented } - - diff --git a/aci/containers.go b/aci/containers.go index 08cacbc3..afc48ec3 100644 --- a/aci/containers.go +++ b/aci/containers.go @@ -62,7 +62,6 @@ func (cs *aciContainerService) List(ctx context.Context, all bool) ([]containers return res, nil } - func (cs *aciContainerService) Run(ctx context.Context, r containers.ContainerConfig) error { if strings.Contains(r.ID, composeContainerSeparator) { return errors.New(fmt.Sprintf("invalid container name. ACI container name cannot include %q", composeContainerSeparator)) diff --git a/aci/login/storagelogin.go b/aci/login/storagelogin.go index 446eb56d..13a3923f 100644 --- a/aci/login/storagelogin.go +++ b/aci/login/storagelogin.go @@ -46,4 +46,4 @@ func (helper StorageLogin) GetAzureStorageAccountKey(ctx context.Context, accoun key := (*result.Keys)[0] return *key.Value, nil -} \ No newline at end of file +} diff --git a/aci/volumes.go b/aci/volumes.go index 0b992a52..78039d66 100644 --- a/aci/volumes.go +++ b/aci/volumes.go @@ -19,7 +19,9 @@ package aci import ( "context" "fmt" + "github.com/Azure/go-autorest/autorest/to" + "github.com/docker/compose-cli/aci/login" "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2019-06-01/storage" @@ -76,6 +78,13 @@ type VolumeCreateOptions struct { Fileshare string } +//VolumeDeleteOptions options to create a new ACI volume +type VolumeDeleteOptions struct { + Account string + Fileshare string + DeleteAccount bool +} + func (cs *aciVolumeService) Create(ctx context.Context, options interface{}) (volumes.Volume, error) { opts, ok := options.(VolumeCreateOptions) if !ok { @@ -91,6 +100,16 @@ func (cs *aciVolumeService) Create(ctx context.Context, options interface{}) (vo return volumes.Volume{}, err } //TODO confirm storage account creation + result, err := accountClient.CheckNameAvailability(ctx, storage.AccountCheckNameAvailabilityParameters{ + Name: to.StringPtr(opts.Account), + Type: to.StringPtr("Microsoft.Storage/storageAccounts"), + }) + if err != nil { + return volumes.Volume{}, err + } + if !*result.NameAvailable { + return volumes.Volume{}, errors.New("error: " + *result.Message) + } parameters := defaultStorageAccountParams(cs.aciContext) // TODO progress account creation future, err := accountClient.Create(ctx, cs.aciContext.ResourceGroup, opts.Account, parameters) @@ -125,6 +144,32 @@ func (cs *aciVolumeService) Create(ctx context.Context, options interface{}) (vo return toVolume(account, *fileShare.Name), nil } +func (cs *aciVolumeService) Delete(ctx context.Context, options interface{}) error { + opts, ok := options.(VolumeDeleteOptions) + if !ok { + return errors.New("Could not read azure VolumeDeleteOptions struct from generic parameter") + } + if opts.DeleteAccount { + //TODO check if there are other shares on this account + //TODO flag account and only delete ours + storageAccountsClient, err := login.NewStorageAccountsClient(cs.aciContext.SubscriptionID) + if err != nil { + return err + } + + _, err = storageAccountsClient.Delete(ctx, cs.aciContext.ResourceGroup, opts.Account) + return err + } + + fileShareClient, err := login.NewFileShareClient(cs.aciContext.SubscriptionID) + if err != nil { + return err + } + + _, err = fileShareClient.Delete(ctx, cs.aciContext.ResourceGroup, opts.Account, opts.Fileshare) + return err +} + func toVolume(account storage.Account, fileShareName string) volumes.Volume { return volumes.Volume{ ID: fmt.Sprintf("%s@%s", *account.Name, fileShareName), @@ -134,12 +179,12 @@ func toVolume(account storage.Account, fileShareName string) volumes.Volume { } func defaultStorageAccountParams(aciContext store.AciContext) storage.AccountCreateParameters { + tags := map[string]*string{dockerVolumeTag: to.StringPtr(dockerVolumeTag)} return storage.AccountCreateParameters{ Location: to.StringPtr(aciContext.Location), Sku: &storage.Sku{ Name: storage.StandardLRS, }, - Kind:storage.StorageV2, - AccountPropertiesCreateParameters: &storage.AccountPropertiesCreateParameters{}, + Tags: tags, } } diff --git a/api/client/volume.go b/api/client/volume.go index 34adfae9..ea0ee378 100644 --- a/api/client/volume.go +++ b/api/client/volume.go @@ -26,12 +26,14 @@ import ( type volumeService struct { } -// List list volumes func (c *volumeService) List(ctx context.Context) ([]volumes.Volume, error) { return nil, errdefs.ErrNotImplemented } -// Create creates a volume func (c *volumeService) Create(ctx context.Context, options interface{}) (volumes.Volume, error) { return volumes.Volume{}, errdefs.ErrNotImplemented } + +func (c *volumeService) Delete(ctx context.Context, options interface{}) error { + return errdefs.ErrNotImplemented +} diff --git a/api/volumes/api.go b/api/volumes/api.go index 59a8b1d7..29562f7e 100644 --- a/api/volumes/api.go +++ b/api/volumes/api.go @@ -31,5 +31,8 @@ type Volume struct { type Service interface { // List returns all available volumes List(ctx context.Context) ([]Volume, error) + // Create creates a new volume Create(ctx context.Context, options interface{}) (Volume, error) + // Delete deletes an existing volume + Delete(ctx context.Context, options interface{}) error } diff --git a/cli/cmd/secrets_test.go b/cli/cmd/secrets_test.go index 37bb25c3..70968136 100644 --- a/cli/cmd/secrets_test.go +++ b/cli/cmd/secrets_test.go @@ -35,5 +35,5 @@ func TestPrintList(t *testing.T) { } out := &bytes.Buffer{} printList(out, secrets) - golden.Assert(t, out.String(), "volumes-out.golden") + golden.Assert(t, out.String(), "secrets-out.golden") } diff --git a/cli/cmd/volume/list.go b/cli/cmd/volume/list.go index 7e8c4e33..3158cce9 100644 --- a/cli/cmd/volume/list.go +++ b/cli/cmd/volume/list.go @@ -1,5 +1,3 @@ -package volume - /* Copyright 2020 Docker, Inc. @@ -16,13 +14,17 @@ package volume limitations under the License. */ +package volume + import ( "fmt" "io" "os" "strings" "text/tabwriter" + "github.com/spf13/cobra" + "github.com/docker/compose-cli/api/client" "github.com/docker/compose-cli/api/volumes" ) diff --git a/cli/cmd/volume/list_test.go b/cli/cmd/volume/list_test.go index ab21c75d..3096ef2a 100644 --- a/cli/cmd/volume/list_test.go +++ b/cli/cmd/volume/list_test.go @@ -1,10 +1,28 @@ +/* + Copyright 2020 Docker, Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + package volume import ( "bytes" - "github.com/docker/compose-cli/api/volumes" - "gotest.tools/v3/golden" "testing" + + "gotest.tools/v3/golden" + + "github.com/docker/compose-cli/api/volumes" ) func TestPrintList(t *testing.T) { @@ -19,4 +37,3 @@ func TestPrintList(t *testing.T) { printList(out, secrets) golden.Assert(t, out.String(), "volumes-out.golden") } - diff --git a/cli/cmd/volume/volume.go b/cli/cmd/volume/volume.go index 77362467..560be536 100644 --- a/cli/cmd/volume/volume.go +++ b/cli/cmd/volume/volume.go @@ -1,5 +1,3 @@ -package volume - /* Copyright 2020 Docker, Inc. @@ -16,9 +14,13 @@ package volume limitations under the License. */ +package volume + import ( "fmt" + "github.com/spf13/cobra" + "github.com/docker/compose-cli/aci" "github.com/docker/compose-cli/api/client" ) @@ -33,6 +35,7 @@ func Command() *cobra.Command { cmd.AddCommand( createVolume(), listVolume(), + rmVolume(), ) return cmd } @@ -60,4 +63,25 @@ func createVolume() *cobra.Command { cmd.Flags().StringVar(&opts.Account, "storage-account", "", "Storage account name") cmd.Flags().StringVar(&opts.Fileshare, "fileshare", "", "Fileshare name") return cmd -} \ No newline at end of file +} + +func rmVolume() *cobra.Command { + opts := aci.VolumeDeleteOptions{} + cmd := &cobra.Command{ + Use: "rm", + Short: "Deletes an Azure file share and/or the Azure storage account.", + Args: cobra.ExactArgs(0), + RunE: func(cmd *cobra.Command, args []string) error { + c, err := client.New(cmd.Context()) + if err != nil { + return err + } + return c.VolumeService().Delete(cmd.Context(), opts) + }, + } + + cmd.Flags().StringVar(&opts.Account, "storage-account", "", "Storage account name") + cmd.Flags().StringVar(&opts.Fileshare, "fileshare", "", "Fileshare name") + cmd.Flags().BoolVar(&opts.DeleteAccount, "delete-storage-account", false, "Also delete storage account") + return cmd +} diff --git a/tests/aci-e2e/e2e-aci_test.go b/tests/aci-e2e/e2e-aci_test.go index 2a360f6e..cb3d1f09 100644 --- a/tests/aci-e2e/e2e-aci_test.go +++ b/tests/aci-e2e/e2e-aci_test.go @@ -47,7 +47,6 @@ import ( "github.com/docker/compose-cli/api/containers" "github.com/docker/compose-cli/context/store" "github.com/docker/compose-cli/errdefs" - "github.com/docker/compose-cli/tests/aci-e2e/storage" . "github.com/docker/compose-cli/tests/framework" ) @@ -151,30 +150,54 @@ func TestContainerRunVolume(t *testing.T) { accountName = "e2e" + strconv.Itoa(int(time.Now().UnixNano())) ) - t.Run("Create volumes", func(t *testing.T) { + t.Run("check volume name validity", func(t *testing.T) { + invalidName := "some-storage-123" + res := c.RunDockerOrExitError("volume", "create", "--storage-account", invalidName, "--fileshare", fileshareName) + res.Assert(t, icmd.Expected{ + ExitCode: 1, + Err: "some-storage-123 is not a valid storage account name. Storage account name must be between 3 and 24 characters in length and use numbers and lower-case letters only.", + }) + }) + + t.Run("create volumes", func(t *testing.T) { c.RunDockerCmd("volume", "create", "--storage-account", accountName, "--fileshare", fileshareName) }) - t.Cleanup(func() { deleteStorageAccount(t, aciContext, accountName) }) - t.Run("Create second fileshare", func(t *testing.T) { + t.Cleanup(func() { + c.RunDockerCmd("volume", "rm", "--storage-account", accountName, "--delete-storage-account") + res := c.RunDockerCmd("volume", "ls") + lines := lines(res.Stdout()) + assert.Equal(t, len(lines), 1) + }) + + t.Run("create second fileshare", func(t *testing.T) { c.RunDockerCmd("volume", "create", "--storage-account", accountName, "--fileshare", "dockertestshare2") }) t.Run("list volumes", func(t *testing.T) { res := c.RunDockerCmd("volume", "ls") - out := strings.Split(strings.TrimSpace(res.Stdout()), "\n") - firstAccount := out[1] + lines := lines(res.Stdout()) + assert.Equal(t, len(lines), 3) + firstAccount := lines[1] fields := strings.Fields(firstAccount) volumeID = accountName + "@" + fileshareName assert.Equal(t, fields[0], volumeID) assert.Equal(t, fields[1], fileshareName) - secondAccount := out[2] + secondAccount := lines[2] fields = strings.Fields(secondAccount) - assert.Equal(t, fields[0], accountName + "@dockertestshare2") + assert.Equal(t, fields[0], accountName+"@dockertestshare2") assert.Equal(t, fields[1], "dockertestshare2") //assert.Assert(t, fields[2], strings.Contains(firstAccount, fmt.Sprintf("Fileshare %s in %s storage account", fileshareName, accountName))) }) + t.Run("delete only fileshare", func(t *testing.T) { + c.RunDockerCmd("volume", "rm", "--storage-account", accountName, "--fileshare", "dockertestshare2") + res := c.RunDockerCmd("volume", "ls") + lines := lines(res.Stdout()) + assert.Equal(t, len(lines), 2) + assert.Assert(t, !strings.Contains(res.Stdout(), "dockertestshare2"), "second fileshare still visible after rm") + }) + t.Run("upload file", func(t *testing.T) { storageLogin := login.StorageLogin{AciContext: aciContext} @@ -213,7 +236,7 @@ func TestContainerRunVolume(t *testing.T) { t.Run("ps", func(t *testing.T) { res := c.RunDockerCmd("ps") - out := strings.Split(strings.TrimSpace(res.Stdout()), "\n") + out := lines(res.Stdout()) l := out[len(out)-1] assert.Assert(t, strings.Contains(l, container), "Looking for %q in line: %s", container, l) assert.Assert(t, strings.Contains(l, "nginx")) @@ -308,6 +331,10 @@ func TestContainerRunVolume(t *testing.T) { }) } +func lines(output string) []string { + return strings.Split(strings.TrimSpace(output), "\n") +} + func TestContainerRunAttached(t *testing.T) { c := NewParallelE2eCLI(t, binDir) _, _ = setupTestResourceGroup(t, c) @@ -398,11 +425,11 @@ func TestContainerRunAttached(t *testing.T) { t.Run("ps stopped container with --all", func(t *testing.T) { res := c.RunDockerCmd("ps", container) - out := strings.Split(strings.TrimSpace(res.Stdout()), "\n") + out := lines(res.Stdout()) assert.Assert(t, is.Len(out, 1)) res = c.RunDockerCmd("ps", "--all", container) - out = strings.Split(strings.TrimSpace(res.Stdout()), "\n") + out = lines(res.Stdout()) assert.Assert(t, is.Len(out, 2)) }) @@ -439,7 +466,7 @@ func TestComposeUpUpdate(t *testing.T) { // Name of Compose project is taken from current folder "acie2e" c.RunDockerCmd("compose", "up", "-f", composeFile) res := c.RunDockerCmd("ps") - out := strings.Split(strings.TrimSpace(res.Stdout()), "\n") + out := lines(res.Stdout()) // Check three containers are running assert.Assert(t, is.Len(out, 4)) webRunning := false @@ -468,7 +495,7 @@ func TestComposeUpUpdate(t *testing.T) { t.Run("compose ps", func(t *testing.T) { res := c.RunDockerCmd("compose", "ps", "--project-name", composeProjectName) - lines := strings.Split(strings.TrimSpace(res.Stdout()), "\n") + lines := lines(res.Stdout()) assert.Assert(t, is.Len(lines, 4)) var wordsDisplayed, webDisplayed, dbDisplayed bool for _, line := range lines { @@ -492,7 +519,7 @@ func TestComposeUpUpdate(t *testing.T) { t.Run("compose ls", func(t *testing.T) { res := c.RunDockerCmd("compose", "ls") - lines := strings.Split(strings.TrimSpace(res.Stdout()), "\n") + lines := lines(res.Stdout()) assert.Equal(t, 2, len(lines)) fields := strings.Fields(lines[1]) @@ -509,7 +536,7 @@ func TestComposeUpUpdate(t *testing.T) { t.Run("update", func(t *testing.T) { c.RunDockerCmd("compose", "up", "-f", composeFileMultiplePorts, "--project-name", composeProjectName) res := c.RunDockerCmd("ps") - out := strings.Split(strings.TrimSpace(res.Stdout()), "\n") + out := lines(res.Stdout()) // Check three containers are running assert.Assert(t, is.Len(out, 4)) @@ -551,7 +578,7 @@ func TestComposeUpUpdate(t *testing.T) { t.Run("down", func(t *testing.T) { c.RunDockerCmd("compose", "down", "--project-name", composeProjectName) res := c.RunDockerCmd("ps") - out := strings.Split(strings.TrimSpace(res.Stdout()), "\n") + out := lines(res.Stdout()) assert.Equal(t, len(out), 1) }) } @@ -572,7 +599,7 @@ func TestRunEnvVars(t *testing.T) { cmd.Env = append(cmd.Env, "MYSQL_USER=user1") res := icmd.RunCmd(cmd) res.Assert(t, icmd.Success) - out := strings.Split(strings.TrimSpace(res.Stdout()), "\n") + out := lines(res.Stdout()) container := strings.TrimSpace(out[len(out)-1]) res = c.RunDockerCmd("inspect", container) @@ -607,7 +634,7 @@ func setupTestResourceGroup(t *testing.T, c *E2eCLI) (string, string) { createAciContextAndUseIt(t, c, sID, rg) // Check nothing is running res := c.RunDockerCmd("ps") - assert.Assert(t, is.Len(strings.Split(strings.TrimSpace(res.Stdout()), "\n"), 1)) + assert.Assert(t, is.Len(lines(res.Stdout()), 1)) return sID, rg } @@ -661,14 +688,6 @@ func createAciContextAndUseIt(t *testing.T, c *E2eCLI, sID, rgName string) { res.Assert(t, icmd.Expected{Out: contextName + " *"}) } -func deleteStorageAccount(t *testing.T, aciContext store.AciContext, name string) { - fmt.Printf(" [%s] deleting storage account %s\n", t.Name(), name) - _, err := storage.DeleteStorageAccount(context.TODO(), aciContext, name) - if err != nil { - t.Error(err) - } -} - func uploadFile(t *testing.T, cred azfile.SharedKeyCredential, baseURL, fileName, content string) { fURL, err := url.Parse(baseURL + "/" + fileName) assert.NilError(t, err) @@ -678,7 +697,7 @@ func uploadFile(t *testing.T, cred azfile.SharedKeyCredential, baseURL, fileName } func getContainerName(stdout string) string { - out := strings.Split(strings.TrimSpace(stdout), "\n") + out := lines(stdout) return strings.TrimSpace(out[len(out)-1]) } diff --git a/tests/aci-e2e/storage/storage.go b/tests/aci-e2e/storage/storage.go deleted file mode 100644 index 9e83d1f3..00000000 --- a/tests/aci-e2e/storage/storage.go +++ /dev/null @@ -1,38 +0,0 @@ -/* - Copyright 2020 Docker, Inc. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. -*/ - -package storage - -import ( - "context" - "github.com/Azure/go-autorest/autorest" - "github.com/docker/compose-cli/aci/login" - "github.com/docker/compose-cli/context/store" -) - - -// DeleteStorageAccount deletes a given storage account -func DeleteStorageAccount(ctx context.Context, aciContext store.AciContext, accountName string) (autorest.Response, error) { - storageAccountsClient, err := login.NewStorageAccountsClient(aciContext.SubscriptionID) - if err != nil { - return autorest.Response{}, err - } - response, err := storageAccountsClient.Delete(ctx, aciContext.ResourceGroup, accountName) - if err != nil { - return autorest.Response{}, err - } - return response, err -} \ No newline at end of file From b96a6d108662ac74756e9ed07cb564023e63c9f2 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Tue, 8 Sep 2020 17:19:42 +0200 Subject: [PATCH 08/13] Add progress on volume creation Signed-off-by: Guillaume Tardif --- aci/volumes.go | 43 ++++++++++++++++++++++++++++++++++++---- cli/cmd/volume/volume.go | 20 +++++++++++++------ 2 files changed, 53 insertions(+), 10 deletions(-) diff --git a/aci/volumes.go b/aci/volumes.go index 78039d66..87791923 100644 --- a/aci/volumes.go +++ b/aci/volumes.go @@ -20,6 +20,8 @@ import ( "context" "fmt" + "github.com/docker/compose-cli/progress" + "github.com/Azure/go-autorest/autorest/to" "github.com/docker/compose-cli/aci/login" @@ -90,12 +92,16 @@ func (cs *aciVolumeService) Create(ctx context.Context, options interface{}) (vo if !ok { return volumes.Volume{}, errors.New("Could not read azure LoginParams struct from generic parameter") } + w := progress.ContextWriter(ctx) + w.Event(event(opts.Account, progress.Working, "Validating")) accountClient, err := login.NewStorageAccountsClient(cs.aciContext.SubscriptionID) if err != nil { return volumes.Volume{}, err } account, err := accountClient.GetProperties(ctx, cs.aciContext.ResourceGroup, opts.Account, "") - if err != nil { + if err == nil { + w.Event(event(opts.Account, progress.Done, "Use existing")) + } else { if account.StatusCode != 404 { return volumes.Volume{}, err } @@ -111,20 +117,27 @@ func (cs *aciVolumeService) Create(ctx context.Context, options interface{}) (vo return volumes.Volume{}, errors.New("error: " + *result.Message) } parameters := defaultStorageAccountParams(cs.aciContext) - // TODO progress account creation + + w.Event(event(opts.Account, progress.Working, "Creating")) + future, err := accountClient.Create(ctx, cs.aciContext.ResourceGroup, opts.Account, parameters) if err != nil { + w.Event(errorEvent(opts.Account)) return volumes.Volume{}, err } err = future.WaitForCompletionRef(ctx, accountClient.Client) if err != nil { + w.Event(errorEvent(opts.Account)) return volumes.Volume{}, err } account, err = future.Result(accountClient) if err != nil { + w.Event(errorEvent(opts.Account)) return volumes.Volume{}, err } + w.Event(event(opts.Account, progress.Done, "Created")) } + w.Event(event(opts.Fileshare, progress.Working, "Creating")) fileShareClient, err := login.NewFileShareClient(cs.aciContext.SubscriptionID) if err != nil { return volumes.Volume{}, err @@ -132,26 +145,48 @@ func (cs *aciVolumeService) Create(ctx context.Context, options interface{}) (vo fileShare, err := fileShareClient.Get(ctx, cs.aciContext.ResourceGroup, *account.Name, opts.Fileshare, "") if err == nil { + w.Event(errorEvent(opts.Fileshare)) return volumes.Volume{}, errors.Wrapf(errdefs.ErrAlreadyExists, "Azure fileshare %q already exists", opts.Fileshare) } if fileShare.StatusCode != 404 { + w.Event(errorEvent(opts.Fileshare)) return volumes.Volume{}, err } + //TODO tag fileshare fileShare, err = fileShareClient.Create(ctx, cs.aciContext.ResourceGroup, *account.Name, opts.Fileshare, storage.FileShare{}) if err != nil { + w.Event(errorEvent(opts.Fileshare)) return volumes.Volume{}, err } + w.Event(event(opts.Fileshare, progress.Done, "Created")) return toVolume(account, *fileShare.Name), nil } +func event(resource string, status progress.EventStatus, text string) progress.Event { + return progress.Event{ + ID: resource, + Status: status, + StatusText: text, + } +} + +func errorEvent(resource string) progress.Event { + return progress.Event{ + ID: resource, + Status: progress.Error, + StatusText: "Error", + } +} + func (cs *aciVolumeService) Delete(ctx context.Context, options interface{}) error { opts, ok := options.(VolumeDeleteOptions) if !ok { return errors.New("Could not read azure VolumeDeleteOptions struct from generic parameter") } if opts.DeleteAccount { - //TODO check if there are other shares on this account - //TODO flag account and only delete ours + //TODO check if there are other fileshares on this account + //TODO flag account and only delete ours? + //TODO error when not found storageAccountsClient, err := login.NewStorageAccountsClient(cs.aciContext.SubscriptionID) if err != nil { return err diff --git a/cli/cmd/volume/volume.go b/cli/cmd/volume/volume.go index 560be536..1f9b4b95 100644 --- a/cli/cmd/volume/volume.go +++ b/cli/cmd/volume/volume.go @@ -17,12 +17,13 @@ package volume import ( + "context" "fmt" - "github.com/spf13/cobra" - "github.com/docker/compose-cli/aci" "github.com/docker/compose-cli/api/client" + "github.com/docker/compose-cli/progress" + "github.com/spf13/cobra" ) // Command manage volumes @@ -47,16 +48,23 @@ func createVolume() *cobra.Command { Short: "Creates an Azure file share to use as ACI volume.", Args: cobra.ExactArgs(0), RunE: func(cmd *cobra.Command, args []string) error { - c, err := client.New(cmd.Context()) + ctx := cmd.Context() + c, err := client.New(ctx) if err != nil { return err } - id, err := c.VolumeService().Create(cmd.Context(), opts) + err = progress.Run(ctx, func(ctx context.Context) error { + _, err := c.VolumeService().Create(ctx, opts) + if err != nil { + return err + } + return nil + }) if err != nil { return err } - fmt.Println(id) - return nil + fmt.Printf("volume successfully created\n") + return err }, } From 18ad20f1c52410f2c34af5f64e9f2f141ee6cbb6 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Wed, 9 Sep 2020 10:26:30 +0200 Subject: [PATCH 09/13] Display errors if resource not found when deleting volumes (file share or storage account) Signed-off-by: Guillaume Tardif --- aci/volumes.go | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/aci/volumes.go b/aci/volumes.go index 87791923..df7ab108 100644 --- a/aci/volumes.go +++ b/aci/volumes.go @@ -152,7 +152,6 @@ func (cs *aciVolumeService) Create(ctx context.Context, options interface{}) (vo w.Event(errorEvent(opts.Fileshare)) return volumes.Volume{}, err } - //TODO tag fileshare fileShare, err = fileShareClient.Create(ctx, cs.aciContext.ResourceGroup, *account.Name, opts.Fileshare, storage.FileShare{}) if err != nil { w.Event(errorEvent(opts.Fileshare)) @@ -185,14 +184,16 @@ func (cs *aciVolumeService) Delete(ctx context.Context, options interface{}) err } if opts.DeleteAccount { //TODO check if there are other fileshares on this account - //TODO flag account and only delete ours? - //TODO error when not found storageAccountsClient, err := login.NewStorageAccountsClient(cs.aciContext.SubscriptionID) if err != nil { return err } - _, err = storageAccountsClient.Delete(ctx, cs.aciContext.ResourceGroup, opts.Account) + result, err := storageAccountsClient.Delete(ctx, cs.aciContext.ResourceGroup, opts.Account) + if result.StatusCode == 204 { + return errors.Wrapf(errdefs.ErrNotFound, "storage account %s does not exist", opts.Account) + } + return err } @@ -201,7 +202,13 @@ func (cs *aciVolumeService) Delete(ctx context.Context, options interface{}) err return err } - _, err = fileShareClient.Delete(ctx, cs.aciContext.ResourceGroup, opts.Account, opts.Fileshare) + result, err := fileShareClient.Delete(ctx, cs.aciContext.ResourceGroup, opts.Account, opts.Fileshare) + if result.StatusCode == 204 { + return errors.Wrapf(errdefs.ErrNotFound, "fileshare %s does not exist", opts.Fileshare) + } + if result.StatusCode == 404 { + return errors.Wrapf(errdefs.ErrNotFound, "storage account %s does not exist", opts.Account) + } return err } From 38a8f5310b64574cb980de2b2a6a94659239c62e Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Wed, 9 Sep 2020 10:46:11 +0200 Subject: [PATCH 10/13] Volume command is only available in aci context type, with ACI specific flags Signed-off-by: Guillaume Tardif --- cli/cmd/volume/{list.go => acilist.go} | 0 .../volume/{list_test.go => acilist_test.go} | 0 cli/cmd/volume/{volume.go => acivolume.go} | 25 ++++++++++--------- cli/main.go | 6 ++++- 4 files changed, 18 insertions(+), 13 deletions(-) rename cli/cmd/volume/{list.go => acilist.go} (100%) rename cli/cmd/volume/{list_test.go => acilist_test.go} (100%) rename cli/cmd/volume/{volume.go => acivolume.go} (72%) diff --git a/cli/cmd/volume/list.go b/cli/cmd/volume/acilist.go similarity index 100% rename from cli/cmd/volume/list.go rename to cli/cmd/volume/acilist.go diff --git a/cli/cmd/volume/list_test.go b/cli/cmd/volume/acilist_test.go similarity index 100% rename from cli/cmd/volume/list_test.go rename to cli/cmd/volume/acilist_test.go diff --git a/cli/cmd/volume/volume.go b/cli/cmd/volume/acivolume.go similarity index 72% rename from cli/cmd/volume/volume.go rename to cli/cmd/volume/acivolume.go index 1f9b4b95..de6c0a46 100644 --- a/cli/cmd/volume/volume.go +++ b/cli/cmd/volume/acivolume.go @@ -20,14 +20,15 @@ import ( "context" "fmt" + "github.com/spf13/cobra" + "github.com/docker/compose-cli/aci" "github.com/docker/compose-cli/api/client" "github.com/docker/compose-cli/progress" - "github.com/spf13/cobra" ) -// Command manage volumes -func Command() *cobra.Command { +// ACICommand manage volumes +func ACICommand() *cobra.Command { cmd := &cobra.Command{ Use: "volume", Short: "Manages volumes", @@ -42,7 +43,7 @@ func Command() *cobra.Command { } func createVolume() *cobra.Command { - opts := aci.VolumeCreateOptions{} + aciOpts := aci.VolumeCreateOptions{} cmd := &cobra.Command{ Use: "create", Short: "Creates an Azure file share to use as ACI volume.", @@ -54,7 +55,7 @@ func createVolume() *cobra.Command { return err } err = progress.Run(ctx, func(ctx context.Context) error { - _, err := c.VolumeService().Create(ctx, opts) + _, err := c.VolumeService().Create(ctx, aciOpts) if err != nil { return err } @@ -68,13 +69,13 @@ func createVolume() *cobra.Command { }, } - cmd.Flags().StringVar(&opts.Account, "storage-account", "", "Storage account name") - cmd.Flags().StringVar(&opts.Fileshare, "fileshare", "", "Fileshare name") + cmd.Flags().StringVar(&aciOpts.Account, "storage-account", "", "Storage account name") + cmd.Flags().StringVar(&aciOpts.Fileshare, "fileshare", "", "Fileshare name") return cmd } func rmVolume() *cobra.Command { - opts := aci.VolumeDeleteOptions{} + aciOpts := aci.VolumeDeleteOptions{} cmd := &cobra.Command{ Use: "rm", Short: "Deletes an Azure file share and/or the Azure storage account.", @@ -84,12 +85,12 @@ func rmVolume() *cobra.Command { if err != nil { return err } - return c.VolumeService().Delete(cmd.Context(), opts) + return c.VolumeService().Delete(cmd.Context(), aciOpts) }, } - cmd.Flags().StringVar(&opts.Account, "storage-account", "", "Storage account name") - cmd.Flags().StringVar(&opts.Fileshare, "fileshare", "", "Fileshare name") - cmd.Flags().BoolVar(&opts.DeleteAccount, "delete-storage-account", false, "Also delete storage account") + cmd.Flags().StringVar(&aciOpts.Account, "storage-account", "", "Storage account name") + cmd.Flags().StringVar(&aciOpts.Fileshare, "fileshare", "", "Fileshare name") + cmd.Flags().BoolVar(&aciOpts.DeleteAccount, "delete-storage-account", false, "Also delete storage account") return cmd } diff --git a/cli/main.go b/cli/main.go index 69d84324..db88150e 100644 --- a/cli/main.go +++ b/cli/main.go @@ -135,7 +135,6 @@ func main() { // Place holders cmd.EcsCommand(), - volume.Command(), ) helpFunc := root.HelpFunc() @@ -185,6 +184,11 @@ func main() { ctype = cc.Type() } + 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()) + } + metrics.Track(ctype, os.Args[1:], root.PersistentFlags()) ctx = apicontext.WithCurrentContext(ctx, currentContext) From 80d23a60977befc11817980a764ce0f4aed8d258 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Wed, 9 Sep 2020 15:49:43 +0200 Subject: [PATCH 11/13] Removed NAME from `volume ls` output, allow `volume delete ` using IDs from `volume ls`. Signed-off-by: Guillaume Tardif --- aci/volumes.go | 76 ++++++++++++---------- api/client/volume.go | 2 +- api/volumes/api.go | 3 +- cli/cmd/volume/acilist.go | 4 +- cli/cmd/volume/acilist_test.go | 1 - cli/cmd/volume/acivolume.go | 40 ++++++++---- cli/cmd/volume/testdata/volumes-out.golden | 4 +- tests/aci-e2e/e2e-aci_test.go | 12 ++-- 8 files changed, 81 insertions(+), 61 deletions(-) diff --git a/aci/volumes.go b/aci/volumes.go index df7ab108..3a43d68a 100644 --- a/aci/volumes.go +++ b/aci/volumes.go @@ -19,6 +19,7 @@ package aci import ( "context" "fmt" + "strings" "github.com/docker/compose-cli/progress" @@ -80,17 +81,10 @@ type VolumeCreateOptions struct { Fileshare string } -//VolumeDeleteOptions options to create a new ACI volume -type VolumeDeleteOptions struct { - Account string - Fileshare string - DeleteAccount bool -} - func (cs *aciVolumeService) Create(ctx context.Context, options interface{}) (volumes.Volume, error) { opts, ok := options.(VolumeCreateOptions) if !ok { - return volumes.Volume{}, errors.New("Could not read azure LoginParams struct from generic parameter") + return volumes.Volume{}, errors.New("Could not read azure VolumeCreateOptions struct from generic parameter") } w := progress.ContextWriter(ctx) w.Event(event(opts.Account, progress.Working, "Validating")) @@ -105,7 +99,6 @@ func (cs *aciVolumeService) Create(ctx context.Context, options interface{}) (vo if account.StatusCode != 404 { return volumes.Volume{}, err } - //TODO confirm storage account creation result, err := accountClient.CheckNameAvailability(ctx, storage.AccountCheckNameAvailabilityParameters{ Name: to.StringPtr(opts.Account), Type: to.StringPtr("Microsoft.Storage/storageAccounts"), @@ -177,49 +170,62 @@ func errorEvent(resource string) progress.Event { } } -func (cs *aciVolumeService) Delete(ctx context.Context, options interface{}) error { - opts, ok := options.(VolumeDeleteOptions) - if !ok { - return errors.New("Could not read azure VolumeDeleteOptions struct from generic parameter") - } - if opts.DeleteAccount { - //TODO check if there are other fileshares on this account - storageAccountsClient, err := login.NewStorageAccountsClient(cs.aciContext.SubscriptionID) - if err != nil { - return err - } - - result, err := storageAccountsClient.Delete(ctx, cs.aciContext.ResourceGroup, opts.Account) - if result.StatusCode == 204 { - return errors.Wrapf(errdefs.ErrNotFound, "storage account %s does not exist", opts.Account) - } - - return err +func (cs *aciVolumeService) Delete(ctx context.Context, id string, options interface{}) error { + tokens := strings.Split(id, "@") + if len(tokens) != 2 { + return errors.New("wrong format for volume ID : should be storageaccount@fileshare") } + storageAccount := tokens[0] + fileshare := tokens[1] fileShareClient, err := login.NewFileShareClient(cs.aciContext.SubscriptionID) if err != nil { return err } - - result, err := fileShareClient.Delete(ctx, cs.aciContext.ResourceGroup, opts.Account, opts.Fileshare) - if result.StatusCode == 204 { - return errors.Wrapf(errdefs.ErrNotFound, "fileshare %s does not exist", opts.Fileshare) + fileShareItemsPage, err := fileShareClient.List(ctx, cs.aciContext.ResourceGroup, storageAccount, "", "", "") + if err != nil { + return err } - if result.StatusCode == 404 { - return errors.Wrapf(errdefs.ErrNotFound, "storage account %s does not exist", opts.Account) + fileshares := fileShareItemsPage.Values() + if len(fileshares) == 1 && *fileshares[0].Name == fileshare { + storageAccountsClient, err := login.NewStorageAccountsClient(cs.aciContext.SubscriptionID) + if err != nil { + return err + } + account, err := storageAccountsClient.GetProperties(ctx, cs.aciContext.ResourceGroup, storageAccount, "") + if err != nil { + return err + } + if err == nil { + if _, ok := account.Tags[dockerVolumeTag]; ok { + result, err := storageAccountsClient.Delete(ctx, cs.aciContext.ResourceGroup, storageAccount) + if result.StatusCode == 204 { + return errors.Wrapf(errdefs.ErrNotFound, "storage account %s does not exist", storageAccount) + } + return err + } + } + } + + result, err := fileShareClient.Delete(ctx, cs.aciContext.ResourceGroup, storageAccount, fileshare) + if result.StatusCode == 204 { + return errors.Wrapf(errdefs.ErrNotFound, "fileshare %s does not exist", fileshare) } return err } func toVolume(account storage.Account, fileShareName string) volumes.Volume { return volumes.Volume{ - ID: fmt.Sprintf("%s@%s", *account.Name, fileShareName), - Name: fileShareName, + ID: VolumeID(*account.Name, fileShareName), Description: fmt.Sprintf("Fileshare %s in %s storage account", fileShareName, *account.Name), } } +// VolumeID generate volume ID from azure storage accoun & fileshare +func VolumeID(storageAccount string, fileShareName string) string { + return fmt.Sprintf("%s@%s", storageAccount, fileShareName) +} + func defaultStorageAccountParams(aciContext store.AciContext) storage.AccountCreateParameters { tags := map[string]*string{dockerVolumeTag: to.StringPtr(dockerVolumeTag)} return storage.AccountCreateParameters{ diff --git a/api/client/volume.go b/api/client/volume.go index ea0ee378..58c89ed8 100644 --- a/api/client/volume.go +++ b/api/client/volume.go @@ -34,6 +34,6 @@ func (c *volumeService) Create(ctx context.Context, options interface{}) (volume return volumes.Volume{}, errdefs.ErrNotImplemented } -func (c *volumeService) Delete(ctx context.Context, options interface{}) error { +func (c *volumeService) Delete(ctx context.Context, id string, options interface{}) error { return errdefs.ErrNotImplemented } diff --git a/api/volumes/api.go b/api/volumes/api.go index 29562f7e..a753fcea 100644 --- a/api/volumes/api.go +++ b/api/volumes/api.go @@ -23,7 +23,6 @@ import ( // Volume volume info type Volume struct { ID string - Name string Description string } @@ -34,5 +33,5 @@ type Service interface { // Create creates a new volume Create(ctx context.Context, options interface{}) (Volume, error) // Delete deletes an existing volume - Delete(ctx context.Context, options interface{}) error + Delete(ctx context.Context, volumeID string, options interface{}) error } diff --git a/cli/cmd/volume/acilist.go b/cli/cmd/volume/acilist.go index 3158cce9..7261f342 100644 --- a/cli/cmd/volume/acilist.go +++ b/cli/cmd/volume/acilist.go @@ -53,9 +53,9 @@ func listVolume() *cobra.Command { func printList(out io.Writer, volumes []volumes.Volume) { printSection(out, func(w io.Writer) { for _, vol := range volumes { - fmt.Fprintf(w, "%s\t%s\t%s\n", vol.ID, vol.Name, vol.Description) // nolint:errcheck + fmt.Fprintf(w, "%s\t%s\n", vol.ID, vol.Description) // nolint:errcheck } - }, "ID", "NAME", "DESCRIPTION") + }, "ID", "DESCRIPTION") } func printSection(out io.Writer, printer func(io.Writer), headers ...string) { diff --git a/cli/cmd/volume/acilist_test.go b/cli/cmd/volume/acilist_test.go index 3096ef2a..a6a1e73d 100644 --- a/cli/cmd/volume/acilist_test.go +++ b/cli/cmd/volume/acilist_test.go @@ -29,7 +29,6 @@ func TestPrintList(t *testing.T) { secrets := []volumes.Volume{ { ID: "volume@123", - Name: "123", Description: "volume 123", }, } diff --git a/cli/cmd/volume/acivolume.go b/cli/cmd/volume/acivolume.go index de6c0a46..a4e4dd6d 100644 --- a/cli/cmd/volume/acivolume.go +++ b/cli/cmd/volume/acivolume.go @@ -19,6 +19,9 @@ package volume import ( "context" "fmt" + "strings" + + "github.com/hashicorp/go-multierror" "github.com/spf13/cobra" @@ -64,8 +67,8 @@ func createVolume() *cobra.Command { if err != nil { return err } - fmt.Printf("volume successfully created\n") - return err + fmt.Println(aci.VolumeID(aciOpts.Account, aciOpts.Fileshare)) + return nil }, } @@ -75,22 +78,37 @@ func createVolume() *cobra.Command { } func rmVolume() *cobra.Command { - aciOpts := aci.VolumeDeleteOptions{} cmd := &cobra.Command{ - Use: "rm", - Short: "Deletes an Azure file share and/or the Azure storage account.", - Args: cobra.ExactArgs(0), + Use: "rm [OPTIONS] VOLUME [VOLUME...]", + Short: "Remove one or more volumes.", + Args: cobra.MinimumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { c, err := client.New(cmd.Context()) if err != nil { return err } - return c.VolumeService().Delete(cmd.Context(), aciOpts) + var errs *multierror.Error + for _, id := range args { + err = c.VolumeService().Delete(cmd.Context(), id, nil) + if err != nil { + errs = multierror.Append(errs, err) + continue + } + fmt.Println(id) + } + if errs != nil { + errs.ErrorFormat = formatErrors + } + return errs.ErrorOrNil() }, } - - cmd.Flags().StringVar(&aciOpts.Account, "storage-account", "", "Storage account name") - cmd.Flags().StringVar(&aciOpts.Fileshare, "fileshare", "", "Fileshare name") - cmd.Flags().BoolVar(&aciOpts.DeleteAccount, "delete-storage-account", false, "Also delete storage account") return cmd } + +func formatErrors(errs []error) string { + messages := make([]string, len(errs)) + for i, err := range errs { + messages[i] = "Error: " + err.Error() + } + return strings.Join(messages, "\n") +} diff --git a/cli/cmd/volume/testdata/volumes-out.golden b/cli/cmd/volume/testdata/volumes-out.golden index ed27e449..9a4039ec 100644 --- a/cli/cmd/volume/testdata/volumes-out.golden +++ b/cli/cmd/volume/testdata/volumes-out.golden @@ -1,2 +1,2 @@ -ID NAME DESCRIPTION -volume@123 123 volume 123 +ID DESCRIPTION +volume@123 volume 123 diff --git a/tests/aci-e2e/e2e-aci_test.go b/tests/aci-e2e/e2e-aci_test.go index cb3d1f09..bd6911b8 100644 --- a/tests/aci-e2e/e2e-aci_test.go +++ b/tests/aci-e2e/e2e-aci_test.go @@ -162,9 +162,10 @@ func TestContainerRunVolume(t *testing.T) { t.Run("create volumes", func(t *testing.T) { c.RunDockerCmd("volume", "create", "--storage-account", accountName, "--fileshare", fileshareName) }) + volumeID = accountName + "@" + fileshareName t.Cleanup(func() { - c.RunDockerCmd("volume", "rm", "--storage-account", accountName, "--delete-storage-account") + c.RunDockerCmd("volume", "rm", volumeID) res := c.RunDockerCmd("volume", "ls") lines := lines(res.Stdout()) assert.Equal(t, len(lines), 1) @@ -173,6 +174,7 @@ func TestContainerRunVolume(t *testing.T) { t.Run("create second fileshare", func(t *testing.T) { c.RunDockerCmd("volume", "create", "--storage-account", accountName, "--fileshare", "dockertestshare2") }) + volumeID2 := accountName + "@dockertestshare2" t.Run("list volumes", func(t *testing.T) { res := c.RunDockerCmd("volume", "ls") @@ -180,18 +182,14 @@ func TestContainerRunVolume(t *testing.T) { assert.Equal(t, len(lines), 3) firstAccount := lines[1] fields := strings.Fields(firstAccount) - volumeID = accountName + "@" + fileshareName assert.Equal(t, fields[0], volumeID) - assert.Equal(t, fields[1], fileshareName) secondAccount := lines[2] fields = strings.Fields(secondAccount) - assert.Equal(t, fields[0], accountName+"@dockertestshare2") - assert.Equal(t, fields[1], "dockertestshare2") - //assert.Assert(t, fields[2], strings.Contains(firstAccount, fmt.Sprintf("Fileshare %s in %s storage account", fileshareName, accountName))) + assert.Equal(t, fields[0], volumeID2) }) t.Run("delete only fileshare", func(t *testing.T) { - c.RunDockerCmd("volume", "rm", "--storage-account", accountName, "--fileshare", "dockertestshare2") + c.RunDockerCmd("volume", "rm", volumeID2) res := c.RunDockerCmd("volume", "ls") lines := lines(res.Stdout()) assert.Equal(t, len(lines), 2) From 099b64935b446a65b36f442cdfee6e581a95421a Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Thu, 10 Sep 2020 14:17:49 +0200 Subject: [PATCH 12/13] Minor fixes Co-authored-by: Chris Crone Signed-off-by: Guillaume Tardif --- aci/cloud.go | 2 +- aci/compose.go | 12 +++---- aci/containers.go | 20 +++++------ aci/volumes.go | 34 ++++++++----------- api/client/client.go | 3 +- cli/cmd/volume/acivolume.go | 3 +- cli/cmd/volume/{acilist.go => list.go} | 8 ++--- .../volume/{acilist_test.go => list_test.go} | 0 cli/main.go | 17 ++++------ ecs/backend.go | 4 +-- ecs/local/backend.go | 7 ++-- 11 files changed, 47 insertions(+), 63 deletions(-) rename cli/cmd/volume/{acilist.go => list.go} (86%) rename cli/cmd/volume/{acilist_test.go => list_test.go} (100%) diff --git a/aci/cloud.go b/aci/cloud.go index 0dee380f..ff761800 100644 --- a/aci/cloud.go +++ b/aci/cloud.go @@ -31,7 +31,7 @@ type aciCloudService struct { func (cs *aciCloudService) Login(ctx context.Context, params interface{}) error { opts, ok := params.(LoginParams) if !ok { - return errors.New("Could not read azure LoginParams struct from generic parameter") + return errors.New("could not read Azure LoginParams struct from generic parameter") } if opts.ClientID != "" { return cs.loginService.LoginServicePrincipal(opts.ClientID, opts.ClientSecret, opts.TenantID) diff --git a/aci/compose.go b/aci/compose.go index d7fd4e32..fe6a3e1e 100644 --- a/aci/compose.go +++ b/aci/compose.go @@ -37,7 +37,7 @@ type aciComposeService struct { } func (cs *aciComposeService) Up(ctx context.Context, project *types.Project) error { - logrus.Debugf("Up on project with name %q\n", project.Name) + logrus.Debugf("Up on project with name %q", project.Name) groupDefinition, err := convert.ToContainerGroup(ctx, cs.ctx, *project) addTag(&groupDefinition, composeContainerTag) @@ -48,7 +48,7 @@ func (cs *aciComposeService) Up(ctx context.Context, project *types.Project) err } func (cs *aciComposeService) Down(ctx context.Context, project string) error { - logrus.Debugf("Down on project with name %q\n", project) + logrus.Debugf("Down on project with name %q", project) cg, err := deleteACIContainerGroup(ctx, cs.ctx, project) if err != nil { @@ -69,11 +69,11 @@ func (cs *aciComposeService) Ps(ctx context.Context, project string) ([]compose. group, err := groupsClient.Get(ctx, cs.ctx.ResourceGroup, project) if err != nil { - return []compose.ServiceStatus{}, err + return nil, err } - if group.Containers == nil || len(*group.Containers) < 1 { - return []compose.ServiceStatus{}, fmt.Errorf("no containers found in ACI container group %s", project) + if group.Containers == nil || len(*group.Containers) == 0 { + return nil, fmt.Errorf("no containers found in ACI container group %s", project) } res := []compose.ServiceStatus{} @@ -89,7 +89,7 @@ func (cs *aciComposeService) Ps(ctx context.Context, project string) ([]compose. func (cs *aciComposeService) List(ctx context.Context, project string) ([]compose.Stack, error) { containerGroups, err := getACIContainerGroups(ctx, cs.ctx.SubscriptionID, cs.ctx.ResourceGroup) if err != nil { - return []compose.Stack{}, err + return nil, err } stacks := []compose.Stack{} diff --git a/aci/containers.go b/aci/containers.go index afc48ec3..61c50b65 100644 --- a/aci/containers.go +++ b/aci/containers.go @@ -43,12 +43,12 @@ type aciContainerService struct { func (cs *aciContainerService) List(ctx context.Context, all bool) ([]containers.Container, error) { containerGroups, err := getACIContainerGroups(ctx, cs.ctx.SubscriptionID, cs.ctx.ResourceGroup) if err != nil { - return []containers.Container{}, err + return nil, err } var res []containers.Container for _, group := range containerGroups { - if group.Containers == nil || len(*group.Containers) < 1 { - return []containers.Container{}, fmt.Errorf("no containers found in ACI container group %s", *group.Name) + if group.Containers == nil || len(*group.Containers) == 0 { + return nil, fmt.Errorf("no containers found in ACI container group %s", *group.Name) } for _, container := range *group.Containers { @@ -64,7 +64,7 @@ func (cs *aciContainerService) List(ctx context.Context, all bool) ([]containers func (cs *aciContainerService) Run(ctx context.Context, r containers.ContainerConfig) error { if strings.Contains(r.ID, composeContainerSeparator) { - return errors.New(fmt.Sprintf("invalid container name. ACI container name cannot include %q", composeContainerSeparator)) + return fmt.Errorf("invalid container name. ACI container name cannot include %q", composeContainerSeparator) } project, err := convert.ContainerToComposeProject(r) @@ -72,7 +72,7 @@ func (cs *aciContainerService) Run(ctx context.Context, r containers.ContainerCo return err } - logrus.Debugf("Running container %q with name %q\n", r.Image, r.ID) + logrus.Debugf("Running container %q with name %q", r.Image, r.ID) groupDefinition, err := convert.ToContainerGroup(ctx, cs.ctx, project) if err != nil { return err @@ -86,7 +86,7 @@ func (cs *aciContainerService) Start(ctx context.Context, containerID string) er groupName, containerName := getGroupAndContainerName(containerID) if groupName != containerID { msg := "cannot start specified service %q from compose application %q, you can update and restart the entire compose app with docker compose up --project-name %s" - return errors.New(fmt.Sprintf(msg, containerName, groupName, groupName)) + return fmt.Errorf(msg, containerName, groupName, groupName) } containerGroupsClient, err := login.NewContainerGroupsClient(cs.ctx.SubscriptionID) @@ -110,12 +110,12 @@ func (cs *aciContainerService) Start(ctx context.Context, containerID string) er func (cs *aciContainerService) Stop(ctx context.Context, containerID string, timeout *uint32) error { if timeout != nil && *timeout != uint32(0) { - return errors.Errorf("ACI integration does not support setting a timeout to stop a container before killing it.") + return fmt.Errorf("the ACI integration does not support setting a timeout to stop a container before killing it") } groupName, containerName := getGroupAndContainerName(containerID) if groupName != containerID { msg := "cannot stop service %q from compose application %q, you can stop the entire compose app with docker stop %s" - return errors.New(fmt.Sprintf(msg, containerName, groupName, groupName)) + return fmt.Errorf(msg, containerName, groupName, groupName) } return stopACIContainerGroup(ctx, cs.ctx, groupName) } @@ -124,7 +124,7 @@ func (cs *aciContainerService) Kill(ctx context.Context, containerID string, _ s groupName, containerName := getGroupAndContainerName(containerID) if groupName != containerID { msg := "cannot kill service %q from compose application %q, you can kill the entire compose app with docker kill %s" - return errors.New(fmt.Sprintf(msg, containerName, groupName, groupName)) + return fmt.Errorf(msg, containerName, groupName, groupName) } return stopACIContainerGroup(ctx, cs.ctx, groupName) // As ACI doesn't have a kill command, we are using the stop implementation instead } @@ -187,7 +187,7 @@ func (cs *aciContainerService) Delete(ctx context.Context, containerID string, r groupName, containerName := getGroupAndContainerName(containerID) if groupName != containerID { msg := "cannot delete service %q from compose application %q, you can delete the entire compose app with docker compose down --project-name %s" - return errors.New(fmt.Sprintf(msg, containerName, groupName, groupName)) + return fmt.Errorf(msg, containerName, groupName, groupName) } if !request.Force { diff --git a/aci/volumes.go b/aci/volumes.go index 3a43d68a..250f0ffa 100644 --- a/aci/volumes.go +++ b/aci/volumes.go @@ -19,22 +19,19 @@ package aci import ( "context" "fmt" + "net/http" "strings" - "github.com/docker/compose-cli/progress" - - "github.com/Azure/go-autorest/autorest/to" - - "github.com/docker/compose-cli/aci/login" - - "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2019-06-01/storage" - - "github.com/docker/compose-cli/api/volumes" - "github.com/docker/compose-cli/errdefs" - "github.com/pkg/errors" + "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2019-06-01/storage" + "github.com/Azure/go-autorest/autorest/to" + + "github.com/docker/compose-cli/aci/login" + "github.com/docker/compose-cli/api/volumes" "github.com/docker/compose-cli/context/store" + "github.com/docker/compose-cli/errdefs" + "github.com/docker/compose-cli/progress" ) type aciVolumeService struct { @@ -75,7 +72,7 @@ func (cs *aciVolumeService) List(ctx context.Context) ([]volumes.Volume, error) return fileShares, nil } -//VolumeCreateOptions options to create a new ACI volume +// VolumeCreateOptions options to create a new ACI volume type VolumeCreateOptions struct { Account string Fileshare string @@ -84,7 +81,7 @@ type VolumeCreateOptions struct { func (cs *aciVolumeService) Create(ctx context.Context, options interface{}) (volumes.Volume, error) { opts, ok := options.(VolumeCreateOptions) if !ok { - return volumes.Volume{}, errors.New("Could not read azure VolumeCreateOptions struct from generic parameter") + return volumes.Volume{}, errors.New("could not read Azure VolumeCreateOptions struct from generic parameter") } w := progress.ContextWriter(ctx) w.Event(event(opts.Account, progress.Working, "Validating")) @@ -96,7 +93,7 @@ func (cs *aciVolumeService) Create(ctx context.Context, options interface{}) (vo if err == nil { w.Event(event(opts.Account, progress.Done, "Use existing")) } else { - if account.StatusCode != 404 { + if account.StatusCode != http.StatusNotFound { return volumes.Volume{}, err } result, err := accountClient.CheckNameAvailability(ctx, storage.AccountCheckNameAvailabilityParameters{ @@ -118,8 +115,7 @@ func (cs *aciVolumeService) Create(ctx context.Context, options interface{}) (vo w.Event(errorEvent(opts.Account)) return volumes.Volume{}, err } - err = future.WaitForCompletionRef(ctx, accountClient.Client) - if err != nil { + if err := future.WaitForCompletionRef(ctx, accountClient.Client); err != nil { w.Event(errorEvent(opts.Account)) return volumes.Volume{}, err } @@ -141,7 +137,7 @@ func (cs *aciVolumeService) Create(ctx context.Context, options interface{}) (vo w.Event(errorEvent(opts.Fileshare)) return volumes.Volume{}, errors.Wrapf(errdefs.ErrAlreadyExists, "Azure fileshare %q already exists", opts.Fileshare) } - if fileShare.StatusCode != 404 { + if fileShare.StatusCode != http.StatusNotFound { w.Event(errorEvent(opts.Fileshare)) return volumes.Volume{}, err } @@ -199,7 +195,7 @@ func (cs *aciVolumeService) Delete(ctx context.Context, id string, options inter if err == nil { if _, ok := account.Tags[dockerVolumeTag]; ok { result, err := storageAccountsClient.Delete(ctx, cs.aciContext.ResourceGroup, storageAccount) - if result.StatusCode == 204 { + if result.StatusCode == http.StatusNoContent { return errors.Wrapf(errdefs.ErrNotFound, "storage account %s does not exist", storageAccount) } return err @@ -209,7 +205,7 @@ func (cs *aciVolumeService) Delete(ctx context.Context, id string, options inter result, err := fileShareClient.Delete(ctx, cs.aciContext.ResourceGroup, storageAccount, fileshare) if result.StatusCode == 204 { - return errors.Wrapf(errdefs.ErrNotFound, "fileshare %s does not exist", fileshare) + return errors.Wrapf(errdefs.ErrNotFound, "fileshare %q", fileshare) } return err } diff --git a/api/client/client.go b/api/client/client.go index 632dc325..fc7c1c27 100644 --- a/api/client/client.go +++ b/api/client/client.go @@ -19,11 +19,10 @@ package client import ( "context" - "github.com/docker/compose-cli/api/volumes" - "github.com/docker/compose-cli/api/compose" "github.com/docker/compose-cli/api/containers" "github.com/docker/compose-cli/api/secrets" + "github.com/docker/compose-cli/api/volumes" "github.com/docker/compose-cli/backend" apicontext "github.com/docker/compose-cli/context" "github.com/docker/compose-cli/context/cloud" diff --git a/cli/cmd/volume/acivolume.go b/cli/cmd/volume/acivolume.go index a4e4dd6d..d419b8e1 100644 --- a/cli/cmd/volume/acivolume.go +++ b/cli/cmd/volume/acivolume.go @@ -58,8 +58,7 @@ func createVolume() *cobra.Command { return err } err = progress.Run(ctx, func(ctx context.Context) error { - _, err := c.VolumeService().Create(ctx, aciOpts) - if err != nil { + if _, err := c.VolumeService().Create(ctx, aciOpts); err != nil { return err } return nil diff --git a/cli/cmd/volume/acilist.go b/cli/cmd/volume/list.go similarity index 86% rename from cli/cmd/volume/acilist.go rename to cli/cmd/volume/list.go index 7261f342..bca77596 100644 --- a/cli/cmd/volume/acilist.go +++ b/cli/cmd/volume/list.go @@ -32,7 +32,7 @@ import ( func listVolume() *cobra.Command { cmd := &cobra.Command{ Use: "ls", - Short: "list Azure file shares usable as ACI volumes.", + Short: "list available volumes in context.", Args: cobra.ExactArgs(0), RunE: func(cmd *cobra.Command, args []string) error { c, err := client.New(cmd.Context()) @@ -53,14 +53,14 @@ func listVolume() *cobra.Command { func printList(out io.Writer, volumes []volumes.Volume) { printSection(out, func(w io.Writer) { for _, vol := range volumes { - fmt.Fprintf(w, "%s\t%s\n", vol.ID, vol.Description) // nolint:errcheck + _, _ = fmt.Fprintf(w, "%s\t%s\n", vol.ID, vol.Description) } }, "ID", "DESCRIPTION") } func printSection(out io.Writer, printer func(io.Writer), headers ...string) { w := tabwriter.NewWriter(out, 20, 1, 3, ' ', 0) - fmt.Fprintln(w, strings.Join(headers, "\t")) // nolint:errcheck + _, _ = fmt.Fprintln(w, strings.Join(headers, "\t")) printer(w) - w.Flush() // nolint:errcheck + _ = w.Flush() } diff --git a/cli/cmd/volume/acilist_test.go b/cli/cmd/volume/list_test.go similarity index 100% rename from cli/cmd/volume/acilist_test.go rename to cli/cmd/volume/list_test.go diff --git a/cli/main.go b/cli/main.go index db88150e..a4eadd1b 100644 --- a/cli/main.go +++ b/cli/main.go @@ -27,26 +27,16 @@ import ( "syscall" "time" - volume "github.com/docker/compose-cli/cli/cmd/volume" - "github.com/docker/compose-cli/cli/cmd/compose" - "github.com/docker/compose-cli/cli/cmd/logout" - + volume "github.com/docker/compose-cli/cli/cmd/volume" "github.com/docker/compose-cli/errdefs" - "github.com/pkg/errors" "github.com/sirupsen/logrus" "github.com/spf13/cobra" // Backend registrations _ "github.com/docker/compose-cli/aci" - _ "github.com/docker/compose-cli/ecs" - _ "github.com/docker/compose-cli/ecs/local" - _ "github.com/docker/compose-cli/example" - _ "github.com/docker/compose-cli/local" - "github.com/docker/compose-cli/metrics" - "github.com/docker/compose-cli/cli/cmd" contextcmd "github.com/docker/compose-cli/cli/cmd/context" "github.com/docker/compose-cli/cli/cmd/login" @@ -56,6 +46,11 @@ import ( "github.com/docker/compose-cli/config" apicontext "github.com/docker/compose-cli/context" "github.com/docker/compose-cli/context/store" + _ "github.com/docker/compose-cli/ecs" + _ "github.com/docker/compose-cli/ecs/local" + _ "github.com/docker/compose-cli/example" + _ "github.com/docker/compose-cli/local" + "github.com/docker/compose-cli/metrics" ) var ( diff --git a/ecs/backend.go b/ecs/backend.go index a615cbeb..8cc0a392 100644 --- a/ecs/backend.go +++ b/ecs/backend.go @@ -19,14 +19,12 @@ package ecs import ( "context" - "github.com/docker/compose-cli/api/volumes" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/session" - "github.com/docker/compose-cli/api/compose" "github.com/docker/compose-cli/api/containers" "github.com/docker/compose-cli/api/secrets" + "github.com/docker/compose-cli/api/volumes" "github.com/docker/compose-cli/backend" apicontext "github.com/docker/compose-cli/context" "github.com/docker/compose-cli/context/cloud" diff --git a/ecs/local/backend.go b/ecs/local/backend.go index cec7c104..c62ed640 100644 --- a/ecs/local/backend.go +++ b/ecs/local/backend.go @@ -19,17 +19,14 @@ package local import ( "context" - "github.com/docker/compose-cli/api/volumes" - - "github.com/docker/docker/client" - "github.com/docker/compose-cli/api/compose" "github.com/docker/compose-cli/api/containers" "github.com/docker/compose-cli/api/secrets" - + "github.com/docker/compose-cli/api/volumes" "github.com/docker/compose-cli/backend" "github.com/docker/compose-cli/context/cloud" "github.com/docker/compose-cli/context/store" + "github.com/docker/docker/client" ) const backendType = store.EcsLocalSimulationContextType From 07547d7b87817f11e732653ef3ee695ca66774fd Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Thu, 10 Sep 2020 15:08:47 +0200 Subject: [PATCH 13/13] ACI Volume create flags are required Signed-off-by: Guillaume Tardif --- cli/cmd/volume/acivolume.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cli/cmd/volume/acivolume.go b/cli/cmd/volume/acivolume.go index d419b8e1..4216c36d 100644 --- a/cli/cmd/volume/acivolume.go +++ b/cli/cmd/volume/acivolume.go @@ -48,7 +48,7 @@ func ACICommand() *cobra.Command { func createVolume() *cobra.Command { aciOpts := aci.VolumeCreateOptions{} cmd := &cobra.Command{ - Use: "create", + Use: "create --storage-account ACCOUNT --fileshare FILESHARE", Short: "Creates an Azure file share to use as ACI volume.", Args: cobra.ExactArgs(0), RunE: func(cmd *cobra.Command, args []string) error { @@ -73,6 +73,8 @@ func createVolume() *cobra.Command { cmd.Flags().StringVar(&aciOpts.Account, "storage-account", "", "Storage account name") cmd.Flags().StringVar(&aciOpts.Fileshare, "fileshare", "", "Fileshare name") + _ = cmd.MarkFlagRequired("fileshare") + _ = cmd.MarkFlagRequired("storage-account") return cmd }