(kedge init): add flag --controller #353

surajssd · 2017-10-13T10:47:08Z

If this flag is specified with value being OnFailure or Never,
a controller of type Job is created.

Fixes #325

kadel

If this is creating a job, then maybe a better name for the flag would be --job what do you think?

kadel · 2017-10-13T11:32:22Z

It would be better if the name would be in the first place in generated Kedge output.

the output is correct, but looks a bit weird in this order

containers:
- image: image
controller: Job
name: name
restartPolicy: Never

kadel · 2017-10-13T11:43:40Z

The thing is that output of kedge init should be human consumable. The expectation is that users will edit and add stuff to it so it should be well formatted.

surajssd · 2017-10-13T12:00:31Z

@kadel

If this is creating a job, then maybe a better name for the flag would be --job what do you think?

--job would be too specific of a flag. Later I envision of add flag called --controller, which basically changes the controller of the init file. Using which folks can generate DeploymentConfig or whatever controller support Kedge will have.

The thing is that output of kedge init should be human consumable. The expectation is that users will edit and add stuff to it so it should be well formatted.

Yeah true that, the golang template was being hard to maintain and add more things to it.

kadel · 2017-10-13T12:09:27Z

--job would be too specific of a flag. Later I envision of add flag called --controller, which basically changes the controller of the init file. Using which folks can generate DeploymentConfig or whatever controller support Kedge will have.

Then --restart OnFailure shouldn't create Job. Instead, regular DeploymentConfig should be used.

--controller job when implemented will create Job and set restartPolicy=OnFailure

kadel · 2017-10-13T12:10:47Z

Yeah true that, the golang template was being hard to maintain and add more things to it.

But output from this PR is complicating life for Kedge users, as generated kedge.yaml is going to be confusing to edit. Especially when we add more to it.

Templating might be better here, even if it is pain to maintain.

surajssd · 2017-10-13T12:12:39Z

Then --restart OnFailure shouldn't create Job. Instead, regular DeploymentConfig should be used.

Deployment and DeploymentConfig should have restartPolicy as Always they can't have anything else!

surajssd · 2017-10-13T12:13:56Z

@kadel how can I generate output of the yaml marshaller that is consistent with the structs that are backing it, any pointers?

kadel · 2017-10-13T12:16:02Z

Deployment and DeploymentConfig should have restartPolicy as Always they can't have anything else!

Ah, you are right.

Than we dont need restart flag for deployment.

The best thing would be to have this only for Job controller.

I think that it is better to have option that explicitly says that Job will be creted, rather than deciding what will be created based on restart flag.

kadel · 2017-10-13T12:16:59Z

@kadel how can I generate output of the yaml marshaller that is consistent with the structs that are backing it, any pointers?

don't understand the question :-(

surajssd · 2017-10-13T12:33:44Z

@kadel yeah make sense what you said, so removing the flag --restart rather adding flag --controller.

surajnarwade · 2017-10-16T05:15:32Z

+1 for replacing template with structs

surajnarwade · 2017-10-16T05:23:15Z

works for me & LGTM

kadel · 2017-10-16T14:05:59Z

+1 for for --controller flag.
Is it possible to control an order of files in yaml file?

It would be great to at least have name as the first field.

right now it looks like this:

containers:
- image: foobar
controller: DeploymentConfig
name: foobar
services:
- portMappings:
  - 80
  - 88

this would be much better:

name: foobar
controller: DeploymentConfig
containers:
- image: foobar

services:
- portMappings:
  - 80
  - 88

kadel

Can you please extend user guide and add more information about kedge init? Especially --controller flag

surajssd · 2017-10-23T02:35:00Z

@kadel

Can you please extend user guide and add more information about kedge init? Especially --controller flag

done

surajssd · 2017-10-23T02:36:40Z

@kadel

Is it possible to control an order of files in yaml file?

not sure how do I do that because right now I am marshalling all those structs into yaml, not sure how to make the yaml library "github.com/ghodss/yaml" respect the struct field ordering!

kadel · 2017-10-23T10:08:14Z

I'm not sure what to do with this :-(

