SQUASH: fixes and cleanup

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

Author: sttts

pkg/cluster/cluster.go

@@ -69,7 +69,9 @@ type ByNameGetterFunc func(ctx context.Context, clusterName string) (Cluster, er
 
 // Cluster provides various methods to interact with a cluster.
 type Cluster interface {
-	// Name returns the identifying name of the cluster.
+	// Name returns the name of the cluster. It identifies the cluster in the
+	// manager if that is attached to a cluster provider. The value is usually
+	// empty for the default cluster of a manager.
 	Name() string
 
 	// GetHTTPClient returns an HTTP client that can be used to talk to the apiserver
@@ -110,7 +112,9 @@ type Cluster interface {
 
 // Options are the possible options that can be configured for a Cluster.
 type Options struct {
-	// Name is the identifying name of the cluster.
+	// name is the name of the cluster. It identifies the cluster in the manager
+	// if that is attached to a cluster provider. The value is usually empty for
+	// the default cluster of a manager.
 	Name string
 
 	// Scheme is the scheme used to resolve runtime.Objects to GroupVersionKinds / Resources
@@ -195,6 +199,10 @@ func New(config *rest.Config, opts ...Option) (Cluster, error) {
 		return nil, errors.New("must specify Config")
 	}
 
+	// the config returned by GetConfig() is not to be modified. Hence, we have
+	// copy it before modifying it.
+	originalConfig := config
+
 	config = rest.CopyConfig(config)
 	if config.UserAgent == "" {
 		config.UserAgent = rest.DefaultKubernetesUserAgent()
@@ -284,7 +292,7 @@ func New(config *rest.Config, opts ...Option) (Cluster, error) {
 
 	return &cluster{
 		name:             options.Name,
-		config:           config,
+		config:           originalConfig,
 		httpClient:       options.HTTPClient,
 		scheme:           options.Scheme,
 		cache:            cache,
@@ -351,7 +359,7 @@ func setOptionsDefaults(options Options, config *rest.Config) (Options, error) {
 	return options, nil
 }
 
-// WithName sets the name of the cluster.
+// WithName sets the name of the cluster. The name can only be set once.
 func WithName(name string) Option {
 	return func(o *Options) {
 		if o.Name != "" {

pkg/cluster/internal.go

@@ -32,6 +32,9 @@ import (
 )
 
 type cluster struct {
+	// name is the name of the cluster. It identifies the cluster in the manager
+	// if that is attached to a cluster provider. The value is usually empty for
+	// the default cluster of a manager.
 	name string
 
 	// config is the rest.config used to talk to the apiserver.  Required.

pkg/handler/enqueue_mapped.go

@@ -49,6 +49,7 @@ func EnqueueRequestsFromMapFunc(fn MapFunc) EventHandler {
 var _ EventHandler = &enqueueRequestsFromMapFunc{}
 
 type enqueueRequestsFromMapFunc struct {
+	// cluster is the source of the requeue request.
 	cluster cluster.Cluster
 
 	// Mapper transforms the argument into a slice of keys to be reconciled

pkg/handler/enqueue_owner.go

@@ -70,6 +70,7 @@ func OnlyControllerOwner() OwnerOption {
 }
 
 type enqueueRequestForOwner struct {
+	// cluster is the source of the requeue request.
 	cluster cluster.Cluster
 
 	// ownerType is the type of the Owner object to look for in OwnerReferences.  Only Group and Kind are compared.

pkg/manager/internal.go

@@ -331,7 +331,7 @@ func (cm *controllerManager) getCluster(ctx context.Context, clusterName string)
 		ctx, cancel := context.WithTimeout(ctx, 10*time.Second)
 		defer cancel()
 		var watchErr error
-		if err := wait.PollImmediateUntilWithContext(ctx, 1*time.Second, func(ctx context.Context) (done bool, err error) {
+		if err := wait.PollUntilContextCancel(ctx, 1*time.Second, true, func(ctx context.Context) (done bool, err error) {
 			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.
@@ -350,8 +350,10 @@ func (cm *controllerManager) getCluster(ctx context.Context, clusterName string)
 	if err := cm.Add(cl); err != nil {
 		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 = &engagedCluster{
 		Cluster: cl,
 		ctx:     ctx,
@@ -361,8 +363,8 @@ func (cm *controllerManager) getCluster(ctx context.Context, clusterName string)
 	return c, nil
 }
 
-func (cm *controllerManager) removeLogicalCluster(clusterName string) error {
-	// Check if the manager was configured with a logical adapter,
+func (cm *controllerManager) removeNamedCluster(clusterName string) error {
+	// Check if the manager was configured with a cluster provider,
 	// otherwise we cannot retrieve the cluster.
 	if cm.clusterProvider == nil {
 		return fmt.Errorf("manager was not configured with a cluster provider, cannot retrieve %q", clusterName)
@@ -596,7 +598,7 @@ func (cm *controllerManager) Start(ctx context.Context) (err error) { //nolint:g
 					if clusterListSet.Has(name) {
 						continue
 					}
-					if err := cm.removeLogicalCluster(name); err != nil {
+					if err := cm.removeNamedCluster(name); err != nil {
 						return err
 					}
 				}
@@ -637,7 +639,7 @@ func (cm *controllerManager) Start(ctx context.Context) (err error) { //nolint:g
 						}
 						cm.engageClusterAwareRunnables()
 					case watch.Deleted, watch.Error:
-						if err := cm.removeLogicalCluster(event.ClusterName); err != nil {
+						if err := cm.removeNamedCluster(event.ClusterName); err != nil {
 							return err
 						}
 					case watch.Bookmark:

pkg/manager/manager.go

@@ -280,6 +280,7 @@ type Options struct {
 	// clusterProvider is an EXPERIMENTAL feature that allows the manager to
 	// operate against many Kubernetes clusters at once.
 	// It can be used by invoking WithExperimentalClusterProvider(adapter) on Options.
+	// Individual clusters can be accessed by calling GetCluster on the Manager.
 	clusterProvider cluster.Provider
 }
 

pkg/reconcile/reconcile.go

@@ -60,7 +60,7 @@ func (r Request) String() string {
 	if r.ClusterName == "" {
 		return r.NamespacedName.String()
 	}
-	return "logical://" + string(r.ClusterName) + string(types.Separator) + r.NamespacedName.String()
+	return "cluster://" + string(r.ClusterName) + string(types.Separator) + r.NamespacedName.String()
 }
 
 /*