Added volume delete, can delete juste a file share or the storage account if confirmed

Signed-off-by: Guillaume Tardif <guillaume.tardif@docker.com>

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
 }
-

aci/cloud.go

@@ -18,7 +18,9 @@ package aci
 
 import (
 	"context"
+
 	"github.com/pkg/errors"
+
 	"github.com/docker/compose-cli/aci/login"
 )
 

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
 }
-
-

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))

aci/login/storagelogin.go

@@ -46,4 +46,4 @@ func (helper StorageLogin) GetAzureStorageAccountKey(ctx context.Context, accoun
 
 	key := (*result.Keys)[0]
 	return *key.Value, nil
-}
+}

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,
+		Tags: tags,
-		AccountPropertiesCreateParameters: &storage.AccountPropertiesCreateParameters{},
 	}
 }

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
+}

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
 }

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")
 }

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"
 )

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")
 }
-

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
-}
+}
+
+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
+}

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")
+		lines := lines(res.Stdout())
-		firstAccount := out[1]
+		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])
 }
 

tests/aci-e2e/storage/storage.go

@@ -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
-}