fix(main): Make Ctrl+C actually work: Do not use context.TODO in setup controllers

main.go

@@ -1,11 +1,13 @@
 // Copyright 2021 VMware, Inc. All Rights Reserved.
+// Copyright 2026 Platform9, Inc. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package main
 
 import (
 	"context"
 	"flag"
+	"fmt"
 	"os"
 	"time"
 
@@ -50,6 +52,19 @@ var (
 	byohostAgentHeartbeatTimeout time.Duration
 )
 
+// controllerOptions is the flag- and environment-derived configuration
+// setupControllers needs.
+type controllerOptions struct {
+	heartbeatTimeout time.Duration
+
+	// Leaves overlay and br_netfilter loaded during uninstall.
+	skipKernelModuleCleanup bool
+
+	// Nil disables the ByoAdmission controller, leaving CSRs to be approved
+	// manually.
+	csrClientSet clientset.Interface
+}
+
 func init() {
 	klog.InitFlags(nil)
 	// clear any discard loggers set by dependecies
@@ -72,136 +87,148 @@ func setFlags() {
 	flag.Parse()
 }
 
-// TODO:
-// main() will have lots of 'if', '&&' and '||' which will
-// increase its cyclometric complexity. Ignoring it for now.
-
-// nolint: funlen, gocyclo
-func main() {
-	setFlags()
-	ctrl.SetLogger(klogr.New()) //nolint: staticcheck // klogr predates the textlogger migration; swapping formats is a separate, deliberate change outside this dependency bump's scope
-
-	mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
-		Scheme:                 scheme,
-		Metrics:                metricsserver.Options{BindAddress: metricsAddr},
-		WebhookServer:          webhook.NewServer(webhook.Options{Port: 9443}),
-		HealthProbeBindAddress: probeAddr,
-		LeaderElection:         enableLeaderElection,
-		LeaderElectionID:       "controller-leader-election-caph",
-	})
-	if err != nil {
-		setupLog.Error(err, "unable to start manager")
-		os.Exit(1)
-	}
-
-	// remote.ClusterCacheTracker/ClusterCacheReconciler are deprecated in favor of
-	// controllers/clustercache as of CAPI v1.10, but still fully supported at v1.10.10 (the
-	// version this repo targets). Migrating to clustercache.SetupWithManager changes the
-	// underlying workload-cluster connection/cache lifecycle and deserves its own dedicated
-	// verification pass rather than riding along with this dependency bump.
+// setupControllers registers every controller and webhook with mgr. ctx must be
+// the context that also stops mgr: the controllers built here capture it for
+// their watches and map functions, so one with a different lifetime leaves
+// those watches running past the manager, or dead before it.
+func setupControllers(ctx context.Context, mgr ctrl.Manager, opts controllerOptions) error {
 	remoteLogger := ctrl.Log.WithName("remote").WithName("ClusterCacheTracker")
 	options := remote.ClusterCacheTrackerOptions{Log: &remoteLogger} //nolint: staticcheck
 	tracker, err := remote.NewClusterCacheTracker(mgr, options)      //nolint: staticcheck
 	if err != nil {
-		setupLog.Error(err, "unable to create cluster cache tracker")
-		os.Exit(1)
+		return fmt.Errorf("create cluster cache tracker: %w", err)
 	}
 
-	if err = (&remote.ClusterCacheReconciler{ //nolint: staticcheck
+	if err := (&remote.ClusterCacheReconciler{ //nolint: staticcheck
 		Client:  mgr.GetClient(),
 		Tracker: tracker,
-	}).SetupWithManager(context.TODO(), mgr, concurrency(0)); err != nil {
-		setupLog.Error(err, "unable to create controller", "controller", "ClusterCacheReconciler")
-		os.Exit(1)
+	}).SetupWithManager(ctx, mgr, concurrency(0)); err != nil {
+		return fmt.Errorf("create ClusterCacheReconciler controller: %w", err)
 	}
 
-	if err = (&byohcontrollers.ByoMachineReconciler{
+	if err := (&byohcontrollers.ByoMachineReconciler{
 		Client:   mgr.GetClient(),
 		Scheme:   mgr.GetScheme(),
 		Tracker:  tracker,
 		Recorder: mgr.GetEventRecorderFor("byomachine-controller"),
-	}).SetupWithManager(context.TODO(), mgr); err != nil {
-		setupLog.Error(err, "unable to create controller", "controller", "ByoMachine")
-		os.Exit(1)
+	}).SetupWithManager(ctx, mgr); err != nil {
+		return fmt.Errorf("create ByoMachine controller: %w", err)
 	}
 
-	if err = (&byohcontrollers.ByoHostReconciler{
+	if err := (&byohcontrollers.ByoHostReconciler{
 		Client:                 mgr.GetClient(),
 		Scheme:                 mgr.GetScheme(),
-		HeartbeatTimeoutPeriod: byohostAgentHeartbeatTimeout,
+		HeartbeatTimeoutPeriod: opts.heartbeatTimeout,
 		Recorder:               mgr.GetEventRecorderFor("byohost-controller"),
 	}).SetupWithManager(mgr); err != nil {
-		setupLog.Error(err, "unable to create controller", "controller", "ByoHost")
-		os.Exit(1)
+		return fmt.Errorf("create ByoHost controller: %w", err)
 	}
-	if err = (&byohcontrollers.ByoHostAgentUpgradeReconciler{
+
+	if err := (&byohcontrollers.ByoHostAgentUpgradeReconciler{
 		Client:   mgr.GetClient(),
 		Scheme:   mgr.GetScheme(),
 		Recorder: mgr.GetEventRecorderFor("byohostagentupgrade-controller"),
 	}).SetupWithManager(mgr); err != nil {
-		setupLog.Error(err, "unable to create controller", "controller", "ByoHostAgentUpgrade")
-		os.Exit(1)
+		return fmt.Errorf("create ByoHostAgentUpgrade controller: %w", err)
 	}
-	if err = (&byohcontrollers.ByoMachineTemplateReconciler{
+
+	if err := (&byohcontrollers.ByoMachineTemplateReconciler{
 		Client: mgr.GetClient(),
 		Scheme: mgr.GetScheme(),
 	}).SetupWithManager(mgr); err != nil {
-		setupLog.Error(err, "unable to create controller", "controller", "ByoMachineTemplate")
-		os.Exit(1)
+		return fmt.Errorf("create ByoMachineTemplate controller: %w", err)
 	}
-	if err = (&byohcontrollers.ByoClusterReconciler{
+
+	if err := (&byohcontrollers.ByoClusterReconciler{
 		Client: mgr.GetClient(),
 		Scheme: mgr.GetScheme(),
-	}).SetupWithManager(context.TODO(), mgr); err != nil {
-		setupLog.Error(err, "unable to create controller", "controller", "ByoCluster")
-		os.Exit(1)
+	}).SetupWithManager(ctx, mgr); err != nil {
+		return fmt.Errorf("create ByoCluster controller: %w", err)
 	}
 
-	// Set 'MANUAL_CSR_APPROVAL=enable' to disable ByoAdmission controller. Now CSRs should be approved manually.
-	if os.Getenv("MANUAL_CSR_APPROVAL") != "enable" {
-		if err = (&byohcontrollers.ByoAdmissionReconciler{
-			ClientSet: clientset.NewForConfigOrDie(ctrl.GetConfigOrDie()),
+	if opts.csrClientSet != nil {
+		if err := (&byohcontrollers.ByoAdmissionReconciler{
+			ClientSet: opts.csrClientSet,
 		}).SetupWithManager(mgr); err != nil {
-			setupLog.Error(err, "unable to create controller", "controller", "ByoAdmission")
-			os.Exit(1)
+			return fmt.Errorf("create ByoAdmission controller: %w", err)
 		}
 	}
-	// Set 'BYOH_SKIP_KERNEL_MODULE_CLEANUP=enable' to skip unloading overlay/br_netfilter kernel
-	// modules during uninstall. Real BYO hosts own their kernel and must unload these modules;
-	// e2e's containerized hosts share Docker's kernel, so unloading them there breaks Docker's
-	// own bridge networking and hangs cluster deletion.
-	//
-	// Uses 'enable'/'disable' (matching MANUAL_CSR_APPROVAL above), not 'true'/'false': kustomize
-	// re-serializes manager.yaml and drops the quotes around "${VAR:=default}", so an unquoted
-	// true/false would parse as a YAML bool instead of a string, breaking clusterctl's conversion
-	// of the rendered Deployment's env value back into a typed corev1.EnvVar.
-	skipKernelModuleCleanup := os.Getenv("BYOH_SKIP_KERNEL_MODULE_CLEANUP") == "enable"
-	if err = (&byohcontrollers.K8sInstallerConfigReconciler{
+
+	if err := (&byohcontrollers.K8sInstallerConfigReconciler{
 		Client:                  mgr.GetClient(),
 		Scheme:                  mgr.GetScheme(),
-		SkipKernelModuleCleanup: skipKernelModuleCleanup,
+		SkipKernelModuleCleanup: opts.skipKernelModuleCleanup,
 	}).SetupWithManager(mgr); err != nil {
-		setupLog.Error(err, "unable to create controller", "controller", "K8sInstallerConfig")
-		os.Exit(1)
+		return fmt.Errorf("create K8sInstallerConfig controller: %w", err)
 	}
 
 	mgr.GetWebhookServer().Register("/validate-infrastructure-cluster-x-k8s-io-v1beta1-byohost", &webhook.Admission{Handler: &infrastructurev1beta1.ByoHostValidator{
 		Client:  mgr.GetClient(),
 		Decoder: admission.NewDecoder(mgr.GetScheme()),
 	}})
 
-	if err = (&byohcontrollers.BootstrapKubeconfigReconciler{
+	if err := (&byohcontrollers.BootstrapKubeconfigReconciler{
 		Client: mgr.GetClient(),
 		Scheme: mgr.GetScheme(),
 	}).SetupWithManager(mgr); err != nil {
-		setupLog.Error(err, "unable to create controller", "controller", "BootstrapKubeconfig")
+		return fmt.Errorf("create BootstrapKubeconfig controller: %w", err)
+	}
+
+	if err := (&infrastructurev1beta1.BootstrapKubeconfig{}).SetupWebhookWithManager(mgr); err != nil {
+		return fmt.Errorf("create BootstrapKubeconfig webhook: %w", err)
+	}
+	//+kubebuilder:scaffold:builder
+
+	return nil
+}
+
+func concurrency(c int) controller.Options {
+	return controller.Options{MaxConcurrentReconciles: c}
+}
+
+func main() {
+	setFlags()
+	ctrl.SetLogger(klogr.New()) //nolint: staticcheck // klogr predates the textlogger migration;
+
+	// SetupSignalHandler panics on a second call, so one context is built
+	// here for both the controller registrations and mgr.Start.
+	ctx := ctrl.SetupSignalHandler()
+
+	mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
+		Scheme:                 scheme,
+		Metrics:                metricsserver.Options{BindAddress: metricsAddr},
+		WebhookServer:          webhook.NewServer(webhook.Options{Port: 9443}),
+		HealthProbeBindAddress: probeAddr,
+		LeaderElection:         enableLeaderElection,
+		LeaderElectionID:       "controller-leader-election-caph",
+	})
+	if err != nil {
+		setupLog.Error(err, "unable to start manager")
 		os.Exit(1)
 	}
-	if err = (&infrastructurev1beta1.BootstrapKubeconfig{}).SetupWebhookWithManager(mgr); err != nil {
-		setupLog.Error(err, "unable to create webhook", "webhook", "BootstrapKubeconfig")
+
+	opts := controllerOptions{
+		heartbeatTimeout: byohostAgentHeartbeatTimeout,
+		// Set 'BYOH_SKIP_KERNEL_MODULE_CLEANUP=enable' to skip unloading overlay/br_netfilter kernel
+		// modules during uninstall. Real BYO hosts own their kernel and must unload these modules;
+		// e2e's containerized hosts share Docker's kernel, so unloading them there breaks Docker's
+		// own bridge networking and hangs cluster deletion.
+		//
+		// Uses 'enable'/'disable' (matching MANUAL_CSR_APPROVAL below), not 'true'/'false': kustomize
+		// re-serializes manager.yaml and drops the quotes around "${VAR:=default}", so an unquoted
+		// true/false would parse as a YAML bool instead of a string, breaking clusterctl's conversion
+		// of the rendered Deployment's env value back into a typed corev1.EnvVar.
+		skipKernelModuleCleanup: os.Getenv("BYOH_SKIP_KERNEL_MODULE_CLEANUP") == "enable",
+	}
+	// Set 'MANUAL_CSR_APPROVAL=enable' to disable ByoAdmission controller. Now CSRs should be approved manually.
+	if os.Getenv("MANUAL_CSR_APPROVAL") != "enable" {
+		opts.csrClientSet = clientset.NewForConfigOrDie(ctrl.GetConfigOrDie())
+	}
+
+	if err := setupControllers(ctx, mgr, opts); err != nil {
+		setupLog.Error(err, "unable to set up controllers")
 		os.Exit(1)
 	}
-	//+kubebuilder:scaffold:builder
 
 	if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil {
 		setupLog.Error(err, "unable to set up health check")
@@ -213,12 +240,8 @@ func main() {
 	}
 
 	setupLog.Info("starting manager")
-	if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil {
+	if err := mgr.Start(ctx); err != nil {
 		setupLog.Error(err, "problem running manager")
 		os.Exit(1)
 	}
 }
-
-func concurrency(c int) controller.Options {
-	return controller.Options{MaxConcurrentReconciles: c}
-}