Merge pull request #128197 from aojea/extract_provider_flags

disable cloud-provider code from kube-controller-manager
This commit is contained in:
Kubernetes Prow Robot 2024-10-24 03:34:59 +01:00 committed by GitHub
commit 1af81c223d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 93 additions and 195 deletions

View File

@ -2224,9 +2224,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
@ -2235,10 +2232,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

@ -36,9 +36,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"
@ -92,26 +89,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{
@ -133,11 +112,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)
@ -182,7 +157,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,
@ -266,27 +241,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 {
@ -300,32 +256,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)