Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup kubectl generators docs #17609

Merged

Conversation

sftim
Copy link
Contributor

@sftim sftim commented Nov 15, 2019

Tidy https://kubernetes.io/docs/reference/kubectl/conventions/#generators (preview)

v1.12 is going out of support, so it's OK to drop mentions of changes introduced in v1.12 and earlier.

Plus general tidying. Separate commits so it's easy to cherry-pick / omit any of these.

/kind cleanup
/milestone 1.17

@k8s-ci-robot
Copy link
Contributor

@sftim: You must be a member of the kubernetes/website-milestone-maintainers GitHub team to set the milestone. If you believe you should be able to issue the /milestone command, please contact your Website milestone maintainers and have them propose you as an additional delegate for this responsibility.

In response to this:

Tidy https://kubernetes.io/docs/reference/kubectl/conventions/#generators

v1.12 is going out of support, so it's OK to drop mentions of changes introduced in v1.12 and earlier.

Plus general tidying. Separate commits so it's easy to cherry-pick / omit any of these.

/kind cleanup
/milestone 1.17

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.

@k8s-ci-robot k8s-ci-robot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Nov 15, 2019
@k8sio-netlify-preview-bot
Copy link
Collaborator

k8sio-netlify-preview-bot commented Nov 15, 2019

Deploy preview for kubernetes-io-vnext-staging processing.

Building with commit 5e8dec0

https://app.netlify.com/sites/kubernetes-io-vnext-staging/deploys/5de27925c6aabe00082a4c3f

@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 15, 2019
@k8s-ci-robot k8s-ci-robot added language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Nov 15, 2019
@sftim sftim force-pushed the 20191115_kubectl_generators_cleanup branch from b16b198 to eee5ada Compare November 15, 2019 15:44
@sftim
Copy link
Contributor Author

sftim commented Nov 15, 2019

/sig cli

@k8s-ci-robot k8s-ci-robot added the sig/cli Categorizes an issue or PR as relevant to SIG CLI. label Nov 15, 2019
@sftim
Copy link
Contributor Author

sftim commented Nov 15, 2019

Reviewers: I haven't checked if the Resource | API group | kubectl command table wants further tidying

@tengqm
Copy link
Contributor

tengqm commented Nov 18, 2019

