From 07ed564b4e5933c729e5d7ae99a11d84b4fcaedc Mon Sep 17 00:00:00 2001 From: Igor Fominykh Date: Thu, 2 Apr 2026 18:54:54 +0200 Subject: [PATCH] feat: support multiple InitTargets for the same WorkspaceType MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, init-agent documentation stated that only a single InitTarget per WorkspaceType was supported — a second InitTarget referencing the same WorkspaceType would race the first and one set of templates could be silently skipped. This change makes multiple InitTargets per WorkspaceType a first-class feature. The targetcontroller now keys controllers by WorkspaceType reference (path:name) instead of InitTarget UID. Sources from all matching InitTargets are aggregated into a single initialization pass, and the initializer is only removed after every source from every target is applied. - internal/controller/targetcontroller: key by WSType ref, track InitTarget names per key, aggregate provider returns all matching targets, cancel manager only when the last target is deleted. - internal/controller/initcontroller: InitTargetsProvider returns []*InitTarget; reconcile iterates sources from all of them. - Rename InitTargetProvider -> InitTargetsProvider (plural) per review feedback from @xrstf. Fixes #17 Signed-off-by: Igor Fominykh --- cmd/init-agent/main.go | 2 +- docs/content/setup/development.md | 7 +- .../controller/initcontroller/controller.go | 6 +- .../controller/initcontroller/reconciler.go | 60 +++++++-------- .../controller/targetcontroller/controller.go | 73 ++++++++++++++----- 5 files changed, 96 insertions(+), 52 deletions(-) diff --git a/cmd/init-agent/main.go b/cmd/init-agent/main.go index 5b7a01a..49d2255 100644 --- a/cmd/init-agent/main.go +++ b/cmd/init-agent/main.go @@ -118,7 +118,7 @@ func run(ctx context.Context, log *zap.SugaredLogger, opts *Options) error { // wrap this controller creation in a closure to prevent giving all the initcontroller // dependencies to the targetcontroller - newInitController := func(remoteManager mcmanager.Manager, targetProvider initcontroller.InitTargetProvider, initializer kcpcorev1alpha1.LogicalClusterInitializer) error { + newInitController := func(remoteManager mcmanager.Manager, targetProvider initcontroller.InitTargetsProvider, initializer kcpcorev1alpha1.LogicalClusterInitializer) error { return initcontroller.Create(remoteManager, targetProvider, sourceFactory, manifestApplier, initializer, log, numInitWorkers) } diff --git a/docs/content/setup/development.md b/docs/content/setup/development.md index c33f3da..8cbb901 100644 --- a/docs/content/setup/development.md +++ b/docs/content/setup/development.md @@ -21,8 +21,11 @@ that backs a workspace. Initializers are the cluster name + name of the `Workspa removed once from a `LogicalCluster`, it's critical that you use dedicated workspace types for every bootstrapping purpose. - This means there can only be exactly one `InitTarget` in the entire kcp installation that refers - to a `WorkspaceType`. And only a single init-agent may process each `InitTarget`. + Multiple `InitTarget` resources may refer to the same `WorkspaceType`. The init-agent aggregates + the sources from all of them into a single initialization pass and only removes the initializer + after every source from every target has been applied. This lets you compose bootstrapping + behavior from independently authored `InitTargets` (e.g. RBAC, quotas, networking) without + racing on the initializer. Only a single init-agent may process a given `WorkspaceType`. **Do not** use the init-agent with kcp's own `WorkspaceTypes`, as this could interfere with kcp's core functionality. diff --git a/internal/controller/initcontroller/controller.go b/internal/controller/initcontroller/controller.go index 0b3fe41..c09a69c 100644 --- a/internal/controller/initcontroller/controller.go +++ b/internal/controller/initcontroller/controller.go @@ -38,11 +38,11 @@ const ( ControllerName = "initagent-init" ) -type InitTargetProvider func(ctx context.Context) (*initializationv1alpha1.InitTarget, error) +type InitTargetsProvider func(ctx context.Context) ([]*initializationv1alpha1.InitTarget, error) type Reconciler struct { remoteManager mcmanager.Manager - targetProvider InitTargetProvider + targetProvider InitTargetsProvider log *zap.SugaredLogger sourceFactory *source.Factory manifestApplier manifest.Applier @@ -53,7 +53,7 @@ type Reconciler struct { // as this controller is started/stopped by the syncmanager controller instead. func Create( remoteManager mcmanager.Manager, - targetProvider InitTargetProvider, + targetProvider InitTargetsProvider, sourceFactory *source.Factory, manifestApplier manifest.Applier, initializer kcpcorev1alpha1.LogicalClusterInitializer, diff --git a/internal/controller/initcontroller/reconciler.go b/internal/controller/initcontroller/reconciler.go index ce8f08f..e588fa1 100644 --- a/internal/controller/initcontroller/reconciler.go +++ b/internal/controller/initcontroller/reconciler.go @@ -89,38 +89,40 @@ func (r *Reconciler) Reconcile(ctx context.Context, request mcreconcile.Request) } func (r *Reconciler) reconcile(ctx context.Context, logger *zap.SugaredLogger, client ctrlruntimeclient.Client, lc *kcpcorev1alpha1.LogicalCluster) (requeue bool, err error) { - // Dynamically fetch the latest InitTarget, so that we do not have to restart - // (and re-cache) this controller everytime an InitTarget changes. - target, err := r.targetProvider(ctx) + // Dynamically fetch all InitTargets for this WorkspaceType, so that we do not + // have to restart (and re-cache) this controller everytime an InitTarget changes. + targets, err := r.targetProvider(ctx) if err != nil { - return requeue, fmt.Errorf("failed to get InitTarget: %w", err) + return requeue, fmt.Errorf("failed to get InitTargets: %w", err) } - for idx, ref := range target.Spec.Sources { - sourceLog := logger.With("init-target", target.Name, "source-idx", idx) - sourceCtx := log.WithLog(ctx, sourceLog) - - src, err := r.sourceFactory.NewForInitSource(sourceCtx, kcp.ClusterNameFromObject(target), ref) - if err != nil { - return requeue, fmt.Errorf("failed to initialize source #%d: %w", idx, err) - } - - objects, err := src.Manifests(lc) - if err != nil { - return requeue, fmt.Errorf("failed to render source #%d: %w", idx, err) - } - - sourceLog.Debugf("Source yielded %d manifests", len(objects)) - - srcNeedRequeue, err := r.manifestApplier.Apply(sourceCtx, client, objects) - if err != nil { - return requeue, fmt.Errorf("failed to apply source #%d: %w", idx, err) - } - - // If one source cannot be completed at this time, continue with the others. - if srcNeedRequeue { - sourceLog.Debug("Source requires requeuing") - requeue = true + for _, target := range targets { + for idx, ref := range target.Spec.Sources { + sourceLog := logger.With("init-target", target.Name, "source-idx", idx) + sourceCtx := log.WithLog(ctx, sourceLog) + + src, err := r.sourceFactory.NewForInitSource(sourceCtx, kcp.ClusterNameFromObject(target), ref) + if err != nil { + return requeue, fmt.Errorf("failed to initialize source #%d from target %s: %w", idx, target.Name, err) + } + + objects, err := src.Manifests(lc) + if err != nil { + return requeue, fmt.Errorf("failed to render source #%d from target %s: %w", idx, target.Name, err) + } + + sourceLog.Debugf("Source yielded %d manifests", len(objects)) + + srcNeedRequeue, err := r.manifestApplier.Apply(sourceCtx, client, objects) + if err != nil { + return requeue, fmt.Errorf("failed to apply source #%d from target %s: %w", idx, target.Name, err) + } + + // If one source cannot be completed at this time, continue with the others. + if srcNeedRequeue { + sourceLog.Debug("Source requires requeuing") + requeue = true + } } } diff --git a/internal/controller/targetcontroller/controller.go b/internal/controller/targetcontroller/controller.go index 3685c70..e3598f3 100644 --- a/internal/controller/targetcontroller/controller.go +++ b/internal/controller/targetcontroller/controller.go @@ -50,7 +50,7 @@ const ( ControllerName = "initagent-target-controller" ) -type NewInitControllerFunc func(remoteManager mcmanager.Manager, targetProvider initcontroller.InitTargetProvider, initializer kcpcorev1alpha1.LogicalClusterInitializer) error +type NewInitControllerFunc func(remoteManager mcmanager.Manager, targetProvider initcontroller.InitTargetsProvider, initializer kcpcorev1alpha1.LogicalClusterInitializer) error type Reconciler struct { // Choose to break good practice of never storing a context in a struct, @@ -65,8 +65,10 @@ type Reconciler struct { newInitController NewInitControllerFunc // A map of cancel funcs for the multicluster managers - // that we launch for each InitTarget. + // that we launch for each WorkspaceType. ctrlCancels map[string]context.CancelCauseFunc + // Tracks which InitTarget names belong to each WorkspaceType key. + ctrlTargets map[string]map[string]bool ctrlLock sync.Mutex } @@ -86,6 +88,7 @@ func Add( clusterClient: clusterClient, newInitController: newInitController, ctrlCancels: map[string]context.CancelCauseFunc{}, + ctrlTargets: map[string]map[string]bool{}, ctrlLock: sync.Mutex{}, } @@ -125,10 +128,17 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco func (r *Reconciler) ensureInitController(ctx context.Context, log *zap.SugaredLogger, target *initializationv1alpha1.InitTarget) (reconcile.Result, error) { key := getInitTargetKey(target) - // controller already exists + r.ctrlLock.Lock() if _, exists := r.ctrlCancels[key]; exists { + // Controller already exists for this WorkspaceType, just track the target. + if r.ctrlTargets[key] == nil { + r.ctrlTargets[key] = map[string]bool{} + } + r.ctrlTargets[key][target.Name] = true + r.ctrlLock.Unlock() return reconcile.Result{}, nil } + r.ctrlLock.Unlock() ctrlog := log.With("ctrlkey", key, "name", target.Name) @@ -148,7 +158,7 @@ func (r *Reconciler) ensureInitController(ctx context.Context, log *zap.SugaredL return reconcile.Result{}, fmt.Errorf("failed to create multicluster manager: %w", err) } - if err := r.newInitController(mgr, r.newInitTargetProvider(target.Name), initializer); err != nil { + if err := r.newInitController(mgr, r.newInitTargetsProvider(key), initializer); err != nil { return reconcile.Result{}, fmt.Errorf("failed to create init controller: %w", err) } @@ -156,7 +166,13 @@ func (r *Reconciler) ensureInitController(ctx context.Context, log *zap.SugaredL // context, which might get cancelled right after Reconcile() is done. ctrlCtx, ctrlCancel := context.WithCancelCause(r.ctx) + r.ctrlLock.Lock() r.ctrlCancels[key] = ctrlCancel + if r.ctrlTargets[key] == nil { + r.ctrlTargets[key] = map[string]bool{} + } + r.ctrlTargets[key][target.Name] = true + r.ctrlLock.Unlock() // cleanup when the context is done go func() { @@ -166,6 +182,7 @@ func (r *Reconciler) ensureInitController(ctx context.Context, log *zap.SugaredL defer r.ctrlLock.Unlock() delete(r.ctrlCancels, key) + delete(r.ctrlTargets, key) }() // time to start the manager @@ -181,15 +198,22 @@ func (r *Reconciler) ensureInitController(ctx context.Context, log *zap.SugaredL func (r *Reconciler) cleanupController(log *zap.SugaredLogger, target *initializationv1alpha1.InitTarget) error { key := getInitTargetKey(target) - log.Infow("Stopping init controller…", "ctrlkey", key) + log.Infow("Removing InitTarget from controller…", "ctrlkey", key, "target", target.Name) r.ctrlLock.Lock() defer r.ctrlLock.Unlock() - cancel, ok := r.ctrlCancels[key] - if ok { - cancel(errors.New("controller is no longer needed")) - delete(r.ctrlCancels, key) + if targets, ok := r.ctrlTargets[key]; ok { + delete(targets, target.Name) + if len(targets) == 0 { + // Last target removed, stop the controller. + log.Infow("Stopping init controller (last InitTarget removed)…", "ctrlkey", key) + if cancel, ok := r.ctrlCancels[key]; ok { + cancel(errors.New("controller is no longer needed")) + delete(r.ctrlCancels, key) + } + delete(r.ctrlTargets, key) + } } return nil @@ -250,17 +274,32 @@ func (r *Reconciler) createMulticlusterManager(wst *kcptenancyv1alpha1.Workspace return mgr, nil } -func (r *Reconciler) newInitTargetProvider(name string) initcontroller.InitTargetProvider { - return func(ctx context.Context) (*initializationv1alpha1.InitTarget, error) { - target := &initializationv1alpha1.InitTarget{} - if err := r.localClient.Get(ctx, types.NamespacedName{Name: name}, target); err != nil { - return nil, err +func (r *Reconciler) newInitTargetsProvider(wstKey string) initcontroller.InitTargetsProvider { + return func(ctx context.Context) ([]*initializationv1alpha1.InitTarget, error) { + r.ctrlLock.Lock() + targetNames := r.ctrlTargets[wstKey] + names := make([]string, 0, len(targetNames)) + for name := range targetNames { + names = append(names, name) } - - return target, nil + r.ctrlLock.Unlock() + + var targets []*initializationv1alpha1.InitTarget + for _, name := range names { + target := &initializationv1alpha1.InitTarget{} + if err := r.localClient.Get(ctx, types.NamespacedName{Name: name}, target); err != nil { + if ctrlruntimeclient.IgnoreNotFound(err) == nil { + continue // target was deleted + } + return nil, err + } + targets = append(targets, target) + } + return targets, nil } } func getInitTargetKey(target *initializationv1alpha1.InitTarget) string { - return string(target.UID) + ref := target.Spec.WorkspaceTypeReference + return ref.Path + ":" + ref.Name }