Merge pull request #637 from docker/volume_tests

Added tests on ACI volume conversion, mock storageLogin required to get storage account keys

aci/backend.go

@@ -88,13 +88,11 @@ func getCloudService() (cloud.Service, error) {
 }
 
 func getAciAPIService(aciCtx store.AciContext) *aciAPIService {
+	containerService := newContainerService(aciCtx)
+	composeService := newComposeService(aciCtx)
 	return &aciAPIService{
-		aciContainerService: &aciContainerService{
-			ctx: aciCtx,
-		},
-		aciComposeService: &aciComposeService{
-			ctx: aciCtx,
-		},
+		aciContainerService: &containerService,
+		aciComposeService:   &composeService,
 		aciVolumeService: &aciVolumeService{
 			aciContext: aciCtx,
 		},

aci/compose.go

@@ -33,12 +33,20 @@ import (
 )
 
 type aciComposeService struct {
-	ctx store.AciContext
+	ctx          store.AciContext
+	storageLogin login.StorageLoginImpl
+}
+
+func newComposeService(ctx store.AciContext) aciComposeService {
+	return aciComposeService{
+		ctx:          ctx,
+		storageLogin: login.StorageLoginImpl{AciContext: ctx},
+	}
 }
 
 func (cs *aciComposeService) Up(ctx context.Context, project *types.Project) error {
 	logrus.Debugf("Up on project with name %q", project.Name)
-	groupDefinition, err := convert.ToContainerGroup(ctx, cs.ctx, *project)
+	groupDefinition, err := convert.ToContainerGroup(ctx, cs.ctx, *project, cs.storageLogin)
 	addTag(&groupDefinition, composeContainerTag)
 
 	if err != nil {

aci/containers.go

@@ -37,7 +37,15 @@ import (
 )
 
 type aciContainerService struct {
-	ctx store.AciContext
+	ctx          store.AciContext
+	storageLogin login.StorageLoginImpl
+}
+
+func newContainerService(ctx store.AciContext) aciContainerService {
+	return aciContainerService{
+		ctx:          ctx,
+		storageLogin: login.StorageLoginImpl{AciContext: ctx},
+	}
 }
 
 func (cs *aciContainerService) List(ctx context.Context, all bool) ([]containers.Container, error) {
@@ -73,7 +81,7 @@ func (cs *aciContainerService) Run(ctx context.Context, r containers.ContainerCo
 	}
 
 	logrus.Debugf("Running container %q with name %q", r.Image, r.ID)
-	groupDefinition, err := convert.ToContainerGroup(ctx, cs.ctx, project)
+	groupDefinition, err := convert.ToContainerGroup(ctx, cs.ctx, project, cs.storageLogin)
 	if err != nil {
 		return err
 	}

aci/convert/convert.go

@@ -26,17 +26,16 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/docker/compose-cli/api/compose"
-	"github.com/docker/compose-cli/utils/formatter"
-
 	"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/pkg/errors"
 
 	"github.com/docker/compose-cli/aci/login"
+	"github.com/docker/compose-cli/api/compose"
 	"github.com/docker/compose-cli/api/containers"
 	"github.com/docker/compose-cli/context/store"
+	"github.com/docker/compose-cli/utils/formatter"
 )
 
 const (
@@ -54,12 +53,9 @@ const (
 )
 
 // ToContainerGroup converts a compose project into a ACI container group
-func ToContainerGroup(ctx context.Context, aciContext store.AciContext, p types.Project) (containerinstance.ContainerGroup, error) {
+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)
-	storageHelper := login.StorageLogin{
-		AciContext: aciContext,
-	}
 	volumesCache, volumesSlice, err := project.getAciFileVolumes(ctx, storageHelper)
 	if err != nil {
 		return containerinstance.ContainerGroup{}, err

aci/convert/convert_test.go

@@ -21,29 +21,33 @@ import (
 	"os"
 	"testing"
 
-	"github.com/docker/compose-cli/api/compose"
+	"github.com/stretchr/testify/mock"
 
-	"github.com/Azure/azure-sdk-for-go/profiles/latest/containerinstance/mgmt/containerinstance"
+	"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"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
 
+	"github.com/docker/compose-cli/api/compose"
 	"github.com/docker/compose-cli/api/containers"
 	"github.com/docker/compose-cli/context/store"
 )
 
-var convertCtx = store.AciContext{
-	SubscriptionID: "subID",
-	ResourceGroup:  "rg",
-	Location:       "eu",
-}
+var (
+	convertCtx = store.AciContext{
+		SubscriptionID: "subID",
+		ResourceGroup:  "rg",
+		Location:       "eu",
+	}
+	mockStorageHelper = &mockStorageLogin{}
+)
 
 func TestProjectName(t *testing.T) {
 	project := types.Project{
 		Name: "TEST",
 	}
-	containerGroup, err := ToContainerGroup(context.TODO(), convertCtx, project)
+	containerGroup, err := ToContainerGroup(context.TODO(), convertCtx, project, mockStorageHelper)
 	assert.NilError(t, err)
 	assert.Equal(t, *containerGroup.Name, "test")
 }
@@ -157,7 +161,7 @@ func TestComposeContainerGroupToContainerWithDnsSideCarSide(t *testing.T) {
 		},
 	}
 
-	group, err := ToContainerGroup(context.TODO(), convertCtx, project)
+	group, err := ToContainerGroup(context.TODO(), convertCtx, project, mockStorageHelper)
 	assert.NilError(t, err)
 	assert.Assert(t, is.Len(*group.Containers, 3))
 
@@ -182,7 +186,7 @@ func TestComposeSingleContainerGroupToContainerNoDnsSideCarSide(t *testing.T) {
 		},
 	}
 
-	group, err := ToContainerGroup(context.TODO(), convertCtx, project)
+	group, err := ToContainerGroup(context.TODO(), convertCtx, project, mockStorageHelper)
 	assert.NilError(t, err)
 
 	assert.Assert(t, is.Len(*group.Containers, 1))
@@ -190,6 +194,96 @@ 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{
@@ -205,7 +299,7 @@ func TestComposeSingleContainerRestartPolicy(t *testing.T) {
 		},
 	}
 
-	group, err := ToContainerGroup(context.TODO(), convertCtx, project)
+	group, err := ToContainerGroup(context.TODO(), convertCtx, project, mockStorageHelper)
 	assert.NilError(t, err)
 
 	assert.Assert(t, is.Len(*group.Containers, 1))
@@ -237,7 +331,7 @@ func TestComposeMultiContainerRestartPolicy(t *testing.T) {
 		},
 	}
 
-	group, err := ToContainerGroup(context.TODO(), convertCtx, project)
+	group, err := ToContainerGroup(context.TODO(), convertCtx, project, mockStorageHelper)
 	assert.NilError(t, err)
 
 	assert.Assert(t, is.Len(*group.Containers, 3))
@@ -271,7 +365,7 @@ func TestComposeInconsistentMultiContainerRestartPolicy(t *testing.T) {
 		},
 	}
 
-	_, err := ToContainerGroup(context.TODO(), convertCtx, project)
+	_, err := ToContainerGroup(context.TODO(), convertCtx, project, mockStorageHelper)
 	assert.Error(t, err, "ACI integration does not support specifying different restart policies on containers in the same compose application")
 }
 
@@ -288,7 +382,7 @@ func TestLabelsErrorMessage(t *testing.T) {
 		},
 	}
 
