Add deploy: mode: global support #771

cdrage · 2017-08-16T19:36:15Z

Adds support for deploy: mode.

For example:

version: "3"

services:
  foo:
    deploy:
      resources:
        mode: global
        replicas: 6
    image: redis

Will only generate replicas: 1 in Kubernetes pods as "global" limits
replicas to only one.

kompose-bot · 2017-08-16T19:36:18Z

@cdrage, thank you for the pull request! We'll request some people to review your PR. @surajnarwade and @procrypt, please review this.

surajnarwade · 2017-08-18T07:12:07Z

documentation of mode says global spin up exactly one container per swarm node, can we achieve same with kubernetes ?

cdrage · 2017-08-18T14:14:38Z

@surajnarwade nope, unless it's a daemonset. For now it's just replicas: 1. I really want to add DaemonSet however.

But you are right.

kadel · 2017-08-22T12:15:23Z

pkg/transformer/kubernetes/kubernetes.go

@@ -496,6 +496,12 @@ func (k *Kubernetes) CreateKubernetesObjects(name string, service kobject.Servic
 		replica = service.Replicas
 	}

+	// Check to see if Docker Compose v3 Deploy.Mode has been set to "global"
+	if service.DeployMode == "global" {
+		log.Infof("Deploy mode has been set to global. Setting Kubernetes container replicas to 0 for service %s", name)


Message says replicas to 0 but it sets replicas to 1

kadel · 2017-08-22T12:20:04Z

script/test/fixtures/v3/docker-compose-deploy-mode.yaml

+services:
+  foo:
+    deploy:
+      resources:


mode: is not under resource but directly under deploy key and the same thing for replicas

kadel · 2017-08-22T12:21:42Z

script/test/cmd/tests.sh

+# Test deploy mode: global
+cmd="kompose convert --stdout -j -f $KOMPOSE_ROOT/script/test/fixtures/v3/docker-compose-deploy-mode.yaml"
+sed -e "s;%VERSION%;$version;g" -e "s;%CMD%;$cmd;g"  "$KOMPOSE_ROOT/script/test/fixtures/v3/output-deploy-mode-k8s-template.json" > /tmp/output-k8s.json
+convert::expect_success "kompose convert --stdout -j -f $KOMPOSE_ROOT/script/test/fixtures/v3/docker-compose-deploy-mode.yaml" "/tmp/output-k8s.json"


use convert::expect_success_and_warning to also check warning to avoid false positives with the wrong docker compose file

cdrage · 2017-08-22T14:15:27Z

Thanks @kadel I've gone ahead and implemented the changes.

kadel · 2017-08-23T12:46:10Z

script/test/fixtures/v3/docker-compose-deploy-mode.yaml

+    deploy:
+      mode: global
+      resources:
+        replicas: 6


there is not replicas in the resources field ;-)

Woops, again another error!

kadel · 2017-08-23T12:47:08Z

pkg/transformer/kubernetes/kubernetes.go

@@ -496,6 +496,12 @@ func (k *Kubernetes) CreateKubernetesObjects(name string, service kobject.Servic
 		replica = service.Replicas
 	}

