Add v3 support of Docker Compose #600

cdrage · 2017-05-12T18:46:58Z

This does a major refactor on the compose.go functions as well as brings
in a new era of v3 support to Kompose.

Similar to how we utilize libcompose, we utilize docker/cli's "stack
deploy" code which has a built-in v3 parser. We convert the parsed
structure to our own and then convert it to Kubernetes/OpenShift
artifacts.

cdrage · 2017-05-12T18:48:58Z

FYI, this is a work-in-progress.

jritsema · 2017-05-12T19:20:56Z

Hey @cdrage. Just curious, did you guys decide to not wait for libcompose to update to v3 and just start consuming code directly from docker? Have you found it to be robust enough to consume as a library so far?

cdrage · 2017-05-15T14:49:06Z

@jritsema Yes, and no.

Ideally we would like to use libcompose. Essentially we only use libcompose in order to correctly parse the docker compose file into a proper Go struct, that's it. We could actually possibly integrate our own code in the future to parse the docker-compose file into a proper structure.

So essentially, it ends up being easy to simply use http://github.com/docker/cli instead of http://github.com/docker/libcompose because we don't need the "deployment" functionality of libcompose, only the parsing capability.

Funny enough, the code is very similar!

See: https://github.com/docker/cli/blob/master/cli/compose/types/types.go#L72 vs https://github.com/docker/libcompose/blob/56b0613aac7d6d524d29dacb6b4221b5e4618d45/config/types.go#L87

jritsema · 2017-05-16T16:56:02Z

Interesting, thanks @cdrage. So does it support all of the compose semantics like multiple compose files and extends, etc?

https://docs.docker.com/compose/extends/

cdrage · 2017-05-16T17:15:07Z

@jritsema At the moment, no, but eventually...

cdrage · 2017-05-16T21:06:13Z

So the POC works now! You're able to go ahead and convert your v3 Docker Compose file to Kubernetes / OpenShift artifacts.

Feel free to pull / build this PR and use it!

At the moment there are some missing parts (mem_limit, CPUQuota, etc.) that I need to figure out before it's 100% complete.

cdrage · 2017-05-19T17:37:08Z

Unfortunately I have to also update the vendoring to a higher version of docker/docker due to the cli being split into docker/cli. Need to resolve conflicts!

cdrage · 2017-06-06T17:12:03Z

So I've gone ahead and abstracted everything to it's own folder so it's a tad more organized as per @surajssd 's comments!

It looks like he'll be away for a while however, @surajnarwade or @kadel wanna take a look at this?

concaf

Except a couple of nits, LGTM :)

concaf · 2017-06-07T09:14:23Z

glide.yaml

+- package: github.com/docker/cli
+  version: 1fc7eb5d644599f30d0c6cc350a4d84ff528c864
+
+


nit: extra \n?

concaf · 2017-06-07T11:08:08Z

pkg/loader/compose/compose.go

