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.

Sign up for GitHub

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

Jump to bottom

kube-apiserver: output flags in logical sections #64517

Merged
merged 1 commit into from
Aug 6, 2018
Merged

kube-apiserver: output flags in logical sections #64517

merged 1 commit into from
Aug 6, 2018

Conversation

sttts
Copy link
Contributor

@sttts sttts commented May 30, 2018 •
edited
Loading

bildschirmfoto 2018-06-07 um 16 21 17

Print kube-apiserver --help flag help in sections.

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels May 30, 2018
@k8s-ci-robot k8s-ci-robot requested review from deads2k and wojtek-t May 30, 2018 14:23
@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 30, 2018
@sttts
Copy link
Contributor Author

sttts commented May 30, 2018

/cc @deads2k @hzxuzhonghu @mtaufen

hzxuzhonghu
hzxuzhonghu reviewed May 31, 2018
View reviewed changes
cmd/kube-apiserver/app/options/options.go Outdated
// AddFlags adds flags for a specific APIServer to the specified FlagSet
func (s *ServerRunOptions) AddFlags(fs *pflag.FlagSet) {
// Flags returns flags for a specific APIServer by section name
func (s *ServerRunOptions) Flags() (fss map[string]*pflag.FlagSet) {
Copy link
Member

Choose a reason for hiding this comment

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

redundant fss

@hzxuzhonghu
Copy link
Member

LGTM, it works fine.

The flags help info are well grouped instead of in alphabetical order.

The Kubernetes API server validates and configures data
for the api objects which include pods, services, replicationcontrollers, and
others. The API Server services REST operations and provides the frontend to the
cluster's shared state through which all other components interact.

Usage:
  kube-apiserver [flags]

Generic flags:
      --advertise-address ip                 The IP address on which to advertise the apiserver to members of the cluster. This address must be reachable by the rest of the cluster. If blank, the --bind-address will be used. If --bind-address is unspecified, the host's default interface will be used.
      --cors-allowed-origins strings         List of allowed origins for CORS, comma separated.  An allowed origin can be a regular expression to support subdomain matching. If this list is empty CORS will not be enabled.
      --external-hostname string             The hostname to use when generating externalized URLs for this master (e.g. Swagger API Docs).
      --feature-gates mapStringBool          A set of key=value pairs that describe feature gates for alpha/experimental features. Options are:
                APIListChunking=true|false (BETA - default=true)
                APIResponseCompression=true|false (ALPHA - default=false)
                AdvancedAuditing=true|false (BETA - default=true)
                AllAlpha=true|false (ALPHA - default=false)

@sttts
Copy link
Contributor Author

sttts commented May 31, 2018

It's a bit ugly that we cannot pass a uniform indention to cobra for all sections. Actually, I like the indention with the newline best:

    --foo-very-long-flag-so-that-not-space-is-left
         This does some thing cool, over many many many many many many many many
         many many many many many many many many lines.

This indention is only chosen implicitly by cobra if there is really not enough space. We would need to contribute something upstream to get this feature under control.

@sttts sttts force-pushed the sttts-apiserver-sectioned-flags branch from cc48ea7 to d63901e Compare June 7, 2018 14:18
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 7, 2018
@sttts
Copy link
Contributor Author

sttts commented Jun 7, 2018

Fixed the order. Enforcing the "newline mode" now via a fake zzzzzzz flag.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 24, 2018
@sttts sttts changed the title [WIP] apiserver: output flags in logical sections kube-apiserver: output flags in logical sections Aug 2, 2018
@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 Aug 2, 2018
@sttts sttts force-pushed the sttts-apiserver-sectioned-flags branch from d63901e to 4b7016e Compare August 2, 2018 09:18
@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Aug 2, 2018
@sttts sttts added the kind/feature Categorizes issue or PR as related to a new feature. label Aug 2, 2018
@sttts sttts force-pushed the sttts-apiserver-sectioned-flags branch from 4b7016e to 4aeef27 Compare August 2, 2018 09:29
@sttts sttts force-pushed the sttts-apiserver-sectioned-flags branch from 4aeef27 to efc48e9 Compare August 2, 2018 09:34
@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

15 similar comments
@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

@k8s-ci-robot
Copy link
Contributor

New changes are detected. LGTM label has been removed.

@sttts sttts force-pushed the sttts-apiserver-sectioned-flags branch from 45d0b0a to 42d533e Compare August 6, 2018 06:55
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 6, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sttts, yue9944882

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

@sttts sttts added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 6, 2018
@k8s-github-robot
Copy link

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

@k8s-github-robot
Copy link

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-github-robot k8s-github-robot merged commit a160fe9 into kubernetes:master Aug 6, 2018
@stewart-yu stewart-yu mentioned this pull request Sep 5, 2018
Merged
This was referenced Oct 24, 2018
Closed
Merged
@yue9944882 yue9944882 mentioned this pull request Dec 13, 2018
Closed
@soltysh soltysh mentioned this pull request Jan 9, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hzxuzhonghu hzxuzhonghu hzxuzhonghu left review comments

@yue9944882 yue9944882 yue9944882 left review comments

@deads2k deads2k Awaiting requested review from deads2k

@wojtek-t wojtek-t Awaiting requested review from wojtek-t

@mtaufen mtaufen Awaiting requested review from mtaufen

Assignees

@yue9944882 yue9944882

@hzxuzhonghu hzxuzhonghu

@stewart-yu stewart-yu

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/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@sttts @hzxuzhonghu @yue9944882 @fejta-bot @k8s-ci-robot @k8s-github-robot @stewart-yu