disable cloud-provider code from kube-controller-manager

Since 1.31 the core component cloud provider logic should not exist,
this disables the existing code in the kube-controller-manager that still
expects to work with the cloud-provider logic to avoid having time bombs
in the code base that can break the component.

The code can not be completely removed because this will impact existing
users that may be using some of the flags breaking their deployments, so
this just removes the code that is no longer to be used becuase it
depends on options that no longer are exposed to users.

It also adds validation on the configuration/flag level to ensure that
the --cloud-provider flag can only be set to external or empty.
This commit is contained in:
Antonio Ojea 2024-10-22 14:04:48 +00:00
parent 81ce66f059
commit 8d6769f62c
7 changed files with 93 additions and 195 deletions

View File

@ -2223,9 +2223,6 @@ function start-kube-controller-manager {
if [[ -n "${SERVICE_CLUSTER_IP_RANGE:-}" ]]; then
params+=("--service-cluster-ip-range=${SERVICE_CLUSTER_IP_RANGE}")
fi
if [[ -n "${CONCURRENT_SERVICE_SYNCS:-}" ]]; then
params+=("--concurrent-service-syncs=${CONCURRENT_SERVICE_SYNCS}")
fi
if [[ "${NETWORK_PROVIDER:-}" == "kubenet" ]]; then
params+=("--allocate-node-cidrs=true")
elif [[ -n "${ALLOCATE_NODE_CIDRS:-}" ]]; then
@ -2234,10 +2231,6 @@ function start-kube-controller-manager {
if [[ -n "${TERMINATED_POD_GC_THRESHOLD:-}" ]]; then
params+=("--terminated-pod-gc-threshold=${TERMINATED_POD_GC_THRESHOLD}")
fi
if [[ "${ENABLE_IP_ALIASES:-}" == 'true' ]]; then
params+=("--cidr-allocator-type=${NODE_IPAM_MODE}")
params+=("--configure-cloud-routes=false")
fi
if [[ -n "${FEATURE_GATES:-}" ]]; then
params+=("--feature-gates=${FEATURE_GATES}")
fi

View File

@ -1,72 +0,0 @@
/*
Copyright 2018 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package app
import (
"fmt"
"k8s.io/klog/v2"
"k8s.io/client-go/informers"
cloudprovider "k8s.io/cloud-provider"
)
// createCloudProvider helps consolidate what is needed for cloud providers, we explicitly list the things
// that the cloud providers need as parameters, so we can control
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 err error
loopMode := ExternalLoops
if cloudprovider.IsExternal(cloudProvider) {
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 {
// 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)
}
if cloud != nil && !cloud.HasClusterID() {
if allowUntaggedCloud {
logger.Info("Warning: detected a cluster without a ClusterID. A ClusterID will be required in the future. Please tag your cluster to avoid any future issues")
} else {
return nil, loopMode, fmt.Errorf("no ClusterID Found. A ClusterID is required for the cloud provider to function properly. This check can be bypassed by setting the allow-untagged-cloud option")
}
}
if informerUserCloud, ok := cloud.(cloudprovider.InformerUser); ok {
informerUserCloud.SetInformers(sharedInformers)
}
return cloud, loopMode, err
}

View File

@ -53,7 +53,6 @@ import (
"k8s.io/client-go/tools/leaderelection/resourcelock"
certutil "k8s.io/client-go/util/cert"
"k8s.io/client-go/util/keyutil"
cloudprovider "k8s.io/cloud-provider"
cliflag "k8s.io/component-base/cli/flag"
"k8s.io/component-base/cli/globalflag"
"k8s.io/component-base/configz"
@ -95,16 +94,6 @@ const (
ConfigzName = "kubecontrollermanager.config.k8s.io"
)
// ControllerLoopMode is the kube-controller-manager's mode of running controller loops that are cloud provider dependent
type ControllerLoopMode int
const (
// IncludeCloudLoops means the kube-controller-manager include the controller loops that are cloud provider dependent
IncludeCloudLoops ControllerLoopMode = iota
// ExternalLoops means the kube-controller-manager exclude the controller loops that are cloud provider dependent
ExternalLoops
)
// NewControllerManagerCommand creates a *cobra.Command object with default parameters
func NewControllerManagerCommand() *cobra.Command {
_, _ = utilversion.DefaultComponentGlobalsRegistry.ComponentGlobalsOrRegister(
@ -396,15 +385,6 @@ type ControllerContext struct {
// requested.
RESTMapper *restmapper.DeferredDiscoveryRESTMapper
// Cloud is the cloud provider interface for the controllers to use.
// It must be initialized and ready to use.
Cloud cloudprovider.Interface
// Control for which control loops to be run
// IncludeCloudLoops is for a kube-controller-manager running all loops
// ExternalLoops is for a kube-controller-manager running with a cloud-controller-manager
LoopMode ControllerLoopMode
// InformersStarted is closed after all of the controllers have been initialized and are running. After this point it is safe,
// for an individual controller to start the shared informers. Before it is closed, they should not.
InformersStarted chan struct{}
@ -644,20 +624,12 @@ func CreateControllerContext(ctx context.Context, s *config.CompletedConfig, roo
restMapper.Reset()
}, 30*time.Second, ctx.Done())
cloud, loopMode, err := createCloudProvider(klog.FromContext(ctx), s.ComponentConfig.KubeCloudShared.CloudProvider.Name, s.ComponentConfig.KubeCloudShared.ExternalCloudVolumePlugin,
s.ComponentConfig.KubeCloudShared.CloudProvider.CloudConfigFile, s.ComponentConfig.KubeCloudShared.AllowUntaggedCloud, sharedInformers)
if err != nil {
return ControllerContext{}, err
}
controllerContext := ControllerContext{
ClientBuilder: clientBuilder,
InformerFactory: sharedInformers,
ObjectOrMetadataInformerFactory: informerfactory.NewInformerFactory(sharedInformers, metadataInformers),
ComponentConfig: s.ComponentConfig,
RESTMapper: restMapper,
Cloud: cloud,
LoopMode: loopMode,
InformersStarted: make(chan struct{}),
ResyncPeriod: ResyncPeriod(s),
ControllerManagerMetrics: controllersmetrics.NewControllerManagerMetrics("kube-controller-manager"),
@ -702,12 +674,6 @@ func StartControllers(ctx context.Context, controllerCtx ControllerContext, cont
}
}
// Initialize the cloud provider with a reference to the clientBuilder only after token controller
// has started in case the cloud provider uses the client builder.
if controllerCtx.Cloud != nil {
controllerCtx.Cloud.Initialize(controllerCtx.ClientBuilder, ctx.Done())
}
// Each controller is passed a context where the logger has the name of
// the controller set through WithName. That name then becomes the prefix of
// of all log messages emitted by that controller.
@ -751,8 +717,8 @@ func StartController(ctx context.Context, controllerCtx ControllerContext, contr
}
}
if controllerDescriptor.IsCloudProviderController() && controllerCtx.LoopMode != IncludeCloudLoops {
logger.Info("Skipping a cloud provider controller", "controller", controllerName, "loopMode", controllerCtx.LoopMode)
if controllerDescriptor.IsCloudProviderController() {
logger.Info("Skipping a cloud provider controller", "controller", controllerName)
return nil, nil
}

View File

@ -225,10 +225,7 @@ func TestNoCloudProviderControllerStarted(t *testing.T) {
ctx, cancel := context.WithCancel(ctx)
defer cancel()
controllerCtx := ControllerContext{
Cloud: nil,
LoopMode: IncludeCloudLoops,
}
controllerCtx := ControllerContext{}
controllerCtx.ComponentConfig.Generic.Controllers = []string{"*"}
for _, controller := range NewControllerDescriptors() {
if !controller.IsCloudProviderController() {

View File

@ -37,9 +37,6 @@ import (
clientset "k8s.io/client-go/kubernetes"
"k8s.io/client-go/metadata"
restclient "k8s.io/client-go/rest"
cloudnodelifecyclecontroller "k8s.io/cloud-provider/controllers/nodelifecycle"
routecontroller "k8s.io/cloud-provider/controllers/route"
servicecontroller "k8s.io/cloud-provider/controllers/service"
cpnames "k8s.io/cloud-provider/names"
"k8s.io/component-base/featuregate"
"k8s.io/controller-manager/controller"
@ -93,26 +90,8 @@ func newServiceLBControllerDescriptor() *ControllerDescriptor {
func startServiceLBController(ctx context.Context, controllerContext ControllerContext, controllerName string) (controller.Interface, bool, error) {
logger := klog.FromContext(ctx)
if controllerContext.Cloud == nil {
logger.Info("Warning: service-controller is set, but no cloud provider specified. Will not configure service controller.")
return nil, false, nil
}
serviceController, err := servicecontroller.New(
controllerContext.Cloud,
controllerContext.ClientBuilder.ClientOrDie("service-controller"),
controllerContext.InformerFactory.Core().V1().Services(),
controllerContext.InformerFactory.Core().V1().Nodes(),
controllerContext.ComponentConfig.KubeCloudShared.ClusterName,
utilfeature.DefaultFeatureGate,
)
if err != nil {
// This error shouldn't fail. It lives like this as a legacy.
logger.Error(err, "Failed to start service controller.")
return nil, false, nil
}
go serviceController.Run(ctx, int(controllerContext.ComponentConfig.ServiceController.ConcurrentServiceSyncs), controllerContext.ControllerManagerMetrics)
return nil, true, nil
logger.Info("Warning: service-controller is set, but no cloud provider functionality is available in kube-controller-manger (KEP-2395). Will not configure service controller.")
return nil, false, nil
}
func newNodeIpamControllerDescriptor() *ControllerDescriptor {
return &ControllerDescriptor{
@ -134,11 +113,7 @@ func startNodeIpamController(ctx context.Context, controllerContext ControllerCo
if controllerContext.ComponentConfig.KubeCloudShared.CIDRAllocatorType == string(ipam.CloudAllocatorType) {
// Cannot run cloud ipam controller if cloud provider is nil (--cloud-provider not set or set to 'external')
if controllerContext.Cloud == nil {
return nil, false, errors.New("--cidr-allocator-type is set to 'CloudAllocator' but cloud provider is not configured")
}
// As part of the removal of all the cloud providers from kubernetes, this support will be removed as well
klog.Warningf("DEPRECATED: 'CloudAllocator' bas been deprecated and will be removed in a future release.")
return nil, false, errors.New("--cidr-allocator-type is set to 'CloudAllocator' but cloud provider is not configured")
}
clusterCIDRs, err := validateCIDRs(controllerContext.ComponentConfig.KubeCloudShared.ClusterCIDR)
@ -183,7 +158,7 @@ func startNodeIpamController(ctx context.Context, controllerContext ControllerCo
nodeIpamController, err := nodeipamcontroller.NewNodeIpamController(
ctx,
controllerContext.InformerFactory.Core().V1().Nodes(),
controllerContext.Cloud,
nil, // no cloud provider on kube-controller-manager since v1.31 (KEP-2395)
controllerContext.ClientBuilder.ClientOrDie("node-controller"),
clusterCIDRs,
serviceCIDR,
@ -267,27 +242,8 @@ func newCloudNodeLifecycleControllerDescriptor() *ControllerDescriptor {
func startCloudNodeLifecycleController(ctx context.Context, controllerContext ControllerContext, controllerName string) (controller.Interface, bool, error) {
logger := klog.FromContext(ctx)
if controllerContext.Cloud == nil {
logger.Info("Warning: node-controller is set, but no cloud provider specified. Will not configure node lifecyle controller.")
return nil, false, nil
}
cloudNodeLifecycleController, err := cloudnodelifecyclecontroller.NewCloudNodeLifecycleController(
controllerContext.InformerFactory.Core().V1().Nodes(),
// cloud node lifecycle controller uses existing cluster role from node-controller
controllerContext.ClientBuilder.ClientOrDie("node-controller"),
controllerContext.Cloud,
controllerContext.ComponentConfig.KubeCloudShared.NodeMonitorPeriod.Duration,
)
if err != nil {
// the controller manager should continue to run if the "Instances" interface is not
// supported, though it's unlikely for a cloud provider to not support it
logger.Error(err, "Failed to start cloud node lifecycle controller")
return nil, false, nil
}
go cloudNodeLifecycleController.Run(ctx, controllerContext.ControllerManagerMetrics)
return nil, true, nil
logger.Info("Warning: node-controller is set, but no cloud provider functionality is available in kube-controller-manger (KEP-2395). Will not configure node lifecyle controller.")
return nil, false, nil
}
func newNodeRouteControllerDescriptor() *ControllerDescriptor {
@ -301,32 +257,8 @@ func newNodeRouteControllerDescriptor() *ControllerDescriptor {
func startNodeRouteController(ctx context.Context, controllerContext ControllerContext, controllerName string) (controller.Interface, bool, error) {
logger := klog.FromContext(ctx)
if !controllerContext.ComponentConfig.KubeCloudShared.AllocateNodeCIDRs || !controllerContext.ComponentConfig.KubeCloudShared.ConfigureCloudRoutes {
logger.Info("Will not configure cloud provider routes for allocate-node-cidrs", "CIDRs", controllerContext.ComponentConfig.KubeCloudShared.AllocateNodeCIDRs, "routes", controllerContext.ComponentConfig.KubeCloudShared.ConfigureCloudRoutes)
return nil, false, nil
}
if controllerContext.Cloud == nil {
logger.Info("Warning: configure-cloud-routes is set, but no cloud provider specified. Will not configure cloud provider routes.")
return nil, false, nil
}
routes, ok := controllerContext.Cloud.Routes()
if !ok {
logger.Info("Warning: configure-cloud-routes is set, but cloud provider does not support routes. Will not configure cloud provider routes.")
return nil, false, nil
}
clusterCIDRs, err := validateCIDRs(controllerContext.ComponentConfig.KubeCloudShared.ClusterCIDR)
if err != nil {
return nil, false, err
}
routeController := routecontroller.New(routes,
controllerContext.ClientBuilder.ClientOrDie("route-controller"),
controllerContext.InformerFactory.Core().V1().Nodes(),
controllerContext.ComponentConfig.KubeCloudShared.ClusterName,
clusterCIDRs)
go routeController.Run(ctx, controllerContext.ComponentConfig.KubeCloudShared.RouteReconciliationPeriod.Duration, controllerContext.ControllerManagerMetrics)
return nil, true, nil
logger.Info("Warning: configure-cloud-routes is set, but no cloud provider functionality is available in kube-controller-manger (KEP-2395). Will not configure cloud provider routes.")
return nil, false, nil
}
func newPersistentVolumeBinderControllerDescriptor() *ControllerDescriptor {

View File

@ -33,6 +33,7 @@ import (
restclient "k8s.io/client-go/rest"
"k8s.io/client-go/tools/clientcmd"
"k8s.io/client-go/tools/record"
cloudprovider "k8s.io/cloud-provider"
cpnames "k8s.io/cloud-provider/names"
cpoptions "k8s.io/cloud-provider/options"
cliflag "k8s.io/component-base/cli/flag"
@ -50,6 +51,7 @@ import (
kubectrlmgrconfigscheme "k8s.io/kubernetes/pkg/controller/apis/config/scheme"
"k8s.io/kubernetes/pkg/controller/garbagecollector"
garbagecollectorconfig "k8s.io/kubernetes/pkg/controller/garbagecollector/config"
"k8s.io/kubernetes/pkg/controller/nodeipam/ipam"
netutils "k8s.io/utils/net"
// add the kubernetes feature gates
@ -452,6 +454,16 @@ func (s *KubeControllerManagerOptions) Validate(allControllers []string, disable
errs = append(errs, s.Metrics.Validate()...)
errs = append(errs, utilversion.ValidateKubeEffectiveVersion(s.ComponentGlobalsRegistry.EffectiveVersionFor(utilversion.DefaultKubeComponent)))
// in-tree cloud providers are disabled since v1.31 (KEP-2395)
if len(s.KubeCloudShared.CloudProvider.Name) > 0 && !cloudprovider.IsExternal(s.KubeCloudShared.CloudProvider.Name) {
cloudprovider.DisableWarningForProvider(s.KubeCloudShared.CloudProvider.Name)
errs = append(errs, cloudprovider.ErrorForDisabledProvider(s.KubeCloudShared.CloudProvider.Name))
}
if len(s.KubeCloudShared.CIDRAllocatorType) > 0 && s.KubeCloudShared.CIDRAllocatorType != string(ipam.RangeAllocatorType) {
errs = append(errs, fmt.Errorf("built-in cloud providers are disabled. The ipam %s is not available", s.KubeCloudShared.CIDRAllocatorType))
}
// TODO: validate component config, master and kubeconfig
return utilerrors.NewAggregate(errs)

View File

@ -461,6 +461,76 @@ func TestAddFlags(t *testing.T) {
}
}
func TestValidateFlags(t *testing.T) {
testcases := []struct {
name string
flags []string // not a good place to test flagParse error
wantErr bool // this won't apply to flagParse, it only apply to KubeControllerManagerOptions.Validate
}{
{
name: "empty flags",
flags: []string{},
wantErr: false,
},
{
name: "cloud provider empty flag",
flags: []string{
"--cloud-provider", "",
},
wantErr: false,
},
{
name: "cloud provider set but not external",
flags: []string{
"--cloud-provider=gce",
},
wantErr: true,
},
{
name: "cloud provider set to external",
flags: []string{
"--cloud-provider=external",
},
wantErr: false,
},
{
name: "nodeipam to cloudAllocator",
flags: []string{
"--cidr-allocator-type=CloudAllocator",
},
wantErr: true,
},
{
name: "nodeipam to rangeAllocator",
flags: []string{
"--cidr-allocator-type=RangeAllocator",
},
wantErr: false,
},
{
name: "nodeipam to IPAMFromCloud",
flags: []string{
"--cidr-allocator-type=IPAMFromCloud",
},
wantErr: true,
},
}
for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
fs, s := setupControllerManagerFlagSet(t)
err := fs.Parse(tc.flags)
checkTestError(t, err, false, "")
err = s.Validate([]string{""}, []string{""}, nil)
if !tc.wantErr && err != nil {
t.Fatal(fmt.Errorf("expected no error, got %w", err))
} else if tc.wantErr && err == nil {
t.Fatal("expected error, got nil")
}
})
}
}
func TestApplyTo(t *testing.T) {
fs, s := setupControllerManagerFlagSet(t)