Merge pull request #899 from docker/aci_volume_error_message

Specific error message when specifying path as a volume source

aci/convert/convert.go

@@ -50,7 +50,7 @@ const (
 func ToContainerGroup(ctx context.Context, aciContext store.AciContext, p types.Project, storageHelper login.StorageLogin) (containerinstance.ContainerGroup, error) {
 	project := projectAciHelper(p)
 	containerGroupName := strings.ToLower(project.Name)
-	volumesCache, volumesSlice, err := project.getAciFileVolumes(ctx, storageHelper)
+	volumesSlice, err := project.getAciFileVolumes(ctx, storageHelper)
 	if err != nil {
 		return containerinstance.ContainerGroup{}, err
 	}
@@ -90,7 +90,7 @@ func ToContainerGroup(ctx context.Context, aciContext store.AciContext, p types.
 	var dnsLabelName *string
 	for _, s := range project.Services {
 		service := serviceConfigAciHelper(s)
-		containerDefinition, err := service.getAciContainer(volumesCache)
+		containerDefinition, err := service.getAciContainer()
 		if err != nil {
 			return containerinstance.ContainerGroup{}, err
 		}
@@ -162,8 +162,8 @@ type projectAciHelper types.Project
 
 type serviceConfigAciHelper types.ServiceConfig
 
-func (s serviceConfigAciHelper) getAciContainer(volumesCache map[string]bool) (containerinstance.Container, error) {
+func (s serviceConfigAciHelper) getAciContainer() (containerinstance.Container, error) {
-	aciServiceVolumes, err := s.getAciFileVolumeMounts(volumesCache)
+	aciServiceVolumes, err := s.getAciFileVolumeMounts()
 	if err != nil {
 		return containerinstance.Container{}, err
 	}

aci/convert/volume.go

@@ -38,18 +38,17 @@ const (
 	volumeReadOnly                 = "read_only"
 )
 
-func (p projectAciHelper) getAciFileVolumes(ctx context.Context, helper login.StorageLogin) (map[string]bool, []containerinstance.Volume, error) {
+func (p projectAciHelper) getAciFileVolumes(ctx context.Context, helper login.StorageLogin) ([]containerinstance.Volume, error) {
-	azureFileVolumesMap := make(map[string]bool, len(p.Volumes))
 	var azureFileVolumesSlice []containerinstance.Volume
 	for name, v := range p.Volumes {
 		if v.Driver == azureFileDriverName {
 			shareName, ok := v.DriverOpts[volumeDriveroptsShareNameKey]
 			if !ok {
-				return nil, nil, fmt.Errorf("cannot retrieve fileshare name for Azurefile")
+				return nil, fmt.Errorf("cannot retrieve fileshare name for Azurefile")
 			}
 			accountName, ok := v.DriverOpts[volumeDriveroptsAccountNameKey]
 			if !ok {
-				return nil, nil, fmt.Errorf("cannot retrieve account name for Azurefile")
+				return nil, fmt.Errorf("cannot retrieve account name for Azurefile")
 			}
 			readOnly, ok := v.DriverOpts[volumeReadOnly]
 			if !ok {
@@ -57,11 +56,11 @@ func (p projectAciHelper) getAciFileVolumes(ctx context.Context, helper login.St
 			}
 			ro, err := strconv.ParseBool(readOnly)
 			if err != nil {
-				return nil, nil, fmt.Errorf("invalid mode %q for volume", readOnly)
+				return nil, fmt.Errorf("invalid mode %q for volume", readOnly)
 			}
 			accountKey, err := helper.GetAzureStorageAccountKey(ctx, accountName)
 			if err != nil {
-				return nil, nil, err
+				return nil, err
 			}
 			aciVolume := containerinstance.Volume{
 				Name: to.StringPtr(name),
@@ -72,18 +71,17 @@ func (p projectAciHelper) getAciFileVolumes(ctx context.Context, helper login.St
 					ReadOnly:           &ro,
 				},
 			}
-			azureFileVolumesMap[name] = true
 			azureFileVolumesSlice = append(azureFileVolumesSlice, aciVolume)
 		}
 	}
-	return azureFileVolumesMap, azureFileVolumesSlice, nil
+	return azureFileVolumesSlice, nil
 }
 
-func (s serviceConfigAciHelper) getAciFileVolumeMounts(volumesCache map[string]bool) ([]containerinstance.VolumeMount, error) {
+func (s serviceConfigAciHelper) getAciFileVolumeMounts() ([]containerinstance.VolumeMount, error) {
 	var aciServiceVolumes []containerinstance.VolumeMount
 	for _, sv := range s.Volumes {
-		if !volumesCache[sv.Source] {
+		if sv.Type == string(types.VolumeTypeBind) {
-			return []containerinstance.VolumeMount{}, fmt.Errorf("could not find volume source %q", sv.Source)
+			return []containerinstance.VolumeMount{}, fmt.Errorf("host path (%q) not allowed as volume source, you need to reference an Azure File Share defined in the 'volumes' section", sv.Source)
 		}
 		aciServiceVolumes = append(aciServiceVolumes, containerinstance.VolumeMount{
 			Name:      to.StringPtr(sv.Source),

aci/convert/volume_test.go

@@ -119,6 +119,30 @@ func TestComposeVolumes(t *testing.T) {
 	assert.DeepEqual(t, (*group.Volumes)[0], expectedGroupVolume)
 }
 
+func TestPathVolumeErrorMessage(t *testing.T) {
+	ctx := context.TODO()
+	accountName := "myAccount"
+	mockStorageHelper.On("GetAzureStorageAccountKey", ctx, accountName).Return("123456", nil)
+	project := types.Project{
+		Services: []types.ServiceConfig{
+			{
+				Name:  "service1",
+				Image: "image1",
+				Volumes: []types.ServiceVolumeConfig{
+					{
+						Source: "/path",
+						Target: "/target",
+						Type:   string(types.VolumeTypeBind),
+					},
+				},
+			},
+		},
+	}
+
+	_, err := ToContainerGroup(ctx, convertCtx, project, mockStorageHelper)
+	assert.ErrorContains(t, err, `host path ("/path") not allowed as volume source, you need to reference an Azure File Share defined in the 'volumes' section`)
+}
+
 func TestComposeVolumesRO(t *testing.T) {
 	ctx := context.TODO()
 	accountName := "myAccount"