From 99186b1d047df7bbcb909b924469b0879854c1c6 Mon Sep 17 00:00:00 2001 From: Ulysses Souza Date: Thu, 8 Oct 2020 10:47:27 +0200 Subject: [PATCH] Look for all mounts before removing a volume This checks the volume mounts of all container groups from all resource groups before removing a volume/fileshare Signed-off-by: Ulysses Souza --- aci/volumes.go | 32 ++++++++++++++++++++++++++++++++ tests/aci-e2e/e2e-aci_test.go | 19 ++++++++++++++----- 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/aci/volumes.go b/aci/volumes.go index dc2e2dbed..e3408ca34 100644 --- a/aci/volumes.go +++ b/aci/volumes.go @@ -24,6 +24,7 @@ import ( "github.com/pkg/errors" + "github.com/Azure/azure-sdk-for-go/services/containerinstance/mgmt/2018-10-01/containerinstance" "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2019-06-01/storage" "github.com/Azure/go-autorest/autorest/to" @@ -164,7 +165,38 @@ func errorEvent(resource string) progress.Event { } } +func checkVolumeUsage(ctx context.Context, aciContext store.AciContext, id string) error { + containerGroups, err := getACIContainerGroups(ctx, aciContext.SubscriptionID, aciContext.ResourceGroup) + if err != nil { + return err + } + for _, cg := range containerGroups { + if hasVolume(cg.Volumes, id) { + return errors.Errorf("volume %q is used in container group %q", + id, *cg.Name) + } + } + return nil +} + +func hasVolume(volumes *[]containerinstance.Volume, id string) bool { + if volumes == nil { + return false + } + for _, v := range *volumes { + if v.AzureFile != nil && v.AzureFile.StorageAccountName != nil && v.AzureFile.ShareName != nil && + (*v.AzureFile.StorageAccountName+"/"+*v.AzureFile.ShareName) == id { + return true + } + } + return false +} + func (cs *aciVolumeService) Delete(ctx context.Context, id string, options interface{}) error { + err := checkVolumeUsage(ctx, cs.aciContext, id) + if err != nil { + return err + } tokens := strings.Split(id, "/") if len(tokens) != 2 { return errors.New("invalid format for volume ID, expected storageaccount/fileshare") diff --git a/tests/aci-e2e/e2e-aci_test.go b/tests/aci-e2e/e2e-aci_test.go index 98544f19b..d6fda9a11 100644 --- a/tests/aci-e2e/e2e-aci_test.go +++ b/tests/aci-e2e/e2e-aci_test.go @@ -545,17 +545,15 @@ func TestUpUpdate(t *testing.T) { overwriteFileStorageAccount(t, singlePortVolumesComposefile, composeAccountName) multiPortComposefile = filepath.Join(dstDir, multiPortComposefile) + volumeID := composeAccountName + "/" + fileshareName t.Run("compose up", func(t *testing.T) { const ( testFileName = "msg.txt" testFileContent = "VOLUME_OK" + projectName = "acidemo" ) c.RunDockerCmd("volume", "create", "--storage-account", composeAccountName, fileshareName) - volumeID := composeAccountName + "/" + fileshareName - t.Cleanup(func() { - c.RunDockerCmd("volume", "rm", volumeID) - }) // Bootstrap volume aciContext := store.AciContext{ @@ -568,7 +566,7 @@ func TestUpUpdate(t *testing.T) { dnsLabelName := "nginx-" + groupID fqdn := dnsLabelName + "." + location + ".azurecontainer.io" // Name of Compose project is taken from current folder "acie2e" - c.RunDockerCmd("compose", "up", "-f", singlePortVolumesComposefile, "--domainname", dnsLabelName, "--project-name", "acidemo") + c.RunDockerCmd("compose", "up", "-f", singlePortVolumesComposefile, "--domainname", dnsLabelName, "--project-name", projectName) res := c.RunDockerCmd("ps") out := lines(res.Stdout()) @@ -599,6 +597,17 @@ func TestUpUpdate(t *testing.T) { body := HTTPGetWithRetry(t, endpoint+"/volume_test/"+testFileName, http.StatusOK, 2*time.Second, 20*time.Second) assert.Assert(t, strings.Contains(body, testFileContent)) + + // Try to remove the volume while it's still in use + res = c.RunDockerOrExitError("volume", "rm", volumeID) + res.Assert(t, icmd.Expected{ + ExitCode: 1, + Err: fmt.Sprintf(`Error: volume "%s/%s" is used in container group %q`, + composeAccountName, fileshareName, projectName), + }) + }) + t.Cleanup(func() { + c.RunDockerCmd("volume", "rm", volumeID) }) t.Run("compose ps", func(t *testing.T) {