Merge pull request #603 from docker/aci_volume_ids

Change volume IDs from “storageaccount@fileshare“ to “storageaccount/fileshare”

aci/convert/convert.go

@@ -207,7 +207,7 @@ func (p projectAciHelper) getAciFileVolumes(ctx context.Context, helper login.St
 		if v.Driver == azureFileDriverName {
 			shareName, ok := v.DriverOpts[volumeDriveroptsShareNameKey]
 			if !ok {
-				return nil, nil, fmt.Errorf("cannot retrieve share name for Azurefile")
+				return nil, nil, fmt.Errorf("cannot retrieve fileshare name for Azurefile")
 			}
 			accountName, ok := v.DriverOpts[volumeDriveroptsAccountNameKey]
 			if !ok {

aci/convert/volume.go

@@ -42,8 +42,8 @@ func GetRunVolumes(volumes []string) (map[string]types.VolumeConfig, []types.Ser
 			Name:   vi.name,
 			Driver: azureFileDriverName,
 			DriverOpts: map[string]string{
-				volumeDriveroptsAccountNameKey: vi.username,
-				volumeDriveroptsShareNameKey:   vi.share,
+				volumeDriveroptsAccountNameKey: vi.storageAccount,
+				volumeDriveroptsShareNameKey:   vi.fileshare,
 			},
 		}
 		sv := types.ServiceVolumeConfig{
@@ -59,28 +59,29 @@ func GetRunVolumes(volumes []string) (map[string]types.VolumeConfig, []types.Ser
 
 type volumeInput struct {
 	name           string
-	username string
-	share    string
+	storageAccount string
+	fileshare      string
 	target         string
 }
 
 func (v *volumeInput) parse(name string, s string) error {
 	v.name = name
-	tokens := strings.Split(s, "@")
-	if len(tokens) < 2 || tokens[0] == "" {
-		return errors.Wrapf(errdefs.ErrParsingFailed, "volume specification %q does not include a storage account before '@'", v)
+	tokens := strings.Split(s, ":")
+	source := tokens[0]
+	sourceTokens := strings.Split(source, "/")
+	if len(sourceTokens) < 2 || sourceTokens[0] == "" {
+		return errors.Wrapf(errdefs.ErrParsingFailed, "volume specification %q does not include a storage account before '/'", v)
 	}
-	v.username = tokens[0]
-	remaining := tokens[1]
-	tokens = strings.Split(remaining, ":")
-	if tokens[0] == "" {
-		return errors.Wrapf(errdefs.ErrParsingFailed, "volume specification %q does not include a storage file share after '@'", v)
+	v.storageAccount = sourceTokens[0]
+	if sourceTokens[1] == "" {
+		return errors.Wrapf(errdefs.ErrParsingFailed, "volume specification %q does not include a storage file fileshare after '/'", v)
 	}
-	v.share = tokens[0]
+	v.fileshare = sourceTokens[1]
+
 	if len(tokens) > 1 {
 		v.target = tokens[1]
 	} else {
-		v.target = "/run/volumes/" + v.share
+		v.target = "/run/volumes/" + v.fileshare
 	}
 	return nil
 }

aci/convert/volume_test.go

@@ -30,9 +30,9 @@ const (
 
 func TestGetRunVolumes(t *testing.T) {
 	volumeStrings := []string{
-		"myuser1@myshare1:/my/path/to/target1",
-		"myuser2@myshare2:/my/path/to/target2",
-		"myuser3@mydefaultsharename", // Use default placement at '/run/volumes/<share_name>'
+		"myuser1/myshare1:/my/path/to/target1",
+		"myuser2/myshare2:/my/path/to/target2",
+		"myuser3/mydefaultsharename", // Use default placement at '/run/volumes/<share_name>'
 	}
 	var goldenVolumeConfigs = map[string]types.VolumeConfig{
 		"volume-0": {
@@ -89,16 +89,16 @@ func TestGetRunVolumes(t *testing.T) {
 }
 
 func TestGetRunVolumesMissingFileShare(t *testing.T) {
-	_, _, err := GetRunVolumes([]string{"myaccount@"})
-	assert.ErrorContains(t, err, "does not include a storage file share after '@'")
+	_, _, err := GetRunVolumes([]string{"myaccount/"})
+	assert.ErrorContains(t, err, "does not include a storage file fileshare after '/'")
 }
 
 func TestGetRunVolumesMissingUser(t *testing.T) {
-	_, _, err := GetRunVolumes([]string{"@myshare"})
-	assert.ErrorContains(t, err, "does not include a storage account before '@'")
+	_, _, err := GetRunVolumes([]string{"/myshare"})
+	assert.ErrorContains(t, err, "does not include a storage account before '/'")
 }
 
 func TestGetRunVolumesNoShare(t *testing.T) {
 	_, _, err := GetRunVolumes([]string{"noshare"})
-	assert.ErrorContains(t, err, "does not include a storage account before '@'")
+	assert.ErrorContains(t, err, "does not include a storage account before '/'")
 }

aci/volumes.go

@@ -167,7 +167,7 @@ func errorEvent(resource string) progress.Event {
 }
 
 func (cs *aciVolumeService) Delete(ctx context.Context, id string, options interface{}) error {
-	tokens := strings.Split(id, "@")
+	tokens := strings.Split(id, "/")
 	if len(tokens) != 2 {
 		return errors.New("wrong format for volume ID : should be storageaccount@fileshare")
 	}
@@ -219,7 +219,7 @@ func toVolume(account storage.Account, fileShareName string) volumes.Volume {
 
 // VolumeID generate volume ID from azure storage accoun & fileshare
 func VolumeID(storageAccount string, fileShareName string) string {
-	return fmt.Sprintf("%s@%s", storageAccount, fileShareName)
+	return fmt.Sprintf("%s/%s", storageAccount, fileShareName)
 }
 
 func defaultStorageAccountParams(aciContext store.AciContext) storage.AccountCreateParameters {

cli/cmd/volume/list_test.go

@@ -28,7 +28,7 @@ import (
 func TestPrintList(t *testing.T) {
 	secrets := []volumes.Volume{
 		{
-			ID:          "volume@123",
+			ID:          "volume/123",
 			Description: "volume 123",
 		},
 	}

cli/cmd/volume/testdata/volumes-out.golden

@@ -1,2 +1,2 @@
 ID                  DESCRIPTION
-volume@123          volume 123
+volume/123          volume 123

tests/aci-e2e/e2e-aci_test.go

@@ -162,7 +162,7 @@ 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
+	volumeID = accountName + "/" + fileshareName
 
 	t.Cleanup(func() {
 		c.RunDockerCmd("volume", "rm", volumeID)
@@ -174,7 +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"
+	volumeID2 := accountName + "/dockertestshare2"
 
 	t.Run("list volumes", func(t *testing.T) {
 		res := c.RunDockerCmd("volume", "ls")