From 5d2e55446d58bc47b262b7aeda3dc130fe59c62b Mon Sep 17 00:00:00 2001 From: Robert Vasek Date: Tue, 28 Oct 2025 20:12:04 +0100 Subject: [PATCH] CachedResourceEndpointSlice reconciler shouldn't retry on success On-behalf-of: @SAP robert.vasek@sap.com Signed-off-by: Robert Vasek --- .../cachedresourceendpointslice_controller.go | 17 ++++------ .../cachedresourceendpointslice_reconcile.go | 33 +++++-------------- ...hedresourceendpointslice_reconcile_test.go | 2 +- 3 files changed, 17 insertions(+), 35 deletions(-) diff --git a/pkg/reconciler/cache/cachedresourceendpointslice/cachedresourceendpointslice_controller.go b/pkg/reconciler/cache/cachedresourceendpointslice/cachedresourceendpointslice_controller.go index 6abce533538..dd3228dab03 100644 --- a/pkg/reconciler/cache/cachedresourceendpointslice/cachedresourceendpointslice_controller.go +++ b/pkg/reconciler/cache/cachedresourceendpointslice/cachedresourceendpointslice_controller.go @@ -253,30 +253,27 @@ func (c *controller) processNextWorkItem(ctx context.Context) bool { // other workers. defer c.queue.Done(key) - if requeue, err := c.process(ctx, key); err != nil { + if err := c.process(ctx, key); err != nil { utilruntime.HandleError(fmt.Errorf("%q controller failed to sync %q, err: %w", ControllerName, key, err)) c.queue.AddRateLimited(key) return true - } else if requeue { - c.queue.Add(key) - return true } c.queue.Forget(key) return true } -func (c *controller) process(ctx context.Context, key string) (bool, error) { +func (c *controller) process(ctx context.Context, key string) error { clusterName, _, name, err := kcpcache.SplitMetaClusterNamespaceKey(key) if err != nil { utilruntime.HandleError(err) - return false, nil + return nil } obj, err := c.getCachedResourceEndpointSlice(clusterName.Path(), name) if err != nil { if errors.IsNotFound(err) { - return false, nil // object deleted before we handled it + return nil // object deleted before we handled it } - return false, err + return err } old := obj @@ -286,7 +283,7 @@ func (c *controller) process(ctx context.Context, key string) (bool, error) { ctx = klog.NewContext(ctx, logger) var errs []error - requeue, err := c.reconcile(ctx, obj) + err = c.reconcile(ctx, obj) if err != nil { errs = append(errs, err) } @@ -302,7 +299,7 @@ func (c *controller) process(ctx context.Context, key string) (bool, error) { errs = append(errs, err) } - return requeue, utilerrors.NewAggregate(errs) + return utilerrors.NewAggregate(errs) } // InstallIndexers adds the additional indexers that this controller requires to the informers. diff --git a/pkg/reconciler/cache/cachedresourceendpointslice/cachedresourceendpointslice_reconcile.go b/pkg/reconciler/cache/cachedresourceendpointslice/cachedresourceendpointslice_reconcile.go index 535ed2d313c..f79a5e964d9 100644 --- a/pkg/reconciler/cache/cachedresourceendpointslice/cachedresourceendpointslice_reconcile.go +++ b/pkg/reconciler/cache/cachedresourceendpointslice/cachedresourceendpointslice_reconcile.go @@ -27,25 +27,10 @@ import ( cachev1alpha1 "github.com/kcp-dev/sdk/apis/cache/v1alpha1" conditionsv1alpha1 "github.com/kcp-dev/sdk/apis/third_party/conditions/apis/conditions/v1alpha1" "github.com/kcp-dev/sdk/apis/third_party/conditions/util/conditions" - topologyv1alpha1 "github.com/kcp-dev/sdk/apis/topology/v1alpha1" ) -func (c *controller) reconcile(ctx context.Context, endpoints *cachev1alpha1.CachedResourceEndpointSlice) (bool, error) { - r := &endpointsReconciler{ - getCachedResource: c.getCachedResource, - getPartition: c.getPartition, - } - - return r.reconcile(ctx, endpoints) -} - -type endpointsReconciler struct { - getCachedResource func(path logicalcluster.Path, name string) (*cachev1alpha1.CachedResource, error) - getPartition func(clusterName logicalcluster.Name, name string) (*topologyv1alpha1.Partition, error) -} - -func (r *endpointsReconciler) reconcile(ctx context.Context, endpoints *cachev1alpha1.CachedResourceEndpointSlice) (bool, error) { - _, err := r.getCachedResource(logicalcluster.From(endpoints).Path(), endpoints.Spec.CachedResource.Name) +func (c *controller) reconcile(ctx context.Context, endpoints *cachev1alpha1.CachedResourceEndpointSlice) error { + _, err := c.getCachedResource(logicalcluster.From(endpoints).Path(), endpoints.Spec.CachedResource.Name) if err != nil { if apierrors.IsNotFound(err) { // Don't keep the endpoints if the CachedResource has been deleted. @@ -60,7 +45,7 @@ func (r *endpointsReconciler) reconcile(ctx context.Context, endpoints *cachev1a endpoints.Spec.CachedResource.Name, ) // No need to try again. - return false, nil + return nil } else { conditions.MarkFalse( endpoints, @@ -71,7 +56,7 @@ func (r *endpointsReconciler) reconcile(ctx context.Context, endpoints *cachev1a logicalcluster.From(endpoints), endpoints.Spec.CachedResource.Name, ) - return true, err + return err } } conditions.MarkTrue(endpoints, cachev1alpha1.CachedResourceValid) @@ -79,7 +64,7 @@ func (r *endpointsReconciler) reconcile(ctx context.Context, endpoints *cachev1a // Check the partition selector. var selector labels.Selector if endpoints.Spec.Partition != "" { - partition, err := r.getPartition(logicalcluster.From(endpoints), endpoints.Spec.Partition) + partition, err := c.getPartition(logicalcluster.From(endpoints), endpoints.Spec.Partition) if err != nil { if apierrors.IsNotFound(err) { // Don't keep the endpoints if the Partition has been deleted and is still referenced. @@ -93,7 +78,7 @@ func (r *endpointsReconciler) reconcile(ctx context.Context, endpoints *cachev1a err, ) // No need to try again. - return false, nil + return nil } else { conditions.MarkFalse( endpoints, @@ -103,7 +88,7 @@ func (r *endpointsReconciler) reconcile(ctx context.Context, endpoints *cachev1a "%v", err, ) - return true, err + return err } } selector, err = metav1.LabelSelectorAsSelector(partition.Spec.Selector) @@ -116,7 +101,7 @@ func (r *endpointsReconciler) reconcile(ctx context.Context, endpoints *cachev1a "%v", err, ) - return true, err + return err } } if selector == nil { @@ -127,5 +112,5 @@ func (r *endpointsReconciler) reconcile(ctx context.Context, endpoints *cachev1a endpoints.Status.ShardSelector = selector.String() - return true, err + return nil } diff --git a/pkg/reconciler/cache/cachedresourceendpointslice/cachedresourceendpointslice_reconcile_test.go b/pkg/reconciler/cache/cachedresourceendpointslice/cachedresourceendpointslice_reconcile_test.go index 9d2d5e68939..54eb1ac712f 100644 --- a/pkg/reconciler/cache/cachedresourceendpointslice/cachedresourceendpointslice_reconcile_test.go +++ b/pkg/reconciler/cache/cachedresourceendpointslice/cachedresourceendpointslice_reconcile_test.go @@ -124,7 +124,7 @@ func TestReconcile(t *testing.T) { Partition: "my-partition", }, } - _, err := c.reconcile(context.Background(), cachedResourceEndpointSlice) + err := c.reconcile(context.Background(), cachedResourceEndpointSlice) if tc.wantError { require.Error(t, err, "expected an error") } else {