Added support for `replicas` keys in v3 #664

surajnarwade · 2017-06-22T04:53:46Z

resolves #644 replicas key

surajnarwade · 2017-06-22T04:55:18Z

if replica is not mentioned in docker-compose file, it will look for --replicas

surajnarwade · 2017-06-22T09:29:33Z

@cdrage @kadel @procrypt, needs review :)

cdrage · 2017-06-22T13:06:15Z

pkg/kobject/kobject.go

@@ -92,6 +92,7 @@ type ServiceConfig struct {
 	MemLimit        yaml.MemStringorInt `compose:"mem_limit" bundle:""`
 	TmpFs           []string            `compose:"tmpfs" bundle:""`
 	Dockerfile      string              `compose:"dockerfile" bundle:""`
+	Replicas        int                 `compose:"replicas" bundle:""`


We want to replicate how we do it similarly with kobject in libcompose / docker/cli.

I ask that you create a separate struct, similar to https://github.com/docker/cli/blob/master/cli/compose/types/types.go#L159 in order to implement the Deploy key.

Actually... this would be more difficult than it's worth. Ignore me! Leave this as it is 👍 Just add a test for testing replicas within deploy.

cdrage · 2017-06-22T13:08:22Z

pkg/transformer/kubernetes/kubernetes.go

@@ -489,15 +489,21 @@ func (k *Kubernetes) ConfigEnvs(name string, service kobject.ServiceConfig) []ap
 // CreateKubernetesObjects generates a Kubernetes artifact for each input type service
 func (k *Kubernetes) CreateKubernetesObjects(name string, service kobject.ServiceConfig, opt kobject.ConvertOptions) []runtime.Object {
 	var objects []runtime.Object
+	var replica int
+	if service.Replicas == 0 {
+		replica = opt.Replicas


This doesn't make sense. We should check to see if the default replicas value has been changed, rather than checking for 0.

See: https://github.com/kubernetes-incubator/kompose/blob/cf39f784355b384dbcd02657a7a2906ebd9c299f/pkg/app/app.go#L57

I didn't get it?

We need to check if --replicas = VALUE has been passed in (which would override what we do here).

cdrage · 2017-06-22T13:10:04Z

script/test/fixtures/v3/output-k8s-full-example.json

@@ -160,7 +160,7 @@
        }
      },
      "spec": {
-        "replicas": 1,
+        "replicas": 6,


no need to change previous test. please include a NEW test.

but its failing, that's why, changed. but it's replicas=6 in docker-compose v3 file ?

ah crap, i didn't realize that. thanks!

cdrage

needs tests!

cdrage · 2017-06-22T13:49:14Z

pkg/kobject/kobject.go

@@ -92,6 +92,7 @@ type ServiceConfig struct {
 	MemLimit        yaml.MemStringorInt `compose:"mem_limit" bundle:""`
 	TmpFs           []string            `compose:"tmpfs" bundle:""`
 	Dockerfile      string              `compose:"dockerfile" bundle:""`
+	Replicas        int                 `compose:"replicas" bundle:""`


Actually... this would be more difficult than it's worth. Ignore me! Leave this as it is 👍 Just add a test for testing replicas within deploy.

surajnarwade · 2017-06-27T09:38:40Z

@cdrage review please as tests are added

cdrage · 2017-06-27T12:44:45Z

pkg/loader/compose/v3.go

@@ -192,6 +192,11 @@ func dockerComposeToKomposeMapping(composeObject *types.Config) (kobject.Kompose
 		serviceConfig.Command = composeServiceConfig.Entrypoint
 		serviceConfig.Args = composeServiceConfig.Command

+		//Handling replicas
+		if composeServiceConfig.Deploy.Replicas != nil {


no need to add != nil. Just leave as if composeServiceConfig.Deploy.Replicas {

@cdrage thats not working as composeServiceConfig.Deploy.Replicas is not boolean but it's pointer

cdrage · 2017-06-27T12:48:16Z

pkg/transformer/kubernetes/kubernetes.go

@@ -489,15 +489,23 @@ func (k *Kubernetes) ConfigEnvs(name string, service kobject.ServiceConfig) []ap
 // CreateKubernetesObjects generates a Kubernetes artifact for each input type service
 func (k *Kubernetes) CreateKubernetesObjects(name string, service kobject.ServiceConfig, opt kobject.ConvertOptions) []runtime.Object {
 	var objects []runtime.Object
+	var replica int
+	if opt.Replicas > 1 {


like what I mentioned before, would be better to do:

if opt.IsReplicaSetFlag { replica = opt.Replicas }

So that means that if it has been changed from the default value, it'd pick it up, even if the user puts replicas=1.

You will have to add another flag similar to the other ones here: https://github.com/kubernetes-incubator/kompose/blob/master/cmd/convert.go#L84

you mean something like this,

kompose convert --replicas=1 --replica-set

I am confused because, --replicas is int flag and what you are suggesting is boolean flag

Yes, it is a boolean flag. We are checking if the default value has been changed, and if it has been changed, we MAKE SURE that we use opt.Replicas.

Currently, opt.Replicas only checks > 1. What if the user sets --replicas=1, but in the docker-compose.yaml file it has it set otherwise? The value from --replicas won't be used.

By checking to see if the default value has been changed, see: https://github.com/kubernetes-incubator/kompose/blob/master/cmd/convert.go#L84 we can make sure that the CLI paramter takes priority over the contents of the docker-compose.yaml file.

cdrage · 2017-06-27T12:49:13Z

pkg/transformer/openshift/openshift.go

-
+		//replicas
+		var replica int
+		if opt.Replicas > 1 {


Similar to above comment in regards to kubernetes.go, make the changes here as well!

surajnarwade · 2017-06-29T10:09:14Z

@cdrage added changes as per your suggestions, please review :)

cdrage

Thanks for the changes!!!

LGTM!

cdrage · 2017-06-29T15:11:05Z

pkg/transformer/kubernetes/kubernetes.go

@@ -489,15 +489,21 @@ func (k *Kubernetes) ConfigEnvs(name string, service kobject.ServiceConfig) []ap
 // CreateKubernetesObjects generates a Kubernetes artifact for each input type service
 func (k *Kubernetes) CreateKubernetesObjects(name string, service kobject.ServiceConfig, opt kobject.ConvertOptions) []runtime.Object {
 	var objects []runtime.Object
+	var replica int
+	if opt.IsReplicaSetFlag || service.Replicas == 0 {
+		replica = opt.Replicas


surajnarwade · 2017-06-30T12:37:16Z

@kadel @surajssd needed one more +1 here :)

resolves kubernetes#644 `replicas` key

surajssd

LGTM

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

surajnarwade force-pushed the add-replicas-v3 branch 2 times, most recently from 5e21507 to 94f4352 Compare June 22, 2017 05:18

cdrage requested changes Jun 22, 2017

View reviewed changes

surajnarwade changed the title ~~Added support for replicas keys in v3~~ [WIP]Added support for replicas keys in v3 Jun 23, 2017

surajnarwade force-pushed the add-replicas-v3 branch 3 times, most recently from deb7f00 to ebd4b0b Compare June 27, 2017 06:30

cdrage requested changes Jun 27, 2017

View reviewed changes

surajnarwade changed the title ~~[WIP]Added support for replicas keys in v3~~ Added support for replicas keys in v3 Jun 27, 2017

surajnarwade force-pushed the add-replicas-v3 branch from ebd4b0b to f09ecce Compare June 29, 2017 09:05

cdrage approved these changes Jun 29, 2017

View reviewed changes

Added support for replicas keys in v3

041d7f7

resolves kubernetes#644 `replicas` key

surajnarwade force-pushed the add-replicas-v3 branch from f09ecce to 041d7f7 Compare July 4, 2017 07:10

surajssd approved these changes Jul 6, 2017

View reviewed changes

surajssd merged commit 06543f7 into kubernetes:master Jul 6, 2017

surajssd mentioned this pull request Jul 6, 2017

Tracking card for new deploy keys in v3 #644

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added support for `replicas` keys in v3 #664

Added support for `replicas` keys in v3 #664

surajnarwade commented Jun 22, 2017

surajnarwade commented Jun 22, 2017

surajnarwade commented Jun 22, 2017

cdrage Jun 22, 2017

cdrage Jun 22, 2017

cdrage Jun 22, 2017

cdrage Jun 22, 2017

surajnarwade Jun 23, 2017

cdrage Jun 23, 2017 •

edited

Loading

cdrage Jun 22, 2017

surajnarwade Jun 22, 2017

cdrage Jun 22, 2017

cdrage left a comment

cdrage Jun 22, 2017

surajnarwade commented Jun 27, 2017

cdrage Jun 27, 2017

surajnarwade Jun 27, 2017

cdrage Jun 27, 2017

cdrage Jun 27, 2017

surajnarwade Jun 28, 2017

cdrage Jun 28, 2017 •

edited

Loading

cdrage Jun 27, 2017

surajnarwade commented Jun 29, 2017

cdrage left a comment

cdrage Jun 29, 2017

surajnarwade commented Jun 30, 2017

surajssd left a comment

Added support for replicas keys in v3 #664

Added support for replicas keys in v3 #664

Conversation

surajnarwade commented Jun 22, 2017

surajnarwade commented Jun 22, 2017

surajnarwade commented Jun 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 Jun 23, 2017 • edited Loading

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 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

surajnarwade commented Jun 27, 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 Jun 28, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

surajnarwade commented Jun 29, 2017

cdrage left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

surajnarwade commented Jun 30, 2017

surajssd left a comment

Choose a reason for hiding this comment

Added support for `replicas` keys in v3 #664

Added support for `replicas` keys in v3 #664

cdrage Jun 23, 2017 •

edited

Loading

cdrage Jun 28, 2017 •

edited

Loading