Merge pull request #645 from docker/volume_name

ACI Volumes : create takes one required arg, instead of required flag `--fileshare`

aci/volumes.go

@@ -74,11 +74,10 @@ func (cs *aciVolumeService) List(ctx context.Context) ([]volumes.Volume, error)
 
 // VolumeCreateOptions options to create a new ACI volume
 type VolumeCreateOptions struct {
-	Account   string
-	Fileshare string
+	Account string
 }
 
-func (cs *aciVolumeService) Create(ctx context.Context, options interface{}) (volumes.Volume, error) {
+func (cs *aciVolumeService) Create(ctx context.Context, name string, options interface{}) (volumes.Volume, error) {
 	opts, ok := options.(VolumeCreateOptions)
 	if !ok {
 		return volumes.Volume{}, errors.New("could not read Azure VolumeCreateOptions struct from generic parameter")
@@ -125,27 +124,27 @@ func (cs *aciVolumeService) Create(ctx context.Context, options interface{}) (vo
 		}
 		w.Event(event(opts.Account, progress.Done, "Created"))
 	}
-	w.Event(event(opts.Fileshare, progress.Working, "Creating"))
+	w.Event(event(name, progress.Working, "Creating"))
 	fileShareClient, err := login.NewFileShareClient(cs.aciContext.SubscriptionID)
 	if err != nil {
 		return volumes.Volume{}, err
 	}
 
-	fileShare, err := fileShareClient.Get(ctx, cs.aciContext.ResourceGroup, *account.Name, opts.Fileshare, "")
+	fileShare, err := fileShareClient.Get(ctx, cs.aciContext.ResourceGroup, *account.Name, name, "")
 	if err == nil {
-		w.Event(errorEvent(opts.Fileshare))
-		return volumes.Volume{}, errors.Wrapf(errdefs.ErrAlreadyExists, "Azure fileshare %q already exists", opts.Fileshare)
+		w.Event(errorEvent(name))
+		return volumes.Volume{}, errors.Wrapf(errdefs.ErrAlreadyExists, "Azure fileshare %q already exists", name)
 	}
 	if !fileShare.HasHTTPStatus(http.StatusNotFound) {
-		w.Event(errorEvent(opts.Fileshare))
+		w.Event(errorEvent(name))
 		return volumes.Volume{}, err
 	}
-	fileShare, err = fileShareClient.Create(ctx, cs.aciContext.ResourceGroup, *account.Name, opts.Fileshare, storage.FileShare{})
+	fileShare, err = fileShareClient.Create(ctx, cs.aciContext.ResourceGroup, *account.Name, name, storage.FileShare{})
 	if err != nil {
-		w.Event(errorEvent(opts.Fileshare))
+		w.Event(errorEvent(name))
 		return volumes.Volume{}, err
 	}
-	w.Event(event(opts.Fileshare, progress.Done, "Created"))
+	w.Event(event(name, progress.Done, "Created"))
 	return toVolume(account, *fileShare.Name), nil
 }
 

api/client/volume.go

@@ -30,7 +30,7 @@ func (c *volumeService) List(ctx context.Context) ([]volumes.Volume, error) {
 	return nil, errdefs.ErrNotImplemented
 }
 
-func (c *volumeService) Create(ctx context.Context, options interface{}) (volumes.Volume, error) {
+func (c *volumeService) Create(ctx context.Context, name string, options interface{}) (volumes.Volume, error) {
 	return volumes.Volume{}, errdefs.ErrNotImplemented
 }
 

api/volumes/api.go

@@ -31,7 +31,7 @@ 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)
+	Create(ctx context.Context, name string, options interface{}) (Volume, error)
 	// Delete deletes an existing volume
 	Delete(ctx context.Context, volumeID string, options interface{}) error
 }

cli/cmd/volume/acivolume.go

@@ -47,9 +47,9 @@ func ACICommand() *cobra.Command {
 func createVolume() *cobra.Command {
 	aciOpts := aci.VolumeCreateOptions{}
 	cmd := &cobra.Command{
-		Use:   "create --storage-account ACCOUNT --fileshare FILESHARE",
+		Use:   "create --storage-account ACCOUNT VOLUME",
 		Short: "Creates an Azure file share to use as ACI volume.",
-		Args:  cobra.ExactArgs(0),
+		Args:  cobra.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			ctx := cmd.Context()
 			c, err := client.New(ctx)
@@ -57,7 +57,7 @@ func createVolume() *cobra.Command {
 				return err
 			}
 			result, err := progress.Run(ctx, func(ctx context.Context) (string, error) {
-				volume, err := c.VolumeService().Create(ctx, aciOpts)
+				volume, err := c.VolumeService().Create(ctx, args[0], aciOpts)
 				if err != nil {
 					return "", err
 				}
@@ -72,8 +72,6 @@ func createVolume() *cobra.Command {
 	}
 
 	cmd.Flags().StringVar(&aciOpts.Account, "storage-account", "", "Storage account name")
-	cmd.Flags().StringVar(&aciOpts.Fileshare, "fileshare", "", "Fileshare name")
-	_ = cmd.MarkFlagRequired("fileshare")
 	_ = cmd.MarkFlagRequired("storage-account")
 	return cmd
 }

tests/aci-e2e/e2e-aci_test.go

@@ -152,7 +152,7 @@ func TestContainerRunVolume(t *testing.T) {
 
 	t.Run("check empty volume name validity", func(t *testing.T) {
 		invalidName := ""
-		res := c.RunDockerOrExitError("volume", "create", "--storage-account", invalidName, "--fileshare", fileshareName)
+		res := c.RunDockerOrExitError("volume", "create", "--storage-account", invalidName, fileshareName)
 		res.Assert(t, icmd.Expected{
 			ExitCode: 1,
 			Err:      `parameter=accountName constraint=MinLength value="" details: value length must be greater than or equal to 3`,
@@ -161,7 +161,7 @@ func TestContainerRunVolume(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 := c.RunDockerOrExitError("volume", "create", "--storage-account", invalidName, 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.",
@@ -169,7 +169,7 @@ func TestContainerRunVolume(t *testing.T) {
 	})
 
 	t.Run("create volumes", func(t *testing.T) {
-		c.RunDockerCmd("volume", "create", "--storage-account", accountName, "--fileshare", fileshareName)
+		c.RunDockerCmd("volume", "create", "--storage-account", accountName, fileshareName)
 	})
 	volumeID = accountName + "/" + fileshareName
 
@@ -181,7 +181,7 @@ func TestContainerRunVolume(t *testing.T) {
 	})
 
 	t.Run("create second fileshare", func(t *testing.T) {
-		c.RunDockerCmd("volume", "create", "--storage-account", accountName, "--fileshare", "dockertestshare2")
+		c.RunDockerCmd("volume", "create", "--storage-account", accountName, "dockertestshare2")
 	})
 	volumeID2 := accountName + "/dockertestshare2"