From 442183a9298e37cb655b4a3f48462ee30e475069 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=A8=E6=9C=B1=20=C2=B7=20Kiki?= Date: Tue, 22 Oct 2024 00:38:52 +0800 Subject: [PATCH] Fix crash on kube manager's service-lb-controller after v1.31.0. (#128182) * Fix crash on kube manager's service-lb-controller after v1.31.0. * Update cmd/kube-controller-manager/app/controllermanager_test.go Co-authored-by: Antonio Ojea --------- Co-authored-by: Antonio Ojea --- .../app/controllermanager_test.go | 26 +++++++++++++++++++ cmd/kube-controller-manager/app/core.go | 13 +++++++++- .../controllers/service/controller.go | 9 ++++--- 3 files changed, 43 insertions(+), 5 deletions(-) diff --git a/cmd/kube-controller-manager/app/controllermanager_test.go b/cmd/kube-controller-manager/app/controllermanager_test.go index 2458d06bd70..be77788b4c9 100644 --- a/cmd/kube-controller-manager/app/controllermanager_test.go +++ b/cmd/kube-controller-manager/app/controllermanager_test.go @@ -219,3 +219,29 @@ func TestTaintEvictionControllerGating(t *testing.T) { }) } } + +func TestNoCloudProviderControllerStarted(t *testing.T) { + _, ctx := ktesting.NewTestContext(t) + ctx, cancel := context.WithCancel(ctx) + defer cancel() + + controllerCtx := ControllerContext{ + Cloud: nil, + LoopMode: IncludeCloudLoops, + } + controllerCtx.ComponentConfig.Generic.Controllers = []string{"*"} + for _, controller := range NewControllerDescriptors() { + if !controller.IsCloudProviderController() { + continue + } + + controllerName := controller.Name() + checker, err := StartController(ctx, controllerCtx, controller, nil) + if err != nil { + t.Errorf("Error starting controller %q: %v", controllerName, err) + } + if checker != nil { + t.Errorf("Controller %q should not be started", controllerName) + } + } +} diff --git a/cmd/kube-controller-manager/app/core.go b/cmd/kube-controller-manager/app/core.go index d0a967edaeb..6d348317ee3 100644 --- a/cmd/kube-controller-manager/app/core.go +++ b/cmd/kube-controller-manager/app/core.go @@ -92,6 +92,12 @@ 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"), @@ -102,7 +108,7 @@ func startServiceLBController(ctx context.Context, controllerContext ControllerC ) if err != nil { // This error shouldn't fail. It lives like this as a legacy. - klog.FromContext(ctx).Error(err, "Failed to start service controller") + logger.Error(err, "Failed to start service controller.") return nil, false, nil } go serviceController.Run(ctx, int(controllerContext.ComponentConfig.ServiceController.ConcurrentServiceSyncs), controllerContext.ControllerManagerMetrics) @@ -261,6 +267,11 @@ 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 diff --git a/staging/src/k8s.io/cloud-provider/controllers/service/controller.go b/staging/src/k8s.io/cloud-provider/controllers/service/controller.go index 4d3d30c8b1d..78376e16e12 100644 --- a/staging/src/k8s.io/cloud-provider/controllers/service/controller.go +++ b/staging/src/k8s.io/cloud-provider/controllers/service/controller.go @@ -108,6 +108,7 @@ func New( featureGate featuregate.FeatureGate, ) (*Controller, error) { registerMetrics() + s := &Controller{ cloud: cloud, kubeClient: kubeClient, @@ -126,6 +127,10 @@ func New( lastSyncedNodes: make(map[string][]*v1.Node), } + if err := s.init(); err != nil { + return nil, err + } + serviceInformer.Informer().AddEventHandlerWithResyncPeriod( cache.ResourceEventHandlerFuncs{ AddFunc: func(cur interface{}) { @@ -180,10 +185,6 @@ func New( nodeSyncPeriod, ) - if err := s.init(); err != nil { - return nil, err - } - return s, nil }