From eb207cb84507ce4889dfe334f5974bed9288dfc3 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Tue, 26 Mar 2024 11:14:45 +0100 Subject: [PATCH 1/5] Add designs/multi-cluster.md Signed-off-by: Dr. Stefan Schimanski --- designs/multi-cluster.md | 281 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 281 insertions(+) create mode 100644 designs/multi-cluster.md diff --git a/designs/multi-cluster.md b/designs/multi-cluster.md new file mode 100644 index 0000000000..5d630a1901 --- /dev/null +++ b/designs/multi-cluster.md @@ -0,0 +1,281 @@ +# Multi-Cluster Support +Author: @sttts +Initial implementation: @vincepri + +Last Updated on: 03/26/2024 + +## Table of Contents + + +- [Multi-Cluster Support](#multi-cluster-support) + - [Table of Contents](#table-of-contents) + - [Summary](#summary) + - [Motivation](#motivation) + - [Goals](#goals) + - [Examples](#examples) + - [Non-Goals/Future Work](#non-goalsfuture-work) + - [Proposal](#proposal) + - [Multi-Cluster-Compatible Reconcilers](#multi-cluster-compatible-reconcilers) + - [User Stories](#user-stories) + - [Controller Author with no interest in multi-cluster wanting to old behaviour.](#controller-author-with-no-interest-in-multi-cluster-wanting-to-old-behaviour) + - [Multi-Cluster Integrator wanting to support cluster managers like Cluster-API or kind](#multi-cluster-integrator-wanting-to-support-cluster-managers-like-cluster-api-or-kind) + - [Multi-Cluster Integrator wanting to support apiservers with logical cluster (like kcp)](#multi-cluster-integrator-wanting-to-support-apiservers-with-logical-cluster-like-kcp) + - [Controller Author without self-interest in multi-cluster, but open for adoption in multi-cluster setups](#controller-author-without-self-interest-in-multi-cluster-but-open-for-adoption-in-multi-cluster-setups) + - [Controller Author who wants to support certain multi-cluster setups](#controller-author-who-wants-to-support-certain-multi-cluster-setups) + - [Risks and Mitigations](#risks-and-mitigations) + - [Alternatives](#alternatives) + - [Implementation History](#implementation-history) + + + +## Summary + +Controller-runtime today allows to write controllers against one cluster only. +Multi-cluster use-cases require the creation of multiple managers and/or cluster +objects. This proposal is about adding native support for multi-cluster use-cases +to controller-runtime. + +## Motivation + +This change is important because: +- multi-cluster use-cases are becoming more and more common, compare projects + like Kamarda, Crossplane or kcp. They all need to write (controller-runtime) + controllers that operate on multiple clusters. +- writing controllers for upper systems in a **portable** way is hard today. + Consequently, there is no multi-cluster controller ecosystem, but could and + should be. +- kcp maintains a [controller-runtime fork with multi-cluster support](https://github.com/kcp-dev/controller-runtime) + because adding support on-top leads to an inefficient controller design, and + even more important leads of divergence in the ecosystem. + +### Goals + +- Provide a way to natively write controllers that + 1. (UNIFORM MULTI-CLUSTER CONTROLLER) operate on multiple clusters in a uniform way, + i.e. reconciling the same resources on multiple clusters, **optionally** + - sourcing information from one central hub cluster + - sourcing information cross-cluster. + + Example: distributed `ReplicaSet` controller, reconciling `ReplicaSets` on multiple clusters. + 2. (AGGREGATING MULTI-CLUSTER CONTROLLER) operate on one central hub cluster aggregating information from multiple clusters. + + Example: distributed `Deployment` controller, aggregating `ReplicaSets` back into the `Deployment` object. +- Allow clusters to dynamically join and leave the set of clusters a controller operates on. +- Allow event sources to be cross-cluster: + 1. Multi-cluster events that trigger reconciliation in the one central hub cluster. + 2. Central hub cluster events to trigger reconciliation on multiple clusters. +- Allow (informer) indexes that span multiple clusters. +- Allow logical clusters where a set of clusters is actually backed by one physical informer store. +- Allow 3rd-parties to plug in their multi-cluster adapter (in source code) into + an existing multi-cluster-compatible code-base. +- Minimize the amount of changes to make a controller-runtime controller + multi-cluster-compatible, in a way that 3rd-party projects have no reason to + object these kind of changes. + +Here we call a controller to be multi-cluster-compatible if the reconcilers get +reconcile requests in cluster `X` and do all reconciliation in cluster `X`. This +is less than being multi-cluster-aware, where reconcilers implement cross-cluster +logic. + +### Examples + +- Run a controller-runtime controller against a kubeconfig with arbitrary many contexts, all being reconciled. +- Run a controller-runtime controller against cluster-managers like kind, Cluster-API, Open-Cluster-Manager or Hypershift. +- Run a controller-runtime controller against a kcp shard with a wildcard watch. + +### Non-Goals/Future Work + +- Ship integration for different multi-cluster setups. This should become + out-of-tree subprojects that can individually evolve and vendor'ed by controller authors. +- Make controller-runtime controllers "binary pluggable". +- Manage one manager per cluster. +- Manage one controller per cluster with dedicated workqueues. + +## Proposal + +The `ctrl.Manager` _SHOULD_ be extended to get an optional `cluster.Provider` via +`ctrl.Options` implementing + +```golang +// pkg/cluster +type Provider interface { + Get(ctx context.Context, clusterName string, opts ...Option) (Cluster, error) + List(ctx context.Context) ([]string, error) + Watch(ctx context.Context) (Watcher, error) +} +``` +The `cluster.Cluster` _SHOULD_ be extended with a unique name identifier: +```golang +// pkg/cluster: +type Cluster interface { + Name() string + ... +} +``` + +The `ctrl.Manager` will use the provider to watch clusters coming and going, and +will inform runnables implementing the `cluster.AwareRunnable` interface: + +```golang +// pkg/cluster +type AwareRunnable interface { + Engage(context.Context, Cluster) error + Disengage(context.Context, Cluster) error +} +``` +In particular, controllers implement the `AwareRunnable` interface. They react +to engaged clusters by duplicating and starting their registered `source.Source`s +and `handler.EventHandler`s for each cluster through implementation of +```golang +// pkg/source +type DeepCopyableSyncingSource interface { + SyncingSource + DeepCopyFor(cluster cluster.Cluster) DeepCopyableSyncingSource +} + +// pkg/handler +type DeepCopyableEventHandler interface { + EventHandler + DeepCopyFor(c cluster.Cluster) DeepCopyableEventHandler +} +``` +The standard implementing types, in particular `internal.Kind` will adhere to +these interfaces. + +The `ctrl.Manager` _SHOULD_ be extended by a `cluster.Cluster` getter: +```golang +// pkg/manager +type Manager interface { + // ... + GetCluster(ctx context.Context, clusterName string) (cluster.Cluster, error) +} +``` +The embedded `cluster.Cluster` corresponds to `GetCluster(ctx, "")`. We call the +clusters with non-empty name "provider clusters" or "enganged clusters", while +the embedded cluster of the manager is called the "default cluster" or "hub +cluster". + +The `reconcile.Request` _SHOULD_ be extended by an optional `ClusterName` field: +```golang +// pkg/reconile +type Request struct { + ClusterName string + types.NamespacedName +} +``` + +With these changes, the behaviour of controller-runtime without a set cluster +provider will be unchanged. + +### Multi-Cluster-Compatible Reconcilers + +Reconcilers can be made multi-cluster-compatible by changing client and cache +accessing code from directly accessing `mgr.GetClient()` and `mgr.GetCache()` to +going through `mgr.GetCluster(req.ClusterName).GetClient()` and +`mgr.GetCluster(req.ClusterName).GetCache()`. + +When building a controller like +```golang +builder.NewControllerManagedBy(mgr). + For(&appsv1.ReplicaSet{}). + Owns(&v1.Pod{}). + Complete(reconciler) +``` +with the described change to use `GetCluster(ctx, req.ClusterName)` will automatically +act as *uniform multi-cluster controller*. It will reconcile resources from cluster `X` +in cluster `X`. + +For a manager with `cluster.Provider`, the builder _SHOULD_ create a controller +that sources events **ONLY** from the provider clusters that got engaged with +the controller. + +Controllers that should be triggered by events on the hub cluster will have to +opt-in like in this example: + +```golang +builder.NewControllerManagedBy(mgr). + For(&appsv1.Deployment{}, builder.InDefaultCluster). + Owns(&v1.ReplicaSet{}). + Complete(reconciler) +``` +A mixed set of sources is possible as shown here in the example. + +## User Stories + +### Controller Author with no interest in multi-cluster wanting to old behaviour. + +- Do nothing. Controller-runtime behaviour is unchanged. + +### Multi-Cluster Integrator wanting to support cluster managers like Cluster-API or kind + +- Implement the `cluster.Provider` interface, either via polling of the cluster registry + or by watching objects in the hub cluster. +- For every new cluster create an instance of `cluster.Cluster`. + +### Multi-Cluster Integrator wanting to support apiservers with logical cluster (like kcp) + +- Implement the `cluster.Provider` interface by watching the apiserver for logical cluster objects + (`LogicalCluster` CRD in kcp). +- Return a facade `cluster.Cluster` that scopes all operations (client, cache, indexers) + to the logical cluster, but backed by one physical `cluster.Cluster` resource. +- Add cross-cluster indexers to the physical `cluster.Cluster` object. + +### Controller Author without self-interest in multi-cluster, but open for adoption in multi-cluster setups + +- Replace `mgr.GetClient()` and `mgr.GetCache` with `mgr.GetCluster(req.ClusterName).GetClient()` and `mgr.GetCluster(req.ClusterName).GetCache()`. +- Make manager and controller plumbing vendor'able to allow plugging in multi-cluster provider. + +### Controller Author who wants to support certain multi-cluster setups + +- Do the `GetCluster` plumbing as described above. +- Vendor 3rd-party multi-cluster providers and wire them up in `main.go` + +## Risks and Mitigations + +- The standard behaviour of controller-runtime is unchanged for single-cluster controllers. +- The activation of the multi-cluster mode is through attaching the `cluster.Provider` to the manager. + To make it clear that the semantics are experimental, we make the `Options.provider` field private + and adds `Options.WithExperimentalClusterProvider` method. +- We only extend these interfaces and structs: + - `ctrl.Manager` with `GetCluster(ctx, clusterName string) (cluster.Cluster, error)` + - `cluster.Cluster` with `Name() string` + - `reconcile.Request` with `ClusterName string` + We think that the behaviour of these extensions is well understood and hence low risk. + Everything else behind the scenes is an implementation detail that can be changed + at any time. + +## Alternatives + +- Multi-cluster support could be built outside of core controller-runtime. This would + lead likely to a design with one manager per cluster. This has a number of problems: + - only one manager can serve webhooks or metrics + - cluster management must be custom built + - logical cluster support would still require a fork of controller-runtime and + with that a divergence in the ecosystem. The reason is that logical clusters + require a shared workqueue because they share the same apiserver. So for + fair queueing, this needs deep integration into one manager. + - informers facades are not supported in today's cluster/cache implementation. +- We could deepcopy the builder instead of the sources and handlers. This would + lead to one controller and one workqueue per cluster. For the reason outlined + in the previous alternative, this is not desireable. +- We could skip adding `ClusterName` to `reconcile.Request` and instead pass the + cluster through in the context. On the one hand, this looks attractive as it + would avoid having to touch reconcilers at all to make them multi-cluster-compatible. + On the other hand, with `cluster.Cluster` embedded into `manager.Manager`, not + every method of `cluster.Cluster` carries a context. So virtualizing the cluster + in the manager leads to contradictions in the semantics. + + For example, it can well be that every cluster has different REST mapping because + installed CRDs are different. Without a context, we cannot return the right + REST mapper. + + An alternative would be to add a context to every method of `cluster.Cluster`, + which is a much bigger and uglier change than what is proposed here. + + +## Implementation History + +- [PR #2207 by @vincepri : WIP: ✨ Cluster Provider and cluster-aware controllers](https://github.com/kubernetes-sigs/controller-runtime/pull/2207) – with extensive review +- [PR #2208 by @sttts replace #2207: WIP: ✨ Cluster Provider and cluster-aware controllers](https://github.com/kubernetes-sigs/controller-runtime/pull/2726) – + picking up #2207, addressing lots of comments and extending the approach to what kcp needs, with a `fleet-namespace` example that demonstrates a similar setup as kcp with real logical clusters. +- [github.com/kcp-dev/controller-runtime](https://github.com/kcp-dev/controller-runtime) – the kcp controller-runtime fork From 120cef5ec9dffa72e841f7f1cd16221ad7ce5b4e Mon Sep 17 00:00:00 2001 From: Marvin Beckers Date: Wed, 4 Dec 2024 15:02:43 +0100 Subject: [PATCH 2/5] Update multi-cluster proposal with new implementation details Signed-off-by: Marvin Beckers --- designs/multi-cluster.md | 220 ++++++++++++++++++++++++++------------- 1 file changed, 147 insertions(+), 73 deletions(-) diff --git a/designs/multi-cluster.md b/designs/multi-cluster.md index 5d630a1901..9afc4bef0f 100644 --- a/designs/multi-cluster.md +++ b/designs/multi-cluster.md @@ -1,8 +1,10 @@ # Multi-Cluster Support -Author: @sttts + +Author: @sttts @embik + Initial implementation: @vincepri -Last Updated on: 03/26/2024 +Last Updated on: 12/04/2024 ## Table of Contents @@ -35,6 +37,10 @@ Multi-cluster use-cases require the creation of multiple managers and/or cluster objects. This proposal is about adding native support for multi-cluster use-cases to controller-runtime. +With this change, it will be possible to implement pluggable cluster providers +that automatically start and stop watches (and thus, cluster-aware reconcilers) when +the cluster provider adds ("engages") or removes ("disengages") a cluster. + ## Motivation This change is important because: @@ -50,6 +56,7 @@ This change is important because: ### Goals +- Provide an interface for plugging in a "cluster provider", which provides a dynamic set of clusters that should be reconciled by registered controllers. - Provide a way to natively write controllers that 1. (UNIFORM MULTI-CLUSTER CONTROLLER) operate on multiple clusters in a uniform way, i.e. reconciling the same resources on multiple clusters, **optionally** @@ -59,12 +66,8 @@ This change is important because: Example: distributed `ReplicaSet` controller, reconciling `ReplicaSets` on multiple clusters. 2. (AGGREGATING MULTI-CLUSTER CONTROLLER) operate on one central hub cluster aggregating information from multiple clusters. - Example: distributed `Deployment` controller, aggregating `ReplicaSets` back into the `Deployment` object. + Example: distributed `Deployment` controller, aggregating `ReplicaSets` across multiple clusters back into a central `Deployment` object. - Allow clusters to dynamically join and leave the set of clusters a controller operates on. -- Allow event sources to be cross-cluster: - 1. Multi-cluster events that trigger reconciliation in the one central hub cluster. - 2. Central hub cluster events to trigger reconciliation on multiple clusters. -- Allow (informer) indexes that span multiple clusters. - Allow logical clusters where a set of clusters is actually backed by one physical informer store. - Allow 3rd-parties to plug in their multi-cluster adapter (in source code) into an existing multi-cluster-compatible code-base. @@ -80,7 +83,7 @@ logic. ### Examples - Run a controller-runtime controller against a kubeconfig with arbitrary many contexts, all being reconciled. -- Run a controller-runtime controller against cluster-managers like kind, Cluster-API, Open-Cluster-Manager or Hypershift. +- Run a controller-runtime controller against cluster managers like kind, Cluster API, Open-Cluster-Manager or Hypershift. - Run a controller-runtime controller against a kcp shard with a wildcard watch. ### Non-Goals/Future Work @@ -94,17 +97,32 @@ logic. ## Proposal The `ctrl.Manager` _SHOULD_ be extended to get an optional `cluster.Provider` via -`ctrl.Options` implementing +`ctrl.Options`, implementing: ```golang // pkg/cluster + +// Provider defines methods to retrieve clusters by name. The provider is +// responsible for discovering and managing the lifecycle of each cluster. +// +// Example: A Cluster API provider would be responsible for discovering and +// managing clusters that are backed by Cluster API resources, which can live +// in multiple namespaces in a single management cluster. type Provider interface { - Get(ctx context.Context, clusterName string, opts ...Option) (Cluster, error) - List(ctx context.Context) ([]string, error) - Watch(ctx context.Context) (Watcher, error) + // Get returns a cluster for the given identifying cluster name. Get + // returns an existing cluster if it has been created before. + Get(ctx context.Context, clusterName string) (Cluster, error) } ``` + +A cluster provider is responsible for constructing a `cluster.Cluster` instance +upon calls to `Get(ctx, clusterName)` and returning it. Providers should keep track +of created clusters and return them again if the same name is requested. Since +providers are responsible for constructing the `cluster.Cluster` instance, they +can make decisions about e.g. reusing existing informers. + The `cluster.Cluster` _SHOULD_ be extended with a unique name identifier: + ```golang // pkg/cluster: type Cluster interface { @@ -113,36 +131,74 @@ type Cluster interface { } ``` -The `ctrl.Manager` will use the provider to watch clusters coming and going, and -will inform runnables implementing the `cluster.AwareRunnable` interface: +A new interface for cluster-aware runnables will be provided: ```golang // pkg/cluster -type AwareRunnable interface { +type Aware interface { + // Engage gets called when the component should start operations for the given Cluster. + // The given context is tied to the Cluster's lifecycle and will be cancelled when the + // Cluster is removed or an error occurs. + // + // Implementers should return an error if they cannot start operations for the given Cluster, + // and should ensure this operation is re-entrant and non-blocking. + // + // \_________________|)____.---'--`---.____ + // || \----.________.----/ + // || / / `--' + // __||____/ /_ + // |___ \ + // `--------' Engage(context.Context, Cluster) error + + // Disengage gets called when the component should stop operations for the given Cluster. Disengage(context.Context, Cluster) error } ``` -In particular, controllers implement the `AwareRunnable` interface. They react -to engaged clusters by duplicating and starting their registered `source.Source`s -and `handler.EventHandler`s for each cluster through implementation of + +`ctrl.Manager` will implement `cluster.Aware`. It is the cluster provider's responsibility +to call `Engage` and `Disengage` on a `ctrl.Manager` instance when clusters join or leave +the set of target clusters that should be reconciled. + +The internal `ctrl.Manager` implementation in turn will call `Engage` and `Disengage` on all +its runnables that are cluster-aware (i.e. that implement the `cluster.Aware` interface). + +In particular, cluster-aware controllers implement the `cluster.Aware` interface and are +responsible for starting watches on clusters when they are engaged. This is expressed through +the interface below: + ```golang -// pkg/source -type DeepCopyableSyncingSource interface { - SyncingSource - DeepCopyFor(cluster cluster.Cluster) DeepCopyableSyncingSource +// pkg/controller +type TypedMultiClusterController[request comparable] interface { + cluster.Aware + TypedController[request] } +``` + +The multi-cluster controller implementation reacts to engaged clusters by starting +a new `TypedSyncingSource` that also wraps the context passed down from the call to `Engage`, +which _MUST_ be canceled by the cluster provider at the end of a cluster's lifecycle. + +Instead of extending `reconcile.Request`, implementations _SHOULD_ bring their +own request type through the generics support in `Typed*` types (`request comparable`). +Optionally, a passed `TypedEventHandler` will be duplicated per engaged cluster if they +fullfil the following interface: + +```golang // pkg/handler -type DeepCopyableEventHandler interface { - EventHandler - DeepCopyFor(c cluster.Cluster) DeepCopyableEventHandler +type TypedDeepCopyableEventHandler[object any, request comparable] interface { + TypedEventHandler[object, request] + DeepCopyFor(c cluster.Cluster) TypedDeepCopyableEventHandler[object, request] } ``` -The standard implementing types, in particular `internal.Kind` will adhere to -these interfaces. + +This might be necessary if a BYO `TypedEventHandler` needs to store information about +the engaged cluster (e.g. because the events do not supply information about the cluster in +object annotations) that it has been started for. The `ctrl.Manager` _SHOULD_ be extended by a `cluster.Cluster` getter: + ```golang // pkg/manager type Manager interface { @@ -150,55 +206,85 @@ type Manager interface { GetCluster(ctx context.Context, clusterName string) (cluster.Cluster, error) } ``` + The embedded `cluster.Cluster` corresponds to `GetCluster(ctx, "")`. We call the clusters with non-empty name "provider clusters" or "enganged clusters", while the embedded cluster of the manager is called the "default cluster" or "hub cluster". -The `reconcile.Request` _SHOULD_ be extended by an optional `ClusterName` field: + +### Multi-Cluster-Compatible Reconcilers + +Reconcilers can be made multi-cluster-compatible by changing client and cache +accessing code from directly accessing `mgr.GetClient()` and `mgr.GetCache()` to +going through `mgr.GetCluster(clusterName).GetClient()` and +`mgr.GetCluster(clusterName).GetCache()`. `clusterName` needs to be extracted from +the BYO `request` type (e.g. a `clusterName` field in the type itself). + +A typical snippet at the beginning of a reconciler to fetch the client could look like this: + ```golang -// pkg/reconile -type Request struct { - ClusterName string - types.NamespacedName +cl, err := mgr.GetCluster(ctx, req.ClusterName) +if err != nil { + return reconcile.Result{}, err } +client := cl.GetClient() ``` -With these changes, the behaviour of controller-runtime without a set cluster -provider will be unchanged. +Due to the BYO `request` type, controllers need to be built like this: -### Multi-Cluster-Compatible Reconcilers +```golang +builder.TypedControllerManagedBy[ClusterRequest](mgr). + Named("multi-cluster-controller"). + Watches(&corev1.Pod{}, &ClusterRequestEventHandler{}). + Complete(reconciler) +``` -Reconcilers can be made multi-cluster-compatible by changing client and cache -accessing code from directly accessing `mgr.GetClient()` and `mgr.GetCache()` to -going through `mgr.GetCluster(req.ClusterName).GetClient()` and -`mgr.GetCluster(req.ClusterName).GetCache()`. +With `ClusterRequest` and `ClusterRequestEventHandler` being BYO types. `reconciler` +can be e.g. of type `reconcile.TypedFunc[ClusterRequest]`. + +`ClusterRequest` will likely often look like this: -When building a controller like ```golang -builder.NewControllerManagedBy(mgr). - For(&appsv1.ReplicaSet{}). - Owns(&v1.Pod{}). - Complete(reconciler) +type ClusterRequest struct { + reconcile.Request + ClusterName string +} ``` -with the described change to use `GetCluster(ctx, req.ClusterName)` will automatically -act as *uniform multi-cluster controller*. It will reconcile resources from cluster `X` + +Controllers that use `For` or `Owns` cannot be converted to multi-cluster controllers +without changing to `Watches` as the BYO `request` type cannot be used with them: + +```golang +// pkg/builder/controller.go +if reflect.TypeFor[request]() != reflect.TypeOf(reconcile.Request{}) { + return fmt.Errorf("For() can only be used with reconcile.Request, got %T", *new(request)) +} +``` + +With the described changes (use `GetCluster(ctx, clusterName)` and making `reconciler` +a `TypedFunc[ClusterRequest`) an existing controller will automatically act as +*uniform multi-cluster controller*. It will reconcile resources from cluster `X` in cluster `X`. For a manager with `cluster.Provider`, the builder _SHOULD_ create a controller that sources events **ONLY** from the provider clusters that got engaged with the controller. -Controllers that should be triggered by events on the hub cluster will have to -opt-in like in this example: +Controllers that should be triggered by events on the hub cluster can continue +to use `For` and `Owns` and explicitly pass the intention to engage only with the +"default" cluster: ```golang builder.NewControllerManagedBy(mgr). - For(&appsv1.Deployment{}, builder.InDefaultCluster). + WithOptions(controller.TypedOptions{ + EngageWithDefaultCluster: ptr.To(true), + EngageWithProviderClusters: ptr.To(false), + }). + For(&appsv1.Deployment{}). Owns(&v1.ReplicaSet{}). Complete(reconciler) ``` -A mixed set of sources is possible as shown here in the example. ## User Stories @@ -206,11 +292,11 @@ A mixed set of sources is possible as shown here in the example. - Do nothing. Controller-runtime behaviour is unchanged. -### Multi-Cluster Integrator wanting to support cluster managers like Cluster-API or kind +### Multi-Cluster Integrator wanting to support cluster managers like Cluster API or kind - Implement the `cluster.Provider` interface, either via polling of the cluster registry or by watching objects in the hub cluster. -- For every new cluster create an instance of `cluster.Cluster`. +- For every new cluster create an instance of `cluster.Cluster` and call `mgr.Engage`. ### Multi-Cluster Integrator wanting to support apiservers with logical cluster (like kcp) @@ -223,7 +309,8 @@ A mixed set of sources is possible as shown here in the example. ### Controller Author without self-interest in multi-cluster, but open for adoption in multi-cluster setups - Replace `mgr.GetClient()` and `mgr.GetCache` with `mgr.GetCluster(req.ClusterName).GetClient()` and `mgr.GetCluster(req.ClusterName).GetCache()`. -- Make manager and controller plumbing vendor'able to allow plugging in multi-cluster provider. +- Switch from `For` and `Owns` builder calls to `watches` +- Make manager and controller plumbing vendor'able to allow plugging in multi-cluster provider and BYO request type. ### Controller Author who wants to support certain multi-cluster setups @@ -234,12 +321,11 @@ A mixed set of sources is possible as shown here in the example. - The standard behaviour of controller-runtime is unchanged for single-cluster controllers. - The activation of the multi-cluster mode is through attaching the `cluster.Provider` to the manager. - To make it clear that the semantics are experimental, we make the `Options.provider` field private - and adds `Options.WithExperimentalClusterProvider` method. + To make it clear that the semantics are experimental, we name the `manager.Options` field + `ExperimentalClusterProvider`. - We only extend these interfaces and structs: - - `ctrl.Manager` with `GetCluster(ctx, clusterName string) (cluster.Cluster, error)` - - `cluster.Cluster` with `Name() string` - - `reconcile.Request` with `ClusterName string` + - `ctrl.Manager` with `GetCluster(ctx, clusterName string) (cluster.Cluster, error)` and `cluster.Aware`. + - `cluster.Cluster` with `Name() string`. We think that the behaviour of these extensions is well understood and hence low risk. Everything else behind the scenes is an implementation detail that can be changed at any time. @@ -258,24 +344,12 @@ A mixed set of sources is possible as shown here in the example. - We could deepcopy the builder instead of the sources and handlers. This would lead to one controller and one workqueue per cluster. For the reason outlined in the previous alternative, this is not desireable. -- We could skip adding `ClusterName` to `reconcile.Request` and instead pass the - cluster through in the context. On the one hand, this looks attractive as it - would avoid having to touch reconcilers at all to make them multi-cluster-compatible. - On the other hand, with `cluster.Cluster` embedded into `manager.Manager`, not - every method of `cluster.Cluster` carries a context. So virtualizing the cluster - in the manager leads to contradictions in the semantics. - - For example, it can well be that every cluster has different REST mapping because - installed CRDs are different. Without a context, we cannot return the right - REST mapper. - - An alternative would be to add a context to every method of `cluster.Cluster`, - which is a much bigger and uglier change than what is proposed here. - ## Implementation History - [PR #2207 by @vincepri : WIP: ✨ Cluster Provider and cluster-aware controllers](https://github.com/kubernetes-sigs/controller-runtime/pull/2207) – with extensive review -- [PR #2208 by @sttts replace #2207: WIP: ✨ Cluster Provider and cluster-aware controllers](https://github.com/kubernetes-sigs/controller-runtime/pull/2726) – +- [PR #2726 by @sttts replacing #2207: WIP: ✨ Cluster Provider and cluster-aware controllers](https://github.com/kubernetes-sigs/controller-runtime/pull/2726) – picking up #2207, addressing lots of comments and extending the approach to what kcp needs, with a `fleet-namespace` example that demonstrates a similar setup as kcp with real logical clusters. +- [PR #3019 by @embik, replacing #2726: ✨ WIP: Cluster provider and cluster-aware controllers](https://github.com/kubernetes-sigs/controller-runtime/pull/3019) - + picking up #2726, reworking existing code to support the recent `Typed*` generic changes of the codebase. - [github.com/kcp-dev/controller-runtime](https://github.com/kcp-dev/controller-runtime) – the kcp controller-runtime fork From 799a91183b409bf48843606ad3130186f7ace37c Mon Sep 17 00:00:00 2001 From: Marvin Beckers Date: Wed, 18 Dec 2024 13:37:36 +0100 Subject: [PATCH 3/5] Update with feedback Signed-off-by: Marvin Beckers --- designs/multi-cluster.md | 56 +++++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 26 deletions(-) diff --git a/designs/multi-cluster.md b/designs/multi-cluster.md index 9afc4bef0f..c033c514b9 100644 --- a/designs/multi-cluster.md +++ b/designs/multi-cluster.md @@ -4,7 +4,7 @@ Author: @sttts @embik Initial implementation: @vincepri -Last Updated on: 12/04/2024 +Last Updated on: 2025-01-06 ## Table of Contents @@ -38,7 +38,7 @@ objects. This proposal is about adding native support for multi-cluster use-case to controller-runtime. With this change, it will be possible to implement pluggable cluster providers -that automatically start and stop watches (and thus, cluster-aware reconcilers) when +that automatically start and stop sources (and thus, cluster-aware reconcilers) when the cluster provider adds ("engages") or removes ("disengages") a cluster. ## Motivation @@ -56,24 +56,27 @@ This change is important because: ### Goals -- Provide an interface for plugging in a "cluster provider", which provides a dynamic set of clusters that should be reconciled by registered controllers. -- Provide a way to natively write controllers that - 1. (UNIFORM MULTI-CLUSTER CONTROLLER) operate on multiple clusters in a uniform way, +- Allow 3rd-parties to implement an (optional) multi-cluster provider Go interface that controller-runtime will use (if configured on the manager) to dynamically attach and detach registered controllers to clusters that come and go. +- With that, provide a way to natively write controllers for these patterns: + 1. (UNIFORM MULTI-CLUSTER CONTROLLERS) operate on multiple clusters in a uniform way, i.e. reconciling the same resources on multiple clusters, **optionally** - sourcing information from one central hub cluster - sourcing information cross-cluster. - Example: distributed `ReplicaSet` controller, reconciling `ReplicaSets` on multiple clusters. - 2. (AGGREGATING MULTI-CLUSTER CONTROLLER) operate on one central hub cluster aggregating information from multiple clusters. + Example: distributed `ReplicaSet` controller, reconciling `ReplicaSets` on multiple clusters. + 2. (AGGREGATING MULTI-CLUSTER CONTROLLERS) operate on one central hub cluster aggregating information from multiple clusters. Example: distributed `Deployment` controller, aggregating `ReplicaSets` across multiple clusters back into a central `Deployment` object. -- Allow clusters to dynamically join and leave the set of clusters a controller operates on. -- Allow logical clusters where a set of clusters is actually backed by one physical informer store. -- Allow 3rd-parties to plug in their multi-cluster adapter (in source code) into - an existing multi-cluster-compatible code-base. + +#### Low-Level Requirements + +- Allow event sources to be cross-cluster such that: + 1. Multi-cluster events can trigger reconciliation in the one central hub cluster. + 2. Central hub cluster events can trigger reconciliation on multiple clusters. +- Allow reconcilers to look up objects through (informer) indexes from specific other clusters. - Minimize the amount of changes to make a controller-runtime controller multi-cluster-compatible, in a way that 3rd-party projects have no reason to - object these kind of changes. + object to these kind of changes. Here we call a controller to be multi-cluster-compatible if the reconcilers get reconcile requests in cluster `X` and do all reconciliation in cluster `X`. This @@ -231,9 +234,16 @@ if err != nil { client := cl.GetClient() ``` -Due to the BYO `request` type, controllers need to be built like this: +Due to the BYO `request` type, controllers using the `For` builder function need to be built/changed like this: ```golang +// previous +builder.TypedControllerManagedBy[reconcile.Request](mgr). + Named("single-cluster-controller"). + For(&corev1.Pod{}). + Complete(reconciler) + +// new builder.TypedControllerManagedBy[ClusterRequest](mgr). Named("multi-cluster-controller"). Watches(&corev1.Pod{}, &ClusterRequestEventHandler{}). @@ -243,7 +253,7 @@ builder.TypedControllerManagedBy[ClusterRequest](mgr). With `ClusterRequest` and `ClusterRequestEventHandler` being BYO types. `reconciler` can be e.g. of type `reconcile.TypedFunc[ClusterRequest]`. -`ClusterRequest` will likely often look like this: +`ClusterRequest` will likely often look like this (but since it is a BYO type, it could store other information as well): ```golang type ClusterRequest struct { @@ -252,18 +262,12 @@ type ClusterRequest struct { } ``` -Controllers that use `For` or `Owns` cannot be converted to multi-cluster controllers -without changing to `Watches` as the BYO `request` type cannot be used with them: - -```golang -// pkg/builder/controller.go -if reflect.TypeFor[request]() != reflect.TypeOf(reconcile.Request{}) { - return fmt.Errorf("For() can only be used with reconcile.Request, got %T", *new(request)) -} -``` +Controllers that use `Owns` cannot be converted to multi-cluster controllers +without a BYO type re-implementation of `handler.EnqueueRequestForOwner` matching +the BYO type, which is considered out of scope for now. -With the described changes (use `GetCluster(ctx, clusterName)` and making `reconciler` -a `TypedFunc[ClusterRequest`) an existing controller will automatically act as +With the described changes (use `GetCluster(ctx, clusterName)`, making `reconciler` +a `TypedFunc[ClusterRequest]` and migrating to `Watches`) an existing controller will automatically act as *uniform multi-cluster controller*. It will reconcile resources from cluster `X` in cluster `X`. @@ -273,7 +277,7 @@ the controller. Controllers that should be triggered by events on the hub cluster can continue to use `For` and `Owns` and explicitly pass the intention to engage only with the -"default" cluster: +"default" cluster (this is only necessary if a cluster provider is plugged in): ```golang builder.NewControllerManagedBy(mgr). From 3f2fa4f1631c3ab320593afb2798daf206066ea3 Mon Sep 17 00:00:00 2001 From: Marvin Beckers Date: Tue, 7 Jan 2025 13:22:32 +0100 Subject: [PATCH 4/5] Adjust for reconcile.ClusterAwareRequest Signed-off-by: Marvin Beckers --- designs/multi-cluster.md | 115 ++++++++++++++++++++------------------- 1 file changed, 59 insertions(+), 56 deletions(-) diff --git a/designs/multi-cluster.md b/designs/multi-cluster.md index c033c514b9..538b7e8b94 100644 --- a/designs/multi-cluster.md +++ b/designs/multi-cluster.md @@ -4,7 +4,7 @@ Author: @sttts @embik Initial implementation: @vincepri -Last Updated on: 2025-01-06 +Last Updated on: 2025-01-07 ## Table of Contents @@ -118,11 +118,10 @@ type Provider interface { } ``` -A cluster provider is responsible for constructing a `cluster.Cluster` instance -upon calls to `Get(ctx, clusterName)` and returning it. Providers should keep track -of created clusters and return them again if the same name is requested. Since -providers are responsible for constructing the `cluster.Cluster` instance, they -can make decisions about e.g. reusing existing informers. +A cluster provider is responsible for constructing `cluster.Cluster` instances and returning +upon calls to `Get(ctx, clusterName)`. Providers should keep track of created clusters and +return them again if the same name is requested. Since providers are responsible for constructing +the `cluster.Cluster` instance, they can make decisions about e.g. reusing existing informers. The `cluster.Cluster` _SHOULD_ be extended with a unique name identifier: @@ -159,9 +158,9 @@ type Aware interface { } ``` -`ctrl.Manager` will implement `cluster.Aware`. It is the cluster provider's responsibility -to call `Engage` and `Disengage` on a `ctrl.Manager` instance when clusters join or leave -the set of target clusters that should be reconciled. +`ctrl.Manager` will implement `cluster.Aware`. As specified in the `Provider` interface, +it is the cluster provider's responsibility to call `Engage` and `Disengage` on a `ctrl.Manager` +instance when clusters join or leave the set of target clusters that should be reconciled. The internal `ctrl.Manager` implementation in turn will call `Engage` and `Disengage` on all its runnables that are cluster-aware (i.e. that implement the `cluster.Aware` interface). @@ -182,7 +181,42 @@ The multi-cluster controller implementation reacts to engaged clusters by starti a new `TypedSyncingSource` that also wraps the context passed down from the call to `Engage`, which _MUST_ be canceled by the cluster provider at the end of a cluster's lifecycle. -Instead of extending `reconcile.Request`, implementations _SHOULD_ bring their +The `ctrl.Manager` _SHOULD_ be extended by a `cluster.Cluster` getter: + +```golang +// pkg/manager +type Manager interface { + // ... + GetCluster(ctx context.Context, clusterName string) (cluster.Cluster, error) +} +``` + +The embedded `cluster.Cluster` corresponds to `GetCluster(ctx, "")`. We call the +clusters with non-empty name "provider clusters" or "enganged clusters", while +the embedded cluster of the manager is called the "default cluster" or "hub +cluster". + +To provide information about the source cluster of a request, a new type +`reconcile.ClusterAwareRequest` _SHOULD_ be added: + +```golang +// pkg/reconcile +type ClusterAwareRequest struct { + Request + ClusterName string +} +``` + +This struct embeds a `reconcile.Request` to store the "usual" information (name and namespace) +about an object, plus the name of the originating cluster. + +Given that an empty cluster name represents the "default cluster", a `reconcile.ClusterAwareRequest` +can be used as `request` type even for controllers that do not have an active cluster provider. +The cluster name will simply be an empty string, which is compatible with calls to `mgr.GetCluster`. + +### BYO Request Type + +Instead of using the new `reconcile.ClusterAwareRequest`, implementations _CAN_ also bring their own request type through the generics support in `Typed*` types (`request comparable`). Optionally, a passed `TypedEventHandler` will be duplicated per engaged cluster if they @@ -200,29 +234,12 @@ This might be necessary if a BYO `TypedEventHandler` needs to store information the engaged cluster (e.g. because the events do not supply information about the cluster in object annotations) that it has been started for. -The `ctrl.Manager` _SHOULD_ be extended by a `cluster.Cluster` getter: - -```golang -// pkg/manager -type Manager interface { - // ... - GetCluster(ctx context.Context, clusterName string) (cluster.Cluster, error) -} -``` - -The embedded `cluster.Cluster` corresponds to `GetCluster(ctx, "")`. We call the -clusters with non-empty name "provider clusters" or "enganged clusters", while -the embedded cluster of the manager is called the "default cluster" or "hub -cluster". - - ### Multi-Cluster-Compatible Reconcilers Reconcilers can be made multi-cluster-compatible by changing client and cache accessing code from directly accessing `mgr.GetClient()` and `mgr.GetCache()` to -going through `mgr.GetCluster(clusterName).GetClient()` and -`mgr.GetCluster(clusterName).GetCache()`. `clusterName` needs to be extracted from -the BYO `request` type (e.g. a `clusterName` field in the type itself). +going through `mgr.GetCluster(req.ClusterName).GetClient()` and +`mgr.GetCluster(req.ClusterName).GetCache()`. A typical snippet at the beginning of a reconciler to fetch the client could look like this: @@ -234,7 +251,7 @@ if err != nil { client := cl.GetClient() ``` -Due to the BYO `request` type, controllers using the `For` builder function need to be built/changed like this: +Due to `request.ClusterAwareRequest`, changes to the controller builder process are minimal: ```golang // previous @@ -244,32 +261,19 @@ builder.TypedControllerManagedBy[reconcile.Request](mgr). Complete(reconciler) // new -builder.TypedControllerManagedBy[ClusterRequest](mgr). +builder.TypedControllerManagedBy[reconcile.ClusterAwareRequest](mgr). Named("multi-cluster-controller"). - Watches(&corev1.Pod{}, &ClusterRequestEventHandler{}). + For(&corev1.Pod{}). Complete(reconciler) ``` -With `ClusterRequest` and `ClusterRequestEventHandler` being BYO types. `reconciler` -can be e.g. of type `reconcile.TypedFunc[ClusterRequest]`. - -`ClusterRequest` will likely often look like this (but since it is a BYO type, it could store other information as well): - -```golang -type ClusterRequest struct { - reconcile.Request - ClusterName string -} -``` - -Controllers that use `Owns` cannot be converted to multi-cluster controllers -without a BYO type re-implementation of `handler.EnqueueRequestForOwner` matching -the BYO type, which is considered out of scope for now. +The builder will chose the correct `EventHandler` implementation for both `For` and `Owns` +depending on the `request` type used. -With the described changes (use `GetCluster(ctx, clusterName)`, making `reconciler` -a `TypedFunc[ClusterRequest]` and migrating to `Watches`) an existing controller will automatically act as -*uniform multi-cluster controller*. It will reconcile resources from cluster `X` -in cluster `X`. +With the described changes (use `GetCluster(ctx, req.ClusterName)`, making `reconciler` +a `TypedFunc[reconcile.ClusterAwareRequest]`) an existing controller will automatically act as +*uniform multi-cluster controller* if a cluster provider is configured. +It will reconcile resources from cluster `X` in cluster `X`. For a manager with `cluster.Provider`, the builder _SHOULD_ create a controller that sources events **ONLY** from the provider clusters that got engaged with @@ -313,20 +317,19 @@ builder.NewControllerManagedBy(mgr). ### Controller Author without self-interest in multi-cluster, but open for adoption in multi-cluster setups - Replace `mgr.GetClient()` and `mgr.GetCache` with `mgr.GetCluster(req.ClusterName).GetClient()` and `mgr.GetCluster(req.ClusterName).GetCache()`. -- Switch from `For` and `Owns` builder calls to `watches` - Make manager and controller plumbing vendor'able to allow plugging in multi-cluster provider and BYO request type. ### Controller Author who wants to support certain multi-cluster setups - Do the `GetCluster` plumbing as described above. -- Vendor 3rd-party multi-cluster providers and wire them up in `main.go` +- Vendor 3rd-party multi-cluster providers and wire them up in `main.go`. ## Risks and Mitigations - The standard behaviour of controller-runtime is unchanged for single-cluster controllers. -- The activation of the multi-cluster mode is through attaching the `cluster.Provider` to the manager. - To make it clear that the semantics are experimental, we name the `manager.Options` field - `ExperimentalClusterProvider`. +- The activation of the multi-cluster mode is through usage of a `request.ClusterAwareRequest` request type and + attaching the `cluster.Provider` to the manager. To make it clear that the semantics are experimental, we name + the `manager.Options` field `ExperimentalClusterProvider`. - We only extend these interfaces and structs: - `ctrl.Manager` with `GetCluster(ctx, clusterName string) (cluster.Cluster, error)` and `cluster.Aware`. - `cluster.Cluster` with `Name() string`. From 7469ce922a9b5c0f14d9a87827ce837bd06b4fba Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Tue, 7 Jan 2025 15:35:52 +0100 Subject: [PATCH 5/5] Add notes about uniform controllers Signed-off-by: Dr. Stefan Schimanski --- designs/multi-cluster.md | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/designs/multi-cluster.md b/designs/multi-cluster.md index 538b7e8b94..c46e6c5b8b 100644 --- a/designs/multi-cluster.md +++ b/designs/multi-cluster.md @@ -106,7 +106,9 @@ The `ctrl.Manager` _SHOULD_ be extended to get an optional `cluster.Provider` vi // pkg/cluster // Provider defines methods to retrieve clusters by name. The provider is -// responsible for discovering and managing the lifecycle of each cluster. +// responsible for discovering and managing the lifecycle of each cluster, +// and to engage or disengage clusters with the manager the provider is +// run against. // // Example: A Cluster API provider would be responsible for discovering and // managing clusters that are backed by Cluster API resources, which can live @@ -196,6 +198,8 @@ clusters with non-empty name "provider clusters" or "enganged clusters", while the embedded cluster of the manager is called the "default cluster" or "hub cluster". +### Cluster-Aware Request + To provide information about the source cluster of a request, a new type `reconcile.ClusterAwareRequest` _SHOULD_ be added: @@ -214,11 +218,21 @@ Given that an empty cluster name represents the "default cluster", a `reconcile. can be used as `request` type even for controllers that do not have an active cluster provider. The cluster name will simply be an empty string, which is compatible with calls to `mgr.GetCluster`. +**Note:** controller-runtime must provide this cluster-aware request type to +allow writing *uniform* multi-cluster controllers (see goals), i.e. controllers +that both work as single-cluster and multi-cluster controllers against arbitrary +cluster providers. Think of generic CNCF projects like cert-manager wanting to +support multi-cluster setups generically without forking the codebase. + ### BYO Request Type Instead of using the new `reconcile.ClusterAwareRequest`, implementations _CAN_ also bring their own request type through the generics support in `Typed*` types (`request comparable`). +**Note:** these kind of controllers won't be uniform in the sense of compatibility +with arbitrary cluster providers, but for use-cases that are tiedly integrated +with specific cluster providers, this might be useful. + Optionally, a passed `TypedEventHandler` will be duplicated per engaged cluster if they fullfil the following interface: