You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by pc...@apache.org on 2026/04/19 07:13:46 UTC
(camel-k) 04/06: chore(controller): move integration action declarations from reconcile to init
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 <ch...@163.com>
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])
-}