Merge pull request #5507 from twz123/more-unique-controller-names

Use unique controller names for Autopilot

Author: twz123

pkg/autopilot/controller/plans/init.go

@@ -46,15 +46,15 @@ func RegisterControllers(ctx context.Context, logger *logrus.Entry, mgr crman.Ma
 
 	if leaderMode {
 		if err := registerNewPlanStateController(logger, mgr, cmdProviders); err != nil {
-			return fmt.Errorf("unable to register 'newplan' controller: %w", err)
+			return fmt.Errorf("unable to register newplan controller: %w", err)
 		}
 
 		if err := registerSchedulableWaitStateController(logger, mgr, cmdProviders); err != nil {
-			return fmt.Errorf("unable to register 'schedulablewait' controller: %w", err)
+			return fmt.Errorf("unable to register schedulablewait controller: %w", err)
 		}
 
 		if err := registerSchedulableStateController(logger, mgr, cmdProviders); err != nil {
-			return fmt.Errorf("unable to register 'schedulable' controller: %w", err)
+			return fmt.Errorf("unable to register schedulable controller: %w", err)
 		}
 	}
 
@@ -108,7 +108,7 @@ func registerSchedulableStateController(logger *logrus.Entry, mgr crman.Manager,
 // controller-runtime.
 func registerPlanStateController(name string, logger *logrus.Entry, mgr crman.Manager, eventFilter crpred.Predicate, handler appc.PlanStateHandler) error {
 	return cr.NewControllerManagedBy(mgr).
-		Named("planstate-" + name).
+		Named("planstate_" + name).
 		For(&apv1beta2.Plan{}).
 		WithEventFilter(eventFilter).
 		Complete(

pkg/autopilot/controller/root_controller.go

@@ -25,10 +25,10 @@ import (
 	apcli "github.com/k0sproject/k0s/pkg/autopilot/client"
 	apconst "github.com/k0sproject/k0s/pkg/autopilot/constant"
 	apdel "github.com/k0sproject/k0s/pkg/autopilot/controller/delegate"
-	applan "github.com/k0sproject/k0s/pkg/autopilot/controller/plans"
+	"github.com/k0sproject/k0s/pkg/autopilot/controller/plans"
 	aproot "github.com/k0sproject/k0s/pkg/autopilot/controller/root"
-	apsig "github.com/k0sproject/k0s/pkg/autopilot/controller/signal"
-	apupdate "github.com/k0sproject/k0s/pkg/autopilot/controller/updates"
+	"github.com/k0sproject/k0s/pkg/autopilot/controller/signal"
+	"github.com/k0sproject/k0s/pkg/autopilot/controller/updates"
 	"github.com/k0sproject/k0s/pkg/kubernetes"
 
 	"github.com/sirupsen/logrus"
@@ -60,6 +60,8 @@ type rootController struct {
 	stopSubHandler         subControllerStopFunc
 	leaseWatcherCreator    leaseWatcherCreatorFunc
 	setupHandler           setupFunc
+
+	initialized bool
 }
 
 var _ aproot.Root = (*rootController)(nil)
@@ -153,15 +155,14 @@ func (c *rootController) startSubControllerRoutine(ctx context.Context, logger *
 	managerOpts := crman.Options{
 		Scheme: scheme,
 		Controller: crconfig.Controller{
-			// Controller-runtime maintains a global checklist of controller
-			// names and does not currently provide a way to unregister the
-			// controller names used by discarded managers. The autopilot
-			// controller and worker components accidentally share some
-			// controller names. So it's necessary to suppress the global name
-			// check because the order in which components are started is not
-			// fully guaranteed for k0s controller nodes running an embedded
-			// worker.
-			SkipNameValidation: ptr.To(true),
+			// If this controller is already initialized, this means that all
+			// controller-runtime controllers have already been successfully
+			// registered to another manager. However, controller-runtime
+			// maintains a global checklist of controller names and doesn't
+			// currently provide a way to unregister names from discarded
+			// managers. So it's necessary to suppress the global name check
+			// whenever things are restarted for reconfiguration.
+			SkipNameValidation: ptr.To(c.initialized),
 		},
 		WebhookServer: crwebhook.NewServer(crwebhook.Options{
 			Port: c.cfg.ManagerPort,
@@ -222,21 +223,24 @@ func (c *rootController) startSubControllerRoutine(ctx context.Context, logger *
 	}
 	clusterID := string(ns.UID)
 
-	if err := apsig.RegisterControllers(ctx, logger, mgr, delegateMap[apdel.ControllerDelegateController], c.cfg.K0sDataDir, clusterID); err != nil {
-		logger.WithError(err).Error("unable to register 'signal' controllers")
+	if err := signal.RegisterControllers(ctx, logger, mgr, delegateMap[apdel.ControllerDelegateController], c.cfg.K0sDataDir, clusterID); err != nil {
+		logger.WithError(err).Error("unable to register signal controllers")
 		return err
 	}
 
-	if err := applan.RegisterControllers(ctx, logger, mgr, c.kubeClientFactory, leaderMode, delegateMap, c.cfg.ExcludeFromPlans); err != nil {
-		logger.WithError(err).Error("unable to register 'plans' controllers")
+	if err := plans.RegisterControllers(ctx, logger, mgr, c.kubeClientFactory, leaderMode, delegateMap, c.cfg.ExcludeFromPlans); err != nil {
+		logger.WithError(err).Error("unable to register plans controllers")
 		return err
 	}
 
-	if err := apupdate.RegisterControllers(ctx, logger, mgr, c.autopilotClientFactory, leaderMode, clusterID); err != nil {
-		logger.WithError(err).Error("unable to register 'update' controllers")
+	if err := updates.RegisterControllers(ctx, logger, mgr, c.autopilotClientFactory, leaderMode, clusterID); err != nil {
+		logger.WithError(err).Error("unable to register updates controllers")
 		return err
 	}
 
+	// All the controller-runtime controllers have been registered.
+	c.initialized = true
+
 	// The controller-runtime start blocks until the context is cancelled.
 	if err := mgr.Start(ctx); err != nil {
 		logger.WithError(err).Error("unable to run controller-runtime manager")

pkg/autopilot/controller/root_worker.go

@@ -24,7 +24,7 @@ import (
 	apcli "github.com/k0sproject/k0s/pkg/autopilot/client"
 	apdel "github.com/k0sproject/k0s/pkg/autopilot/controller/delegate"
 	aproot "github.com/k0sproject/k0s/pkg/autopilot/controller/root"
-	apsig "github.com/k0sproject/k0s/pkg/autopilot/controller/signal"
+	"github.com/k0sproject/k0s/pkg/autopilot/controller/signal"
 
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/wait"
@@ -43,6 +43,8 @@ type rootWorker struct {
 	cfg           aproot.RootConfig
 	log           *logrus.Entry
 	clientFactory apcli.FactoryInterface
+
+	initialized bool
 }
 
 var _ aproot.Root = (*rootWorker)(nil)
@@ -64,15 +66,14 @@ func (w *rootWorker) Run(ctx context.Context) error {
 	managerOpts := crman.Options{
 		Scheme: scheme,
 		Controller: crconfig.Controller{
-			// Controller-runtime maintains a global checklist of controller
-			// names and does not currently provide a way to unregister the
-			// controller names used by discarded managers. The autopilot
-			// controller and worker components accidentally share some
-			// controller names. So it's necessary to suppress the global name
-			// check because the order in which components are started is not
-			// fully guaranteed for k0s controller nodes running an embedded
-			// worker.
-			SkipNameValidation: ptr.To(true),
+			// If this controller is already initialized, this means that all
+			// controller-runtime controllers have already been successfully
+			// registered to another manager. However, controller-runtime
+			// maintains a global checklist of controller names and doesn't
+			// currently provide a way to unregister names from discarded
+			// managers. So it's necessary to suppress the global name check
+			// whenever things retried.
+			SkipNameValidation: ptr.To(w.initialized),
 		},
 		WebhookServer: crwebhook.NewServer(crwebhook.Options{
 			Port: w.cfg.ManagerPort,
@@ -120,9 +121,13 @@ func (w *rootWorker) Run(ctx context.Context) error {
 			return fmt.Errorf("unable to register indexers: %w", err)
 		}
 
-		if err := apsig.RegisterControllers(ctx, logger, mgr, apdel.NodeControllerDelegate(), w.cfg.K0sDataDir, clusterID); err != nil {
-			return fmt.Errorf("unable to register 'controlnodes' controllers: %w", err)
+		if err := signal.RegisterControllers(ctx, logger, mgr, apdel.NodeControllerDelegate(), w.cfg.K0sDataDir, clusterID); err != nil {
+			return fmt.Errorf("unable to register signal controllers: %w", err)
 		}
+
+		// All the controller-runtime controllers have been registered.
+		w.initialized = true
+
 		// The controller-runtime start blocks until the context is cancelled.
 		if err := mgr.Start(ctx); err != nil {
 			return fmt.Errorf("unable to run controller-runtime manager for workers: %w", err)

pkg/autopilot/controller/signal/airgap/download.go

@@ -17,6 +17,7 @@ package airgap
 import (
 	"crypto/sha256"
 	"path"
+	"strings"
 
 	apdel "github.com/k0sproject/k0s/pkg/autopilot/controller/delegate"
 	apsigcomm "github.com/k0sproject/k0s/pkg/autopilot/controller/signal/common"
@@ -43,10 +44,11 @@ var _ apsigcomm.DownloadManifestBuilder = (*downloadManfiestBuilderAirgap)(nil)
 // moved to a `Downloading` status. At this point, it will attempt to download
 // the file provided in the update request.
 func registerDownloading(logger *logrus.Entry, mgr crman.Manager, eventFilter crpred.Predicate, delegate apdel.ControllerDelegate, k0sDataDir string) error {
-	logger.Infof("Registering 'airgap-downloading' reconciler for '%s'", delegate.Name())
+	name := strings.ToLower(delegate.Name()) + "_airgap_downloading"
+	logger.Info("Registering reconciler: ", name)
 
 	return cr.NewControllerManagedBy(mgr).
-		Named("airgap-downloading").
+		Named(name).
 		For(delegate.CreateObject()).
 		WithEventFilter(eventFilter).
 		Complete(

pkg/autopilot/controller/signal/airgap/init.go

@@ -38,15 +38,15 @@ func RegisterControllers(ctx context.Context, logger *logrus.Entry, mgr crman.Ma
 
 	hostname, err := apcomm.FindEffectiveHostname()
 	if err != nil {
-		return fmt.Errorf("unable to determine hostname for controlnode airgap 'signal' reconciler: %w", err)
+		return fmt.Errorf("unable to determine hostname: %w", err)
 	}
 
 	if err := registerSignalController(logger, mgr, SignalControllerEventFilter(hostname, apsigpred.DefaultErrorHandler(logger, "signal")), delegate); err != nil {
-		return fmt.Errorf("unable to register 'airgap-signal' controller: %w", err)
+		return fmt.Errorf("unable to register signal controller: %w", err)
 	}
 
 	if err := registerDownloading(logger, mgr, SignalControllerEventFilter(hostname, apsigpred.DefaultErrorHandler(logger, "airgap download")), delegate, k0sDataDir); err != nil {
-		return fmt.Errorf("unable to register 'airgap-downloading' controller: %w", err)
+		return fmt.Errorf("unable to register downloading controller: %w", err)
 	}
 
 	return nil

pkg/autopilot/controller/signal/airgap/signal.go

@@ -17,6 +17,7 @@ package airgap
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	apcomm "github.com/k0sproject/k0s/pkg/autopilot/common"
 	apdel "github.com/k0sproject/k0s/pkg/autopilot/controller/delegate"
@@ -64,11 +65,12 @@ type signalControllerHandler struct {
 // updates.
 func registerSignalController(logger *logrus.Entry, mgr crman.Manager, eventFilter crpred.Predicate, delegate apdel.ControllerDelegate) error {
 	logr := logger.WithFields(logrus.Fields{"updatetype": "airgap"})
+	name := strings.ToLower(delegate.Name()) + "_airgap_signal"
 
-	logr.Infof("Registering 'airgap-signal' reconciler for '%s'", delegate.Name())
+	logr.Info("Registering reconciler: ", name)
 
 	return cr.NewControllerManagedBy(mgr).
-		Named("airgap-signal").
+		Named(name).
 		For(delegate.CreateObject()).
 		WithEventFilter(eventFilter).
 		Complete(

pkg/autopilot/controller/signal/init.go

@@ -21,8 +21,8 @@ import (
 	"fmt"
 
 	apdel "github.com/k0sproject/k0s/pkg/autopilot/controller/delegate"
-	apsigag "github.com/k0sproject/k0s/pkg/autopilot/controller/signal/airgap"
-	apsigk0s "github.com/k0sproject/k0s/pkg/autopilot/controller/signal/k0s"
+	"github.com/k0sproject/k0s/pkg/autopilot/controller/signal/airgap"
+	"github.com/k0sproject/k0s/pkg/autopilot/controller/signal/k0s"
 
 	"github.com/sirupsen/logrus"
 	crman "sigs.k8s.io/controller-runtime/pkg/manager"
@@ -31,11 +31,11 @@ import (
 // RegisterControllers registers all of the autopilot controllers used by both controller
 // and worker modes.
 func RegisterControllers(ctx context.Context, logger *logrus.Entry, mgr crman.Manager, delegate apdel.ControllerDelegate, k0sDataDir, clusterID string) error {
-	if err := apsigk0s.RegisterControllers(ctx, logger, mgr, delegate, clusterID); err != nil {
+	if err := k0s.RegisterControllers(ctx, logger, mgr, delegate, clusterID); err != nil {
 		return fmt.Errorf("unable to register k0s controllers: %w", err)
 	}
 
-	if err := apsigag.RegisterControllers(ctx, logger, mgr, delegate, k0sDataDir); err != nil {
+	if err := airgap.RegisterControllers(ctx, logger, mgr, delegate, k0sDataDir); err != nil {
 		return fmt.Errorf("unable to register airgap controllers: %w", err)
 	}
 

pkg/autopilot/controller/signal/k0s/apply.go

@@ -21,6 +21,7 @@ import (
 	"os"
 	"path"
 	"path/filepath"
+	"strings"
 
 	apcomm "github.com/k0sproject/k0s/pkg/autopilot/common"
 	apconst "github.com/k0sproject/k0s/pkg/autopilot/constant"
@@ -78,15 +79,16 @@ func registerApplyingUpdate(
 	delegate apdel.ControllerDelegate,
 	k0sBinaryDir string,
 ) error {
-	logger.Infof("Registering 'applying-update' reconciler for '%s'", delegate.Name())
+	name := strings.ToLower(delegate.Name()) + "_k0s_applying_update"
+	logger.Info("Registering reconciler: ", name)
 
 	return cr.NewControllerManagedBy(mgr).
-		Named(delegate.Name() + "-applying-update").
+		Named(name).
 		For(delegate.CreateObject()).
 		WithEventFilter(eventFilter).
 		Complete(
 			&applyingUpdate{
-				log:          logger.WithFields(logrus.Fields{"reconciler": "applying-update", "object": delegate.Name()}),
+				log:          logger.WithFields(logrus.Fields{"reconciler": "k0s-applying-update", "object": delegate.Name()}),
 				client:       mgr.GetClient(),
 				delegate:     delegate,
 				k0sBinaryDir: k0sBinaryDir,

pkg/autopilot/controller/signal/k0s/cordon.go

@@ -18,6 +18,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"strings"
 	"time"
 
 	autopilotv1beta2 "github.com/k0sproject/k0s/pkg/apis/autopilot/v1beta2"
@@ -77,7 +78,8 @@ type cordoning struct {
 // moved to a `Cordoning` status. At this point, it will attempt to cordong & drain
 // the node.
 func registerCordoning(logger *logrus.Entry, mgr crman.Manager, eventFilter crpred.Predicate, delegate apdel.ControllerDelegate) error {
-	logger.Infof("Registering 'cordoning' reconciler for '%s'", delegate.Name())
+	name := strings.ToLower(delegate.Name()) + "_k0s_cordoning"
+	logger.Info("Registering reconciler: ", name)
 
 	// create the clientset
 	clientset, err := kubernetes.NewForConfig(mgr.GetConfig())
@@ -86,12 +88,12 @@ func registerCordoning(logger *logrus.Entry, mgr crman.Manager, eventFilter crpr
 	}
 
 	return cr.NewControllerManagedBy(mgr).
-		Named(delegate.Name() + "-cordoning").
+		Named(name).
 		For(delegate.CreateObject()).
 		WithEventFilter(eventFilter).
 		Complete(
 			&cordoning{
-				log:       logger.WithFields(logrus.Fields{"reconciler": "cordoning", "object": delegate.Name()}),
+				log:       logger.WithFields(logrus.Fields{"reconciler": "k0s-cordoning", "object": delegate.Name()}),
 				client:    mgr.GetClient(),
 				delegate:  delegate,
 				clientset: clientset,

pkg/autopilot/controller/signal/k0s/download.go

@@ -16,6 +16,7 @@ package k0s
 
 import (
 	"crypto/sha256"
+	"strings"
 
 	apcomm "github.com/k0sproject/k0s/pkg/autopilot/common"
 	apconst "github.com/k0sproject/k0s/pkg/autopilot/constant"
@@ -69,10 +70,11 @@ var _ apsigcomm.DownloadManifestBuilder = (*downloadManifestBuilderK0s)(nil)
 // moved to a `Downloading` status. At this point, it will attempt to download
 // the file provided in the update request.
 func registerDownloading(logger *logrus.Entry, mgr crman.Manager, eventFilter crpred.Predicate, delegate apdel.ControllerDelegate, k0sBinaryDir string) error {
-	logger.Infof("Registering k0s 'downloading' reconciler for '%s'", delegate.Name())
+	name := strings.ToLower(delegate.Name()) + "_k0s_downloading"
+	logger.Info("Registering reconciler: ", name)
 
 	return cr.NewControllerManagedBy(mgr).
-		Named(delegate.Name() + "-downloading").
+		Named(name).
 		For(delegate.CreateObject()).
 		WithEventFilter(eventFilter).
 		Complete(

pkg/autopilot/controller/signal/k0s/init.go

@@ -38,12 +38,12 @@ func RegisterControllers(ctx context.Context, logger *logrus.Entry, mgr crman.Ma
 
 	hostname, err := apcomm.FindEffectiveHostname()
 	if err != nil {
-		return fmt.Errorf("unable to determine hostname for controlnode 'signal' reconciler: %w", err)
+		return fmt.Errorf("unable to determine hostname: %w", err)
 	}
 
 	k0sBinaryPath, err := os.Executable()
 	if err != nil {
-		return fmt.Errorf("unable to determine k0s binary path for controlnode 'signal' reconciler: %w", err)
+		return fmt.Errorf("unable to determine k0s binary path: %w", err)
 	}
 	k0sBinaryDir := filepath.Dir(k0sBinaryPath)
 
@@ -54,31 +54,31 @@ func RegisterControllers(ctx context.Context, logger *logrus.Entry, mgr crman.Ma
 	}
 
 	if err := registerSignalController(logger, mgr, signalControllerEventFilter(hostname, apsigpred.DefaultErrorHandler(logger, "k0s signal")), delegate, clusterID, k0sVersionHandler); err != nil {
-		return fmt.Errorf("unable to register k0s 'signal' controller: %w", err)
+		return fmt.Errorf("unable to register signal controller: %w", err)
 	}
 
 	if err := registerDownloading(logger, mgr, downloadEventFilter(hostname, apsigpred.DefaultErrorHandler(logger, "k0s downloading")), delegate, k0sBinaryDir); err != nil {
-		return fmt.Errorf("unable to register k0s 'downloading' controller: %w", err)
+		return fmt.Errorf("unable to register downloading controller: %w", err)
 	}
 
 	if err := registerCordoning(logger, mgr, cordoningEventFilter(hostname, apsigpred.DefaultErrorHandler(logger, "k0s cordoning")), delegate); err != nil {
-		return fmt.Errorf("unable to register k0s 'cordoning' controller: %w", err)
+		return fmt.Errorf("unable to register cordoning controller: %w", err)
 	}
 
 	if err := registerApplyingUpdate(logger, mgr, applyingUpdateEventFilter(hostname, apsigpred.DefaultErrorHandler(logger, "k0s applying-update")), delegate, k0sBinaryDir); err != nil {
-		return fmt.Errorf("unable to register k0s 'applying-update' controller: %w", err)
+		return fmt.Errorf("unable to register applying-update controller: %w", err)
 	}
 
 	if err := registerRestart(logger, mgr, restartEventFilter(hostname, apsigpred.DefaultErrorHandler(logger, "k0s restart")), delegate); err != nil {
-		return fmt.Errorf("unable to register k0s 'restart' controller: %w", err)
+		return fmt.Errorf("unable to register restart controller: %w", err)
 	}
 
 	if err := registerRestarted(logger, mgr, restartedEventFilter(hostname, apsigpred.DefaultErrorHandler(logger, "k0s restarted")), delegate); err != nil {
-		return fmt.Errorf("unable to register k0s 'restarted' controller: %w", err)
+		return fmt.Errorf("unable to register restarted controller: %w", err)
 	}
 
 	if err := registerUncordoning(logger, mgr, unCordoningEventFilter(hostname, apsigpred.DefaultErrorHandler(logger, "k0s uncordoning")), delegate); err != nil {
-		return fmt.Errorf("unable to register k0s 'uncordoning' controller: %w", err)
+		return fmt.Errorf("unable to register uncordoning controller: %w", err)
 	}
 
 	return nil