Merge pull request #736 from docker/check-volume-usage-on-delete

Look for all mounts before removing a volume

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

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