/approve

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 18, 2019
@tengqm tengqm added this to the 1.17 milestone Nov 18, 2019
@@ -50,7 +50,7 @@ You can create the following resources using `kubectl run` with the `--generator
Generators other than `run-pod/v1` are deprecated.
{{< /note >}}

If you do not specify a generator flag, other flags prompt you to use a specific generator. The following table lists the flags that force you to use specific generators, depending on the version of the cluster:
If you do not explicitly specify a generator, specifying certain other flags force use of a specific generator. The following table lists the flags that force you to use specific generators:
Copy link
Contributor

@kbhawkey kbhawkey Nov 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sftim , line 53, nits: 4x specify or specific. Could use set or assign?
If you do not explicitly specify a generator, a generator will be implicitly set by other flags. The following table lists the flags and the generators that are activated:

This means that when you combine `--generator` with other flags the generator that you specified later does not change. For example, in a cluster v1.4, if you initially specify
`--restart=Always`, a Deployment is created; if you later specify `--restart=Always`
This means that when you combine `--generator` with other flags the generator that you specified later does not change.

Copy link
Contributor

@kbhawkey kbhawkey Nov 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sftim Generally, the changes look good.
I reread the note in the preview. My remaining questions:

  • What is the default generator?
  • What happens When you combine the --generator option with other flags ? This part still is not clear.
    This means that when you combine --generator with other flags the generator that you "specified later" does not change.

Lines 66 -70 tries to clarify that statement by example (the example seems straightforward, if correct). And this content is after the note:
The flags set the generator in the following order: first the --schedule flag, then the --restart policy flag, and finally the --generator flag.

This last statement seems to imply that a generator value is set depending on whether a given flag or comb of flags is present (with the generator flag overriding the other flags when used in combination)? The generator flag takes precedence.

@daminisatya
Copy link
Contributor

@sftim Updates on this?

@sftim sftim changed the title Cleanup kubectl generators docs [WIP] Cleanup kubectl generators docs Nov 28, 2019
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 28, 2019
@sftim sftim force-pushed the 20191115_kubectl_generators_cleanup branch 2 times, most recently from 5a50a01 to 31d6cc2 Compare November 28, 2019 09:17
@sftim
Copy link
Contributor Author

sftim commented Nov 28, 2019

@daminisatya @kbhawkey reworded; please feel free to take a look

@sftim sftim changed the title [WIP] Cleanup kubectl generators docs Cleanup kubectl generators docs Nov 28, 2019
@k8s-ci-robot 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 Nov 28, 2019
Now, when you run the script, it creates a ReplicationController because you specified the (deprecated) generator that you want kubectl to use.

If you don't specify a generator, kubectl pays attention to other flags in the following order:
1. `--schedule`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sftim. Great work! 😀 I don't want to drag this out; sorry if this is taking up a lot of time.
nit, could fix later: The original sentence covers all the flags. How about adding back the --generator flag as the last flag to be processed?
nit fix now: If you use a list, add a new line at line 85.
Kubectl processes the flags in the following order:

  1. ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had another go.

If this doesn't land via the v1.17 release, I can cherry-pick the same changes against master.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sftim. Last thing: Add a new line before line 67. The list does not render. Thanks!

and `--generator=run/v1`, a ReplicationController is created.
This enables you to pin to a specific behavior with the generator,
even when the default generator is changed later.
This means that when you combine `--generator` with other flags, the generator that you specified later does not change.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: clarify in another PR? This part still seems confusing to me -- default generator vs generated resource.
What happens to the generator -- it is not changed by using or overlaying a number of flag combinations.

@sftim sftim force-pushed the 20191115_kubectl_generators_cleanup branch from 31d6cc2 to 19fcb4f Compare November 28, 2019 22:29
@daminisatya
Copy link
Contributor

@kbhawkey is this pr good to go?

@kbhawkey
Copy link
Contributor

@kbhawkey is this pr good to go?

The list should be fixed, otherwise, Looks good. Thanks!

@sftim sftim force-pushed the 20191115_kubectl_generators_cleanup branch from 19fcb4f to 5e8dec0 Compare November 30, 2019 14:13
@sftim
Copy link
Contributor Author

sftim commented Nov 30, 2019

@kbhawkey fixed

@Rajakavitha1
Copy link
Contributor

@daminisatya !!! This PR is ready @sftim has made the changes suggested by @kbhawkey !!!!

@daminisatya
Copy link
Contributor

/lgtm
/approve

Thank you @Rajakavitha1 @sftim

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 3, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: daminisatya, tengqm

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

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 33c2748 into kubernetes:dev-1.17 Dec 3, 2019
mrbobbytables pushed a commit that referenced this pull request Dec 6, 2019
* Write ReplicationController without a space

* Drop mentioning unsupported cluster versions

* Fix capitalization for “API group”

* Tweak wording

* Avoid using deprecated generator in example
k8s-ci-robot pushed a commit that referenced this pull request Dec 10, 2019
* feat: graduate TaintNodesByCondition to GA (#17073)

* Promote StartupProbe to beta (enabled by default). (#17164)

* Watch bookmarks to GA (#17026)

* feat: graduate ScheduleDaemonSetPods to GA (#17350)

* Update Docker installation instructions (#17405)

* Use exact version numbers for installing Docker in Ubuntu (#17428)

* Move CSIMigration and CSIMigrationGCE to Beta in Kubernetes v1.17 (#17478)

* Promote NodeLease feature to GA (#17189)

* Update docs for csi topology ga (#17408)

* Update RunAsUsername to beta (#17460)

* doc:Update RunAsUsername to beta

* doc: update samples - kubernetes.io/os is no longer beta

* Updating based on review feedback

* Promote Node-specific volume limits to GA (#17432)

* Promote PodShareProcessNamespace to stable (#17192)

* Promote PodShareProcessNamespace to stable

* Add for_k8s_version to feature-state label

Co-Authored-By: Tim Bannister <tim@scalefactory.com>

* Readd version-check to shareProcessNamespace task

* Update service load balancer finalizer doc for GA (#17438)

* Update Topology Manager docs (#17451)

* Added information on how device plugins can take advantage
of Topology Manager
* Updated the Topology Manager documentation to include additionalinformation and update some out of date sections

* Fix broken Topology Manager link (#17746)

Part of What's Next Device Plugin section

* Update CRD defaulting docs for GA (#17450)

* Add documentation for VolumeSnapshot Beta (#17233)

* Updating EndpointSlice documentation for beta release in 1.17 (#17411)

* (docs/dualstack): v1.17 updates (#17457)

* Add placehold doc updates for dualstack in 1.17

Signed-off-by: Lachlan Evenson <lachlan.evenson@microsoft.com>

* Add Downward API and /etc/hosts Pod IP validation

Signed-off-by: Lachlan Evenson <lachlan.evenson@microsoft.com>

* remove addressed known issue via k/k pr 85246

Signed-off-by: Lachlan Evenson <lachlan.evenson@microsoft.com>

* Remove known issue and add flag as part of k/k 79993

Signed-off-by: Lachlan Evenson <lachlan.evenson@microsoft.com>

* remove follow up placeholders

Signed-off-by: Lachlan Evenson <lachlan.evenson@microsoft.com>

* Update verbiage

Signed-off-by: Lachlan Evenson <lachlan.evenson@microsoft.com>

* Make IP addressing consistent throughout the task

Signed-off-by: Lachlan Evenson <lachlan.evenson@microsoft.com>

* Update to status.podIPs

Signed-off-by: Lachlan Evenson <lachlan.evenson@microsoft.com>

* Update content/en/docs/tasks/network/validate-dual-stack.md

Use set instead of env

Co-Authored-By: Khaled Henidak (Kal) <khnidk@outlook.com>

* add topology.kubernetes.io/zone, topology.kubernetes.io/region and node.kubernetes.io/instance-type labels to docs (#17498)

Signed-off-by: Andrew Sy Kim <kiman@vmware.com>

* Service topology alpha documentation (#17459)

* Update list of feature flags for in-tree plugins migrated to CSI (#17533)

Signed-off-by: Deep Debroy <ddebroy@docker.com>

* Update Node concept for TaintNodesByCondition going GA (#17577)

* feat: graduate ResourceQuotaScopeSelectors to GA in 1.17 (#17554)

* kubeadm: update the upgrade documentation for 1.17 (#17587)

* doc: Simplify Windows deployments with RuntimeClass (#16697)

* doc: Simplify Windows deployments with RuntimeClass

* Updating on review feedback

* doc: Adding windows-build label from enhancement 1301

* update doc for kubelet option --reserved-cpus (#17648)

* feat: update TaintNodesByCondition in feature gates table (#17377)

* Update docs for v1 resource quota configuration (#17547)

* AdmissionConfiguration v1 (#17548)

* Update WebhookAdmissionConfiguration examples (#17549)

* Update AWS EBS Migration Feature state (#16126)

* Add resource version section to api-concepts documentation (#16910)

* Add Resource Version semantics section to api concepts

* Clarify risks of going back in time, add details about compaction and watch cache sizes

* Apply suggestions from liggitt

Co-Authored-By: Jordan Liggitt <jordan@liggitt.net>

* remove pesudocode, apply feedback

* Fix typo

* Clarify equality rules

* Cleanup kubectl generators docs (#17609)

* Write ReplicationController without a space

* Drop mentioning unsupported cluster versions

* Fix capitalization for “API group”

* Tweak wording

* Avoid using deprecated generator in example

* add Antrea description in dev-1.17 (#17919)

* Promote VolumeSubpathEnvExpansion to GA

* Reference Documentation for the Kubernetes API for 1.17 (#18019)

* Update feature-gates.md (#18033)

* Reference Documentation for kubectl Commands for 1.17 (#18017)

* Update for v1.17 (#18034)

* Update config.toml(release-1.17) for 1.17 (#18031)
@sftim sftim deleted the 20191115_kubectl_generators_cleanup branch June 9, 2021 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants