Merge pull request kubernetes#127711 from elmiko/correct-provider-deprecation-logic

Correct cloud provider detection logic to be more representative of deprecation and disablement status

Author: k8s-ci-robot

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,29 +30,28 @@ 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.IsDeprecatedInternal(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.
 			return nil, loopMode, nil
 		}
 		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)

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)
 		}
 	}
 

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.IsDeprecatedInternal(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))
 	}

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.IsDeprecatedInternal(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)
 	}

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"
@@ -85,47 +80,19 @@ func IsExternal(name string) bool {
 	return name == externalCloudProvider
 }
 
-// IsDeprecatedInternal is responsible for preventing cloud.Interface
-// from being initialized in kubelet, kube-controller-manager or kube-api-server
-func IsDeprecatedInternal(name string) bool {
-	for _, provider := range deprecatedCloudProviders {
-		if provider.name == name {
-			return true
-		}
-	}
-
-	return false
-}
-
 // 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.