From 5732a555cb908f47cbee379e93b8a63592443c24 Mon Sep 17 00:00:00 2001 From: Chander G Date: Fri, 3 Jan 2020 20:47:29 +0530 Subject: [PATCH] Concat and merge service fields when merging compose files (#1186) (#1226) according to docker compose specs, certain fields are merged and certain others are concatented --- pkg/loader/compose/v3.go | 51 ++++- script/test/cmd/tests.sh | 4 + .../output-compose-new-service-template.json | 8 + .../output-compose-port-template.json | 8 + .../output-openshift-template.json | 8 + .../output-service-merge-concat-template.json | 198 ++++++++++++++++++ .../service-merge-concat-base.yml | 27 +++ .../service-merge-concat-override.yml | 24 +++ 8 files changed, 319 insertions(+), 9 deletions(-) create mode 100644 script/test/fixtures/merge-multiple-compose/output-service-merge-concat-template.json create mode 100644 script/test/fixtures/merge-multiple-compose/service-merge-concat-base.yml create mode 100644 script/test/fixtures/merge-multiple-compose/service-merge-concat-override.yml diff --git a/pkg/loader/compose/v3.go b/pkg/loader/compose/v3.go index 6e88963d..526a441f 100755 --- a/pkg/loader/compose/v3.go +++ b/pkg/loader/compose/v3.go @@ -564,13 +564,18 @@ func mergeComposeObject(oldCompose *types.Config, newCompose *types.Config) (*ty } if len(service.Devices) != 0 { + // merge the 2 sets of values + // TODO: need to merge the sets based on target values + // Not implemented yet as we don't convert devices to k8s anyway tmpOldService.Devices = service.Devices } if len(service.DNS) != 0 { - tmpOldService.DNS = service.DNS + // concat the 2 sets of values + tmpOldService.DNS = append(tmpOldService.DNS, service.DNS...) } if len(service.DNSSearch) != 0 { - tmpOldService.DNSSearch = service.DNSSearch + // concat the 2 sets of values + tmpOldService.DNSSearch = append(tmpOldService.DNSSearch, service.DNSSearch...) } if service.DomainName != "" { tmpOldService.DomainName = service.DomainName @@ -579,16 +584,21 @@ func mergeComposeObject(oldCompose *types.Config, newCompose *types.Config) (*ty tmpOldService.Entrypoint = service.Entrypoint } if len(service.Environment) != 0 { - tmpOldService.Environment = service.Environment + // merge the 2 sets of values + for k, v := range service.Environment { + tmpOldService.Environment[k] = v + } } if len(service.EnvFile) != 0 { tmpOldService.EnvFile = service.EnvFile } if len(service.Expose) != 0 { - tmpOldService.Expose = service.Expose + // concat the 2 sets of values + tmpOldService.Expose = append(tmpOldService.Expose, service.Expose...) } if len(service.ExternalLinks) != 0 { - tmpOldService.ExternalLinks = service.ExternalLinks + // concat the 2 sets of values + tmpOldService.ExternalLinks = append(tmpOldService.ExternalLinks, service.ExternalLinks...) } if len(service.ExtraHosts) != 0 { tmpOldService.ExtraHosts = service.ExtraHosts @@ -606,7 +616,10 @@ func mergeComposeObject(oldCompose *types.Config, newCompose *types.Config) (*ty tmpOldService.Ipc = service.Ipc } if len(service.Labels) != 0 { - tmpOldService.Labels = service.Labels + // merge the 2 sets of values + for k, v := range service.Labels { + tmpOldService.Labels[k] = v + } } if len(service.Links) != 0 { tmpOldService.Links = service.Links @@ -627,7 +640,8 @@ func mergeComposeObject(oldCompose *types.Config, newCompose *types.Config) (*ty tmpOldService.Pid = service.Pid } if len(service.Ports) != 0 { - tmpOldService.Ports = service.Ports + // concat the 2 sets of values + tmpOldService.Ports = append(tmpOldService.Ports, service.Ports...) } if service.Privileged != tmpOldService.Privileged { tmpOldService.Privileged = service.Privileged @@ -654,7 +668,8 @@ func mergeComposeObject(oldCompose *types.Config, newCompose *types.Config) (*ty tmpOldService.StopSignal = service.StopSignal } if len(service.Tmpfs) != 0 { - tmpOldService.Tmpfs = service.Tmpfs + // concat the 2 sets of values + tmpOldService.Tmpfs = append(tmpOldService.Tmpfs, service.Tmpfs...) } if service.Tty != tmpOldService.Tty { tmpOldService.Tty = service.Tty @@ -666,7 +681,25 @@ func mergeComposeObject(oldCompose *types.Config, newCompose *types.Config) (*ty tmpOldService.User = service.User } if len(service.Volumes) != 0 { - tmpOldService.Volumes = service.Volumes + // merge the 2 sets of values + + // Store volumes by Target + volumeConfigsMap := make(map[string]types.ServiceVolumeConfig) + // populate the older values + for _, volConfig := range tmpOldService.Volumes { + volumeConfigsMap[volConfig.Target] = volConfig + } + // add the new values, overriding as needed + for _, volConfig := range service.Volumes { + volumeConfigsMap[volConfig.Target] = volConfig + } + + // get the new list of volume configs + var volumes []types.ServiceVolumeConfig + for _, volConfig := range volumeConfigsMap { + volumes = append(volumes, volConfig) + } + tmpOldService.Volumes = volumes } if service.WorkingDir != "" { tmpOldService.WorkingDir = service.WorkingDir diff --git a/script/test/cmd/tests.sh b/script/test/cmd/tests.sh index 38ba84d4..ad97d1b4 100755 --- a/script/test/cmd/tests.sh +++ b/script/test/cmd/tests.sh @@ -138,6 +138,10 @@ cmd="kompose --provider=openshift convert -f $KOMPOSE_ROOT/script/test/fixtures/ sed -e "s;%VERSION%;$version;g" -e "s;%CMD%;$cmd;g" $KOMPOSE_ROOT/script/test/fixtures/merge-multiple-compose/output-openshift-template.json > /tmp/output-os.json convert::expect_success "$cmd" "/tmp/output-os.json" +cmd="kompose convert -f $KOMPOSE_ROOT/script/test/fixtures/merge-multiple-compose/service-merge-concat-base.yml -f $KOMPOSE_ROOT/script/test/fixtures/merge-multiple-compose/service-merge-concat-override.yml --stdout -j" +sed -e "s;%VERSION%;$version;g" -e "s;%CMD%;$cmd;g" $KOMPOSE_ROOT/script/test/fixtures/merge-multiple-compose/output-service-merge-concat-template.json > /tmp/output-k8s.json +convert::expect_success_and_warning "$cmd" "/tmp/output-k8s.json" "Volume mount on the host \"/override/dir\" isn't supported - ignoring path on the host" + # Test other top level keys # In merge cmd="kompose convert -f $KOMPOSE_ROOT/script/test/fixtures/merge-multiple-compose/other-toplevel-dev.yml -f $KOMPOSE_ROOT/script/test/fixtures/merge-multiple-compose/other-toplevel-ext.yml --stdout -j" diff --git a/script/test/fixtures/merge-multiple-compose/output-compose-new-service-template.json b/script/test/fixtures/merge-multiple-compose/output-compose-new-service-template.json index 1bfc4934..b37a6fbe 100644 --- a/script/test/fixtures/merge-multiple-compose/output-compose-new-service-template.json +++ b/script/test/fixtures/merge-multiple-compose/output-compose-new-service-template.json @@ -19,6 +19,11 @@ }, "spec": { "ports": [ + { + "name": "3000", + "port": 3000, + "targetPort": 3000 + }, { "name": "5000", "port": 5000, @@ -123,6 +128,9 @@ "imagePullPolicy": "", "name": "test-server", "ports": [ + { + "containerPort": 3000 + }, { "containerPort": 5000 } diff --git a/script/test/fixtures/merge-multiple-compose/output-compose-port-template.json b/script/test/fixtures/merge-multiple-compose/output-compose-port-template.json index 6047f263..ad0d840a 100644 --- a/script/test/fixtures/merge-multiple-compose/output-compose-port-template.json +++ b/script/test/fixtures/merge-multiple-compose/output-compose-port-template.json @@ -19,6 +19,11 @@ }, "spec": { "ports": [ + { + "name": "3000", + "port": 3000, + "targetPort": 3000 + }, { "name": "5000", "port": 5000, @@ -73,6 +78,9 @@ "imagePullPolicy": "", "name": "test-server", "ports": [ + { + "containerPort": 3000 + }, { "containerPort": 5000 } diff --git a/script/test/fixtures/merge-multiple-compose/output-openshift-template.json b/script/test/fixtures/merge-multiple-compose/output-openshift-template.json index 36dba3a5..f570241b 100644 --- a/script/test/fixtures/merge-multiple-compose/output-openshift-template.json +++ b/script/test/fixtures/merge-multiple-compose/output-openshift-template.json @@ -19,6 +19,11 @@ }, "spec": { "ports": [ + { + "name": "3000", + "port": 3000, + "targetPort": 3000 + }, { "name": "5000", "port": 5000, @@ -177,6 +182,9 @@ "name": "test-server", "image": " ", "ports": [ + { + "containerPort": 3000 + }, { "containerPort": 5000 } diff --git a/script/test/fixtures/merge-multiple-compose/output-service-merge-concat-template.json b/script/test/fixtures/merge-multiple-compose/output-service-merge-concat-template.json new file mode 100644 index 00000000..ffbc9575 --- /dev/null +++ b/script/test/fixtures/merge-multiple-compose/output-service-merge-concat-template.json @@ -0,0 +1,198 @@ +{ + "kind": "List", + "apiVersion": "v1", + "metadata": {}, + "items": [ + { + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": { + "annotations": { + "complexlabel": "override", + "kompose.cmd": "%CMD%", + "kompose.version": "%VERSION%", + "simplelabel.first": "Foo", + "simplelabel.second": "Bar" + }, + "creationTimestamp": null, + "labels": { + "io.kompose.service": "server" + }, + "name": "server" + }, + "spec": { + "replicas": 1, + "selector": { + "matchLabels": { + "io.kompose.service": "server" + } + }, + "strategy": { + "type": "Recreate" + }, + "template": { + "metadata": { + "annotations": { + "complexlabel": "override", + "kompose.cmd": "%CMD%", + "kompose.version": "%VERSION%", + "simplelabel.first": "Foo", + "simplelabel.second": "Bar" + }, + "creationTimestamp": null, + "labels": { + "io.kompose.service": "server" + } + }, + "spec": { + "containers": [ + { + "env": [ + { + "name": "NEW_ONE", + "value": "1234" + }, + { + "name": "SAMPLE_TO_BE_OVERRIDDEN", + "value": "new" + }, + { + "name": "SIMPLE_ONE", + "value": "true" + } + ], + "image": "test", + "imagePullPolicy": "", + "name": "test-server", + "resources": {}, + "volumeMounts": [ + { + "mountPath": "/simple/base/volume", + "name": "server-claim0" + }, + { + "mountPath": "/common/mount/point", + "name": "server-claim1" + }, + { + "mountPath": "/additional/added/volume", + "name": "server-claim2" + }, + { + "mountPath": "/base-tmpdir", + "name": "server-tmpfs0" + }, + { + "mountPath": "/additional-tmpdir", + "name": "server-tmpfs1" + } + ] + } + ], + "restartPolicy": "Always", + "serviceAccountName": "", + "volumes": [ + { + "name": "server-claim0", + "persistentVolumeClaim": { + "claimName": "server-claim0" + } + }, + { + "name": "server-claim1", + "persistentVolumeClaim": { + "claimName": "server-claim1" + } + }, + { + "name": "server-claim2", + "persistentVolumeClaim": { + "claimName": "server-claim2" + } + }, + { + "emptyDir": { + "medium": "Memory" + }, + "name": "server-tmpfs0" + }, + { + "emptyDir": { + "medium": "Memory" + }, + "name": "server-tmpfs1" + } + ] + } + } + }, + "status": {} + }, + { + "kind": "PersistentVolumeClaim", + "apiVersion": "v1", + "metadata": { + "name": "server-claim0", + "creationTimestamp": null, + "labels": { + "io.kompose.service": "server-claim0" + } + }, + "spec": { + "accessModes": [ + "ReadWriteOnce" + ], + "resources": { + "requests": { + "storage": "100Mi" + } + } + }, + "status": {} + }, + { + "kind": "PersistentVolumeClaim", + "apiVersion": "v1", + "metadata": { + "name": "server-claim1", + "creationTimestamp": null, + "labels": { + "io.kompose.service": "server-claim1" + } + }, + "spec": { + "accessModes": [ + "ReadWriteOnce" + ], + "resources": { + "requests": { + "storage": "100Mi" + } + } + }, + "status": {} + }, + { + "kind": "PersistentVolumeClaim", + "apiVersion": "v1", + "metadata": { + "name": "server-claim2", + "creationTimestamp": null, + "labels": { + "io.kompose.service": "server-claim2" + } + }, + "spec": { + "accessModes": [ + "ReadWriteOnce" + ], + "resources": { + "requests": { + "storage": "100Mi" + } + } + }, + "status": {} + } + ] +} diff --git a/script/test/fixtures/merge-multiple-compose/service-merge-concat-base.yml b/script/test/fixtures/merge-multiple-compose/service-merge-concat-base.yml new file mode 100644 index 00000000..9a6eb47b --- /dev/null +++ b/script/test/fixtures/merge-multiple-compose/service-merge-concat-base.yml @@ -0,0 +1,27 @@ +version: '3' + +services: + server: + image: test + container_name: test_server + environment: + - SIMPLE_ONE=true + - SAMPLE_TO_BE_OVERRIDDEN=original + expose: + - "3000" + dns: + - 1.1.1.1 + - 8.8.8.8 + dns_search: first.search.domain.com + external_links: + - other_service_1 + - common_service:alias1 + labels: + - "simplelabel.first=Foo" + - "complexlabel=original" + tmpfs: + - /base-tmpdir + volumes: + - /simple/base/volume + - /base/dir:/common/mount/point + diff --git a/script/test/fixtures/merge-multiple-compose/service-merge-concat-override.yml b/script/test/fixtures/merge-multiple-compose/service-merge-concat-override.yml new file mode 100644 index 00000000..84e0144f --- /dev/null +++ b/script/test/fixtures/merge-multiple-compose/service-merge-concat-override.yml @@ -0,0 +1,24 @@ +version: '3' + +services: + server: + image: test + container_name: test_server + environment: + - SAMPLE_TO_BE_OVERRIDDEN=new + - NEW_ONE=1234 + expose: + - "5000" + dns: 9.9.9.9 + dns_search: + - second.search.domain.com + external_links: + - other_service_2 + - common_service:alias2 + labels: + simplelabel.second: "Bar" + complexlabel: override + tmpfs: /additional-tmpdir + volumes: + - /additional/added/volume + - /override/dir:/common/mount/point