Clarify docs: rename spec/specification into desired configuration #2542

kenden · 2017-05-10T13:26:14Z

The cluster state in S3 has (among others) two files: cluster.spec and config.
When the documentation mentioned "create or update cluster spec" for example, it was confusing what was actually updated. It's not the cluster.spec file.
As I understand, cluster.spec should only be created/updated after kops update --yes is run.

I changed the docs for kops get, kops create, kops replace, kops edit.
I did NOT change those files: kops_rolling-update.md, kops_rolling-update_cluster.md as I think those actually use cluster.spec.

This change is

k8s-ci-robot · 2017-05-10T13:26:21Z

Hi @kenden. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with @k8s-bot ok to test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

WillemMali · 2017-05-19T12:24:34Z

Hi @kenden, you should rebase onto the current master and then regenerate the docs using make gen-cli-docs.

If you want a tighter feedback loop while making changes, use make ci to run the tests. To automatically run the tests pre-commit, install the Git hook with make hooks, or by copying hack/pre-commit.sh to .git/hooks manually.

kenden · 2017-05-21T00:43:38Z

@WillemMali I rebased. But running the gen-cli-docs target is not that easy.

$ make gen-cli-docs
go build  -ldflags "" -o /Users/myuser/go/bin/go-bindata k8s.io/kops/vendor/github.com/jteeuwen/go-bindata/go-bindata
cd /Users/myuser/go/src/k8s.io/kops; /Users/myuser/go/bin/go-bindata -o upup/models/bindata.go -pkg models -ignore="\\.DS_Store" -ignore="bindata\\.go" -ignore="vfs\\.go" -prefix upup/models/ upup/models/...
cd /Users/myuser/go/src/k8s.io/kops; /Users/myuser/go/bin/go-bindata -o federation/model/bindata.go -pkg model -ignore="\\.DS_Store" -ignore="bindata\\.go" -prefix federation/model/ federation/model/...
go install  -ldflags "-X k8s.io/kops.Version=1.6.0 -X k8s.io/kops.GitVersion=8dc7d2ad " k8s.io/kops/cmd/kops/...
KOPS_STATE_STORE= \
	/Users/myuser/go/bin/kops genhelpdocs --out docs/cli
Error: unknown command "genhelpdocs" for "kops"
Run 'kops --help' for usage.

unknown command "genhelpdocs" for "kops"
make: *** [gen-cli-docs] Error 1

$ /Users/myuser/go/bin/kops version
Version 1.4.1 # ???

$ go version
go version go1.8.1 darwin/amd64

WillemMali · 2017-05-21T11:16:22Z

