Moving code from global convert.go to volumes.go, ports.go

Signed-off-by: Guillaume Tardif <guillaume.tardif@docker.com>

aci/convert/convert.go

@@ -42,11 +42,7 @@ const (
 	// ComposeDNSSidecarName name of the dns sidecar container
 	ComposeDNSSidecarName = "aci--dns--sidecar"
 
-	dnsSidecarImage                = "busybox:1.31.1"
+	dnsSidecarImage = "busybox:1.31.1"
-	azureFileDriverName            = "azure_file"
-	volumeDriveroptsShareNameKey   = "share_name"
-	volumeDriveroptsAccountNameKey = "storage_account_name"
-	volumeReadOnly                 = "read_only"
 )
 
 // ToContainerGroup converts a compose project into a ACI container group
@@ -132,31 +128,6 @@ func ToContainerGroup(ctx context.Context, aciContext store.AciContext, p types.
 	return groupDefinition, nil
 }
 
-func convertPortsToAci(service serviceConfigAciHelper) ([]containerinstance.ContainerPort, []containerinstance.Port, *string, error) {
-	var groupPorts []containerinstance.Port
-	var containerPorts []containerinstance.ContainerPort
-	for _, portConfig := range service.Ports {
-		if portConfig.Published != 0 && portConfig.Published != portConfig.Target {
-			msg := fmt.Sprintf("Port mapping is not supported with ACI, cannot map port %d to %d for container %s",
-				portConfig.Published, portConfig.Target, service.Name)
-			return nil, nil, nil, errors.New(msg)
-		}
-		portNumber := int32(portConfig.Target)
-		containerPorts = append(containerPorts, containerinstance.ContainerPort{
-			Port: to.Int32Ptr(portNumber),
-		})
-		groupPorts = append(groupPorts, containerinstance.Port{
-			Port:     to.Int32Ptr(portNumber),
-			Protocol: containerinstance.TCP,
-		})
-	}
-	var dnsLabelName *string = nil
-	if service.DomainName != "" {
-		dnsLabelName = &service.DomainName
-	}
-	return containerPorts, groupPorts, dnsLabelName, nil
-}
-
 func getDNSSidecar(containers []containerinstance.Container) containerinstance.Container {
 	var commands []string
 	for _, container := range containers {
@@ -184,47 +155,6 @@ func getDNSSidecar(containers []containerinstance.Container) containerinstance.C
 
 type projectAciHelper types.Project
 
-func (p projectAciHelper) getAciFileVolumes(ctx context.Context, helper login.StorageLogin) (map[string]bool, []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")
-			}
-			accountName, ok := v.DriverOpts[volumeDriveroptsAccountNameKey]
-			if !ok {
-				return nil, nil, fmt.Errorf("cannot retrieve account name for Azurefile")
-			}
-			readOnly, ok := v.DriverOpts[volumeReadOnly]
-			if !ok {
-				readOnly = "false"
-			}
-			ro, err := strconv.ParseBool(readOnly)
-			if err != nil {
-				return nil, nil, fmt.Errorf("invalid mode %q for volume", readOnly)
-			}
-			accountKey, err := helper.GetAzureStorageAccountKey(ctx, accountName)
-			if err != nil {
-				return nil, nil, err
-			}
-			aciVolume := containerinstance.Volume{
-				Name: to.StringPtr(name),
-				AzureFile: &containerinstance.AzureFileVolume{
-					ShareName:          to.StringPtr(shareName),
-					StorageAccountName: to.StringPtr(accountName),
-					StorageAccountKey:  to.StringPtr(accountKey),
-					ReadOnly:           &ro,
-				},
-			}
-			azureFileVolumesMap[name] = true
-			azureFileVolumesSlice = append(azureFileVolumesSlice, aciVolume)
-		}
-	}
-	return azureFileVolumesMap, azureFileVolumesSlice, nil
-}
-
 func (p projectAciHelper) getRestartPolicy() (containerinstance.ContainerGroupRestartPolicy, error) {
 	var restartPolicyCondition containerinstance.ContainerGroupRestartPolicy
 	if len(p.Services) >= 1 {
@@ -275,20 +205,6 @@ func toContainerRestartPolicy(aciRestartPolicy containerinstance.ContainerGroupR
 
 type serviceConfigAciHelper types.ServiceConfig
 
-func (s serviceConfigAciHelper) getAciFileVolumeMounts(volumesCache map[string]bool) ([]containerinstance.VolumeMount, error) {
-	var aciServiceVolumes []containerinstance.VolumeMount
-	for _, sv := range s.Volumes {
-		if !volumesCache[sv.Source] {
-			return []containerinstance.VolumeMount{}, fmt.Errorf("could not find volume source %q", sv.Source)
-		}
-		aciServiceVolumes = append(aciServiceVolumes, containerinstance.VolumeMount{
-			Name:      to.StringPtr(sv.Source),
-			MountPath: to.StringPtr(sv.Target),
-		})
-	}
-	return aciServiceVolumes, nil
-}
-
 func (s serviceConfigAciHelper) getAciContainer(volumesCache map[string]bool) (containerinstance.Container, error) {
 	aciServiceVolumes, err := s.getAciFileVolumeMounts(volumesCache)
 	if err != nil {

aci/convert/convert_test.go

@@ -21,8 +21,6 @@ import (
 	"os"
 	"testing"
 
-	"github.com/stretchr/testify/mock"
-
 	"github.com/Azure/azure-sdk-for-go/services/containerinstance/mgmt/2018-10-01/containerinstance"
 	"github.com/Azure/go-autorest/autorest/to"
 	"github.com/compose-spec/compose-go/types"
@@ -206,96 +204,6 @@ func TestComposeSingleContainerGroupToContainerNoDnsSideCarSide(t *testing.T) {
 	assert.Equal(t, *(*group.Containers)[0].Image, "image1")
 }
 
-func TestComposeVolumes(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.Volumes{
-			"vol1": types.VolumeConfig{
-				Driver: "azure_file",
-				DriverOpts: map[string]string{
-					"share_name":           "myFileshare",
-					"storage_account_name": accountName,
-				},
-			},
-		},
-	}
-
-	group, err := ToContainerGroup(ctx, convertCtx, project, mockStorageHelper)
-	assert.NilError(t, err)
-
-	assert.Assert(t, is.Len(*group.Containers, 1))
-	assert.Equal(t, *(*group.Containers)[0].Name, "service1")
-	expectedGroupVolume := containerinstance.Volume{
-		Name: to.StringPtr("vol1"),
-		AzureFile: &containerinstance.AzureFileVolume{
-			ShareName:          to.StringPtr("myFileshare"),
-			StorageAccountName: &accountName,
-			StorageAccountKey:  to.StringPtr("123456"),
-			ReadOnly:           to.BoolPtr(false),
-		},
-	}
-	assert.Equal(t, len(*group.Volumes), 1)
-	assert.DeepEqual(t, (*group.Volumes)[0], expectedGroupVolume)
-}
-
-func TestComposeVolumesRO(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.Volumes{
-			"vol1": types.VolumeConfig{
-				Driver: "azure_file",
-				DriverOpts: map[string]string{
-					"share_name":           "myFileshare",
-					"storage_account_name": accountName,
-					"read_only":            "true",
-				},
-			},
-		},
-	}
-
-	group, err := ToContainerGroup(ctx, convertCtx, project, mockStorageHelper)
-	assert.NilError(t, err)
-
-	assert.Assert(t, is.Len(*group.Containers, 1))
-	assert.Equal(t, *(*group.Containers)[0].Name, "service1")
-	expectedGroupVolume := containerinstance.Volume{
-		Name: to.StringPtr("vol1"),
-		AzureFile: &containerinstance.AzureFileVolume{
-			ShareName:          to.StringPtr("myFileshare"),
-			StorageAccountName: &accountName,
-			StorageAccountKey:  to.StringPtr("123456"),
-			ReadOnly:           to.BoolPtr(true),
-		},
-	}
-	assert.Equal(t, len(*group.Volumes), 1)
-	assert.DeepEqual(t, (*group.Volumes)[0], expectedGroupVolume)
-}
-
-type mockStorageLogin struct {
-	mock.Mock
-}
-
-func (s *mockStorageLogin) GetAzureStorageAccountKey(ctx context.Context, accountName string) (string, error) {
-	args := s.Called(ctx, accountName)
-	return args.String(0), args.Error(1)
-}
-
 func TestComposeSingleContainerRestartPolicy(t *testing.T) {
 	project := types.Project{
 		Services: []types.ServiceConfig{
@@ -416,53 +324,6 @@ func TestComposeSingleContainerGroupToContainerDefaultRestartPolicy(t *testing.T
 	assert.Equal(t, group.RestartPolicy, containerinstance.Always)
 }
 
-func TestComposeContainerGroupToContainerMultiplePorts(t *testing.T) {
-	project := types.Project{
-		Services: []types.ServiceConfig{
-			{
-				Name:  "service1",
-				Image: "image1",
-				Ports: []types.ServicePortConfig{
-					{
-						Published: 80,
-						Target:    80,
-					},
-				},
-			},
-			{
-				Name:  "service2",
-				Image: "image2",
-				Ports: []types.ServicePortConfig{
-					{
-						Published: 8080,
-						Target:    8080,
-					},
-				},
-			},
-		},
-	}
-
-	group, err := ToContainerGroup(context.TODO(), convertCtx, project, mockStorageHelper)
-	assert.NilError(t, err)
-	assert.Assert(t, is.Len(*group.Containers, 3))
-
-	container1 := (*group.Containers)[0]
-	assert.Equal(t, *container1.Name, "service1")
-	assert.Equal(t, *container1.Image, "image1")
-	assert.Equal(t, *(*container1.Ports)[0].Port, int32(80))
-
-	container2 := (*group.Containers)[1]
-	assert.Equal(t, *container2.Name, "service2")
-	assert.Equal(t, *container2.Image, "image2")
-	assert.Equal(t, *(*container2.Ports)[0].Port, int32(8080))
-
-	groupPorts := *group.IPAddress.Ports
-	assert.Assert(t, is.Len(groupPorts, 2))
-	assert.Equal(t, *groupPorts[0].Port, int32(80))
-	assert.Equal(t, *groupPorts[1].Port, int32(8080))
-	assert.Assert(t, group.IPAddress.DNSNameLabel == nil)
-}
-
 func TestComposeContainerGroupToContainerWithDomainName(t *testing.T) {
 	project := types.Project{
 		Services: []types.ServiceConfig{

aci/convert/ports.go

@@ -17,13 +17,41 @@
 package convert
 
 import (
+	"fmt"
 	"strings"
 
 	"github.com/Azure/azure-sdk-for-go/services/containerinstance/mgmt/2018-10-01/containerinstance"
+	"github.com/Azure/go-autorest/autorest/to"
+	"github.com/pkg/errors"
 
 	"github.com/docker/compose-cli/api/containers"
 )
 
+func convertPortsToAci(service serviceConfigAciHelper) ([]containerinstance.ContainerPort, []containerinstance.Port, *string, error) {
+	var groupPorts []containerinstance.Port
+	var containerPorts []containerinstance.ContainerPort
+	for _, portConfig := range service.Ports {
+		if portConfig.Published != 0 && portConfig.Published != portConfig.Target {
+			msg := fmt.Sprintf("Port mapping is not supported with ACI, cannot map port %d to %d for container %s",
+				portConfig.Published, portConfig.Target, service.Name)
+			return nil, nil, nil, errors.New(msg)
+		}
+		portNumber := int32(portConfig.Target)
+		containerPorts = append(containerPorts, containerinstance.ContainerPort{
+			Port: to.Int32Ptr(portNumber),
+		})
+		groupPorts = append(groupPorts, containerinstance.Port{
+			Port:     to.Int32Ptr(portNumber),
+			Protocol: containerinstance.TCP,
+		})
+	}
+	var dnsLabelName *string = nil
+	if service.DomainName != "" {
+		dnsLabelName = &service.DomainName
+	}
+	return containerPorts, groupPorts, dnsLabelName, nil
+}
+
 // ToPorts converts Azure container ports to api ports
 func ToPorts(ipAddr *containerinstance.IPAddress, ports []containerinstance.ContainerPort) []containers.Port {
 	var result []containers.Port

aci/convert/ports_test.go

@@ -17,15 +17,65 @@
 package convert
 
 import (
+	"context"
 	"testing"
 
 	"github.com/Azure/azure-sdk-for-go/profiles/latest/containerinstance/mgmt/containerinstance"
 	"github.com/Azure/go-autorest/autorest/to"
+	"github.com/compose-spec/compose-go/types"
 	"gotest.tools/v3/assert"
+	is "gotest.tools/v3/assert/cmp"
 
 	"github.com/docker/compose-cli/api/containers"
 )
 
+func TestComposeContainerGroupToContainerMultiplePorts(t *testing.T) {
+	project := types.Project{
+		Services: []types.ServiceConfig{
+			{
+				Name:  "service1",
+				Image: "image1",
+				Ports: []types.ServicePortConfig{
+					{
+						Published: 80,
+						Target:    80,
+					},
+				},
+			},
+			{
+				Name:  "service2",
+				Image: "image2",
+				Ports: []types.ServicePortConfig{
+					{
+						Published: 8080,
+						Target:    8080,
+					},
+				},
+			},
+		},
+	}
+
+	group, err := ToContainerGroup(context.TODO(), convertCtx, project, mockStorageHelper)
+	assert.NilError(t, err)
+	assert.Assert(t, is.Len(*group.Containers, 3))
+
+	container1 := (*group.Containers)[0]
+	assert.Equal(t, *container1.Name, "service1")
+	assert.Equal(t, *container1.Image, "image1")
+	assert.Equal(t, *(*container1.Ports)[0].Port, int32(80))
+
+	container2 := (*group.Containers)[1]
+	assert.Equal(t, *container2.Name, "service2")
+	assert.Equal(t, *container2.Image, "image2")
+	assert.Equal(t, *(*container2.Ports)[0].Port, int32(8080))
+
+	groupPorts := *group.IPAddress.Ports
+	assert.Assert(t, is.Len(groupPorts, 2))
+	assert.Equal(t, *groupPorts[0].Port, int32(80))
+	assert.Equal(t, *groupPorts[1].Port, int32(8080))
+	assert.Assert(t, group.IPAddress.DNSNameLabel == nil)
+}
+
 func TestPortConvert(t *testing.T) {
 	expectedPorts := []containers.Port{
 		{

aci/convert/secrets_test.go

@@ -27,7 +27,6 @@ import (
 	"gotest.tools/v3/assert"
 )
 
-
 func TestConvertSecrets(t *testing.T) {
 	serviceName := "testservice"
 	secretName := "testsecret"
@@ -177,4 +176,4 @@ func TestConvertSecrets(t *testing.T) {
 			fmt.Sprintf(`mount paths %q and %q collide. A volume mount cannot include another one.`,
 				path.Dir(targetName1), path.Dir(targetName2)))
 	})
-}
+}

aci/convert/volume.go

@@ -17,17 +17,82 @@
 package convert
 
 import (
+	"context"
 	"fmt"
 	"strconv"
 	"strings"
 
-	"github.com/pkg/errors"
+	"github.com/Azure/azure-sdk-for-go/services/containerinstance/mgmt/2018-10-01/containerinstance"
-
+	"github.com/Azure/go-autorest/autorest/to"
 	"github.com/compose-spec/compose-go/types"
+	"github.com/docker/compose-cli/aci/login"
+	"github.com/pkg/errors"
 
 	"github.com/docker/compose-cli/errdefs"
 )
 
+const (
+	azureFileDriverName            = "azure_file"
+	volumeDriveroptsShareNameKey   = "share_name"
+	volumeDriveroptsAccountNameKey = "storage_account_name"
+	volumeReadOnly                 = "read_only"
+)
+
+func (p projectAciHelper) getAciFileVolumes(ctx context.Context, helper login.StorageLogin) (map[string]bool, []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")
+			}
+			accountName, ok := v.DriverOpts[volumeDriveroptsAccountNameKey]
+			if !ok {
+				return nil, nil, fmt.Errorf("cannot retrieve account name for Azurefile")
+			}
+			readOnly, ok := v.DriverOpts[volumeReadOnly]
+			if !ok {
+				readOnly = "false"
+			}
+			ro, err := strconv.ParseBool(readOnly)
+			if err != nil {
+				return nil, nil, fmt.Errorf("invalid mode %q for volume", readOnly)
+			}
+			accountKey, err := helper.GetAzureStorageAccountKey(ctx, accountName)
+			if err != nil {
+				return nil, nil, err
+			}
+			aciVolume := containerinstance.Volume{
+				Name: to.StringPtr(name),
+				AzureFile: &containerinstance.AzureFileVolume{
+					ShareName:          to.StringPtr(shareName),
+					StorageAccountName: to.StringPtr(accountName),
+					StorageAccountKey:  to.StringPtr(accountKey),
+					ReadOnly:           &ro,
+				},
+			}
+			azureFileVolumesMap[name] = true
+			azureFileVolumesSlice = append(azureFileVolumesSlice, aciVolume)
+		}
+	}
+	return azureFileVolumesMap, azureFileVolumesSlice, nil
+}
+
+func (s serviceConfigAciHelper) getAciFileVolumeMounts(volumesCache map[string]bool) ([]containerinstance.VolumeMount, error) {
+	var aciServiceVolumes []containerinstance.VolumeMount
+	for _, sv := range s.Volumes {
+		if !volumesCache[sv.Source] {
+			return []containerinstance.VolumeMount{}, fmt.Errorf("could not find volume source %q", sv.Source)
+		}
+		aciServiceVolumes = append(aciServiceVolumes, containerinstance.VolumeMount{
+			Name:      to.StringPtr(sv.Source),
+			MountPath: to.StringPtr(sv.Target),
+		})
+	}
+	return aciServiceVolumes, nil
+}
+
 // GetRunVolumes return volume configurations for a project and a single service
 // this is meant to be used as a compose project of a single service
 func GetRunVolumes(volumes []string) (map[string]types.VolumeConfig, []types.ServiceVolumeConfig, error) {

aci/convert/volume_test.go

@@ -17,11 +17,16 @@
 package convert
 
 import (
+	"context"
 	"strconv"
 	"testing"
 
+	"github.com/Azure/azure-sdk-for-go/services/containerinstance/mgmt/2018-10-01/containerinstance"
+	"github.com/Azure/go-autorest/autorest/to"
 	"github.com/compose-spec/compose-go/types"
+	"github.com/stretchr/testify/mock"
 	"gotest.tools/v3/assert"
+	is "gotest.tools/v3/assert/cmp"
 )
 
 func TestGetRunVolumes(t *testing.T) {
@@ -74,6 +79,96 @@ func TestGetRunVolumesInvalidOption(t *testing.T) {
 	assert.ErrorContains(t, err, `volume specification "myuser4/myshare4:/my/path/to/target4:invalid" has an invalid mode "invalid"`)
 }
 
+func TestComposeVolumes(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.Volumes{
+			"vol1": types.VolumeConfig{
+				Driver: "azure_file",
+				DriverOpts: map[string]string{
+					"share_name":           "myFileshare",
+					"storage_account_name": accountName,
+				},
+			},
+		},
+	}
+
+	group, err := ToContainerGroup(ctx, convertCtx, project, mockStorageHelper)
+	assert.NilError(t, err)
+
+	assert.Assert(t, is.Len(*group.Containers, 1))
+	assert.Equal(t, *(*group.Containers)[0].Name, "service1")
+	expectedGroupVolume := containerinstance.Volume{
+		Name: to.StringPtr("vol1"),
+		AzureFile: &containerinstance.AzureFileVolume{
+			ShareName:          to.StringPtr("myFileshare"),
+			StorageAccountName: &accountName,
+			StorageAccountKey:  to.StringPtr("123456"),
+			ReadOnly:           to.BoolPtr(false),
+		},
+	}
+	assert.Equal(t, len(*group.Volumes), 1)
+	assert.DeepEqual(t, (*group.Volumes)[0], expectedGroupVolume)
+}
+
+func TestComposeVolumesRO(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.Volumes{
+			"vol1": types.VolumeConfig{
+				Driver: "azure_file",
+				DriverOpts: map[string]string{
+					"share_name":           "myFileshare",
+					"storage_account_name": accountName,
+					"read_only":            "true",
+				},
+			},
+		},
+	}
+
+	group, err := ToContainerGroup(ctx, convertCtx, project, mockStorageHelper)
+	assert.NilError(t, err)
+
+	assert.Assert(t, is.Len(*group.Containers, 1))
+	assert.Equal(t, *(*group.Containers)[0].Name, "service1")
+	expectedGroupVolume := containerinstance.Volume{
+		Name: to.StringPtr("vol1"),
+		AzureFile: &containerinstance.AzureFileVolume{
+			ShareName:          to.StringPtr("myFileshare"),
+			StorageAccountName: &accountName,
+			StorageAccountKey:  to.StringPtr("123456"),
+			ReadOnly:           to.BoolPtr(true),
+		},
+	}
+	assert.Equal(t, len(*group.Volumes), 1)
+	assert.DeepEqual(t, (*group.Volumes)[0], expectedGroupVolume)
+}
+
+type mockStorageLogin struct {
+	mock.Mock
+}
+
+func (s *mockStorageLogin) GetAzureStorageAccountKey(ctx context.Context, accountName string) (string, error) {
+	args := s.Called(ctx, accountName)
+	return args.String(0), args.Error(1)
+}
+
 func getServiceVolumeConfig(source string, target string, readOnly bool) types.ServiceVolumeConfig {
 	return types.ServiceVolumeConfig{
 		Type:     "azure_file",