From 0092de6df1db580acbab4662b93774eddbb8f1a7 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Tue, 22 Sep 2020 11:45:02 +0200 Subject: [PATCH] Added tests on ACI volume conversion, mock storageLogin required to get storage account keys Signed-off-by: Guillaume Tardif --- aci/backend.go | 10 ++- aci/compose.go | 12 +++- aci/containers.go | 12 +++- aci/convert/convert.go | 10 +-- aci/convert/convert_test.go | 132 +++++++++++++++++++++++++++++----- aci/login/storagelogin.go | 10 ++- tests/aci-e2e/e2e-aci_test.go | 2 +- 7 files changed, 149 insertions(+), 39 deletions(-) diff --git a/aci/backend.go b/aci/backend.go index a5890f33f..50f7a2f0b 100644 --- a/aci/backend.go +++ b/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, }, diff --git a/aci/compose.go b/aci/compose.go index fe6a3e1eb..6eb00d460 100644 --- a/aci/compose.go +++ b/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 { diff --git a/aci/containers.go b/aci/containers.go index 61c50b656..eb73a746a 100644 --- a/aci/containers.go +++ b/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 } diff --git a/aci/convert/convert.go b/aci/convert/convert.go index 17d3ad6c8..c20fb4304 100644 --- a/aci/convert/convert.go +++ b/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 diff --git a/aci/convert/convert_test.go b/aci/convert/convert_test.go index d6c656dd6..c439dc447 100644 --- a/aci/convert/convert_test.go +++ b/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 diff --git a/aci/login/storagelogin.go b/aci/login/storagelogin.go index 13a3923f1..f50a172a7 100644 --- a/aci/login/storagelogin.go +++ b/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 diff --git a/tests/aci-e2e/e2e-aci_test.go b/tests/aci-e2e/e2e-aci_test.go index e697d9f63..ef8f5b58e 100644 --- a/tests/aci-e2e/e2e-aci_test.go +++ b/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)