Hi @kenden, I recently modified the gen-cli-docs target to use kops from the first item in your GOPATH, because previously it took the first one found in your PATH, which was less than ideal (this presumably was a more long-lasting and older copy, as it's not automatically replaced during development because stuff in your PATH generally requires root to change).

You need an up-to-date version of kops to generate the CLI docs. The quick fix is to put a new copy at the /Users/myuser/go/bin/kops (or in "the first item in your GOPATH"/bin/kops), which is where the Makefile gets its copy of kops from (as you can see in the log).

I'm not sure what the minimum version required is, I advise you to use a version built from the current master.

The "right" fix is making sure we know where kops is installed by the kops Makefile target (I strongly, strongly suspect it is GOPATH_1ST/bin/kops, which would match what we have now), and making sure the verify-gendocs target also uses the same kops installation. I'm writing a PR for the latter now, because I think it might benefit from being moved into the main Makefile.

chrislovecnm · 2017-05-21T21:21:31Z

The problem is that you are making changes to documentation files that are generated from the go code. You need to modify the go code and run gen docs make target. These are automatically generated from the code see: https://github.com/kubernetes/kops/blob/master/cmd/kops/get_instancegroups.go#L36

Every command in that directory contains its help definitions. You have to make changes to the code.

Here are some instructions

Setup $GOPATH - src, pkg, bin directories
Kops project needs to be in $GOPATH/src/k8s.io/kops
Edit a go file say get.go
Run make gen-docs target - make help shows our targets

That will generate the new md file.

Mac and Linux are supported dev environments.

justinsb · 2017-06-09T13:12:01Z

(I thought I had commented on this before but I can't find it - sorry)

In general we don't encourage poking around the S3 bucket - the files in there might not be well named, and shouldn't drive this.

The idea of calling it specification rather than configuration is that you specify the desired configurations, called spec in the schema, or specification. The configuration in my mind implies the actual configuration.

It's part of the design of k8s that it is essentially just loops that try to reconcile the actual configuration with the desired configuration (spec). And so that's what I was / we are trying to get at by calling it specification.

That said, some of these changes are separate from this debate, and good (other than being in the codegen) - I'll comment on the PR itself.

justinsb

Some random thoughts based on looking at kubectl & looking at a few cases here. I'd say we follow kubectl, which actually seems to side-step the problem.

justinsb · 2017-06-09T13:13:06Z

docs/cli/kops_create.md

@@ -12,7 +12,8 @@ Create a resource:
  * secret  
  * federation  

-Create a cluster, instancegroup or secret using command line flags or YAML cluster spec. Clusters and instancegroups can be created using the YAML cluster spec.
+Create a cluster, instancegroup or secret using command line parameters or YAML config files.
+(Note: secrets cannot be created from YAML config files yet).


I like this.

Maybe "or YAML configuration specification files" side-steps the issue?

Done, I used what you proposed.

justinsb · 2017-06-09T13:13:46Z

docs/cli/kops_create.md

@@ -21,7 +22,7 @@ kops create -f FILENAME
 ### Examples

 ```
-  # Create a cluster using a cluser spec file
+  # Create a cluster from the configuration in a YAML file


I would stick with "...from the specification in...". Or maybe "configuration specification" again?

I changed to configuration specification, as proposed.

justinsb · 2017-06-09T13:14:49Z

docs/cli/kops_edit.md

@@ -5,7 +5,7 @@ Edit clusters and other resources.
 ### Synopsis


-Edit a resource configuration. This command changes the cloud specification in the registry. 
+Edit a resource configuration. This command changes the cloud configuration in the registry. 


This could be "desired configuration" - it's not really "cloud specification" anyway...

Updated as proposed. "Desired configuration" is really the same as "specification" but it avoids confusion.

justinsb · 2017-06-09T13:15:49Z

docs/cli/kops_edit_cluster.md

@@ -7,7 +7,7 @@ Edit cluster.

 Edit a cluster configuration. 

-This command changes the cluster cloud specification in the registry. 
+This command changes the cluster cloud configuration in the registry. 


I'd say either "cluster specification" or "desired cluster configuration".

And I'm not even sure we need "in the registry", but that's just my opinion.

Updated using "desired configuration" as proposed.
Regarding "in the registry", I think the word "registry" is a bit confusing, but I would still indicate where this is updated, I think it explains better what the command actually does.
"...in the S3 state config"?

justinsb · 2017-06-09T13:17:28Z

docs/cli/kops_get.md

@@ -21,9 +21,12 @@ Display one or many resources.
  # Get a cluster
  kops get cluster k8s-cluster.example.com

-  # Get a cluster YAML cluster spec
+  # Get a cluster YAML configuration


I would probably stick here with cluster specification, IMO.

kubectl get just says e.g. "# List a single pod in JSON output format.", so avoids the whole question.

@justinsb
The problem here is that the command kops get cluster -o yaml
allows to get the desired configuration (the specification) AND also allows to get the actual configuration (cluster.spec, which I'm not 100% sure this is the actual configuration), with -o yaml --full.

To be clearer, I updated "cluster spec" to "cluster desired configuration" and added another example, "Get a cluster YAML cluster actual configuration".
How is that?

justinsb · 2017-06-09T13:18:29Z

docs/cli/kops_replace.md

@@ -5,7 +5,7 @@ Replace cluster resources.
 ### Synopsis


-Replace a resource specification by filename or stdin.
+Replace a resource configuration by filename or stdin.


Looking at what kubectl does, it just says Replace a resource by filename or stdin.

That does avoid the issue :-)

Used "Replace a resource by filename" as proposed as replaced specification by "desired configuration" in the example.

justinsb · 2017-06-09T13:19:37Z

Update: I looked at kubectl help, and it just side-steps the issue with e.g. "Replace a resource by filename or stdin. ". I think we can probably just follow kubectl's examples..

kenden · 2017-06-11T13:25:31Z

@justinsb Even if people shouldn't poke wound the state s3 bucket, they will, and might get confused by the .spec in cluster.spec: "Hmm, if this is the spec file, what's that config file then?"

But also, it is possible to get:

the config file, with kops get cluster -o yaml,
the cluster.spec file, with kops get cluster --full -o yaml

Maybe I am just confused about what the --full flag does. The command line help shows:

Flags:
      --full   Show fully populated configuration

I understand this shows the result of the kops update --yes command, so the "actual" configuration?

WillemMali · 2017-06-11T13:58:30Z

Maybe calling `--generated`, `--effective` or `--populated` would be clearer than calling it `--full`?

…

On Sun, Jun 11, 2017 at 3:25 PM, Quentin Nerden ***@***.***> wrote: @justinsb <https://github.com/justinsb> Even if people shouldn't poke wound the state s3 bucket, they will, and might get confused by the .spec in cluster.spec: "Hmm, if this is the spec file, what's that config file then?" But also, it is possible to get: - the config file, with kops get cluster -o yaml, - the cluster.spec file, with kops get cluster --full -o yaml Maybe I am just confused about what the --full flag does. The command line help shows: Flags: --full Show fully populated configuration I understand this shows the result of the kops update --yes command, so the "actual" configuration? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2542 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEePF74CfhI8ZaK3lUlV7fTIxoT8keInks5sC-rOgaJpZM4NWqVl> .

kenden · 2017-06-11T14:36:13Z

I updated my PR to change the docs directly in the go code and ran make gen-cli-docs to update the .md files. I also updated the texts from comments, basically now replacing "specification" by "desired configuration" (those really mean the same thing, but I think reduce confusion with the cluster.spec file).

chrislovecnm · 2017-06-22T18:54:17Z

@k8s-bot ok to test

chrislovecnm · 2017-06-22T18:54:30Z

/assign @justinsb

@justinsb status on this?

chrislovecnm · 2017-06-22T19:30:48Z

We problem need a rebase on this, and e2e will run again

chrislovecnm · 2017-06-22T20:10:33Z

Nope something is up with e2e.

@justinsb @zmerlynn any ideas?

justinsb · 2017-08-05T22:01:06Z

I think the test is (now) failing because it wants you to run make gen-cli-docs. Otherwise looks good I think!

k8s-github-robot · 2017-08-07T00:00:40Z

@kenden PR needs rebase

kenden · 2017-08-07T10:11:52Z

I rebased and ran make gen-cli-docs.

kenden · 2017-08-08T10:10:45Z

/ok-to-test

k8s-ci-robot · 2017-08-08T10:10:58Z

@kenden: you can't request testing unless you are a kubernetes member.

In response to this:

/ok-to-test

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

kenden · 2017-08-08T10:15:54Z

@chrislovecnm @justinsb Any idea why check "pull-kops-e2e-kubernetes-aws" is "Waiting for status to be reported" ?

justinsb · 2017-08-10T14:13:25Z

Not sure, but I'll close & reopen, that often gives things a kick...

In the S3 bucket, the file cluster.spec is not actually the spec, but the actual configuration. The file config is the spec. To avoid confusion, this commit changes spec/specification into 'desired configuration' in the documentation, to avoid associating cluster.spec with a cluster 'specification' that the users should use.

kenden · 2017-08-10T16:52:37Z

I rebased, to try and re-trigger it

chrislovecnm · 2017-08-10T22:19:53Z

/ok-to-test

chrislovecnm · 2017-08-10T22:21:06Z

@justinsb / @kenden bot needed the /ok-to-test

chrislovecnm · 2017-08-10T22:21:23Z

/approve no-issue

chrislovecnm · 2017-08-11T03:31:45Z

/lgtm

k8s-github-robot · 2017-08-11T03:32:13Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chrislovecnm, kenden

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

~~OWNERS~~ [chrislovecnm]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

k8s-github-robot · 2017-08-11T04:06:54Z

/test all [submit-queue is verifying that this PR is safe to merge]

k8s-github-robot · 2017-08-11T04:37:29Z

Automatic merge from submit-queue

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

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label May 10, 2017

kenden force-pushed the patch-1 branch 2 times, most recently from 0690f2d to 82704e0 Compare May 10, 2017 13:34

kenden force-pushed the patch-1 branch from 82704e0 to dec350c Compare May 20, 2017 23:57

justinsb reviewed Jun 9, 2017

View reviewed changes

kenden force-pushed the patch-1 branch 2 times, most recently from 73d080c to d918ad4 Compare June 11, 2017 14:17

kenden force-pushed the patch-1 branch 2 times, most recently from fe08226 to 891799e Compare June 11, 2017 14:42

kenden changed the title ~~Clarify docs: rename spec/specification into configuration~~ Clarify docs: rename spec/specification into desired configuration Jun 11, 2017

k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jun 22, 2017

k8s-ci-robot assigned justinsb Jun 22, 2017

kenden force-pushed the patch-1 branch from 891799e to a75bbd1 Compare June 22, 2017 19:32

kenden force-pushed the patch-1 branch 2 times, most recently from 38ef7d1 to 535718b Compare July 3, 2017 07:29

kenden force-pushed the patch-1 branch 2 times, most recently from 40f884c to 8e3a1ac Compare July 17, 2017 10:50

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 7, 2017

kenden force-pushed the patch-1 branch from 8e3a1ac to a058338 Compare August 7, 2017 10:09

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 7, 2017

kenden force-pushed the patch-1 branch from a058338 to 37a8c8d Compare August 8, 2017 10:16

justinsb closed this Aug 10, 2017

justinsb reopened this Aug 10, 2017

kenden force-pushed the patch-1 branch from 37a8c8d to 5491f17 Compare August 10, 2017 16:50

chrislovecnm approved these changes Aug 10, 2017

View reviewed changes

k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 10, 2017

k8s-ci-robot assigned chrislovecnm Aug 11, 2017

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 11, 2017

k8s-github-robot merged commit b1aee7a into kubernetes:master Aug 11, 2017

Clarify docs: rename spec/specification into desired configuration #2542

Clarify docs: rename spec/specification into desired configuration #2542

Conversation

kenden commented May 10, 2017 • edited Loading

k8s-ci-robot commented May 10, 2017

WillemMali commented May 19, 2017 • edited Loading

kenden commented May 21, 2017

WillemMali commented May 21, 2017

chrislovecnm commented May 21, 2017

justinsb commented Jun 9, 2017

justinsb 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kenden Jun 11, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

justinsb commented Jun 9, 2017 • edited Loading

kenden commented Jun 11, 2017 • edited Loading

WillemMali commented Jun 11, 2017 via email

kenden commented Jun 11, 2017 • edited Loading

chrislovecnm commented Jun 22, 2017

chrislovecnm commented Jun 22, 2017

chrislovecnm commented Jun 22, 2017

chrislovecnm commented Jun 22, 2017

justinsb commented Aug 5, 2017

k8s-github-robot commented Aug 7, 2017

kenden commented Aug 7, 2017 • edited Loading

kenden commented Aug 8, 2017

k8s-ci-robot commented Aug 8, 2017

kenden commented Aug 8, 2017

justinsb commented Aug 10, 2017

kenden commented Aug 10, 2017

chrislovecnm commented Aug 10, 2017

chrislovecnm commented Aug 10, 2017

chrislovecnm commented Aug 10, 2017

chrislovecnm commented Aug 11, 2017

k8s-github-robot commented Aug 11, 2017

k8s-github-robot commented Aug 11, 2017

k8s-github-robot commented Aug 11, 2017

kenden commented May 10, 2017 •

edited

Loading

WillemMali commented May 19, 2017 •

edited

Loading

kenden Jun 11, 2017 •

edited

Loading

justinsb commented Jun 9, 2017 •

edited

Loading

kenden commented Jun 11, 2017 •

edited

Loading

kenden commented Jun 11, 2017 •

edited

Loading

kenden commented Aug 7, 2017 •

edited

Loading