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

Show deprecated kube-apiserver flags #62621

Merged

Conversation

hzxuzhonghu
Copy link
Member

What this PR does / why we need it:

This PR unhides deprecated kube-apiserver flags, so that the deprecation notice is clearly visible in --help.

Fixes #62617

Release note:

NONE

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 16, 2018
@hzxuzhonghu
Copy link
Member Author

/assign @liggitt @sttts

@hzxuzhonghu
Copy link
Member Author

/retest

@CaoShuFeng
Copy link
Contributor

diff of help message:

>       --address ip                                              DEPRECATED: see --insecure-bind-address instead. (default 127.0.0.1) (DEPRECATED: see --insecure-bind-address instead.)
>       --admission-control strings                               Admission is divided into two phases. In the first phase, only mutating admission plugins run. In the second phase, only validating admission plugins run. The names in the below list may represent a validating plugin, a mutating plugin, or both. The order of plugins in which they are passed to this flag does not matter. Comma-delimited list of: AlwaysAdmit, AlwaysDeny, AlwaysPullImages, DefaultStorageClass, DefaultTolerationSeconds, DenyEscalatingExec, DenyExecOnPrivileged, EventRateLimit, ExtendedResourceToleration, ImagePolicyWebhook, InitialResources, Initializers, LimitPodHardAntiAffinityTopology, LimitRanger, MutatingAdmissionWebhook, NamespaceAutoProvision, NamespaceExists, NamespaceLifecycle, NodeRestriction, OwnerReferencesPermissionEnforcement, PersistentVolumeClaimResize, PersistentVolumeLabel, PodNodeSelector, PodPreset, PodSecurityPolicy, PodTolerationRestriction, Priority, ResourceQuota, SecurityContextDeny, ServiceAccount, StorageObjectInUseProtection, ValidatingAdmissionWebhook. (DEPRECATED: Use --enable-admission-plugins or --disable-admission-plugins instead. Will be removed in a future version.)
30a33
>       --audit-webhook-batch-initial-backoff duration            The amount of time to wait before retrying the first failed request. (default 10s) (DEPRECATED: Deprecated, use --audit-webhook-initial-backoff instead.)
74a78
>       --etcd-quorum-read                                        If true, enable quorum read. It defaults to true and is strongly recommended not setting to false. (default true) (DEPRECATED: This flag is deprecated and the ability to switch off quorum read will be removed in a future release.)
129a134,135
>       --insecure-bind-address ip                                The IP address on which to serve the --insecure-port (set to 0.0.0.0 for all IPv4 interfaces and :: for all IPv6 interfaces). (default 127.0.0.1) (DEPRECATED: This flag will be removed in a future version.)
>       --insecure-port int                                       The port on which to serve unsecured, unauthenticated access. It is assumed that firewall rules are set up such that this port is not reachable from outside of the cluster and that port 443 on the cluster's public address is proxied to this port. This is performed by nginx in the default setup. Set to zero to disable. (default 8080) (DEPRECATED: This flag will be removed in a future version.)
141a148
>       --kubelet-port uint                                       DEPRECATED: kubelet port. (default 10250) (DEPRECATED: kubelet-port is deprecated and will be removed.)
163a171
>       --port int                                                DEPRECATED: see --insecure-port instead. (default 8080) (DEPRECATED: see --insecure-port instead.)
182a191,192
>       --ssh-keyfile string                                      If non-empty, use secure SSH proxy to the nodes, using this user keyfile (DEPRECATED: This flag will be removed in a future version.)
>       --ssh-user string                                         If non-empty, use secure SSH proxy to the nodes, using this user name (DEPRECATED: This flag will be removed in a future version.)

// Unhide deprecated flags. We want deprecated flags to show in kube-apiserver help.
// We have some hidden flags, but we might as well unhide these when they are deprecated,
// as silently deprecating and removing (even hidden) things is unkind to people who use them.
fs.VisitAll(func(flag *pflag.Flag) {
Copy link
Contributor

Choose a reason for hiding this comment

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

didn't know this is possible so easily 👍

@sttts
Copy link
Contributor

sttts commented Apr 16, 2018

There are many flags (also in other components) with "Deprecated" in its description. Can we call MarkDeprectated for all of them and then add code like in this PR to make them visible?

@CaoShuFeng
Copy link
Contributor

There are many flags (also in other components) with "Deprecated" in its description. Can we call MarkDeprectated for all of them and then add code like in this PR to make them visible?

I am working on kubectl.

@sttts
Copy link
Contributor

sttts commented Apr 16, 2018

@CaoShuFeng for easier approval, you can open multiple PRs if that helps.

// as silently deprecating and removing (even hidden) things is unkind to people who use them.
fs.VisitAll(func(flag *pflag.Flag) {
if len(flag.Deprecated) > 0 {
flag.Hidden = false
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi, will this take the hidden flag back?

What should I do if I do want to hide it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ref #62009 (comment)
This is intended.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK.
What should I do if I do want to hide it?

I do not want to hide it. 😆

@liggitt
Copy link
Member

liggitt commented Apr 16, 2018

I don't think we need to make all of the apiserver flags visible like this. The reason we did that for the kubelet was that all flags that could be specified via config file were marked deprecated, which made most of the kubelet flags invisible, despite their function still being supported.

The only ones I would add back are --admission-control, --insecure-bind-address, and --insecure-port

@hzxuzhonghu
Copy link
Member Author

The only ones I would add back are --admission-control, --insecure-bind-address, and --insecure-port

But how can we make it? like this

pflag.VisitAll(func(flag *pflag.Flag) {
	if flag.Name == "admission-control" {
		flag.Hidden = false
        }
})

@liggitt
Copy link
Member

liggitt commented Apr 16, 2018

func (s *InsecureServingOptions) AddFlags(fs *pflag.FlagSet) {
	fs.IPVar(&s.BindAddress, "insecure-bind-address", s.BindAddress, ""+
		"The IP address on which to serve the --insecure-port (set to 0.0.0.0 for all IPv4 interfaces and :: for all IPv6 interfaces).")
	fs.MarkDeprecated("insecure-bind-address", "This flag will be removed in a future version.")
+	fs.Lookup("insecure-bind-address").Hidden = false

	fs.IntVar(&s.BindPort, "insecure-port", s.BindPort, ""+
		"The port on which to serve unsecured, unauthenticated access. It is assumed "+
		"that firewall rules are set up such that this port is not reachable from outside of "+
		"the cluster and that port 443 on the cluster's public address is proxied to this "+
		"port. This is performed by nginx in the default setup. Set to zero to disable.")
	fs.MarkDeprecated("insecure-port", "This flag will be removed in a future version.")
+	fs.Lookup("insecure-port").Hidden = false
}

@hzxuzhonghu hzxuzhonghu force-pushed the kube-api-show-deprecated-flags branch from 31899b3 to 8f98af9 Compare April 16, 2018 13:20
@liggitt
Copy link
Member

liggitt commented Apr 16, 2018

/lgtm

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hzxuzhonghu, liggitt

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 added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 16, 2018
@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 58784, 62057, 62621, 62652, 62656). If you want to cherry-pick this change to another branch, please follow the instructions here.

k8s-github-robot pushed a commit that referenced this pull request May 14, 2018
…pstream-release-1.10

Automatic merge from submit-queue.

Automated cherry pick of #62621: Show deprecated kube-apiserver flags

Cherry pick of #62621 on release-1.10.

#62621: Show deprecated kube-apiserver flags

```release-note
Show help for deprecated kube-apiserver flags
```
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. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants