Deprecation warnings for auto detecting cloud providers #51312
Conversation
k8s-ci-robot
added
size/S
|
/sig node |
k8s-ci-robot
added
the
sig/node
k8s-github-robot
added
the
release-note
cmd/kubelet/app/options/options.go
Outdated
| // with cloud providers instead of in the core repo. | ||
| // More details here: https://github.com/kubernetes/kubernetes/issues/50986 | ||
| CloudProvider: v1alpha1.AutoDetectCloudProvider, | ||
| RootDirectory: v1alpha1.DefaultRootDir, |
There was a problem hiding this comment.
Might be better to move RootDirectory above the comment?
| @@ -220,7 +224,7 @@ func (f *KubeletFlags) AddFlags(fs *pflag.FlagSet) { | |||
| fs.StringVar(&f.CertDirectory, "cert-dir", f.CertDirectory, "The directory where the TLS certs are located. "+ | |||
| "If --tls-cert-file and --tls-private-key-file are provided, this flag will be ignored.") | |||
|
|
|||
| fs.StringVar(&f.CloudProvider, "cloud-provider", f.CloudProvider, "The provider for cloud services. By default, kubelet will attempt to auto-detect the cloud provider. Specify empty string for running with no cloud provider.") | |||
| fs.StringVar(&f.CloudProvider, "cloud-provider", f.CloudProvider, "The provider for cloud services. By default, kubelet will attempt to auto-detect the cloud provider (deprecated). Specify empty string for running with no cloud provider, this will be the default in upcoming releases.") | |||
There was a problem hiding this comment.
Please add a MarkDeprecated e.g. fs.MarkDeprecated("cloud-provider", "Specify empty string for running with no cloud provider, this will be the default in upcoming releases.")
And maybe make the StringVar doc string start with "Deprecated", so it's super-obvious.
There was a problem hiding this comment.
@mtaufen Don't want users thinking we're deprecating the cloud-provider flag if we start the flag descrption with "DEPRECATED". Do you forsee that as a problem?
There was a problem hiding this comment.
Oh duh. I see your point, you aren't deprecating the entire flag.
Can we at least print a deprecation warning when people use the auto-detection feature?
|
PTAL @kubernetes/sig-node-pr-reviews @kubernetes/sig-cluster-lifecycle-pr-reviews @wlan0 @andrewsykim Did any of you send out an email to kubernetes-dev? |
k8s-ci-robot
added
the
sig/cluster-lifecycle
|
@luxas will send the email out today, wanted to open the PR first so we have something concrete to show. |
|
Email sent! |
|
@mtaufen comments addressed |
|
/test pull-kubernetes-e2e-gce-etcd3 |
|
cc @dims |
k8s-ci-robot
added
the
lgtm
|
@kubernetes/sig-node-pr-reviews |
|
/lgtm |
|
/approve |
k8s-github-robot
added
the
approved
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andrewsykim, luxas, mtaufen, vishh, yujuhong Associated issue: 50986 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
/retest Review the full test history for this PR. |
|
Automatic merge from submit-queue (batch tested with PRs 50932, 49610, 51312, 51415, 50705) |
Following up on PR kubernetes#53573 and PR kubernetes#51312, Let's drop all code related to auto-detection for v1.10.
What this PR does / why we need it:
Adds deprecation warnings for auto detecting cloud providers. As part of the initiative for out-of-tree cloud providers, this feature is conflicting since we're shifting the dependency of kubernetes core into cAdvisor. In the future kubelets should be using
--cloud-provider=externalor no cloud provider at all.Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged): fixes #50986Special notes for your reviewer:
NOTE: I still have to coordinate with sig-node and kubernetes-dev to get approval for this deprecation, I'm only opening this PR since we're close to code freeze and it's something presentable.
Release note: