From 96d785a5bd8d9b5f7c137deaf8da743acd15ba9f Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Tue, 8 Sep 2020 15:27:56 +0200 Subject: [PATCH] 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 955c487ac..a5e084253 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 2d2e9757e..0dee380f9 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 07759a735..d7fd4e321 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 08cacbc3e..afc48ec3d 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 446eb56d9..13a3923f1 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 0b992a52c..78039d66e 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 34adfae9e..ea0ee378f 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 59a8b1d71..29562f7ec 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 37bb25c39..709681365 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 7e8c4e33a..3158cce94 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 ab21c75db..3096ef2a2 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 773624671..560be5369 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 2a360f6ec..cb3d1f091 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 9e83d1f30..000000000 --- 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