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
The following commit(s) were added to refs/heads/main by this push:
new c8c513a86 fix(gc): Prevent false negative in dry-build detection with
replicas=0
c8c513a86 is described below
commit c8c513a86134623da5a813c59dc66de2970a648e
Author: Pranjul Kalsi <[email protected]>
AuthorDate: Mon Dec 15 22:15:39 2025 +0530
fix(gc): Prevent false negative in dry-build detection with replicas=0
---
e2e/common/cli/deploy_test.go | 35 +++++++++++++++++--
pkg/trait/gc.go | 37 ++++++++++++++++++--
pkg/trait/gc_test.go | 79 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 146 insertions(+), 5 deletions(-)
diff --git a/e2e/common/cli/deploy_test.go b/e2e/common/cli/deploy_test.go
index 49081bc20..6ed7be58a 100644
--- a/e2e/common/cli/deploy_test.go
+++ b/e2e/common/cli/deploy_test.go
@@ -104,7 +104,6 @@ func TestPipeBuildDontRun(t *testing.T) {
WithTransform(PipeConditionReason,
Equal("BuildComplete")))
})
t.Run("deploy the pipe", func(t *testing.T) {
- t.Skip("Skipping: deploy/undeploy pipe lifecycle needs
further investigation")
g.Expect(Kamel(t, ctx, "deploy", name, "-n",
ns).Execute()).To(Succeed())
g.Eventually(IntegrationPhase(t, ctx, ns, name),
TestTimeoutMedium).Should(Equal(v1.IntegrationPhaseRunning))
g.Eventually(PipePhase(t, ctx, ns, name),
TestTimeoutMedium).Should(Equal(v1.PipePhaseReady))
@@ -115,7 +114,6 @@ func TestPipeBuildDontRun(t *testing.T) {
g.Eventually(IntegrationLogs(t, ctx, ns, name),
TestTimeoutMedium).Should(ContainSubstring("HelloPipe"))
})
t.Run("undeploy the pipe", func(t *testing.T) {
- t.Skip("Skipping: deploy/undeploy pipe lifecycle needs
further investigation")
g.Expect(Kamel(t, ctx, "undeploy", name, "-n",
ns).Execute()).To(Succeed())
g.Eventually(IntegrationPhase(t, ctx, ns, name),
TestTimeoutMedium).Should(Equal(v1.IntegrationPhaseBuildComplete))
g.Eventually(PipePhase(t, ctx, ns, name),
TestTimeoutMedium).Should(Equal(v1.PipePhaseBuildComplete))
@@ -124,3 +122,36 @@ func TestPipeBuildDontRun(t *testing.T) {
})
})
}
+
+func TestHasNeverDeployedLogic(t *testing.T) {
+ t.Parallel()
+ WithNewTestNamespace(t, func(ctx context.Context, g *WithT, ns string) {
+ // Critical edge case: deployment with trait config (e.g.
replicas=0)
+ t.Run("deployment with trait config sets deployment timestamp",
func(t *testing.T) {
+ deployedName := RandomizedSuffixName("deployed")
+ g.Expect(KamelRun(t, ctx, ns, "files/yaml.yaml",
+ "--name", deployedName,
+ "--trait", "deployment.replicas=0",
+ ).Execute()).To(Succeed())
+
+ g.Eventually(func() v1.IntegrationPhase {
+ it := Integration(t, ctx, ns, deployedName)()
+ if it == nil {
+ return ""
+ }
+ return it.Status.Phase
+ }, TestTimeoutMedium).Should(Or(
+ Equal(v1.IntegrationPhaseDeploying),
+ Equal(v1.IntegrationPhaseRunning),
+ ))
+
+ it := Integration(t, ctx, ns, deployedName)()
+ g.Expect(it).NotTo(BeNil())
+ g.Expect(it.Status.DeploymentTimestamp).NotTo(BeNil(),
+ "ANY deployment (regardless of replica count)
MUST set DeploymentTimestamp - hasNeverDeployed returns false")
+
+ t.Logf("Integration deployed with spec.replicas=%v,
DeploymentTimestamp=%v",
+ it.Spec.Replicas, it.Status.DeploymentTimestamp)
+ })
+ })
+}
diff --git a/pkg/trait/gc.go b/pkg/trait/gc.go
index 7520cca9e..d3d8f39c2 100644
--- a/pkg/trait/gc.go
+++ b/pkg/trait/gc.go
@@ -119,7 +119,19 @@ func (t *gcTrait) Configure(e *Environment) (bool,
*TraitCondition, error) {
}
func (t *gcTrait) Apply(e *Environment) error {
- if e.Integration.GetGeneration() > 1 ||
e.IntegrationInPhase(v1.IntegrationPhaseBuildComplete) {
+ // Garbage collection runs when:
+ // 1. Generation > 1: resource was updated, clean up old generation
resources
+ // 2. BuildComplete phase AND integration has previously been deployed:
undeploy scenario
+ shouldRunGC := e.Integration.GetGeneration() > 1
+
+ if !shouldRunGC &&
e.IntegrationInPhase(v1.IntegrationPhaseBuildComplete) {
+ // Only run GC if integration was previously deployed (undeploy
case)
+ if !hasNeverDeployed(e.Integration) {
+ shouldRunGC = true
+ }
+ }
+
+ if shouldRunGC {
// Register a post action that deletes the existing resources
that are labelled
// with the previous integration generation(s).
// We make the assumption generation is a monotonically
increasing strictly positive integer,
@@ -190,8 +202,10 @@ func (t *gcTrait) garbageCollectResources(e *Environment)
error {
selector := labels.NewSelector().
Add(*integration)
- // Skip the generation checking when we undeploy (which requires
therefore to remove all dependent resources)
- if !e.IntegrationInPhase(v1.IntegrationPhaseBuildComplete) {
+ // On undeploy, delete all resources regardless of generation.
+ // On generation upgrade, filter to only delete old resources.
+ isUndeploying := e.IntegrationInPhase(v1.IntegrationPhaseBuildComplete)
&& !hasNeverDeployed(e.Integration)
+ if !isUndeploying {
selector = selector.Add(*generation)
}
@@ -251,6 +265,23 @@ func canBeDeleted(it *v1.Integration, u
unstructured.Unstructured) bool {
return false
}
+// hasNeverDeployed returns true if the integration has never been deployed.
+// Checks both DeploymentTimestamp and Ready condition for reliability.
+func hasNeverDeployed(integration *v1.Integration) bool {
+ // Primary check: DeploymentTimestamp is set when deployment is
triggered
+ if integration.Status.DeploymentTimestamp != nil &&
!integration.Status.DeploymentTimestamp.IsZero() {
+ return false // has been deployed
+ }
+
+ // Secondary check: Ready condition becomes true only after successful
deployment
+ readyCond :=
integration.Status.GetCondition(v1.IntegrationConditionReady)
+ if readyCond != nil && readyCond.FirstTruthyTime != nil &&
!readyCond.FirstTruthyTime.IsZero() {
+ return false
+ }
+
+ return true
+}
+
// getDeletableTypes returns the list of deletable types resources, inspecting
the rules for which the operator SA is allowed in the
// Integration namespace.
func (t *gcTrait) getDeletableTypes(e *Environment)
(map[schema.GroupVersionKind]struct{}, error) {
diff --git a/pkg/trait/gc_test.go b/pkg/trait/gc_test.go
index 40d77f1b4..f81d216f6 100644
--- a/pkg/trait/gc_test.go
+++ b/pkg/trait/gc_test.go
@@ -164,6 +164,10 @@ func TestGarbageCollectUndeploying(t *testing.T) {
gcTrait, environment := createNominalGCTest()
environment.Integration.Status.Phase = v1.IntegrationPhaseBuildComplete
+ // Simulate undeploy scenario: DeploymentTimestamp set means
integration was previously deployed
+ now := metav1.Now()
+ environment.Integration.Status.DeploymentTimestamp = &now
+
deployment := getIntegrationDeployment(environment.Integration)
gcTrait.Client, _ = internal.NewFakeClient(deployment)
@@ -388,3 +392,78 @@ func TestCanResourceBeDeleted(t *testing.T) {
)
assert.True(t, canBeDeleted(it, resThisItOwner))
}
+
+func TestHasNeverDeployed(t *testing.T) {
+ tests := []struct {
+ name string
+ deploymentTimestamp *metav1.Time
+ readyCondition *v1.IntegrationCondition
+ expected bool
+ }{
+ {
+ name: "never deployed - both checks nil",
+ deploymentTimestamp: nil,
+ readyCondition: nil,
+ expected: true,
+ },
+ {
+ name: "deployed - DeploymentTimestamp
set",
+ deploymentTimestamp: ptr.To(metav1.Now()),
+ readyCondition: nil,
+ expected: false,
+ },
+ {
+ name: "deployed - Ready FirstTruthyTime
set",
+ deploymentTimestamp: nil,
+ readyCondition: &v1.IntegrationCondition{
+ Type: v1.IntegrationConditionReady,
+ Status: corev1.ConditionTrue,
+ FirstTruthyTime: ptr.To(metav1.Now()),
+ },
+ expected: false,
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ integration := &v1.Integration{
+ Status: v1.IntegrationStatus{
+ DeploymentTimestamp:
tt.deploymentTimestamp,
+ },
+ }
+ if tt.readyCondition != nil {
+ integration.Status.Conditions =
[]v1.IntegrationCondition{*tt.readyCondition}
+ }
+
+ result := hasNeverDeployed(integration)
+ assert.Equal(t, tt.expected, result)
+ })
+ }
+}
+
+func TestGarbageCollectDryBuildSkipsGC(t *testing.T) {
+ gcTrait, environment := createNominalGCTest()
+ environment.Integration.Status.Phase = v1.IntegrationPhaseBuildComplete
+
+ environment.Integration.Status.DeploymentTimestamp = nil
+ environment.Integration.Status.Conditions = nil
+
+ deployment := getIntegrationDeployment(environment.Integration)
+ gcTrait.Client, _ = internal.NewFakeClient(deployment)
+
+ environment.Client = gcTrait.Client
+ resourceDeleted := false
+ fakeClient := gcTrait.Client.(*internal.FakeClient)
+ fakeClient.Intercept(&interceptor.Funcs{
+ Delete: func(ctx context.Context, client ctrl.WithWatch, obj
ctrl.Object, opts ...ctrl.DeleteOption) error {
+ resourceDeleted = true
+ return nil
+ },
+ })
+
+ err := gcTrait.Apply(environment)
+
+ require.NoError(t, err)
+ assert.Len(t, environment.PostActions, 0)
+ assert.False(t, resourceDeleted)
+}
