Mark Kubelet --cloud-provider and --cloud-config deprecated #90408

knabben · 2020-04-23T14:29:26Z

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

Moving Kubelet flags to configuration file.

/area kubelet
/area kubelet-api

Which issue(s) this PR fixes:

Refs #86843

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

The Kubelet's `--cloud-provider` and `--cloud-config` options are now marked as deprecated.

k8s-ci-robot · 2020-04-23T14:29:35Z

Hi @knabben. 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 /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.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

fejta-bot · 2020-04-23T15:36:13Z

This PR may require API review.

If so, when the changes are ready, complete the pre-review checklist and request an API review.

Status of requested reviews is tracked in the API Review project.

stealthybox · 2020-04-23T18:24:38Z

/lgtm
This seems like a good addition to the existing KubeletConfiguration UX

knabben · 2020-04-23T22:26:14Z

/assign @liggitt

liggitt · 2020-04-27T13:34:12Z

pkg/kubelet/apis/config/types.go

@@ -100,6 +100,8 @@ type KubeletConfiguration struct {
 	// volumePluginDir is the full path of the directory in which to search
 	// for additional third party volume plugins.
 	VolumePluginDir string
+	// cloudProvider is the provider for cloud services.


copy the flag documentation here (meaning of empty string, etc), and maybe also call out the special "external" value?

liggitt · 2020-04-27T13:34:16Z

staging/src/k8s.io/kubelet/config/v1beta1/types.go

@@ -304,6 +304,9 @@ type KubeletConfiguration struct {
 	// Default: "4h"
 	// +optional
 	StreamingConnectionIdleTimeout metav1.Duration `json:"streamingConnectionIdleTimeout,omitempty"`
+	// cloudProvider is the provider for cloud services.


copy the flag documentation here (meaning of empty string, etc), and maybe also call out the special "external" value?

We should include the same details as the flag help, e.g.:
"Specify empty string for running with no cloud provider. If set, the cloud provider determines the name of the node (consult cloud provider documentation to determine if and how the hostname is used)."

liggitt · 2020-04-27T13:34:46Z

cmd/kubelet/app/options/options.go

@@ -368,7 +364,6 @@ func (f *KubeletFlags) AddFlags(mainfs *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. Specify empty string for running with no cloud provider. If set, the cloud provider determines the name of the node (consult cloud provider documentation to determine if and how the hostname is used).")
 	fs.StringVar(&f.CloudConfigFile, "cloud-config", f.CloudConfigFile, "The path to the cloud provider configuration file.  Empty string for no configuration file.")


I expected cloud-provider and cloud-config to move together to stay coherent

liggitt · 2020-04-27T13:35:57Z

pkg/kubelet/apis/config/types.go

@@ -100,6 +100,8 @@ type KubeletConfiguration struct {
 	// volumePluginDir is the full path of the directory in which to search
 	// for additional third party volume plugins.
 	VolumePluginDir string
+	// cloudProvider is the provider for cloud services.
+	CloudProvider string


is there any validation needed in ValidateKubeletConfiguration?

the validation is made on InitCloudProvider, not in the flag, should it be moved to the ValidateKubeletConfiguration?

anything that can be changed via the config file should be validated as much as possible in ValidateKubeletConfiguration... that is what protects us from picking up a bad config value in an dynamic config update. cc @mtaufen

edit: the validation should be limited to syntactic errors in the config

/cc @cheftako
for opinion on what we should validate up-front and what should be left to the cloud-provider integration

My primary opinion is that we should deprecate this. The goal is to get rid of it in a year (From Kubelet, Kube API Server and Kube Controller Manager).

Also I believe the format of the config file is dependent upon the cloud provider implementation. So we could see what it would take to call add and call a validate method from kubelet's validate. Given that its deprecated, that may be overkill.

cheftako · 2020-06-03T16:58:04Z

/lgtm

liggitt · 2020-06-03T17:01:57Z

/approve

k8s-ci-robot · 2020-06-03T17:04:29Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: knabben, liggitt, mtaufen

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:

~~cmd/kubelet/OWNERS~~ [liggitt]
~~staging/src/k8s.io/cloud-provider/OWNERS~~ [liggitt]

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

knabben · 2020-06-04T11:20:42Z

/test all

knabben · 2020-06-04T12:40:29Z

/retest

knabben · 2020-06-04T17:48:50Z

/retest

rtheis · 2020-08-04T20:24:06Z

As far as I can tell, --cloud-provider=external is still needed for a proper CCM deployment. Should this flag really be deprecated since it is still needed so nodes can be tainted for an external cloud provider?

cheftako · 2020-08-06T17:06:41Z

As far as I can tell, --cloud-provider=external is still needed for a proper CCM deployment. Should this flag really be deprecated since it is still needed so nodes can be tainted for an external cloud provider?

The code executed under the --cloud-provider=external case should become the only option for Kubelet, KCM and KAS. There exist today valid configurations of these where cloud provider is set to something other than external. However we are hoping that soon that will no longer be true in a few more releases. Once external is the only valid configuration; any other configuration would be an error. We would like to be able to delete the flag once it can only have 1 legal value but to do that the flag has to have been deprecated for at least a year.

rtheis · 2020-08-06T17:34:15Z

Thanks for the clarification @cheftako.

aojea · 2023-10-19T14:46:33Z

The code executed under the --cloud-provider=external case should become the only option for Kubelet, KCM and KAS. There exist today valid configurations of these where cloud provider is set to something other than external. However we are hoping that soon that will no longer be true in a few more releases. Once external is the only valid configuration; any other configuration would be an error. We would like to be able to delete the flag once it can only have 1 legal value but to do that the flag has to have been deprecated for at least a year.

@cheftako the kubelet should be able to run in two modes, standalone or cloud-provider, how is kubelet supposed to decide which mode is going to use if we remove the flag?

liggitt · 2023-10-19T15:03:52Z

The description of this PR says it was deprecated in favor of the config file, but there's no config file field to specify an external cloud provider.

liggitt · 2023-10-19T15:04:28Z

Once external is the only valid configuration; any other configuration would be an error. We would like to be able to delete the flag once it can only have 1 legal value but to do that the flag has to have been deprecated for at least a year.

There will not be only one legal value. As @aojea mentioned, there is standalone mode and external cloud provider mode.

aojea · 2023-10-19T15:08:45Z

this will break a lot of installers, I suggest we remove the deprecation notice, seems the least resistance path

liggitt · 2023-10-19T15:12:10Z

Opened #121367 to drop the deprecation notice from --cloud-provider

--cloud-config can stay deprecated since that is not used with external cloud providers

cheftako · 2023-10-19T17:34:18Z

The code executed under the --cloud-provider=external case should become the only option for Kubelet, KCM and KAS. There exist today valid configurations of these where cloud provider is set to something other than external. However we are hoping that soon that will no longer be true in a few more releases. Once external is the only valid configuration; any other configuration would be an error. We would like to be able to delete the flag once it can only have 1 legal value but to do that the flag has to have been deprecated for at least a year.

Walter Fender the kubelet should be able to run in two modes, standalone or cloud-provider, how is kubelet supposed to decide which mode is going to use if we remove the flag?

Our goal is to run without any cloud-provider code compiled into the Kubelet, KCM or KAS. The cloud-provider=external is meant to emulate that scenario. So I would hope that standalone is equivalent (or at least orthoganol) to cloud-provider=external. Once that is true we should be able to remove the flag and stop compiling cloud provider into these binaries.

liggitt · 2023-10-19T18:02:11Z

So I would hope that standalone is equivalent (or at least orthoganol) to cloud-provider=external.

--cloud-provider=external turns off node reconciliation of its own addresses and adds a taint on self-registration that the cloud provider has to remove before the kubelet is considered ready. I think that's about it. It's primarily about turning off things that make sense for the kubelet to self-report or self-reconcile when it is not running in a cloud environment.

Once that is true we should be able to remove the flag

You'll still need a way to tell the kubelet something external is responsible for its addresses and initial readiness, everyone is already using --cloud-provider=external for that, I don't see a reason to switch to another flag that has the same meaning / semantics

k8s-ci-robot requested review from gmarek and sjenning April 23, 2020 14:32

k8s-ci-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API sig/node Categorizes an issue or PR as relevant to SIG Node. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Apr 23, 2020

k8s-ci-robot assigned stealthybox Apr 23, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 23, 2020

k8s-ci-robot assigned liggitt Apr 23, 2020

liggitt reviewed Apr 27, 2020

View reviewed changes

knabben force-pushed the kubelet-cloud-provider branch from 35e484f to 8f63e55 Compare April 28, 2020 14:11

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Apr 28, 2020

k8s-ci-robot requested a review from cheftako April 28, 2020 17:10

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 3, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 3, 2020

k8s-ci-robot merged commit ef1bc41 into kubernetes:master Jun 4, 2020

k8s-ci-robot added this to the v1.19 milestone Jun 4, 2020

github-actions bot mentioned this pull request Jun 9, 2020

Week Ending June 7, 2020 dev-obs/actus#175

Open

Danil-Grigorev mentioned this pull request Oct 6, 2020

Describing steps to support out-of-tree providers openshift/enhancements#463

Merged

This was referenced May 21, 2021

Removed default hostname-override kubernetes/kops#10595

Closed

Remove --cloud-provider flag kubernetes/kops#11546

Closed

fhofherr mentioned this pull request Jul 9, 2021

Explain --cloud-provider=external a bit more hetznercloud/hcloud-cloud-controller-manager#217

Merged

ialidzhikov mentioned this pull request Mar 10, 2022

[Failing test] guestbook test is failing on aws with K8s v1.23 gardener/gardener-extension-provider-aws#514

Closed

maxime1907 mentioned this pull request Feb 13, 2023

fix(kubelet): no cloud config for external cloud provider kubernetes-sigs/kubespray#9793

Merged

liggitt mentioned this pull request Oct 19, 2023

Undeprecated kubelet cloud-provider flag #121367

Merged

aojea mentioned this pull request Oct 19, 2023

remove deprecation for kubelet cloud-provider and cloud-config flags #121369

Closed

ahmedwaleedmalik mentioned this pull request Jan 16, 2024

Bring back cloud-provider flag for Kubelet kubermatic/operating-system-manager#348

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mark Kubelet --cloud-provider and --cloud-config deprecated #90408

Mark Kubelet --cloud-provider and --cloud-config deprecated #90408

knabben commented Apr 23, 2020 •

edited

Loading

k8s-ci-robot commented Apr 23, 2020

fejta-bot commented Apr 23, 2020

stealthybox commented Apr 23, 2020

knabben commented Apr 23, 2020

liggitt Apr 27, 2020

liggitt Apr 27, 2020

mtaufen May 12, 2020

liggitt Apr 27, 2020

liggitt Apr 27, 2020

knabben Apr 28, 2020

liggitt Apr 28, 2020 •

edited

Loading

mtaufen Apr 28, 2020

cheftako May 14, 2020

cheftako May 14, 2020

cheftako commented Jun 3, 2020

liggitt commented Jun 3, 2020

k8s-ci-robot commented Jun 3, 2020

knabben commented Jun 4, 2020

knabben commented Jun 4, 2020

knabben commented Jun 4, 2020

rtheis commented Aug 4, 2020

cheftako commented Aug 6, 2020 •

edited

Loading

rtheis commented Aug 6, 2020

aojea commented Oct 19, 2023

liggitt commented Oct 19, 2023

liggitt commented Oct 19, 2023

aojea commented Oct 19, 2023

liggitt commented Oct 19, 2023

cheftako commented Oct 19, 2023

liggitt commented Oct 19, 2023 •

edited

Loading

Mark Kubelet --cloud-provider and --cloud-config deprecated #90408

Mark Kubelet --cloud-provider and --cloud-config deprecated #90408

Conversation

knabben commented Apr 23, 2020 • edited Loading

k8s-ci-robot commented Apr 23, 2020

fejta-bot commented Apr 23, 2020

stealthybox commented Apr 23, 2020

knabben commented Apr 23, 2020

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

liggitt Apr 28, 2020 • edited Loading

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

cheftako commented Jun 3, 2020

liggitt commented Jun 3, 2020

k8s-ci-robot commented Jun 3, 2020

knabben commented Jun 4, 2020

knabben commented Jun 4, 2020

knabben commented Jun 4, 2020

rtheis commented Aug 4, 2020

cheftako commented Aug 6, 2020 • edited Loading

rtheis commented Aug 6, 2020

aojea commented Oct 19, 2023

liggitt commented Oct 19, 2023

liggitt commented Oct 19, 2023

aojea commented Oct 19, 2023

liggitt commented Oct 19, 2023

cheftako commented Oct 19, 2023

liggitt commented Oct 19, 2023 • edited Loading

knabben commented Apr 23, 2020 •

edited

Loading

liggitt Apr 28, 2020 •

edited

Loading

cheftako commented Aug 6, 2020 •

edited

Loading

liggitt commented Oct 19, 2023 •

edited

Loading