Add support for Config, endpoint_mode and 3.3 support #994

xianlubird · 2018-04-27T08:23:46Z

Version 3.3
An upgrade of version 3 that introduces new parameters only available with Docker Engine version 17.06.0+, and higher.

Introduces the following additional parameters:

    build labels
    credential_spec
    configs
    deploy endpoint_mode

Docker compose v3.3 add four attributes. For credential_spec we shouldn't support. I will work on it , support v3.3 ASAP.

Issue: #914 and #865

@cdrage

xianlubird · 2018-05-04T03:14:08Z

@cdrage @hangyan @surajnarwade
A brief introduction:

This PR implement compose v3.3 support for kubernetes transform. For openshift, it will be implemented in another PR later.
credential_spec can only be used in windows container, so we ignore it
configs convert to configmap in kubernetes and can be mount into deployment pod
endpoint_mode vip mode is similar to nodePort type service, dnsrr should ignore

@cdrage @hangyan Pls review this PR if you are free. Review this PR maybe cost much time. If you have any comments , I will refactor my code. Thanks . :-)

youyajie · 2018-05-05T13:12:13Z

What's update on this ? Waiting it to be merged, so we can use it.

xianlubird · 2018-05-05T13:27:28Z

@youyajie Thank you for notice this pr. @cdrage and @hangyan are reviewing code , hope it will be merged soon :-)

cdrage

This is a great start! However, we really need to add more documentation.

With this PR, we are actually adding: Configuration support, 3.3 support as well as new key support (endpoint_mode for deploy).

Please:

Update the conversion documentation (the table)
Change the title of this PR (add support for Config, endpoint_mode and 3.3 support).
Change your commit message (can be modified using git commit --amend

If possible, could you also change this to three separate commits? (one for adding config support, one for adding deployment endpoint_mode support, and one for adding docker compose 3.3 support). I know it's a pain to do, but maybe @hangyan could help you?

Many thanks! 👍

cdrage · 2018-05-08T13:11:18Z

pkg/loader/compose/v3.go

+	noSupKeys := checkUnsupportedKeyForV3(config)
+	for _, keyName := range noSupKeys {
+		log.Warningf("Unsupported %s key - ignoring", keyName)
+	}


cdrage · 2018-05-08T13:14:29Z

script/test/fixtures/compose-v3.3-test/compose-endpoint-mode-1.yaml

+    deploy:
+      mode: replicated
+      replicas: 2
+      endpoint_mode: vip


this needs to be added to the documentation: http://kompose.io/conversion/

Done @cdrage

cdrage · 2018-05-08T13:16:46Z

pkg/loader/compose/v3.go

+			keysFound = append(keysFound, "credential_spec")
+		}
+		if service.Deploy.EndpointMode == "dnsrr" {
+			keysFound = append(keysFound, "dnsrr")


A bit confused, we're manipulating "vip" and that makes sense (using an IP endpoint). But what is the Kubernetes equivalent with dnsrr?

@cdrage dnsrr means user can use service name to connect, so in k8s, a normal service type can connect through service name. Change code to that, dnsrr convert to a normal service type.

ah, that makes sense. thank you.

hangyan · 2018-05-08T13:27:12Z

@cdrage Sure. I am current working on the secret support, separate the commit can also reduce the potentially conflict

xianlubird · 2018-05-09T04:20:27Z

@cdrage @hangyan
Already done these things

Change PR name to add support for Config, endpoint_mode and 3.3 support
Change this pr to three separate commits
Add document

Pls review again if you are free, thanks.

xianlubird · 2018-05-10T01:41:55Z

@cdrage Any update ?

cdrage · 2018-05-10T13:48:29Z

docs/conversion.md

@@ -4,7 +4,7 @@ This document outlines all possible conversion details regarding `docker-compose

 The current table covers all **current** possible Docker Compose keys.

-__Note:__ due to the fast-pace nature of Docker Compose version revisions, minor versions such as 2.1, 2.2, 3.3 are not supported until they are cut into a major version release such as 2 or 3.
+__Note:__ due to the fast-pace nature of Docker Compose version revisions, minor versions such as 2.1, 2.2 are not supported until they are cut into a major version release such as 2 or 3.


Go ahead and completely remove this line.. We should simply state that we don't support anything 3.4 and above at the moment.

cdrage · 2018-05-10T13:49:43Z

pkg/loader/compose/v3.go

+			keysFound = append(keysFound, "credential_spec")
+		}
+		if service.Deploy.EndpointMode == "dnsrr" {
+			keysFound = append(keysFound, "dnsrr")


ah, that makes sense. thank you.

cdrage

Other than one small comment, this LGTM. Although this is a big PR. Could you have a quick-look @hangyan before we merge?

hangyan · 2018-05-10T14:03:41Z

@cdrage @xianlubird

A few thoughts:

OpenShfit support should be added
About deploy mode. In my opinion, vip -> ClusterIP , dnsrr-> Headless

xianlubird · 2018-05-10T14:41:11Z

@hangyan

OpenShfit will be implemented in another pr later, this pr focus on kubernetes
VIP means user can visit any ip:port in the cluster node, so it equals to NodePort service. ClusterIP can't be visited outside the cluster. dnsrr means user can use service name to connect, so in k8s, a normal service type can connect through service name.

hangyan · 2018-05-10T14:49:11Z

@xianlubird

OpenShift support is just a few commands, it would't take many time. I don't think it's a good idea to seperate this
According to the compose doc. vip mode acts like a normal load balancer, the client wouldn't known what the backends are. With NodePort, you have to known how many nodes are in the kubernetes cluster, and what are there ips, I think these principle take preceder over the access from outside of the cluster rule.

dnsrr works very similar to Headless Service. In Headless service, when you do a dns-lookup for the service name, a range of ips will returned(the ip list of the pods).

hangyan · 2018-05-10T14:50:42Z

@cdrage Please take a look at my reviews and share some of your thoughts on this

xianlubird · 2018-05-10T15:03:12Z

@hangyan

For vip mode, I don't agree with you. In vip mode, user should use ip:port to visit service. User also should know node ip, it totally equals to NodePort service. If convert to Cluster IP, how should user to visit service outside cluster? Cloud they use node ip with port ? It can not equals to ClusterIP type.

dnsrr means user visit service name, headless is a external service. I think cluster ip is better. Both of them can visit through service name, but cluster ip type is more used.

I am not similar to OpenShift. So this pr first name is add support 3.3 for kubernetes. It will cost much time for me to implement OpenShift. So I will implement it later in another pr. And for OpenShift, I think it should split to another pr, it will more clear.

Need your opinion @cdrage

xianlubird · 2018-05-11T06:52:21Z

@cdrage @hangyan Discuss with @hangyan , I change some code for this pr:

Ignore short syntax config. Because short syntax config mount path in Deployment must be /, but docker daemon don't allow mount path is /, so we have to ignore it.
For long syntax config, if target path is /, ignore this. The reason is similar to above.
Add openshift support, thanks @hangyan insist
Change document according to @cdrage advice

Now this PR has become a XXL size, very exciting to see that it will be merged.

Thanks all for your review.

xianlubird · 2018-05-13T13:13:31Z

@cdrage
We still have some differences between @hangyan at which service type we should convert.

For compose network_mode: vip, according to docker's document link, vip mode is the swarm mode routing mesh, equals to docker service --publish. So user can visit any node ip in the cluster, the address is ip:port. It is especially like NodePort service in kubernetes cluster which user can visit any node ip in the cluster, the address is also ip:port.
For compose network_mode: dnsrr. This allow user to visit service name in a container in swarm mode cluster. It's similar to Cluster IP type in kubernetes cluster. For Headless service type, it is very similar to compose attribute external_links.

Now we need @cdrage opinion to push this pr going on. Hope you have time to see this and give us some advice. So we can merge this important pr soon. :-)

Many thanks.

cdrage · 2018-05-14T14:10:40Z

@xianlubird For NodePort it's actually the user can visit any node port. You still have to figure out the IP. network_mode: vip in my opinion is closer to LoadBalancer.

I agree however with dnsrr.

I will do another review in the next few days!

xianlubird · 2018-05-18T08:42:30Z

@cdrage any update on this ? .........

hangyan · 2018-05-26T04:55:42Z

@xianlubird @cdrage I have re-check the kubernetes documents, it seems like subPath(https://kubernetes.io/docs/concepts/storage/volumes/#using-subpath) can bring the full support for compose config.

fentas · 2018-07-22T09:45:46Z

@hangyan any progress?

cdrage · 2018-07-24T13:13:44Z

@hangyan @xianlubird Minor conflicting file, otherwise this LGTM. I can do another review after the conflicted file is resolved!

xianlubird · 2018-07-25T08:06:09Z

@cdrage All check have passed. conflicting solved.

hangyan · 2018-07-26T02:54:57Z

@xianlubird As i have point out before, subPath can bring the full support for compose config, otherwise this PR is too limited. @cdrage Or we can merge this first and create a new one to improve it

cdrage · 2018-07-27T14:13:26Z

@hangyan Adding subPath may be best before merging, so that config support is fully supported (short-syntax and all).

Although to be honest, 3.3 support seems to be important as well as endpoint_mode. Maybe we should merge and then open an issue indicating that we should implement subPath / syntax support for config.

hangyan · 2018-07-29T01:24:44Z

@cdrage Let's merge this first so we can move on

cdrage · 2018-08-01T19:08:56Z

@hangyan I agree. It's been way too long. Any issues that we encounter in the future can be resolved.

Merging this massive PR.

Special thanks to @xianlubird ! 💯

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Apr 27, 2018

fentas mentioned this pull request Apr 30, 2018

kompose does not indicate key nor line number when printing error #915

Closed

xianlubird force-pushed the features/v3.3 branch from aca5e54 to edd5a24 Compare May 3, 2018 11:51

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 3, 2018

xianlubird force-pushed the features/v3.3 branch 2 times, most recently from 5f922e4 to 7d4d0fb Compare May 4, 2018 02:16

xianlubird changed the title ~~[WIP] Do Not Merge Support docker compose v3.3~~ Support docker compose v3.3 for kubernetes transform May 4, 2018

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 4, 2018

xianlubird force-pushed the features/v3.3 branch from 7d4d0fb to 4225614 Compare May 4, 2018 02:51

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 4, 2018

k8s-ci-robot assigned hangyan May 4, 2018

xianlubird force-pushed the features/v3.3 branch 2 times, most recently from 20d79d5 to 5d919a7 Compare May 8, 2018 12:10

cdrage requested changes May 8, 2018

View reviewed changes

xianlubird force-pushed the features/v3.3 branch from 5d919a7 to 052ac71 Compare May 9, 2018 04:03

xianlubird changed the title ~~Support docker compose v3.3 for kubernetes transform~~ Add support for Config, endpoint_mode and 3.3 support May 9, 2018

cdrage mentioned this pull request May 10, 2018

Add label 'kompose.controller.type' set service convert controller type #1001

Merged

cdrage reviewed May 10, 2018

View reviewed changes

cdrage approved these changes May 10, 2018

View reviewed changes

hangyan added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 10, 2018

xianlubird force-pushed the features/v3.3 branch 2 times, most recently from ba97063 to 5892a51 Compare May 11, 2018 03:50

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels May 11, 2018

xianlubird force-pushed the features/v3.3 branch 2 times, most recently from 64592be to dccb995 Compare May 13, 2018 12:42

hangyan mentioned this pull request May 15, 2018

Add support for file based secret #1007

Closed

xianlubird added 4 commits July 25, 2018 15:11

Add support for Config

beb73fe

Add deployment endpoint_mode support

b33ffe3

Add docker compose 3.3 support

13bfb1c

Add compose v3.3 support for openshift

45f017d

xianlubird force-pushed the features/v3.3 branch from 50b522a to 45f017d Compare July 25, 2018 07:30

cdrage merged commit 0252213 into kubernetes:master Aug 1, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for Config, endpoint_mode and 3.3 support #994

Add support for Config, endpoint_mode and 3.3 support #994

xianlubird commented Apr 27, 2018 •

edited

Loading

xianlubird commented May 4, 2018 •

edited

Loading

youyajie commented May 5, 2018 •

edited

Loading

xianlubird commented May 5, 2018 •

edited

Loading

cdrage left a comment

cdrage May 8, 2018

cdrage May 8, 2018

xianlubird May 9, 2018

cdrage May 8, 2018

xianlubird May 9, 2018

cdrage May 10, 2018

hangyan commented May 8, 2018

xianlubird commented May 9, 2018

xianlubird commented May 10, 2018

cdrage May 10, 2018

cdrage May 10, 2018

cdrage left a comment

hangyan commented May 10, 2018 •

edited

Loading

xianlubird commented May 10, 2018 •

edited

Loading

hangyan commented May 10, 2018

hangyan commented May 10, 2018

xianlubird commented May 10, 2018 •

edited

Loading

xianlubird commented May 11, 2018 •

edited

Loading

xianlubird commented May 13, 2018

cdrage commented May 14, 2018

xianlubird commented May 18, 2018 •

edited

Loading

hangyan commented May 26, 2018

fentas commented Jul 22, 2018

cdrage commented Jul 24, 2018

xianlubird commented Jul 25, 2018

hangyan commented Jul 26, 2018

cdrage commented Jul 27, 2018

hangyan commented Jul 29, 2018

cdrage commented Aug 1, 2018

Add support for Config, endpoint_mode and 3.3 support #994

Add support for Config, endpoint_mode and 3.3 support #994

Conversation

xianlubird commented Apr 27, 2018 • edited Loading

xianlubird commented May 4, 2018 • edited Loading

youyajie commented May 5, 2018 • edited Loading

xianlubird commented May 5, 2018 • edited Loading

cdrage left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hangyan commented May 8, 2018

xianlubird commented May 9, 2018

xianlubird commented May 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cdrage left a comment

Choose a reason for hiding this comment

hangyan commented May 10, 2018 • edited Loading

xianlubird commented May 10, 2018 • edited Loading

hangyan commented May 10, 2018

hangyan commented May 10, 2018

xianlubird commented May 10, 2018 • edited Loading

xianlubird commented May 11, 2018 • edited Loading

xianlubird commented May 13, 2018

cdrage commented May 14, 2018

xianlubird commented May 18, 2018 • edited Loading

hangyan commented May 26, 2018

fentas commented Jul 22, 2018

cdrage commented Jul 24, 2018

xianlubird commented Jul 25, 2018

hangyan commented Jul 26, 2018

cdrage commented Jul 27, 2018

hangyan commented Jul 29, 2018

cdrage commented Aug 1, 2018

xianlubird commented Apr 27, 2018 •

edited

Loading

xianlubird commented May 4, 2018 •

edited

Loading

youyajie commented May 5, 2018 •

edited

Loading

xianlubird commented May 5, 2018 •

edited

Loading

hangyan commented May 10, 2018 •

edited

Loading

xianlubird commented May 10, 2018 •

edited

Loading

xianlubird commented May 10, 2018 •

edited

Loading

xianlubird commented May 11, 2018 •

edited

Loading

xianlubird commented May 18, 2018 •

edited

Loading