It makes our code a bit better, but on the other side it worsens User Experience a bit :-(

concaf · 2017-10-23T10:22:16Z

docs/user-guide.md

@@ -101,10 +101,32 @@ kedge version

 ## `kedge init`

-Initialize kedge file
+### Basic getting started


nit: I think "getting start" should suffice

concaf · 2017-10-23T10:23:31Z

docs/user-guide.md

+kedge init --name web --image centos/httpd --ports 80
+```
+
+This will create a `kedge.yml` file for a long running application running inside


Why do we need "long running" here?
How about, "file for an httpd web server with container image centos/httpd"

concaf · 2017-10-23T10:24:27Z

docs/user-guide.md

+
+### Create a different controller type
+
+By default the controller type that is created is Kubernetes Deployments, which


"is a Kubernetes Deployment" - also a link to Kubernetes Deployment docs would help

concaf · 2017-10-23T10:24:58Z

docs/user-guide.md

+which is Kubernetes Job, you can do that using the flag `--controller`.
+
+```bash
+kedge init --name myjob --image jobimage --controller Job


lowercase "J" in job?

both are valid

concaf · 2017-10-23T10:29:58Z

docs/user-guide.md

+### Create a different file
+
+```bash
+kedge init --file myapp.yml --name web --image centos/httpd --ports 80


$ kedge init --file myapp.yml --name web --image centos/httpd --ports 80 Error: unknown flag: --file Usage: kedge init [flags] Flags: -c, --controller string The type of controller this application is. Legal values [Deployment, Job, DeploymentConfig]. Default 'Deployment'. -i, --image string The image for the container to run -n, --name string The name of service -o, --out string Output filename (default "kedge.yml") -p, --ports intSlice The ports that this container exposes Global Flags: --provider string Specify a provider. Kubernetes or OpenShift. (default "kubernetes") -v, --verbose verbose output unknown flag: --file

good catch!

You can choose what type of controller is created using flag `--controller`. The values you can give it are [Deployment, DeploymentConfig, Job].

concaf · 2017-10-23T11:46:11Z

@kadel about the UX part, I think kedge init will be used for initial getting started experience, which means very small kedge files. It would not take a lot of effort by the end user to find the name field.
Personally, I can live with it.
But if you think that it's a problem then I understand that too.

surajssd added the status/work-in-progress label Oct 13, 2017

surajssd requested a review from surajnarwade October 13, 2017 10:47

surajssd force-pushed the job-kedge-init branch 2 times, most recently from b6a76ba to df5f39b Compare October 13, 2017 11:12

surajssd added status/needs-review and removed status/work-in-progress labels Oct 13, 2017

kadel suggested changes Oct 13, 2017

View reviewed changes

surajssd force-pushed the job-kedge-init branch from df5f39b to ad48cda Compare October 13, 2017 12:32

surajssd changed the title ~~(kedge init): add flag --restart~~ (kedge init): add flag --controller Oct 13, 2017

surajssd force-pushed the job-kedge-init branch from ad48cda to 1415c48 Compare October 14, 2017 09:32

kadel suggested changes Oct 16, 2017

View reviewed changes

surajssd force-pushed the job-kedge-init branch from 1415c48 to 895ddbc Compare October 23, 2017 02:33

concaf suggested changes Oct 23, 2017

View reviewed changes

(kedge init): add flag --controller

d6c365b

You can choose what type of controller is created using flag `--controller`. The values you can give it are [Deployment, DeploymentConfig, Job].

Add docs for kedge init flag --controller

a68b39c

surajssd force-pushed the job-kedge-init branch from 895ddbc to a68b39c Compare October 23, 2017 11:22

concaf approved these changes Oct 23, 2017

View reviewed changes

kadel approved these changes Oct 26, 2017

View reviewed changes

kadel merged commit 5609de7 into kedgeproject:master Oct 26, 2017

kadel mentioned this pull request Oct 30, 2017

kedge init generates invalid kedge file #396

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(kedge init): add flag --controller #353

(kedge init): add flag --controller #353

surajssd commented Oct 13, 2017

kadel left a comment

kadel commented Oct 13, 2017 •

edited

Loading

kadel commented Oct 13, 2017

surajssd commented Oct 13, 2017

kadel commented Oct 13, 2017

kadel commented Oct 13, 2017 •

edited

Loading

surajssd commented Oct 13, 2017

surajssd commented Oct 13, 2017

kadel commented Oct 13, 2017

kadel commented Oct 13, 2017

surajssd commented Oct 13, 2017

surajnarwade commented Oct 16, 2017

surajnarwade commented Oct 16, 2017 •

edited

Loading

kadel commented Oct 16, 2017 •

edited

Loading

kadel left a comment

surajssd commented Oct 23, 2017

surajssd commented Oct 23, 2017

kadel commented Oct 23, 2017

concaf Oct 23, 2017

surajssd Oct 23, 2017

concaf Oct 23, 2017

surajssd Oct 23, 2017

concaf Oct 23, 2017

surajssd Oct 23, 2017

concaf Oct 23, 2017

surajssd Oct 23, 2017

concaf Oct 23, 2017

concaf Oct 23, 2017

surajssd Oct 23, 2017

concaf commented Oct 23, 2017


		### Create a different controller type

		By default the controller type that is created is Kubernetes Deployments, which

(kedge init): add flag --controller #353

(kedge init): add flag --controller #353

Conversation

surajssd commented Oct 13, 2017

kadel left a comment

Choose a reason for hiding this comment

kadel commented Oct 13, 2017 • edited Loading

kadel commented Oct 13, 2017

surajssd commented Oct 13, 2017

kadel commented Oct 13, 2017

kadel commented Oct 13, 2017 • edited Loading

surajssd commented Oct 13, 2017

surajssd commented Oct 13, 2017

kadel commented Oct 13, 2017

kadel commented Oct 13, 2017

surajssd commented Oct 13, 2017

surajnarwade commented Oct 16, 2017

surajnarwade commented Oct 16, 2017 • edited Loading

kadel commented Oct 16, 2017 • edited Loading

kadel left a comment

Choose a reason for hiding this comment

surajssd commented Oct 23, 2017

surajssd commented Oct 23, 2017

kadel commented Oct 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

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

concaf commented Oct 23, 2017

kadel commented Oct 13, 2017 •

edited

Loading

kadel commented Oct 13, 2017 •

edited

Loading

surajnarwade commented Oct 16, 2017 •

edited

Loading

kadel commented Oct 16, 2017 •

edited

Loading