This is an automated email from the ASF dual-hosted git repository. squakez pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/camel-k.git
commit c457de4b002b753265466ac0ec044d243ca656c8 Author: chaojixinren <[email protected]> AuthorDate: Thu Apr 16 16:49:50 2026 +0800 Avoid mutable reconcile state on shared catalog actions Keep the catalog controller actions initialized once on the reconciler, but stop mutating shared action instances with request-scoped logger state during each reconcile. The client is injected once at action creation time, while the reconcile-specific logger is now passed through context so the concurrent catalog controller can safely reuse its action list. Constraint: catalog-controller runs with MaxConcurrentReconciles enabled Rejected: recreating actions on every reconcile | goes against the requested initialization-time action setup Confidence: high Scope-risk: narrow Reversibility: clean Directive: Shared controller actions must not retain per-request mutable state unless they are concurrency-safe Tested: go test ./pkg/controller/catalog Tested: go test -race ./pkg/controller/catalog Tested: go test ./pkg/controller/integration ./pkg/controller/integrationkit ./pkg/controller/pipe Not-tested: integrationplatform package tests in this environment (missing mvn) --- pkg/controller/catalog/action.go | 17 +++++++-- pkg/controller/catalog/catalog_controller.go | 45 ++++++++++------------- pkg/controller/catalog/catalog_controller_test.go | 25 ++++++------- pkg/controller/catalog/initialize.go | 2 +- 4 files changed, 45 insertions(+), 44 deletions(-) diff --git a/pkg/controller/catalog/action.go b/pkg/controller/catalog/action.go index c8e52f1dc..b4ef17582 100644 --- a/pkg/controller/catalog/action.go +++ b/pkg/controller/catalog/action.go @@ -25,10 +25,11 @@ import ( "github.com/apache/camel-k/v2/pkg/util/log" ) +type actionLoggerKey struct{} + // Action --. type Action interface { client.Injectable - log.Injectable // a user friendly name for the action Name() string @@ -42,13 +43,21 @@ type Action interface { type baseAction struct { client client.Client - L log.Logger } func (action *baseAction) InjectClient(client client.Client) { action.client = client } -func (action *baseAction) InjectLogger(log log.Logger) { - action.L = log +func contextWithLogger(ctx context.Context, logger log.Logger) context.Context { + return context.WithValue(ctx, actionLoggerKey{}, logger) +} + +func loggerFromContext(ctx context.Context, catalog *v1.CamelCatalog) log.Logger { + logger, ok := ctx.Value(actionLoggerKey{}).(log.Logger) + if ok { + return logger + } + + return Log.ForCatalog(catalog) } diff --git a/pkg/controller/catalog/catalog_controller.go b/pkg/controller/catalog/catalog_controller.go index 1294c82ed..dbf36afc9 100644 --- a/pkg/controller/catalog/catalog_controller.go +++ b/pkg/controller/catalog/catalog_controller.go @@ -45,8 +45,6 @@ const ( requeueAfterDuration = 5 * time.Second ) -type actionFactory func() Action - // Add creates a new Catalog Controller and adds it to the Manager. The Manager will set fields on the Controller // and Start it when the Manager is Started. func Add(ctx context.Context, mgr manager.Manager, c client.Client) error { @@ -59,10 +57,7 @@ func newReconciler(mgr manager.Manager, c client.Client) reconcile.Reconciler { client: c, scheme: mgr.GetScheme(), recorder: mgr.GetEventRecorder("camel-k-catalog-controller"), - actionFactories: []actionFactory{ - NewInitializeAction, - NewMonitorAction, - }, + actions: newCatalogActions(c), }, schema.GroupVersionKind{ Group: v1.SchemeGroupVersion.Group, @@ -72,6 +67,19 @@ func newReconciler(mgr manager.Manager, c client.Client) reconcile.Reconciler { ) } +func newCatalogActions(c client.Client) []Action { + actions := []Action{ + NewInitializeAction(), + NewMonitorAction(), + } + + for _, action := range actions { + action.InjectClient(c) + } + + return actions +} + func add(mgr manager.Manager, r reconcile.Reconciler) error { return builder.ControllerManagedBy(mgr). Named("catalog-controller"). @@ -110,19 +118,10 @@ var _ reconcile.Reconciler = &reconcileCatalog{} type reconcileCatalog struct { // This client, initialized using mgr.Client() above, is a split client // that reads objects from the cache and writes to the API server - client client.Client - scheme *runtime.Scheme - recorder events.EventRecorder - actionFactories []actionFactory -} - -func (r *reconcileCatalog) newActions() []Action { - actions := make([]Action, 0, len(r.actionFactories)) - for _, newAction := range r.actionFactories { - actions = append(actions, newAction()) - } - - return actions + client client.Client + scheme *runtime.Scheme + recorder events.EventRecorder + actions []Action } // Reconcile reads that state of the cluster for a catalog object and makes changes based @@ -166,18 +165,14 @@ func (r *reconcileCatalog) Reconcile(ctx context.Context, request reconcile.Requ return reconcile.Result{}, nil } - actions := r.newActions() - var targetPhase v1.CamelCatalogPhase var err error target := instance.DeepCopy() targetLog := rlog.ForCatalog(target) + ctx = contextWithLogger(ctx, targetLog) - for _, a := range actions { - a.InjectClient(r.client) - a.InjectLogger(targetLog) - + for _, a := range r.actions { if !a.CanHandle(target) { continue } diff --git a/pkg/controller/catalog/catalog_controller_test.go b/pkg/controller/catalog/catalog_controller_test.go index 7ed735228..73359471a 100644 --- a/pkg/controller/catalog/catalog_controller_test.go +++ b/pkg/controller/catalog/catalog_controller_test.go @@ -20,25 +20,22 @@ package catalog import ( "testing" + "github.com/apache/camel-k/v2/pkg/internal" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func TestNewActions(t *testing.T) { +func TestActions(t *testing.T) { + c, err := internal.NewFakeClient() + require.NoError(t, err) + reconciler := reconcileCatalog{ - actionFactories: []actionFactory{ - NewInitializeAction, - NewMonitorAction, - }, + actions: newCatalogActions(c), } - actions := reconciler.newActions() - require.Len(t, actions, 2) - - assert.IsType(t, &initializeAction{}, actions[0]) - assert.IsType(t, &monitorAction{}, actions[1]) - - nextActions := reconciler.newActions() - assert.NotSame(t, actions[0], nextActions[0]) - assert.NotSame(t, actions[1], nextActions[1]) + require.Len(t, reconciler.actions, 2) + assert.IsType(t, &initializeAction{}, reconciler.actions[0]) + assert.IsType(t, &monitorAction{}, reconciler.actions[1]) + assert.Same(t, c, reconciler.actions[0].(*initializeAction).client) + assert.Same(t, c, reconciler.actions[1].(*monitorAction).client) } diff --git a/pkg/controller/catalog/initialize.go b/pkg/controller/catalog/initialize.go index e28e4f343..68dec59ff 100644 --- a/pkg/controller/catalog/initialize.go +++ b/pkg/controller/catalog/initialize.go @@ -44,7 +44,7 @@ func (action *initializeAction) CanHandle(catalog *v1.CamelCatalog) bool { } func (action *initializeAction) Handle(ctx context.Context, catalog *v1.CamelCatalog) (*v1.CamelCatalog, error) { - action.L.Info("Initializing CamelCatalog") + loggerFromContext(ctx, catalog).Info("Initializing CamelCatalog") platform, err := platformutil.GetForName(ctx, action.client, catalog.Namespace, defaults.OperatorID()) if err != nil {
