From 7dd3a5a1eb45e0a0601cb10034eace1cd44e1c9e Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Mon, 14 Dec 2020 10:01:45 +0100 Subject: [PATCH 1/3] Remove duplicate compute of image name & add unit test Signed-off-by: Guillaume Tardif --- local/compose/build.go | 12 ++---------- local/compose/create.go | 14 +++++++++----- local/compose/create_test.go | 6 ++++++ 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/local/compose/build.go b/local/compose/build.go index f36af9bcc..9c88430f1 100644 --- a/local/compose/build.go +++ b/local/compose/build.go @@ -36,7 +36,7 @@ func (s *composeService) Build(ctx context.Context, project *types.Project) erro opts := map[string]build.Options{} for _, service := range project.Services { if service.Build != nil { - imageName := getImageName(service, project) + imageName := getImageName(service, project.Name) opts[imageName] = s.toBuildOptions(service, project.WorkingDir, imageName) } } @@ -44,14 +44,6 @@ func (s *composeService) Build(ctx context.Context, project *types.Project) erro return s.build(ctx, project, opts) } -func getImageName(service types.ServiceConfig, project *types.Project) string { - imageName := service.Image - if imageName == "" { - imageName = project.Name + "_" + service.Name - } - return imageName -} - func (s *composeService) ensureImagesExists(ctx context.Context, project *types.Project) error { opts := map[string]build.Options{} for _, service := range project.Services { @@ -59,7 +51,7 @@ func (s *composeService) ensureImagesExists(ctx context.Context, project *types. return fmt.Errorf("invalid service %q. Must specify either image or build", service.Name) } - imageName := getImageName(service, project) + imageName := getImageName(service, project.Name) localImagePresent, err := s.localImagePresent(ctx, imageName) if err != nil { return err diff --git a/local/compose/create.go b/local/compose/create.go index ebea5d9a7..be6daea17 100644 --- a/local/compose/create.go +++ b/local/compose/create.go @@ -123,6 +123,14 @@ func (s *composeService) ensureProjectVolumes(ctx context.Context, project *type return nil } +func getImageName(service types.ServiceConfig, projectName string) string { + imageName := service.Image + if imageName == "" { + imageName = projectName + "_" + service.Name + } + return imageName +} + func getCreateOptions(p *types.Project, s types.ServiceConfig, number int, inherit *moby.Container, autoRemove bool) (*container.Config, *container.HostConfig, *network.NetworkingConfig, error) { hash, err := jsonHash(s) if err != nil { @@ -155,10 +163,6 @@ func getCreateOptions(p *types.Project, s types.ServiceConfig, number int, inher if len(s.Entrypoint) > 0 { entrypoint = strslice.StrSlice(s.Entrypoint) } - image := s.Image - if s.Image == "" { - image = fmt.Sprintf("%s_%s", p.Name, s.Name) - } var ( tty = s.Tty @@ -178,7 +182,7 @@ func getCreateOptions(p *types.Project, s types.ServiceConfig, number int, inher AttachStderr: true, AttachStdout: true, Cmd: runCmd, - Image: image, + Image: getImageName(s, p.Name), WorkingDir: s.WorkingDir, Entrypoint: entrypoint, NetworkDisabled: s.NetworkMode == "disabled", diff --git a/local/compose/create_test.go b/local/compose/create_test.go index fa1b3681e..58b717407 100644 --- a/local/compose/create_test.go +++ b/local/compose/create_test.go @@ -21,6 +21,7 @@ import ( "path/filepath" "testing" + "github.com/compose-spec/compose-go/types" composetypes "github.com/compose-spec/compose-go/types" mountTypes "github.com/docker/docker/api/types/mount" "gotest.tools/v3/assert" @@ -60,3 +61,8 @@ func TestBuildVolumeMount(t *testing.T) { assert.Equal(t, mount.Source, "myProject_myVolume") assert.Equal(t, mount.Type, mountTypes.TypeVolume) } + +func TestServiceImageName(t *testing.T) { + assert.Equal(t, getImageName(types.ServiceConfig{Image: "myImage"}, "myProject"), "myImage") + assert.Equal(t, getImageName(types.ServiceConfig{Name: "aService"}, "myProject"), "myProject_aService") +} From 8ab3149f45d04af901b0643a8bd2329dc4e4f6bf Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Mon, 14 Dec 2020 11:35:53 +0100 Subject: [PATCH 2/3] =?UTF-8?q?Network=20names=20and=20volume=20names=20do?= =?UTF-8?q?n=E2=80=99t=20need=20anymore=20to=20be=20re-adapted=20(and=20pr?= =?UTF-8?q?efixed=20with=20project=20name)=20after=20loaded=20from=20compo?= =?UTF-8?q?se-go?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Guillaume Tardif --- ecs/awsResources.go | 2 +- ecs/volumes.go | 2 +- go.mod | 2 +- go.sum | 4 +- local/compose/create.go | 20 ++++----- local/compose/create_test.go | 13 ++++++ tests/compose-e2e/compose_test.go | 41 ++++++++++++++++++- .../fixtures/network-test/docker-compose.yaml | 25 +++++++++++ .../fixtures/volume-test/docker-compose.yml | 2 + 9 files changed, 95 insertions(+), 16 deletions(-) create mode 100644 tests/compose-e2e/fixtures/network-test/docker-compose.yaml diff --git a/ecs/awsResources.go b/ecs/awsResources.go index a7688a3a4..0ab601465 100644 --- a/ecs/awsResources.go +++ b/ecs/awsResources.go @@ -406,7 +406,7 @@ func (b *ecsAPIService) ensureVolumes(r *awsResources, project *types.Project, t }, { Key: "Name", - Value: fmt.Sprintf("%s_%s", project.Name, name), + Value: volume.Name, }, }, KmsKeyId: kmsKeyID, diff --git a/ecs/volumes.go b/ecs/volumes.go index 49295e454..81aefe738 100644 --- a/ecs/volumes.go +++ b/ecs/volumes.go @@ -72,7 +72,7 @@ func (b *ecsAPIService) createAccessPoints(project *types.Project, r awsResource }, { Key: "Name", - Value: fmt.Sprintf("%s_%s", project.Name, name), + Value: volume.Name, }, }, FileSystemId: r.filesystems[name].ID(), diff --git a/go.mod b/go.mod index 52b183d5e..2bc3d53a6 100644 --- a/go.mod +++ b/go.mod @@ -17,7 +17,7 @@ require ( github.com/awslabs/goformation/v4 v4.15.6 github.com/buger/goterm v0.0.0-20200322175922-2f3e71b85129 github.com/cnabio/cnab-to-oci v0.3.1-beta1 - github.com/compose-spec/compose-go v0.0.0-20210106202047-687be5e0e320 + github.com/compose-spec/compose-go v0.0.0-20210113150448-e0b1ffe70cc5 github.com/containerd/console v1.0.1 github.com/containerd/containerd v1.4.3 github.com/containerd/continuity v0.0.0-20200928162600-f2cc35102c2a // indirect diff --git a/go.sum b/go.sum index abfce2096..da682c1f6 100644 --- a/go.sum +++ b/go.sum @@ -271,8 +271,8 @@ github.com/cnabio/cnab-to-oci v0.3.1-beta1/go.mod h1:8BomA5Vye+3V/Kd2NSFblCBmp1r github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc= github.com/cockroachdb/datadriven v0.0.0-20190809214429-80d97fb3cbaa/go.mod h1:zn76sxSg3SzpJ0PPJaLDCu+Bu0Lg3sKTORVIj19EIF8= github.com/codahale/hdrhistogram v0.0.0-20160425231609-f8ad88b59a58/go.mod h1:sE/e/2PUdi/liOCUjSTXgM1o87ZssimdTWN964YiIeI= -github.com/compose-spec/compose-go v0.0.0-20210106202047-687be5e0e320 h1:PjwzjUYqjto8PLdHLtPX2/JtCbYYsKMs1Zof7/h29YA= -github.com/compose-spec/compose-go v0.0.0-20210106202047-687be5e0e320/go.mod h1:rz7rjxJGA/pWpLdBmDdqymGm2okEDYgBE7yx569xW+I= +github.com/compose-spec/compose-go v0.0.0-20210113150448-e0b1ffe70cc5 h1:YBR7Ds5WrY+FNxN2aRRZyEB1E86PD+g1O5RjK++acx8= +github.com/compose-spec/compose-go v0.0.0-20210113150448-e0b1ffe70cc5/go.mod h1:rz7rjxJGA/pWpLdBmDdqymGm2okEDYgBE7yx569xW+I= github.com/containerd/cgroups v0.0.0-20190919134610-bf292b21730f/go.mod h1:OApqhQ4XNSNC13gXIwDjhOQxjWa/NxkwZXJ1EvqT0ko= github.com/containerd/cgroups v0.0.0-20200531161412-0dbf7f05ba59/go.mod h1:pA0z1pT8KYB3TCXK/ocprsh7MAkoW8bZVzPdih9snmM= github.com/containerd/cgroups v0.0.0-20200710171044-318312a37340 h1:9atoWyI9RtXFwf7UDbme/6M8Ud0rFrx+Q3ZWgSnsxtw= diff --git a/local/compose/create.go b/local/compose/create.go index be6daea17..575c18ca4 100644 --- a/local/compose/create.go +++ b/local/compose/create.go @@ -48,7 +48,9 @@ func (s *composeService) Create(ctx context.Context, project *types.Project, opt return err } - if err := s.ensureProjectNetworks(ctx, project); err != nil { + prepareNetworks(project) + + if err := s.ensureNetworks(ctx, project.Networks); err != nil { return err } @@ -89,15 +91,17 @@ func (s *composeService) Create(ctx context.Context, project *types.Project, opt }) } -func (s *composeService) ensureProjectNetworks(ctx context.Context, project *types.Project) error { +func prepareNetworks(project *types.Project) { for k, network := range project.Networks { - if !network.External.External && network.Name != "" { - network.Name = fmt.Sprintf("%s_%s", project.Name, k) - project.Networks[k] = network - } network.Labels = network.Labels.Add(networkLabel, k) network.Labels = network.Labels.Add(projectLabel, project.Name) network.Labels = network.Labels.Add(versionLabel, ComposeVersion) + project.Networks[k] = network + } +} + +func (s *composeService) ensureNetworks(ctx context.Context, networks types.Networks) error { + for _, network := range networks { err := s.ensureNetwork(ctx, network) if err != nil { return err @@ -108,10 +112,6 @@ func (s *composeService) ensureProjectNetworks(ctx context.Context, project *typ func (s *composeService) ensureProjectVolumes(ctx context.Context, project *types.Project) error { for k, volume := range project.Volumes { - if !volume.External.External && volume.Name != "" { - volume.Name = fmt.Sprintf("%s_%s", project.Name, k) - project.Volumes[k] = volume - } volume.Labels = volume.Labels.Add(volumeLabel, k) volume.Labels = volume.Labels.Add(projectLabel, project.Name) volume.Labels = volume.Labels.Add(versionLabel, ComposeVersion) diff --git a/local/compose/create_test.go b/local/compose/create_test.go index 58b717407..b8cd32a4b 100644 --- a/local/compose/create_test.go +++ b/local/compose/create_test.go @@ -66,3 +66,16 @@ func TestServiceImageName(t *testing.T) { assert.Equal(t, getImageName(types.ServiceConfig{Image: "myImage"}, "myProject"), "myImage") assert.Equal(t, getImageName(types.ServiceConfig{Name: "aService"}, "myProject"), "myProject_aService") } + +func TestPrepareNetworkLabels(t *testing.T) { + project := types.Project{ + Name: "myProject", + Networks: types.Networks(map[string]types.NetworkConfig{"skynet": {}}), + } + prepareNetworks(&project) + assert.DeepEqual(t, project.Networks["skynet"].Labels, types.Labels(map[string]string{ + "com.docker.compose.network": "skynet", + "com.docker.compose.project": "myProject", + "com.docker.compose.version": "1.0-alpha", + })) +} diff --git a/tests/compose-e2e/compose_test.go b/tests/compose-e2e/compose_test.go index 21f0e3bf2..e23f5f61c 100644 --- a/tests/compose-e2e/compose_test.go +++ b/tests/compose-e2e/compose_test.go @@ -158,6 +158,44 @@ func TestLocalComposeRun(t *testing.T) { }) } +func TestNetworks(t *testing.T) { + c := NewParallelE2eCLI(t, binDir) + + const projectName = "network_e2e" + + t.Run("ensure we do not reuse previous networks", func(t *testing.T) { + c.RunDockerOrExitError("network", "rm", projectName+"_dbnet") + c.RunDockerOrExitError("network", "rm", "microservices") + }) + + t.Run("up", func(t *testing.T) { + c.RunDockerCmd("compose", "up", "-d", "-f", "./fixtures/network-test/docker-compose.yaml", "--project-name", projectName, "-d") + }) + + t.Run("check running project", func(t *testing.T) { + res := c.RunDockerCmd("compose", "ps", "-p", projectName) + res.Assert(t, icmd.Expected{Out: `web`}) + + endpoint := "http://localhost:80" + output := HTTPGetWithRetry(t, endpoint+"/words/noun", http.StatusOK, 2*time.Second, 20*time.Second) + assert.Assert(t, strings.Contains(output, `"word":`)) + + res = c.RunDockerCmd("network", "ls") + res.Assert(t, icmd.Expected{Out: projectName + "_dbnet"}) + res.Assert(t, icmd.Expected{Out: "microservices"}) + }) + + t.Run("down", func(t *testing.T) { + _ = c.RunDockerCmd("compose", "down", "--project-name", projectName) + }) + + t.Run("check networks after down", func(t *testing.T) { + res := c.RunDockerCmd("network", "ls") + assert.Assert(t, !strings.Contains(res.Combined(), projectName), res.Combined()) + assert.Assert(t, !strings.Contains(res.Combined(), "microservices"), res.Combined()) + }) +} + func TestLocalComposeBuild(t *testing.T) { c := NewParallelE2eCLI(t, binDir) @@ -213,6 +251,7 @@ func TestLocalComposeVolume(t *testing.T) { //ensure local test run does not reuse previously build image c.RunDockerOrExitError("rmi", "compose-e2e-volume_nginx") c.RunDockerOrExitError("volume", "rm", projectName+"_staticVol") + c.RunDockerOrExitError("volume", "rm", "myvolume") c.RunDockerCmd("compose", "up", "-d", "--workdir", "fixtures/volume-test", "--project-name", projectName) }) @@ -224,7 +263,7 @@ func TestLocalComposeVolume(t *testing.T) { t.Run("check container volume specs", func(t *testing.T) { res := c.RunDockerCmd("inspect", "compose-e2e-volume_nginx2_1", "--format", "{{ json .HostConfig.Mounts }}") //nolint - res.Assert(t, icmd.Expected{Out: `[{"Type":"volume","Source":"compose-e2e-volume_staticVol","Target":"/usr/share/nginx/html","ReadOnly":true},{"Type":"volume","Target":"/usr/src/app/node_modules"}]`}) + res.Assert(t, icmd.Expected{Out: `[{"Type":"volume","Source":"compose-e2e-volume_staticVol","Target":"/usr/share/nginx/html","ReadOnly":true},{"Type":"volume","Target":"/usr/src/app/node_modules"},{"Type":"volume","Source":"myVolume","Target":"/usr/share/nginx/test"}]`}) }) t.Run("cleanup volume project", func(t *testing.T) { diff --git a/tests/compose-e2e/fixtures/network-test/docker-compose.yaml b/tests/compose-e2e/fixtures/network-test/docker-compose.yaml new file mode 100644 index 000000000..e3435fe32 --- /dev/null +++ b/tests/compose-e2e/fixtures/network-test/docker-compose.yaml @@ -0,0 +1,25 @@ +services: + db: + image: gtardif/sentences-db + networks: + - dbnet + words: + image: gtardif/sentences-api + ports: + - "8080:8080" + networks: + - dbnet + - servicenet + web: + image: gtardif/sentences-web + ports: + - "80:80" + labels: + - "my-label=test" + networks: + - servicenet + +networks: + dbnet: + servicenet: + name: microservices diff --git a/tests/compose-e2e/fixtures/volume-test/docker-compose.yml b/tests/compose-e2e/fixtures/volume-test/docker-compose.yml index c9959d745..b5f56309b 100644 --- a/tests/compose-e2e/fixtures/volume-test/docker-compose.yml +++ b/tests/compose-e2e/fixtures/volume-test/docker-compose.yml @@ -11,9 +11,11 @@ services: volumes: - staticVol:/usr/share/nginx/html:ro - /usr/src/app/node_modules + - otherVol:/usr/share/nginx/test ports: - 9090:80 volumes: staticVol: + otherVol: name: myVolume \ No newline at end of file From 8bb4aeb901964ab3fe16bc17a024a3ed6ed80078 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Wed, 13 Jan 2021 18:16:49 +0100 Subject: [PATCH 3/3] Adjust aws test : actual volume name is set as tag, previously was key Signed-off-by: Guillaume Tardif --- ecs/testdata/simple/simple-cloudformation-conversion.golden | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ecs/testdata/simple/simple-cloudformation-conversion.golden b/ecs/testdata/simple/simple-cloudformation-conversion.golden index 6882bf795..7f82eb5f0 100644 --- a/ecs/testdata/simple/simple-cloudformation-conversion.golden +++ b/ecs/testdata/simple/simple-cloudformation-conversion.golden @@ -30,7 +30,7 @@ Resources: - Key: com.docker.compose.project Value: TestSimpleConvert - Key: com.docker.compose.network - Value: default + Value: TestSimpleConvert_default VpcId: vpc-123 Type: AWS::EC2::SecurityGroup DefaultNetworkIngress: