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 463ee55121cd1b5403db2e510703bae2cdf8896b Author: chaojixinren <[email protected]> AuthorDate: Thu Apr 16 23:32:28 2026 +0800 chore(controller): move integration action declarations from reconcile to init Move the integration controller's action arrays from function-scoped variables recreated on every reconcile to struct fields initialized once at reconciler creation. Use a shared baseActions slice to eliminate duplication between the synthetic and non-synthetic action lists. Drop the catalog controller changes from this PR as requested by the reviewer — any concurrency optimization there will be handled in a separate PR. Remove trivial action-type-only tests for integrationkit, integrationplatform, and pipe controllers that provided no real business logic coverage. Restore the original integration controller tests (TestIsIntegrationUpdated, TestReadinessTimestampCalculation, TestDeploymentTimestampIsSet) that were inadvertently removed. --- pkg/controller/catalog/action.go | 17 +- pkg/controller/catalog/catalog_controller.go | 26 +-- pkg/controller/catalog/catalog_controller_test.go | 41 ---- pkg/controller/catalog/initialize.go | 2 +- .../integration/integration_controller.go | 36 ++-- .../integration/integration_controller_test.go | 210 ++++++++++++++++++--- .../integrationkit_controller_test.go | 42 ----- .../integrationplatform_controller_test.go | 40 ---- pkg/controller/pipe/pipe_controller_test.go | 38 ---- 9 files changed, 215 insertions(+), 237 deletions(-) diff --git a/pkg/controller/catalog/action.go b/pkg/controller/catalog/action.go index b4ef17582..c8e52f1dc 100644 --- a/pkg/controller/catalog/action.go +++ b/pkg/controller/catalog/action.go @@ -25,11 +25,10 @@ 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 @@ -43,21 +42,13 @@ type Action interface { type baseAction struct { client client.Client + L log.Logger } func (action *baseAction) InjectClient(client client.Client) { action.client = client } -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) +func (action *baseAction) InjectLogger(log log.Logger) { + action.L = log } diff --git a/pkg/controller/catalog/catalog_controller.go b/pkg/controller/catalog/catalog_controller.go index dbf36afc9..58aa4eacb 100644 --- a/pkg/controller/catalog/catalog_controller.go +++ b/pkg/controller/catalog/catalog_controller.go @@ -57,7 +57,6 @@ func newReconciler(mgr manager.Manager, c client.Client) reconcile.Reconciler { client: c, scheme: mgr.GetScheme(), recorder: mgr.GetEventRecorder("camel-k-catalog-controller"), - actions: newCatalogActions(c), }, schema.GroupVersionKind{ Group: v1.SchemeGroupVersion.Group, @@ -67,19 +66,6 @@ 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"). @@ -121,7 +107,6 @@ type reconcileCatalog struct { 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 @@ -165,14 +150,21 @@ func (r *reconcileCatalog) Reconcile(ctx context.Context, request reconcile.Requ return reconcile.Result{}, nil } + actions := []Action{ + NewInitializeAction(), + NewMonitorAction(), + } + var targetPhase v1.CamelCatalogPhase var err error target := instance.DeepCopy() targetLog := rlog.ForCatalog(target) - ctx = contextWithLogger(ctx, targetLog) - for _, a := range r.actions { + for _, a := range actions { + a.InjectClient(r.client) + a.InjectLogger(targetLog) + if !a.CanHandle(target) { continue } diff --git a/pkg/controller/catalog/catalog_controller_test.go b/pkg/controller/catalog/catalog_controller_test.go deleted file mode 100644 index 73359471a..000000000 --- a/pkg/controller/catalog/catalog_controller_test.go +++ /dev/null @@ -1,41 +0,0 @@ -/* -Licensed to the Apache Software Foundation (ASF) under one or more -contributor license agreements. See the NOTICE file distributed with -this work for additional information regarding copyright ownership. -The ASF licenses this file to You under the Apache License, Version 2.0 -(the "License"); you may not use this file except in compliance with -the License. You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package catalog - -import ( - "testing" - - "github.com/apache/camel-k/v2/pkg/internal" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestActions(t *testing.T) { - c, err := internal.NewFakeClient() - require.NoError(t, err) - - reconciler := reconcileCatalog{ - actions: newCatalogActions(c), - } - - 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 68dec59ff..e28e4f343 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) { - loggerFromContext(ctx, catalog).Info("Initializing CamelCatalog") + action.L.Info("Initializing CamelCatalog") platform, err := platformutil.GetForName(ctx, action.client, catalog.Namespace, defaults.OperatorID()) if err != nil { diff --git a/pkg/controller/integration/integration_controller.go b/pkg/controller/integration/integration_controller.go index 3f0b58084..1b415efe1 100644 --- a/pkg/controller/integration/integration_controller.go +++ b/pkg/controller/integration/integration_controller.go @@ -74,32 +74,20 @@ func Add(ctx context.Context, mgr manager.Manager, c client.Client) error { } func newReconciler(mgr manager.Manager, c client.Client) reconcile.Reconciler { + baseActions := []Action{ + NewPlatformSetupAction(), + NewInitializeAction(), + NewBuildAction(), + newBuildKitAction(), + NewBuildCompleteAction(), + } return monitoring.NewInstrumentedReconciler( &reconcileIntegration{ - client: c, - scheme: mgr.GetScheme(), - recorder: mgr.GetEventRecorder("camel-k-integration-controller"), - syntheticActions: append( - []Action{ - NewPlatformSetupAction(), - NewInitializeAction(), - NewBuildAction(), - newBuildKitAction(), - NewBuildCompleteAction(), - }, - NewMonitorSyntheticAction(), - ), - nonSyntheticActions: append( - []Action{ - NewPlatformSetupAction(), - NewInitializeAction(), - NewBuildAction(), - newBuildKitAction(), - NewBuildCompleteAction(), - }, - NewMonitorAction(), - NewMonitorUnknownAction(), - ), + client: c, + scheme: mgr.GetScheme(), + recorder: mgr.GetEventRecorder("camel-k-integration-controller"), + syntheticActions: append(append([]Action{}, baseActions...), NewMonitorSyntheticAction()), + nonSyntheticActions: append(append([]Action{}, baseActions...), NewMonitorAction(), NewMonitorUnknownAction()), }, schema.GroupVersionKind{ Group: v1.SchemeGroupVersion.Group, diff --git a/pkg/controller/integration/integration_controller_test.go b/pkg/controller/integration/integration_controller_test.go index e6e2fe65b..318dfb528 100644 --- a/pkg/controller/integration/integration_controller_test.go +++ b/pkg/controller/integration/integration_controller_test.go @@ -19,34 +19,27 @@ package integration import ( "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + v1 "github.com/apache/camel-k/v2/pkg/apis/camel/v1" ) func TestActions(t *testing.T) { + baseActions := []Action{ + NewPlatformSetupAction(), + NewInitializeAction(), + NewBuildAction(), + newBuildKitAction(), + NewBuildCompleteAction(), + } reconciler := reconcileIntegration{ - syntheticActions: append( - []Action{ - NewPlatformSetupAction(), - NewInitializeAction(), - NewBuildAction(), - newBuildKitAction(), - NewBuildCompleteAction(), - }, - NewMonitorSyntheticAction(), - ), - nonSyntheticActions: append( - []Action{ - NewPlatformSetupAction(), - NewInitializeAction(), - NewBuildAction(), - newBuildKitAction(), - NewBuildCompleteAction(), - }, - NewMonitorAction(), - NewMonitorUnknownAction(), - ), + syntheticActions: append(append([]Action{}, baseActions...), NewMonitorSyntheticAction()), + nonSyntheticActions: append(append([]Action{}, baseActions...), NewMonitorAction(), NewMonitorUnknownAction()), } t.Run("non-synthetic", func(t *testing.T) { @@ -74,3 +67,178 @@ func TestActions(t *testing.T) { assert.IsType(t, &monitorSyntheticAction{}, actions[5]) }) } + +func TestIsIntegrationUpdated(t *testing.T) { + now := metav1.Now() + + tests := []struct { + name string + it *v1.Integration + previous *v1.IntegrationCondition + next *v1.IntegrationCondition + expected bool + }{ + { + name: "should return true when transitioning to ready", + it: &v1.Integration{ + Status: v1.IntegrationStatus{ + InitializationTimestamp: &now, + }, + }, + previous: nil, + next: &v1.IntegrationCondition{ + Status: corev1.ConditionTrue, + FirstTruthyTime: &now, + }, + expected: true, + }, + { + name: "should return false when no InitializationTimestamp", + it: &v1.Integration{ + Status: v1.IntegrationStatus{}, + }, + previous: nil, + next: &v1.IntegrationCondition{ + Status: corev1.ConditionTrue, + FirstTruthyTime: &now, + }, + expected: false, + }, + { + name: "should return false when already ready", + it: &v1.Integration{ + Status: v1.IntegrationStatus{ + InitializationTimestamp: &now, + }, + }, + previous: &v1.IntegrationCondition{ + Status: corev1.ConditionTrue, + FirstTruthyTime: &now, + }, + next: &v1.IntegrationCondition{ + Status: corev1.ConditionTrue, + FirstTruthyTime: &now, + }, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := isIntegrationUpdated(tt.it, tt.previous, tt.next) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestReadinessTimestampCalculation(t *testing.T) { + initTime := metav1.NewTime(time.Date(2025, 1, 1, 10, 0, 0, 0, time.UTC)) + deployTime := metav1.NewTime(time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC)) + readyTime := metav1.NewTime(time.Date(2025, 1, 1, 12, 5, 0, 0, time.UTC)) + zeroTime := metav1.Time{} + + tests := []struct { + name string + initializationTimestamp *metav1.Time + deploymentTimestamp *metav1.Time + firstTruthyTime *metav1.Time + expectedDuration time.Duration + description string + }{ + { + name: "normal build - uses InitializationTimestamp", + initializationTimestamp: &initTime, + deploymentTimestamp: nil, + firstTruthyTime: &readyTime, + expectedDuration: 2*time.Hour + 5*time.Minute, + description: "Without DeploymentTimestamp, should use InitializationTimestamp", + }, + { + name: "dry build - uses DeploymentTimestamp", + initializationTimestamp: &initTime, + deploymentTimestamp: &deployTime, + firstTruthyTime: &readyTime, + expectedDuration: 5 * time.Minute, + description: "With DeploymentTimestamp, should use it instead of InitializationTimestamp", + }, + { + name: "DeploymentTimestamp is zero - falls back to InitializationTimestamp", + initializationTimestamp: &initTime, + deploymentTimestamp: &zeroTime, + firstTruthyTime: &readyTime, + expectedDuration: 2*time.Hour + 5*time.Minute, + description: "Zero DeploymentTimestamp should fall back to InitializationTimestamp", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + it := &v1.Integration{ + Status: v1.IntegrationStatus{ + InitializationTimestamp: tt.initializationTimestamp, + DeploymentTimestamp: tt.deploymentTimestamp, + }, + } + + startTime := it.Status.InitializationTimestamp.Time + if it.Status.DeploymentTimestamp != nil && !it.Status.DeploymentTimestamp.IsZero() { + startTime = it.Status.DeploymentTimestamp.Time + } + duration := tt.firstTruthyTime.Sub(startTime) + assert.Equal(t, tt.expectedDuration, duration, tt.description) + }) + } +} + +func TestDeploymentTimestampIsSet(t *testing.T) { + tests := []struct { + name string + hasDontRunAnnotation bool + expectedPhase v1.IntegrationPhase + expectDeploymentTimestamp bool + }{ + { + name: "normal build sets DeploymentTimestamp", + hasDontRunAnnotation: false, + expectedPhase: v1.IntegrationPhaseDeploying, + expectDeploymentTimestamp: true, + }, + { + name: "dry build does not set DeploymentTimestamp yet", + hasDontRunAnnotation: true, + expectedPhase: v1.IntegrationPhaseBuildComplete, + expectDeploymentTimestamp: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + it := &v1.Integration{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{}, + }, + Status: v1.IntegrationStatus{}, + } + + if tt.hasDontRunAnnotation { + it.Annotations[v1.IntegrationDontRunAfterBuildAnnotation] = v1.IntegrationDontRunAfterBuildAnnotationTrueValue + } + + if it.Annotations[v1.IntegrationDontRunAfterBuildAnnotation] == v1.IntegrationDontRunAfterBuildAnnotationTrueValue { + it.Status.Phase = v1.IntegrationPhaseBuildComplete + } else { + now := metav1.Now().Rfc3339Copy() + it.Status.DeploymentTimestamp = &now + it.Status.Phase = v1.IntegrationPhaseDeploying + } + + assert.Equal(t, tt.expectedPhase, it.Status.Phase) + if tt.expectDeploymentTimestamp { + assert.NotNil(t, it.Status.DeploymentTimestamp) + assert.False(t, it.Status.DeploymentTimestamp.IsZero()) + } else { + assert.Nil(t, it.Status.DeploymentTimestamp) + } + }) + } +} diff --git a/pkg/controller/integrationkit/integrationkit_controller_test.go b/pkg/controller/integrationkit/integrationkit_controller_test.go deleted file mode 100644 index af47229e4..000000000 --- a/pkg/controller/integrationkit/integrationkit_controller_test.go +++ /dev/null @@ -1,42 +0,0 @@ -/* -Licensed to the Apache Software Foundation (ASF) under one or more -contributor license agreements. See the NOTICE file distributed with -this work for additional information regarding copyright ownership. -The ASF licenses this file to You under the Apache License, Version 2.0 -(the "License"); you may not use this file except in compliance with -the License. You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package integrationkit - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestActions(t *testing.T) { - reconciler := reconcileIntegrationKit{ - actions: []Action{ - NewInitializeAction(), - NewBuildAction(), - NewMonitorAction(), - NewErrorAction(), - }, - } - - require.Len(t, reconciler.actions, 4) - assert.IsType(t, &initializeAction{}, reconciler.actions[0]) - assert.IsType(t, &buildAction{}, reconciler.actions[1]) - assert.IsType(t, &monitorAction{}, reconciler.actions[2]) - assert.IsType(t, &errorAction{}, reconciler.actions[3]) -} diff --git a/pkg/controller/integrationplatform/integrationplatform_controller_test.go b/pkg/controller/integrationplatform/integrationplatform_controller_test.go deleted file mode 100644 index f06ab4698..000000000 --- a/pkg/controller/integrationplatform/integrationplatform_controller_test.go +++ /dev/null @@ -1,40 +0,0 @@ -/* -Licensed to the Apache Software Foundation (ASF) under one or more -contributor license agreements. See the NOTICE file distributed with -this work for additional information regarding copyright ownership. -The ASF licenses this file to You under the Apache License, Version 2.0 -(the "License"); you may not use this file except in compliance with -the License. You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package integrationplatform - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestActions(t *testing.T) { - reconciler := reconcileIntegrationPlatform{ - actions: []Action{ - NewInitializeAction(), - NewCreateAction(), - NewMonitorAction(), - }, - } - - require.Len(t, reconciler.actions, 3) - assert.IsType(t, &initializeAction{}, reconciler.actions[0]) - assert.IsType(t, &createAction{}, reconciler.actions[1]) - assert.IsType(t, &monitorAction{}, reconciler.actions[2]) -} diff --git a/pkg/controller/pipe/pipe_controller_test.go b/pkg/controller/pipe/pipe_controller_test.go deleted file mode 100644 index 56dafa3bc..000000000 --- a/pkg/controller/pipe/pipe_controller_test.go +++ /dev/null @@ -1,38 +0,0 @@ -/* -Licensed to the Apache Software Foundation (ASF) under one or more -contributor license agreements. See the NOTICE file distributed with -this work for additional information regarding copyright ownership. -The ASF licenses this file to You under the Apache License, Version 2.0 -(the "License"); you may not use this file except in compliance with -the License. You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package pipe - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestActions(t *testing.T) { - reconciler := ReconcilePipe{ - actions: []Action{ - NewInitializeAction(), - NewMonitorAction(), - }, - } - - require.Len(t, reconciler.actions, 2) - assert.IsType(t, &initializeAction{}, reconciler.actions[0]) - assert.IsType(t, &monitorAction{}, reconciler.actions[1]) -}
