SQUASH: address comments

Signed-off-by: Dr. Stefan Schimanski <[email protected]>

Author: sttts

examples/fleet/main.go

@@ -110,7 +110,7 @@ func (k *KindClusterProvider) Get(ctx context.Context, clusterName string, opts
 	return cluster.New(cfg, opts...)
 }
 
-func (k *KindClusterProvider) List() ([]string, error) {
+func (k *KindClusterProvider) List(ctx context.Context) ([]string, error) {
 	provider := kind.NewProvider()
 	list, err := provider.List()
 	if err != nil {
@@ -144,6 +144,7 @@ func (k *KindWatcher) Stop() {
 	k.wg.Wait()
 	close(k.ch)
 }
+
 func (k *KindWatcher) ResultChan() <-chan cluster.WatchEvent {
 	k.init.Do(func() {
 		ctx, cancel := context.WithCancel(context.Background())

pkg/builder/controller.go

@@ -332,12 +332,12 @@ func (blder *Builder) doWatch() error {
 			}
 			srcKind.Type = typeForSrc
 		} else if !ok {
-			// If we're building a logical controller, raw watches are not allowed
-			// given that the cache cannot be validated to be coming from the same cluter.
+			// If we're building a cluster-aware controller, raw watches are not allowed
+			// given that the cache cannot be validated to be coming from the same cluster.
 			// In the future, we could consider allowing this by satisfying a new interface
 			// that sets and uses the cluster.
 			if blder.clusterName != "" {
-				return fmt.Errorf("when using a logical adapter, custom raw watches %T are not allowed", w.src)
+				return fmt.Errorf("when using a cluster adapter, custom raw watches %T are not allowed", w.src)
 			}
 		}
 		allPredicates := append([]predicate.Predicate(nil), blder.globalPredicates...)

pkg/builder/controller_test.go

@@ -558,7 +558,7 @@ var _ = Describe("application", func() {
 		})
 	})
 
-	Context("with logical adapter", func() {
+	Context("with cluster provider", func() {
 		It("should support watching across clusters", func() {
 			adapter := &fakeClusterProvider{
 				clusterNameList: []string{
@@ -847,7 +847,7 @@ func (f *fakeClusterProvider) Get(ctx context.Context, clusterName string, opts
 	return cluster.New(testenv.Config, opts...)
 }
 
-func (f *fakeClusterProvider) List() ([]string, error) {
+func (f *fakeClusterProvider) List(ctx context.Context) ([]string, error) {
 	return f.clusterNameList, f.listErr
 }
 

pkg/cluster/cluster.go

@@ -64,7 +64,7 @@ type AwareDeepCopy[T any] interface {
 	DeepCopyFor(Cluster) T
 }
 
-// ByNameGetterFunc is a function that returns a cluster for a given logical cluster name.
+// ByNameGetterFunc is a function that returns a cluster for a given identifying cluster name.
 type ByNameGetterFunc func(ctx context.Context, clusterName string) (Cluster, error)
 
 // Cluster provides various methods to interact with a cluster.

pkg/cluster/provider.go

@@ -14,14 +14,18 @@ import (
 // clusters that are backed by Cluster API resources, which can live
 // in multiple namespaces in a single management cluster.
 type Provider interface {
+	// Get returns a cluster for the given identifying cluster name. The
+	// options are passed to the cluster constructor in case the cluster has
+	// not been created yet. Get returns an existing cluster if it has been
+	// created before.
 	Get(ctx context.Context, clusterName string, opts ...Option) (Cluster, error)
 
-	// List returns a list of logical clusters.
-	// This method is used to discover the initial set of logical clusters
-	// and to refresh the list of logical clusters periodically.
-	List() ([]string, error)
+	// List returns a list of known identifying clusters names.
+	// This method is used to discover the initial set of known cluster names
+	// and to refresh the list of cluster names periodically.
+	List(ctx context.Context) ([]string, error)
 
-	// Watch returns a Watcher that watches for changes to a list of logical clusters
+	// Watch returns a Watcher that watches for changes to a list of known clusters
 	// and react to potential changes.
 	Watch() (Watcher, error)
 }
@@ -44,15 +48,15 @@ type WatchEvent struct {
 	// Type is the type of event that occurred.
 	//
 	// - ADDED or MODIFIED
-	//	 	The logical cluster was added or updated: a new RESTConfig is available, or needs to be refreshed.
+	//	 	The cluster was added or updated: a new RESTConfig is available, or needs to be refreshed.
 	// - DELETED
-	// 		The logical cluster was deleted: the cluster is removed.
+	// 		The cluster was deleted: the cluster is removed.
 	// - ERROR
-	// 		An error occurred while watching the logical cluster: the cluster is removed.
+	// 		An error occurred while watching the cluster: the cluster is removed.
 	// - BOOKMARK
 	// 		A periodic event is sent that contains no new data: ignored.
 	Type watch.EventType
 
-	// ClusterName is the name of the cluster related to the event.
+	// ClusterName is the identifying name of the cluster related to the event.
 	ClusterName string
 }

pkg/handler/enqueue_mapped.go

@@ -86,7 +86,7 @@ func (e *enqueueRequestsFromMapFunc) mapAndEnqueue(ctx context.Context, q workqu
 			// reqs is a map of requests to avoid enqueueing the same request multiple times.
 			continue
 		}
-		// If the request doesn't specify a cluster, use the cluster from the context.
+		// If the request doesn't specify a cluster, use the cluster from the handler.
 		if req.ClusterName == "" && e.cluster != nil {
 			req.ClusterName = e.cluster.Name()
 		}

pkg/internal/controller/controller.go

@@ -84,7 +84,7 @@ type Controller struct {
 	// clusterAwareWatches maintains a list of cluster aware sources, handlers, and predicates to start when the controller is started.
 	clusterAwareWatches []*watchDescription
 
-	// clustersByName is used to manage the logical clustersByName.
+	// clustersByName is used to manage the fleet of clusters.
 	clustersByName map[string]*clusterDescription
 
 	// LogConstructor is used to construct a logger to then log messages to users during reconciliation,

pkg/manager/internal.go

@@ -65,7 +65,7 @@ var _ Runnable = &controllerManager{}
 var _ Manager = &controllerManager{}
 var _ cluster.ByNameGetterFunc = (&controllerManager{}).GetCluster
 
-type logicalCluster struct {
+type engagedCluster struct {
 	cluster.Cluster
 	ctx    context.Context
 	cancel context.CancelFunc
@@ -83,10 +83,10 @@ type controllerManager struct {
 	defaultCluster        cluster.Cluster
 	defaultClusterOptions cluster.Option
 
-	logicalProvider cluster.Provider
+	clusterProvider cluster.Provider
 
 	clusterLock           sync.RWMutex // protects clusters
-	clusters              map[string]*logicalCluster
+	clusters              map[string]*engagedCluster
 	clusterAwareRunnables []cluster.AwareRunnable
 
 	// recorderProvider is used to generate event recorders that will be injected into Controllers
@@ -282,8 +282,8 @@ func (cm *controllerManager) engageClusterAwareRunnables() {
 	cm.Lock()
 	defer cm.Unlock()
 
-	// If we don't have a logical adapter, we cannot sync the runnables.
-	if cm.logicalProvider == nil {
+	// If we don't have a cluster provider, we cannot sync the runnables.
+	if cm.clusterProvider == nil {
 		return
 	}
 
@@ -299,11 +299,11 @@ func (cm *controllerManager) engageClusterAwareRunnables() {
 	}
 }
 
-func (cm *controllerManager) getCluster(ctx context.Context, clusterName string) (c *logicalCluster, err error) {
-	// Check if the manager was configured with a logical adapter,
+func (cm *controllerManager) getCluster(ctx context.Context, clusterName string) (c *engagedCluster, err error) {
+	// Check if the manager was configured with a cluster provider,
 	// otherwise we cannot retrieve the cluster.
-	if cm.logicalProvider == nil {
-		return nil, fmt.Errorf("manager was not configured with a logical adapter, cannot retrieve %q", clusterName)
+	if cm.clusterProvider == nil {
+		return nil, fmt.Errorf("manager was not configured with a cluster provider, cannot retrieve %q", clusterName)
 	}
 
 	// Check if the cluster already exists.
@@ -332,13 +332,13 @@ func (cm *controllerManager) getCluster(ctx context.Context, clusterName string)
 		defer cancel()
 		var watchErr error
 		if err := wait.PollImmediateUntilWithContext(ctx, 1*time.Second, func(ctx context.Context) (done bool, err error) {
-			cl, watchErr = cm.logicalProvider.Get(ctx, clusterName, cm.defaultClusterOptions, cluster.WithName(clusterName))
+			cl, watchErr = cm.clusterProvider.Get(ctx, clusterName, cm.defaultClusterOptions, cluster.WithName(clusterName))
 			if watchErr != nil {
 				return false, nil //nolint:nilerr // We want to keep trying.
 			}
 			return true, nil
 		}); err != nil {
-			return nil, fmt.Errorf("failed  create logical cluster %q: %w", clusterName, kerrors.NewAggregate([]error{err, watchErr}))
+			return nil, fmt.Errorf("failed create cluster %q: %w", clusterName, kerrors.NewAggregate([]error{err, watchErr}))
 		}
 	}
 
@@ -348,11 +348,11 @@ func (cm *controllerManager) getCluster(ctx context.Context, clusterName string)
 	// Once added, if the manager has already started, it waits for the Cache to sync before returning
 	// otherwise it enqueues the Runnable to be started when the manager starts.
 	if err := cm.Add(cl); err != nil {
-		return nil, fmt.Errorf("cannot add logical cluster %q to manager: %w", clusterName, err)
+		return nil, fmt.Errorf("cannot add cluster %q to manager: %w", clusterName, err)
 	}
 	// Create a new context for the Cluster, so that it can be stopped independently.
 	ctx, cancel := context.WithCancel(context.Background())
-	c = &logicalCluster{
+	c = &engagedCluster{
 		Cluster: cl,
 		ctx:     ctx,
 		cancel:  cancel,
@@ -364,8 +364,8 @@ func (cm *controllerManager) getCluster(ctx context.Context, clusterName string)
 func (cm *controllerManager) removeLogicalCluster(clusterName string) error {
 	// Check if the manager was configured with a logical adapter,
 	// otherwise we cannot retrieve the cluster.
-	if cm.logicalProvider == nil {
-		return fmt.Errorf("manager was not configured with a logical adapter, cannot retrieve %q", clusterName)
+	if cm.clusterProvider == nil {
+		return fmt.Errorf("manager was not configured with a cluster provider, cannot retrieve %q", clusterName)
 	}
 
 	cm.clusterLock.Lock()
@@ -578,11 +578,11 @@ func (cm *controllerManager) Start(ctx context.Context) (err error) { //nolint:g
 		}()
 	}
 
-	// If the manager has been configured with a logical adapter, start it.
-	if cm.logicalProvider != nil {
+	// If the manager has been configured with a cluster provider, start it.
+	if cm.clusterProvider != nil {
 		if err := cm.add(RunnableFunc(func(ctx context.Context) error {
 			resync := func() error {
-				clusterList, err := cm.logicalProvider.List()
+				clusterList, err := cm.clusterProvider.List(ctx)
 				if err != nil {
 					return err
 				}
@@ -610,7 +610,7 @@ func (cm *controllerManager) Start(ctx context.Context) (err error) { //nolint:g
 			}
 
 			// Create a watcher and start watching for changes.
-			watcher, err := cm.logicalProvider.Watch()
+			watcher, err := cm.clusterProvider.Watch()
 			if err != nil {
 				return err
 			}
@@ -648,7 +648,7 @@ func (cm *controllerManager) Start(ctx context.Context) (err error) { //nolint:g
 				}
 			}
 		})); err != nil {
-			return fmt.Errorf("failed to add logical adapter to runnables: %w", err)
+			return fmt.Errorf("failed to add cluster provider to runnables: %w", err)
 		}
 	}
 

pkg/manager/manager.go

@@ -79,7 +79,7 @@ type Manager interface {
 	// lock was lost.
 	Start(ctx context.Context) error
 
-	// GetCluster retrieves a Cluster from a given logical name.
+	// GetCluster retrieves a Cluster from a given identifying cluster name.
 	GetCluster(ctx context.Context, clusterName string) (cluster.Cluster, error)
 
 	// GetWebhookServer returns a webhook.Server
@@ -277,10 +277,10 @@ type Options struct {
 	newHealthProbeListener func(addr string) (net.Listener, error)
 	newPprofListener       func(addr string) (net.Listener, error)
 
-	// logicalClusterProvider is an EXPERIMENTAL feature that allows the manager to
+	// clusterProvider is an EXPERIMENTAL feature that allows the manager to
 	// operate against many Kubernetes clusters at once.
-	// It can be used by invoking WithExperimentalLogicalAdapter(adapter) on Options.
-	logicalClusterProvider cluster.Provider
+	// It can be used by invoking WithExperimentalClusterProvider(adapter) on Options.
+	clusterProvider cluster.Provider
 }
 
 // BaseContextFunc is a function used to provide a base Context to Runnables
@@ -422,8 +422,8 @@ func New(config *rest.Config, options Options) (Manager, error) {
 		stopProcedureEngaged:          ptr.To(int64(0)),
 		defaultCluster:                cluster,
 		defaultClusterOptions:         clusterOptions,
-		logicalProvider:               options.logicalClusterProvider,
-		clusters:                      make(map[string]*logicalCluster),
+		clusterProvider:               options.clusterProvider,
+		clusters:                      make(map[string]*engagedCluster),
 		runnables:                     newRunnables(options.BaseContext, errChan),
 		errChan:                       errChan,
 		recorderProvider:              recorderProvider,
@@ -531,13 +531,13 @@ func (o Options) AndFromOrDie(loader config.ControllerManagerConfiguration) Opti
 	return o
 }
 
-// WithExperimentalClusterProvider sets the logical adapter to use for the manager.
+// WithExperimentalClusterProvider sets the cluster adapter to use for the manager.
 // This is an EXPERIMENTAL feature that allows a Manager to be used against a fleet
 // of Kubernetes clusters.
 //
 // NOTE: The method signature may change or be removed in the future.
 func (o Options) WithExperimentalClusterProvider(provider cluster.Provider) Options {
-	o.logicalClusterProvider = provider
+	o.clusterProvider = provider
 	return o
 }
 

pkg/manager/manager_test.go

@@ -1849,8 +1849,8 @@ var _ = Describe("manger.Manager", func() {
 		Expect(m.GetAPIReader()).NotTo(BeNil())
 	})
 
-	Context("with logical clusters", func() {
-		It("should be able to create a manager with a logical cluster", func() {
+	Context("with cluster provider", func() {
+		It("should be able to create a manager with a cluster provider", func() {
 			adapter := &fakeClusterAdapter{
 				clusterNameList: []string{
 					"test-cluster",
@@ -2027,7 +2027,7 @@ func (f *fakeClusterAdapter) Get(ctx context.Context, clusterName string, opts .
 	return cluster.New(testenv.Config, opts...)
 }
 
-func (f *fakeClusterAdapter) List() ([]string, error) {
+func (f *fakeClusterAdapter) List(ctx context.Context) ([]string, error) {
 	return f.clusterNameList, f.listErr
 }