This is an automated email from the ASF dual-hosted git repository. pcongiusti pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/camel-k.git
commit 892f1e86d1ece266cf89cb67c422505c291e5869 Author: Pranjul Kalsi <[email protected]> AuthorDate: Sat Dec 13 12:03:52 2025 +0530 fix: Correct first-readiness metric for dry-build integrations --- e2e/common/cli/deploy_test.go | 16 ++ pkg/apis/camel/v1/integration_types.go | 2 + pkg/cmd/deploy.go | 4 + pkg/controller/integration/build.go | 2 + pkg/controller/integration/build_kit.go | 5 + pkg/controller/integration/initialize.go | 2 + .../integration/integration_controller.go | 7 +- .../integration/integration_controller_test.go | 204 +++++++++++++++++++++ 8 files changed, 241 insertions(+), 1 deletion(-) diff --git a/e2e/common/cli/deploy_test.go b/e2e/common/cli/deploy_test.go index 3be66f585..49081bc20 100644 --- a/e2e/common/cli/deploy_test.go +++ b/e2e/common/cli/deploy_test.go @@ -48,8 +48,18 @@ func TestBuildDontRun(t *testing.T) { g.Eventually(IntegrationPhase(t, ctx, ns, name), TestTimeoutMedium).Should(Equal(v1.IntegrationPhaseBuildComplete)) g.Consistently(IntegrationPhase(t, ctx, ns, name), 10*time.Second).Should(Equal(v1.IntegrationPhaseBuildComplete)) g.Eventually(Deployment(t, ctx, ns, name)).Should(BeNil()) + + // Verify DeploymentTimestamp is NOT set before deploying (dry-build waits) + it := Integration(t, ctx, ns, name)() + g.Expect(it).NotTo(BeNil()) + g.Expect(it.Status.InitializationTimestamp).NotTo(BeNil()) + g.Expect(it.Status.DeploymentTimestamp).To(BeNil()) }) t.Run("deploy the integration", func(t *testing.T) { + // Capture InitializationTimestamp before deploying + itBeforeDeploy := Integration(t, ctx, ns, name)() + initTimestamp := itBeforeDeploy.Status.InitializationTimestamp + g.Expect(Kamel(t, ctx, "deploy", name, "-n", ns).Execute()).To(Succeed()) // The integration should run immediately g.Eventually(IntegrationPhase(t, ctx, ns, name), TestTimeoutShort).Should(Equal(v1.IntegrationPhaseRunning)) @@ -58,6 +68,12 @@ func TestBuildDontRun(t *testing.T) { g.Eventually(IntegrationConditionStatus(t, ctx, ns, name, v1.IntegrationConditionReady)). Should(Equal(corev1.ConditionTrue)) g.Eventually(IntegrationLogs(t, ctx, ns, name)).Should(ContainSubstring("Magicstring!")) + + // Verify DeploymentTimestamp is now set and is after InitializationTimestamp + it := Integration(t, ctx, ns, name)() + g.Expect(it).NotTo(BeNil()) + g.Expect(it.Status.DeploymentTimestamp).NotTo(BeNil()) + g.Expect(it.Status.DeploymentTimestamp.Time).To(BeTemporally(">", initTimestamp.Time)) }) t.Run("undeploy the integration", func(t *testing.T) { g.Expect(Kamel(t, ctx, "undeploy", name, "-n", ns).Execute()).To(Succeed()) diff --git a/pkg/apis/camel/v1/integration_types.go b/pkg/apis/camel/v1/integration_types.go index f52c76efe..da2c93b54 100644 --- a/pkg/apis/camel/v1/integration_types.go +++ b/pkg/apis/camel/v1/integration_types.go @@ -133,6 +133,8 @@ type IntegrationStatus struct { Capabilities []string `json:"capabilities,omitempty"` // the timestamp representing the last time when this integration was initialized. InitializationTimestamp *metav1.Time `json:"lastInitTimestamp,omitempty"` + // the timestamp representing the last time when this integration was deployed. + DeploymentTimestamp *metav1.Time `json:"lastDeploymentTimestamp,omitempty"` } // +kubebuilder:object:root=true diff --git a/pkg/cmd/deploy.go b/pkg/cmd/deploy.go index 51f04f385..f63d36b2c 100644 --- a/pkg/cmd/deploy.go +++ b/pkg/cmd/deploy.go @@ -23,6 +23,7 @@ import ( v1 "github.com/apache/camel-k/v2/pkg/apis/camel/v1" "github.com/spf13/cobra" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -73,6 +74,9 @@ func (o *deployCmdOptions) run(cmd *cobra.Command, args []string) error { } integration := existing.DeepCopy() + // Set DeploymentTimestamp to track when deployment was initiated + now := metav1.Now().Rfc3339Copy() + integration.Status.DeploymentTimestamp = &now integration.Status.Phase = v1.IntegrationPhaseDeploying patch := ctrl.MergeFrom(existing) diff --git a/pkg/controller/integration/build.go b/pkg/controller/integration/build.go index dea25beab..2192e7ca3 100644 --- a/pkg/controller/integration/build.go +++ b/pkg/controller/integration/build.go @@ -231,6 +231,8 @@ func (action *buildAction) handleBuildRunning(ctx context.Context, it *v1.Integr 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 } case v1.BuildPhaseError, v1.BuildPhaseInterrupted, v1.BuildPhaseFailed: diff --git a/pkg/controller/integration/build_kit.go b/pkg/controller/integration/build_kit.go index 7db1cabab..3f18a676d 100644 --- a/pkg/controller/integration/build_kit.go +++ b/pkg/controller/integration/build_kit.go @@ -25,6 +25,7 @@ import ( "github.com/apache/camel-k/v2/pkg/trait" "github.com/apache/camel-k/v2/pkg/util/digest" "github.com/apache/camel-k/v2/pkg/util/kubernetes" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func newBuildKitAction() Action { @@ -150,6 +151,8 @@ kits: if integration.Annotations[v1.IntegrationDontRunAfterBuildAnnotation] == v1.IntegrationDontRunAfterBuildAnnotationTrueValue { integration.Status.Phase = v1.IntegrationPhaseBuildComplete } else { + now := metav1.Now().Rfc3339Copy() + integration.Status.DeploymentTimestamp = &now integration.Status.Phase = v1.IntegrationPhaseDeploying } } @@ -211,6 +214,8 @@ func (action *buildKitAction) checkIntegrationKit(ctx context.Context, integrati if integration.Annotations[v1.IntegrationDontRunAfterBuildAnnotation] == v1.IntegrationDontRunAfterBuildAnnotationTrueValue { integration.Status.Phase = v1.IntegrationPhaseBuildComplete } else { + now := metav1.Now().Rfc3339Copy() + integration.Status.DeploymentTimestamp = &now integration.Status.Phase = v1.IntegrationPhaseDeploying } integration.SetIntegrationKit(kit) diff --git a/pkg/controller/integration/initialize.go b/pkg/controller/integration/initialize.go index ebd5f1ab0..65ae414b8 100644 --- a/pkg/controller/integration/initialize.go +++ b/pkg/controller/integration/initialize.go @@ -76,6 +76,8 @@ func (action *initializeAction) Handle(ctx context.Context, integration *v1.Inte if integration.Annotations[v1.IntegrationDontRunAfterBuildAnnotation] == v1.IntegrationDontRunAfterBuildAnnotationTrueValue { integration.Status.Phase = v1.IntegrationPhaseBuildComplete } else { + now := metav1.Now().Rfc3339Copy() + integration.Status.DeploymentTimestamp = &now integration.Status.Phase = v1.IntegrationPhaseDeploying } diff --git a/pkg/controller/integration/integration_controller.go b/pkg/controller/integration/integration_controller.go index e7f61fa49..96777000e 100644 --- a/pkg/controller/integration/integration_controller.go +++ b/pkg/controller/integration/integration_controller.go @@ -93,7 +93,12 @@ func integrationUpdateFunc(c client.Client, old *v1.Integration, it *v1.Integrat previous := old.Status.GetCondition(v1.IntegrationConditionReady) next := it.Status.GetCondition(v1.IntegrationConditionReady) if isIntegrationUpdated(it, previous, next) { - duration := next.FirstTruthyTime.Sub(it.Status.InitializationTimestamp.Time) + // Use DeploymentTimestamp if available (for dry-build), else use InitializationTimestamp + startTime := it.Status.InitializationTimestamp.Time + if it.Status.DeploymentTimestamp != nil && !it.Status.DeploymentTimestamp.IsZero() { + startTime = it.Status.DeploymentTimestamp.Time + } + duration := next.FirstTruthyTime.Sub(startTime) Log.WithValues("request-namespace", it.Namespace, "request-name", it.Name, "ready-after", duration.Seconds()). ForIntegration(it).Infof("First readiness after %s", duration) timeToFirstReadiness.Observe(duration.Seconds()) diff --git a/pkg/controller/integration/integration_controller_test.go b/pkg/controller/integration/integration_controller_test.go new file mode 100644 index 000000000..31403528b --- /dev/null +++ b/pkg/controller/integration/integration_controller_test.go @@ -0,0 +1,204 @@ +/* +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 integration + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + 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 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) + } + }) + } +}
