From 38fe239ac44050c2ed83ffb8ea7e514db71f903f Mon Sep 17 00:00:00 2001 From: elmiko Date: Fri, 27 Sep 2024 10:03:18 -0400 Subject: [PATCH] factor our cloudprovider.DeprecationWarningForProvider this change removes the deprecation warning function in favor of using the `cloudprovider.DisableWarningForProvider`. it also fixes some of the logic to ensure that non-external providers are properly detected and warned about. --- .../app/cloudproviders.go | 27 ++++++------ cmd/kubelet/app/server.go | 14 ++----- pkg/kubeapiserver/options/cloudprovider.go | 11 +---- pkg/kubelet/kubelet.go | 2 +- staging/src/k8s.io/cloud-provider/plugins.go | 41 +++++-------------- 5 files changed, 28 insertions(+), 67 deletions(-) diff --git a/cmd/kube-controller-manager/app/cloudproviders.go b/cmd/kube-controller-manager/app/cloudproviders.go index 562e478f2d5..ab259f5b17f 100644 --- a/cmd/kube-controller-manager/app/cloudproviders.go +++ b/cmd/kube-controller-manager/app/cloudproviders.go @@ -19,9 +19,7 @@ package app import ( "fmt" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/klog/v2" - "k8s.io/kubernetes/pkg/features" "k8s.io/client-go/informers" cloudprovider "k8s.io/cloud-provider" @@ -32,18 +30,10 @@ import ( func createCloudProvider(logger klog.Logger, cloudProvider string, externalCloudVolumePlugin string, cloudConfigFile string, allowUntaggedCloud bool, sharedInformers informers.SharedInformerFactory) (cloudprovider.Interface, ControllerLoopMode, error) { var cloud cloudprovider.Interface - var loopMode ControllerLoopMode var err error - - if utilfeature.DefaultFeatureGate.Enabled(features.DisableCloudProviders) && !cloudprovider.IsExternal(cloudProvider) { - cloudprovider.DisableWarningForProvider(cloudProvider) - return nil, ExternalLoops, fmt.Errorf( - "cloud provider %q was specified, but built-in cloud providers are disabled. Please set --cloud-provider=external and migrate to an external cloud provider", - cloudProvider) - } + loopMode := ExternalLoops if cloudprovider.IsExternal(cloudProvider) { - loopMode = ExternalLoops if externalCloudVolumePlugin == "" { // externalCloudVolumePlugin is temporary until we split all cloud providers out. // So we just tell the caller that we need to run ExternalLoops without any cloud provider. @@ -51,10 +41,17 @@ func createCloudProvider(logger klog.Logger, cloudProvider string, externalCloud } cloud, err = cloudprovider.InitCloudProvider(externalCloudVolumePlugin, cloudConfigFile) } else { - cloudprovider.DeprecationWarningForProvider(cloudProvider) - - loopMode = IncludeCloudLoops - cloud, err = cloudprovider.InitCloudProvider(cloudProvider, cloudConfigFile) + // in the case where the cloudProvider is not set, we need to inform the caller that there + // is no cloud provider and that the default loops should be used, and there is no error. + // this will cause the kube-controller-manager to start the default controller contexts + // without being attached to a specific cloud. + if len(cloudProvider) == 0 { + loopMode = IncludeCloudLoops + } else { + // for all other cloudProvider values the internal cloud loops are disabled + cloudprovider.DisableWarningForProvider(cloudProvider) + err = cloudprovider.ErrorForDisabledProvider(cloudProvider) + } } if err != nil { return nil, loopMode, fmt.Errorf("cloud provider could not be initialized: %v", err) diff --git a/cmd/kubelet/app/server.go b/cmd/kubelet/app/server.go index 5e2300ddbb8..11bcb069a53 100644 --- a/cmd/kubelet/app/server.go +++ b/cmd/kubelet/app/server.go @@ -651,16 +651,10 @@ func run(ctx context.Context, s *options.KubeletServer, kubeDeps *kubelet.Depend } if kubeDeps.Cloud == nil { - if !cloudprovider.IsExternal(s.CloudProvider) { - cloudprovider.DeprecationWarningForProvider(s.CloudProvider) - cloud, err := cloudprovider.InitCloudProvider(s.CloudProvider, s.CloudConfigFile) - if err != nil { - return err - } - if cloud != nil { - klog.V(2).InfoS("Successfully initialized cloud provider", "cloudProvider", s.CloudProvider, "cloudConfigFile", s.CloudConfigFile) - } - kubeDeps.Cloud = cloud + if !cloudprovider.IsExternal(s.CloudProvider) && len(s.CloudProvider) != 0 { + // internal cloud provider loops are disabled + cloudprovider.DisableWarningForProvider(s.CloudProvider) + return cloudprovider.ErrorForDisabledProvider(s.CloudProvider) } } diff --git a/pkg/kubeapiserver/options/cloudprovider.go b/pkg/kubeapiserver/options/cloudprovider.go index 716e0651e6b..5fd96aa1dbb 100644 --- a/pkg/kubeapiserver/options/cloudprovider.go +++ b/pkg/kubeapiserver/options/cloudprovider.go @@ -42,7 +42,7 @@ func (opts *CloudProviderOptions) Validate() []error { switch { case opts.CloudProvider == "": - case opts.CloudProvider == "external": + case cloudprovider.IsExternal(opts.CloudProvider): if !utilfeature.DefaultFeatureGate.Enabled(features.DisableCloudProviders) { errs = append(errs, fmt.Errorf("when using --cloud-provider set to '%s', "+ "please set DisableCloudProviders feature to true", opts.CloudProvider)) @@ -51,15 +51,6 @@ func (opts *CloudProviderOptions) Validate() []error { errs = append(errs, fmt.Errorf("when using --cloud-provider set to '%s', "+ "please set DisableKubeletCloudCredentialProviders feature to true", opts.CloudProvider)) } - case !cloudprovider.IsExternal(opts.CloudProvider): - if utilfeature.DefaultFeatureGate.Enabled(features.DisableCloudProviders) { - errs = append(errs, fmt.Errorf("when using --cloud-provider set to '%s', "+ - "please set DisableCloudProviders feature to false", opts.CloudProvider)) - } - if utilfeature.DefaultFeatureGate.Enabled(features.DisableKubeletCloudCredentialProviders) { - errs = append(errs, fmt.Errorf("when using --cloud-provider set to '%s', "+ - "please set DisableKubeletCloudCredentialProviders feature to false", opts.CloudProvider)) - } default: errs = append(errs, fmt.Errorf("unknown --cloud-provider: %s", opts.CloudProvider)) } diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index 516f9d5014d..81e9ce4c4cd 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -386,7 +386,7 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration, return nil, fmt.Errorf("invalid sync frequency %d", kubeCfg.SyncFrequency.Duration) } - if utilfeature.DefaultFeatureGate.Enabled(features.DisableCloudProviders) && !cloudprovider.IsExternal(cloudProvider) { + if !cloudprovider.IsExternal(cloudProvider) && len(cloudProvider) != 0 { cloudprovider.DisableWarningForProvider(cloudProvider) return nil, fmt.Errorf("cloud provider %q was specified, but built-in cloud providers are disabled. Please set --cloud-provider=external and migrate to an external cloud provider", cloudProvider) } diff --git a/staging/src/k8s.io/cloud-provider/plugins.go b/staging/src/k8s.io/cloud-provider/plugins.go index 718016a2c87..8ef778b1785 100644 --- a/staging/src/k8s.io/cloud-provider/plugins.go +++ b/staging/src/k8s.io/cloud-provider/plugins.go @@ -33,13 +33,8 @@ type Factory func(config io.Reader) (Interface, error) // All registered cloud providers. var ( - providersMutex sync.Mutex - providers = make(map[string]Factory) - deprecatedCloudProviders = []struct { - name string - external bool - detail string - }{} + providersMutex sync.Mutex + providers = make(map[string]Factory) ) const externalCloudProvider = "external" @@ -87,33 +82,17 @@ func IsExternal(name string) bool { // DisableWarningForProvider logs information about disabled cloud provider state func DisableWarningForProvider(providerName string) { - for _, provider := range deprecatedCloudProviders { - if provider.name == providerName { - klog.Infof("INFO: Please make sure you are running external cloud controller manager binary for provider %q."+ - "In-tree cloud providers are currently disabled. Refer to https://github.com/kubernetes/kubernetes/tree/master/staging/src/k8s.io/cloud-provider/sample"+ - "for example implementation.", providerName) - detail := fmt.Sprintf("Please reach to sig-cloud-provider and use 'external' cloud provider for %q: %s", providerName, provider.detail) - klog.Warningf("WARNING: %q built-in cloud provider is now disabled. %s", providerName, detail) - break - } + if !IsExternal(providerName) { + klog.Infof("INFO: Please make sure you are running an external cloud controller manager binary for provider %q."+ + "In-tree cloud providers are disabled. Refer to https://github.com/kubernetes/kubernetes/tree/master/staging/src/k8s.io/cloud-provider/sample "+ + "for an example implementation.", providerName) + klog.Warningf("WARNING: built-in cloud providers are disabled. Please set \"--cloud-provider=external\" and migrate to an external cloud controller manager for provider %q", providerName) } } -// DeprecationWarningForProvider logs information about deprecated cloud provider state -func DeprecationWarningForProvider(providerName string) { - for _, provider := range deprecatedCloudProviders { - if provider.name != providerName { - continue - } - - detail := provider.detail - if provider.external { - detail = fmt.Sprintf("Please use 'external' cloud provider for %s: %s", providerName, provider.detail) - } - - klog.Warningf("WARNING: %s built-in cloud provider is now deprecated. %s", providerName, detail) - break - } +// ErrorForDisabledProvider returns an error formatted with the supplied provider name +func ErrorForDisabledProvider(providerName string) error { + return fmt.Errorf("cloud provider %q was specified, but built-in cloud providers are disabled. Please set --cloud-provider=external and migrate to an external cloud provider", providerName) } // InitCloudProvider creates an instance of the named cloud provider.