-	_, err := ToContainerGroup(context.TODO(), convertCtx, project)
+	_, err := ToContainerGroup(context.TODO(), convertCtx, project, mockStorageHelper)
 	assert.Error(t, err, "ACI integration does not support labels in compose applications")
 }
 
@@ -302,7 +396,7 @@ func TestComposeSingleContainerGroupToContainerDefaultRestartPolicy(t *testing.T
 		},
 	}
 
-	group, err := ToContainerGroup(context.TODO(), convertCtx, project)
+	group, err := ToContainerGroup(context.TODO(), convertCtx, project, mockStorageHelper)
 	assert.NilError(t, err)
 
 	assert.Assert(t, is.Len(*group.Containers, 1))
@@ -336,7 +430,7 @@ func TestComposeContainerGroupToContainerMultiplePorts(t *testing.T) {
 		},
 	}
 
-	group, err := ToContainerGroup(context.TODO(), convertCtx, project)
+	group, err := ToContainerGroup(context.TODO(), convertCtx, project, mockStorageHelper)
 	assert.NilError(t, err)
 	assert.Assert(t, is.Len(*group.Containers, 3))
 
@@ -375,7 +469,7 @@ func TestComposeContainerGroupToContainerResourceLimits(t *testing.T) {
 		},
 	}
 
-	group, err := ToContainerGroup(context.TODO(), convertCtx, project)
+	group, err := ToContainerGroup(context.TODO(), convertCtx, project, mockStorageHelper)
 	assert.NilError(t, err)
 
 	limits := *((*group.Containers)[0]).Resources.Limits
@@ -401,7 +495,7 @@ func TestComposeContainerGroupToContainerResourceLimitsDefaults(t *testing.T) {
 		},
 	}
 
-	group, err := ToContainerGroup(context.TODO(), convertCtx, project)
+	group, err := ToContainerGroup(context.TODO(), convertCtx, project, mockStorageHelper)
 	assert.NilError(t, err)
 
 	limits := *((*group.Containers)[0]).Resources.Limits
@@ -425,7 +519,7 @@ func TestComposeContainerGroupToContainerenvVar(t *testing.T) {
 		},
 	}
 
-	group, err := ToContainerGroup(context.TODO(), convertCtx, project)
+	group, err := ToContainerGroup(context.TODO(), convertCtx, project, mockStorageHelper)
 	assert.NilError(t, err)
 
 	envVars := *((*group.Containers)[0]).EnvironmentVariables

aci/login/storagelogin.go

@@ -26,12 +26,18 @@ import (
 )
 
 // StorageLogin helper for Azure Storage Login
-type StorageLogin struct {
+type StorageLogin interface {
+	// GetAzureStorageAccountKey retrieves the storage account ket from the current azure login
+	GetAzureStorageAccountKey(ctx context.Context, accountName string) (string, error)
+}
+
+// StorageLoginImpl implementation of StorageLogin
+type StorageLoginImpl struct {
 	AciContext store.AciContext
 }
 
 // GetAzureStorageAccountKey retrieves the storage account ket from the current azure login
-func (helper StorageLogin) GetAzureStorageAccountKey(ctx context.Context, accountName string) (string, error) {
+func (helper StorageLoginImpl) GetAzureStorageAccountKey(ctx context.Context, accountName string) (string, error) {
 	client, err := NewStorageAccountsClient(helper.AciContext.SubscriptionID)
 	if err != nil {
 		return "", err

tests/aci-e2e/e2e-aci_test.go

@@ -197,7 +197,7 @@ func TestContainerRunVolume(t *testing.T) {
 	})
 
 	t.Run("upload file", func(t *testing.T) {
-		storageLogin := login.StorageLogin{AciContext: aciContext}
+		storageLogin := login.StorageLoginImpl{AciContext: aciContext}
 
 		key, err := storageLogin.GetAzureStorageAccountKey(context.TODO(), accountName)
 		assert.NilError(t, err)