+	// Check to see if Docker Compose v3 Deploy.Mode has been set to "global"
+	if service.DeployMode == "global" {
+		log.Infof("Deploy mode has been set to global. Setting Kubernetes container replicas to 1 for service %s", name)


Isn't it a bit confusing to display this message even if replicas are set to 1 or if replicas is not set?

By default DeployMode is "replicated" so this will only be shown when "global" is selected. Should we remove this all-together?

Ah, I see what you mean now. Updated!

cdrage · 2017-08-23T15:38:57Z

Again, another mistake and tests failed. But fixed with @kadel 's suggestions.

Ready for another review!

kadel · 2017-08-23T15:54:51Z

pkg/transformer/kubernetes/kubernetes.go

 	if opt.IsReplicaSetFlag || service.Replicas == 0 {
 		replica = opt.Replicas
 	} else {
 		replica = service.Replicas
 	}

+	// Check to see if Docker Compose v3 Deploy.Mode has been set to "global"
+	if service.DeployMode == "global" {
+		log.Infof("Global mode has been set. Containers will only be replicated once throughout the cluster")


This message is still weird. It doesn't say what it does.
For kubernetes Kompose is ignoring global mode, Kompose just creates regular Deployment with one replica.

So this is not what global mode should do. Should this message just say that global mode is not supported and that regular deployment will be created?

@kadel In the future when we add DaemonSet we could switch it to use that, as that is better reflection on how this should work (one container per node).

Would it be better to output a warning saying that Global is not support, but replicas will be set to 1?

Would it be better to output a warning saying that Global is not support, but replicas will be set to 1?

Yes , I think this would be the best.

We can remove it once we start doing the DaemonSet thing. ( automatically using DaemonSets for services with global mode.)

Luckily I doubt many people use global. So for now let's output the warning and put up an issue to eventually have it switch to daemonset

Adds support for deploy: mode. For example: ```yaml version: "3" services: foo: deploy: resources: mode: global replicas: 6 image: redis ``` Will only generate replicas: 1 in Kubernetes pods as "global" limits replicas to only one.

cdrage · 2017-08-23T16:47:37Z

@kadel Updated with the new warning message

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

kompose-bot requested review from procrypt and surajnarwade August 16, 2017 19:36

cdrage mentioned this pull request Aug 16, 2017

Tracking card for new deploy keys in v3 #644

Closed

9 tasks

cdrage force-pushed the add-global branch from 45cb576 to 90757bd Compare August 16, 2017 19:43

surajnarwade added the rebase needed label Aug 19, 2017

kadel requested changes Aug 22, 2017

View reviewed changes

cdrage force-pushed the add-global branch from 90757bd to b72ba63 Compare August 22, 2017 14:15

cdrage force-pushed the add-global branch from b72ba63 to 74b824f Compare August 22, 2017 14:16

kadel reviewed Aug 23, 2017

View reviewed changes

cdrage force-pushed the add-global branch 2 times, most recently from 7305018 to 7594b58 Compare August 23, 2017 15:38

kadel reviewed Aug 23, 2017

View reviewed changes

Add deploy: mode: global support

bd6d493

Adds support for deploy: mode. For example: ```yaml version: "3" services: foo: deploy: resources: mode: global replicas: 6 image: redis ``` Will only generate replicas: 1 in Kubernetes pods as "global" limits replicas to only one.

cdrage force-pushed the add-global branch from 7594b58 to bd6d493 Compare August 23, 2017 16:47

kadel approved these changes Aug 24, 2017

View reviewed changes

kadel merged commit 2ff2d38 into kubernetes:master Aug 24, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add deploy: mode: global support #771

Add deploy: mode: global support #771

cdrage commented Aug 16, 2017

kompose-bot commented Aug 16, 2017

surajnarwade commented Aug 18, 2017 •

edited

cdrage commented Aug 18, 2017 •

edited

kadel Aug 22, 2017

kadel Aug 22, 2017

kadel Aug 22, 2017

cdrage commented Aug 22, 2017

kadel Aug 23, 2017

cdrage Aug 23, 2017

kadel Aug 23, 2017

cdrage Aug 23, 2017

cdrage Aug 23, 2017

cdrage commented Aug 23, 2017

kadel Aug 23, 2017

cdrage Aug 23, 2017

kadel Aug 23, 2017

cdrage Aug 23, 2017

cdrage commented Aug 23, 2017

Add deploy: mode: global support #771

Add deploy: mode: global support #771

Conversation

cdrage commented Aug 16, 2017

kompose-bot commented Aug 16, 2017

surajnarwade commented Aug 18, 2017 • edited

cdrage commented Aug 18, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cdrage commented Aug 22, 2017

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

cdrage commented Aug 23, 2017

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

cdrage commented Aug 23, 2017

surajnarwade commented Aug 18, 2017 •

edited

cdrage commented Aug 18, 2017 •

edited