-			})
+		// Check that the previous file loaded matches.
+		if len(files) > 0 && version != "" && version != composeVersion {
+			return kobject.KomposeObject{}, errors.New("All files passed must be the same version")


nit: "All Docker Compose files must be of the same version"

cdrage · 2017-06-07T14:24:38Z

@containscafeine DONE! Everything's been separated into it's own file so it's easier to understand / place everything if it's either v1/v2 conversion or v3.

One LGTM 👍 down, @surajssd @kadel @surajnarwade Want to take a second look?

kadel

For some reason env values are not propagated to Kubernetes artifacts.

For

version: '3'
services:
  foo:
    image: quay.io/tomkral/sleeper
    environment:
      FOO: foo

I get env without value.

        - env:
          - name: FOO

If I change version to '2' everything is ok

        - env:
          - name: FOO
            value: foo

this should be caught by tests :-(

kadel · 2017-06-08T09:17:24Z

pkg/loader/compose/v3.go

+	for _, vol := range volumes {
+
+		// There will *always* be Source when parsing
+		v := vol.Source


docker-compose allows '_' in Volume names but Kubernetes does not.
normalizeServiceNames should be used here.

kadel · 2017-06-08T10:03:53Z

pkg/loader/compose/v3.go

+		// DockerCompose uses map[string]*string while we use []string
+		// So let's convert that using this hack
+		var envKeys []string
+		for key := range composeServiceConfig.Environment {


Should this be enough to handle env for v3?

for name, value := range composeServiceConfig.Environment { env := kobject.EnvVar{Name: name, Value: *value} serviceConfig.Environment = append(serviceConfig.Environment, env) }

It should also fix the problem with values not beeing populated.

Good catch. When I previously tested it environment variables were working. I'll fix this and add a test (which I thought worked correctly with my full example yaml file).

cdrage · 2017-06-08T15:23:35Z

@kadel @surajssd

For missing the env variables: they now appear in the script/test/fixtures/v3/output* as well as I have added an explicit test for them.

A full example for v3 support is located here: script/test/fixtures/v3/docker-compose-full-example.yaml.

I've also added the normalizeVolumeNames as well as another explicit test on volumes.

cdrage · 2017-06-12T17:28:40Z

@kadel @surajnarwade @surajssd @containscafeine

For secondary review!

bgruening · 2017-06-12T20:37:13Z

This is great work! Thanks a bunch!

kadel

sorry, forgot to hit submit button yesterday :-(

kadel · 2017-06-12T09:25:08Z

pkg/loader/compose/v3.go

+		v := normalizeServiceNames(vol.Source)
+
+		if vol.Target != "" {
+			v = v + ":" + normalizeServiceNames(vol.Target)


I don't think that target path should be normalized. Target is always path. This will break paths with '_'.

This does a major refactor on the compose.go functions as well as brings in a new era of v3 support to Kompose. Similar to how we utilize libcompose, we utilize docker/cli's "stack deploy" code which has a built-in v3 parser. We convert the parsed structure to our own and then convert it to Kubernetes/OpenShift artifacts.

cdrage · 2017-06-13T13:45:04Z

@kadel made the change for vol target! 👍

surajssd

LGTM

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 12, 2017

cdrage force-pushed the add-v3-support branch from df2de76 to 3e541c6 Compare May 12, 2017 18:48

cdrage changed the title ~~Add v3 support of Docker Compose~~ [WIP] Add v3 support of Docker Compose May 12, 2017

cdrage force-pushed the add-v3-support branch 2 times, most recently from f09dabc to 274c229 Compare May 15, 2017 20:42

cdrage force-pushed the add-v3-support branch 6 times, most recently from 170b203 to 97a06df Compare May 19, 2017 17:27

cdrage force-pushed the add-v3-support branch 12 times, most recently from 692720a to e6231ed Compare May 24, 2017 15:37

cdrage force-pushed the add-v3-support branch from 42178ab to a8b019f Compare June 6, 2017 16:48

cdrage force-pushed the add-v3-support branch 2 times, most recently from 672837b to 989d909 Compare June 6, 2017 18:44

concaf approved these changes Jun 7, 2017

View reviewed changes

cdrage force-pushed the add-v3-support branch 2 times, most recently from 7d3c4ac to bca4c0e Compare June 7, 2017 14:23

Update vendoring

683ae91

cdrage force-pushed the add-v3-support branch from bca4c0e to a41a7f5 Compare June 7, 2017 14:23

kadel requested changes Jun 8, 2017

View reviewed changes

kadel reviewed Jun 8, 2017

View reviewed changes

cdrage force-pushed the add-v3-support branch 2 times, most recently from e939e29 to 08658d3 Compare June 8, 2017 15:23

bgruening mentioned this pull request Jun 10, 2017

[WIP] Enable k8s testing. bgruening/docker-galaxy#347

Closed

cdrage mentioned this pull request Jun 12, 2017

Add build key support for v3 of Docker Compose #636

Closed

kadel reviewed Jun 13, 2017

View reviewed changes

cdrage force-pushed the add-v3-support branch from 08658d3 to 2b58307 Compare June 13, 2017 13:44

kadel approved these changes Jun 13, 2017

View reviewed changes

surajssd approved these changes Jun 14, 2017

View reviewed changes

surajssd merged commit 3be76ff into kubernetes:master Jun 14, 2017

This was referenced Jun 15, 2017

Add openshift tests for docker compose v3 #641

Closed

OpenShift tests for docker compose v3 #642

Merged

cdrage mentioned this pull request Jun 15, 2017

Error to parse docker-compose v3 format #412

Closed

cdrage deleted the add-v3-support branch June 15, 2017 19:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add v3 support of Docker Compose #600

Add v3 support of Docker Compose #600

cdrage commented May 12, 2017

cdrage commented May 12, 2017

jritsema commented May 12, 2017

cdrage commented May 15, 2017

jritsema commented May 16, 2017

cdrage commented May 16, 2017

cdrage commented May 16, 2017

cdrage commented May 19, 2017

cdrage commented Jun 6, 2017

concaf left a comment

concaf Jun 7, 2017

cdrage Jun 7, 2017

concaf Jun 7, 2017

cdrage commented Jun 7, 2017 •

edited

Loading

kadel left a comment •

edited

Loading

kadel Jun 8, 2017

kadel Jun 8, 2017

cdrage Jun 8, 2017

cdrage commented Jun 8, 2017

cdrage commented Jun 12, 2017

bgruening commented Jun 12, 2017

kadel left a comment

kadel Jun 12, 2017

cdrage commented Jun 13, 2017

surajssd left a comment

		- package: github.com/docker/cli
		version: 1fc7eb5d644599f30d0c6cc350a4d84ff528c864

Add v3 support of Docker Compose #600

Add v3 support of Docker Compose #600

Conversation

cdrage commented May 12, 2017

cdrage commented May 12, 2017

jritsema commented May 12, 2017

cdrage commented May 15, 2017

jritsema commented May 16, 2017

cdrage commented May 16, 2017

cdrage commented May 16, 2017

cdrage commented May 19, 2017

cdrage commented Jun 6, 2017

concaf left a comment

Choose a reason for hiding this comment

concaf Jun 7, 2017

Choose a reason for hiding this comment

cdrage Jun 7, 2017

Choose a reason for hiding this comment

concaf Jun 7, 2017

Choose a reason for hiding this comment

cdrage commented Jun 7, 2017 • edited Loading

kadel left a comment • edited Loading

Choose a reason for hiding this comment

kadel Jun 8, 2017

Choose a reason for hiding this comment

kadel Jun 8, 2017

Choose a reason for hiding this comment

cdrage Jun 8, 2017

Choose a reason for hiding this comment

cdrage commented Jun 8, 2017

cdrage commented Jun 12, 2017

bgruening commented Jun 12, 2017

kadel left a comment

Choose a reason for hiding this comment

kadel Jun 12, 2017

Choose a reason for hiding this comment

cdrage commented Jun 13, 2017

surajssd left a comment

Choose a reason for hiding this comment

cdrage commented Jun 7, 2017 •

edited

Loading

kadel left a comment •

edited

Loading