Improve reconciler error handling (#59884)

Consider a "not found" error from a delete call to be a non-error.
This can happen if the resource expires naturally between the time
the reconciler checked for resources and processed the removal.
This commit is contained in:
Zac Bergquist
2025-10-06 16:21:39 +00:00
committed by GitHub
parent ae559fd5bc
commit 3340c142e7
2 changed files with 31 additions and 0 deletions
+10
View File
@@ -137,6 +137,8 @@ func (r *GenericReconciler[K, T]) Reconcile(ctx context.Context) error {
}
}
// TODO(zmb3): with a large number of resources, this can return a lengthy
// error message that is difficult to parse
return trace.NewAggregate(errs...)
}
@@ -154,6 +156,10 @@ func (r *GenericReconciler[K, T]) processRegisteredResource(ctx context.Context,
}
r.logger.InfoContext(ctx, "Resource was removed, deleting", "kind", kind, "name", key)
if err := r.cfg.OnDelete(ctx, registered); err != nil {
if trace.IsNotFound(err) {
r.logger.Log(ctx, logutils.TraceLevel, "Failed to delete resource", "kind", kind, "name", key, "err", err)
return nil
}
return trace.Wrap(err, "failed to delete %v %v", kind, key)
}
@@ -216,6 +222,10 @@ func (r *GenericReconciler[K, T]) processNewResource(ctx context.Context, curren
}
r.logger.InfoContext(ctx, "Existing resource updated and no longer matches, deleting", "name", key)
if err := r.cfg.OnDelete(ctx, registered); err != nil {
if trace.IsNotFound(err) {
r.logger.Log(ctx, logutils.TraceLevel, "Failed to delete resource", "kind", kind, "name", key, "err", err)
return nil
}
return trace.Wrap(err, "failed to delete %v %v", kind, key)
}
return nil
+21
View File
@@ -23,6 +23,7 @@ import (
"maps"
"testing"
"github.com/gravitational/trace"
"github.com/stretchr/testify/require"
headerv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/header/v1"
@@ -106,6 +107,26 @@ func TestReconciler(t *testing.T) {
registeredResources: []testResource{makeDynamicResource("res1", nil)},
onDeleteCalls: []testResource{makeDynamicResource("res1", nil)},
},
{
description: "removing a resource that doesn't exist is not an error",
selectors: []ResourceMatcher{{
Labels: types.Labels{"foo": []string{"bar"}},
}},
// Note the label change below. This means the resource no longer matches and should be removed.
registeredResources: []testResource{makeDynamicResource("res1", map[string]string{"foo": "bar"})},
newResources: []testResource{makeDynamicResource("res1", map[string]string{"baz": "quux"})},
// Simulate the resource having already expired from the backend.
configure: func(cfg *ReconcilerConfig[testResource]) {
originalDelete := cfg.OnDelete
cfg.OnDelete = func(ctx context.Context, tr testResource) error {
originalDelete(ctx, tr)
return trace.NotFound("resource does not exist")
}
},
onDeleteCalls: []testResource{makeDynamicResource("res1", map[string]string{"foo": "bar"})},
},
{
description: "resource with updated matching labels should be updated",
selectors: []ResourceMatcher{{