From 079bc5cef78e9a95160805a05209418430afcfed Mon Sep 17 00:00:00 2001 From: Pasquale Congiusti Date: Thu, 16 May 2024 16:43:22 +0200 Subject: [PATCH 1/2] fix(e2e): strengthen upgrade test We must identify if a rolled out Pod after an upgrade is turning ready as expected. If not we have a possible breaking compatibility change --- e2e/install/upgrade/cli_upgrade_test.go | 6 +++++- e2e/install/upgrade/olm_upgrade_test.go | 11 +++++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/e2e/install/upgrade/cli_upgrade_test.go b/e2e/install/upgrade/cli_upgrade_test.go index 90da1cb5ab..8f607fa925 100644 --- a/e2e/install/upgrade/cli_upgrade_test.go +++ b/e2e/install/upgrade/cli_upgrade_test.go @@ -98,7 +98,11 @@ func TestCLIOperatorUpgrade(t *testing.T) { g.Eventually(PlatformVersion(t, ctx, ns), TestTimeoutMedium).Should(Equal(defaults.Version)) // Check the Integration hasn't been upgraded - g.Consistently(IntegrationVersion(t, ctx, ns, name), 5*time.Second, 1*time.Second).Should(Equal(version)) + g.Consistently(IntegrationVersion(t, ctx, ns, name), 15*time.Second, 3*time.Second).Should(Equal(version)) + // Make sure that any Pod rollout is completing successfully + // otherwise we are probably in front of a non breaking compatibility change + g.Consistently(IntegrationConditionStatus(t, ctx, ns, name, v1.IntegrationConditionReady), + 2*time.Minute, 15*time.Second).Should(Equal(corev1.ConditionTrue)) // Force the Integration upgrade g.Expect(Kamel(t, ctx, "rebuild", name, "-n", ns).Execute()).To(Succeed()) diff --git a/e2e/install/upgrade/olm_upgrade_test.go b/e2e/install/upgrade/olm_upgrade_test.go index ac8bcf4721..7544ce7a8a 100644 --- a/e2e/install/upgrade/olm_upgrade_test.go +++ b/e2e/install/upgrade/olm_upgrade_test.go @@ -205,6 +205,13 @@ func TestOLMOperatorUpgrade(t *testing.T) { g.Consistently(IntegrationVersion(t, ctx, ns, name), 5*time.Second, 1*time.Second). Should(ContainSubstring(prevIPVersionPrefix)) + // Make sure that any Pod rollout is completing successfully + // otherwise we are probably in front of a non breaking compatibility change + g.Consistently(IntegrationConditionStatus(t, ctx, ns, name, v1.IntegrationConditionReady), + 2*time.Minute, 15*time.Second).Should(Equal(corev1.ConditionTrue)) + g.Consistently(IntegrationConditionStatus(t, ctx, ns, kbindName, v1.IntegrationConditionReady), + 2*time.Minute, 15*time.Second).Should(Equal(corev1.ConditionTrue)) + // Rebuild the Integration g.Expect(Kamel(t, ctx, "rebuild", "--all", "-n", ns).Execute()).To(Succeed()) if prevCSVVersion.Version.String() >= "2" { @@ -244,9 +251,9 @@ func TestOLMOperatorUpgrade(t *testing.T) { // Check the Integration runs correctly g.Eventually(IntegrationPodPhase(t, ctx, ns, name)).Should(Equal(corev1.PodRunning)) g.Eventually(IntegrationPodPhase(t, ctx, ns, kbindName)).Should(Equal(corev1.PodRunning)) - g.Eventually(IntegrationConditionStatus(t, ctx, ns, name, v1.IntegrationConditionReady), TestTimeoutLong). + g.Eventually(IntegrationConditionStatus(t, ctx, ns, name, v1.IntegrationConditionReady), TestTimeoutMedium). Should(Equal(corev1.ConditionTrue)) - g.Eventually(IntegrationConditionStatus(t, ctx, ns, kbindName, v1.IntegrationConditionReady), TestTimeoutLong). + g.Eventually(IntegrationConditionStatus(t, ctx, ns, kbindName, v1.IntegrationConditionReady), TestTimeoutMedium). Should(Equal(corev1.ConditionTrue)) // Clean up From 958b0417ab4c41a3834f7c99858ecd7fe20a1a36 Mon Sep 17 00:00:00 2001 From: Pasquale Congiusti Date: Thu, 16 May 2024 16:44:50 +0200 Subject: [PATCH 2/2] chore(trait): revert default health enabled It turned out to be a breaking compatibility change as the switch would require a forcefully rebuild that is not done automatically by the operator during upgrade procedures. --- e2e/common/traits/health_test.go | 33 +++++++++++++++---------------- pkg/apis/camel/v1/trait/health.go | 2 +- pkg/trait/health.go | 2 +- pkg/trait/health_test.go | 14 ++++++++----- pkg/trait/trait_test.go | 2 +- 5 files changed, 28 insertions(+), 25 deletions(-) diff --git a/e2e/common/traits/health_test.go b/e2e/common/traits/health_test.go index 015009fa2d..61d469d7f8 100644 --- a/e2e/common/traits/health_test.go +++ b/e2e/common/traits/health_test.go @@ -52,23 +52,12 @@ func TestHealthTrait(t *testing.T) { g.Eventually(SelectedPlatformPhase(t, ctx, ns, operatorID), TestTimeoutMedium).Should(Equal(v1.IntegrationPlatformPhaseReady)) - t.Run("Disabled health trait", func(t *testing.T) { - name := RandomizedSuffixName("java") - g.Expect(KamelRunWithID(t, ctx, operatorID, ns, "files/Java.java", "-t", "health.enabled=false", "--name", name).Execute()).To(Succeed()) - - g.Eventually(IntegrationPodPhase(t, ctx, ns, name), TestTimeoutLong).Should(Equal(corev1.PodRunning)) - g.Eventually(IntegrationPhase(t, ctx, ns, name), TestTimeoutShort).Should(Equal(v1.IntegrationPhaseRunning)) - g.Eventually(IntegrationConditionStatus(t, ctx, ns, name, v1.IntegrationConditionReady), TestTimeoutShort). - Should(Equal(corev1.ConditionTrue)) - g.Eventually(IntegrationLogs(t, ctx, ns, name), TestTimeoutShort).Should(ContainSubstring("Magicstring!")) - - // Clean-up - g.Expect(Kamel(t, ctx, "delete", "--all", "-n", ns).Execute()).To(Succeed()) - }) - t.Run("Readiness condition with stopped route scaled", func(t *testing.T) { name := RandomizedSuffixName("java") - g.Expect(KamelRunWithID(t, ctx, operatorID, ns, "files/Java.java", "-t", "jolokia.enabled=true", "-t", "jolokia.use-ssl-client-authentication=false", "-t", "jolokia.protocol=http", "--name", name).Execute()).To(Succeed()) + g.Expect(KamelRunWithID(t, ctx, operatorID, ns, "files/Java.java", + "-t", "health.enabled=true", + "-t", "jolokia.enabled=true", "-t", "jolokia.use-ssl-client-authentication=false", + "-t", "jolokia.protocol=http", "--name", name).Execute()).To(Succeed()) g.Eventually(IntegrationPodPhase(t, ctx, ns, name), TestTimeoutLong).Should(Equal(corev1.PodRunning)) g.Eventually(IntegrationPhase(t, ctx, ns, name), TestTimeoutShort).Should(Equal(v1.IntegrationPhaseRunning)) @@ -158,7 +147,10 @@ func TestHealthTrait(t *testing.T) { t.Run("Readiness condition with stopped route", func(t *testing.T) { name := RandomizedSuffixName("java") - g.Expect(KamelRunWithID(t, ctx, operatorID, ns, "files/Java.java", "-t", "jolokia.enabled=true", "-t", "jolokia.use-ssl-client-authentication=false", "-t", "jolokia.protocol=http", "--name", name).Execute()).To(Succeed()) + g.Expect(KamelRunWithID(t, ctx, operatorID, ns, "files/Java.java", + "-t", "health.enabled=true", + "-t", "jolokia.enabled=true", "-t", "jolokia.use-ssl-client-authentication=false", + "-t", "jolokia.protocol=http", "--name", name).Execute()).To(Succeed()) g.Eventually(IntegrationPodPhase(t, ctx, ns, name), TestTimeoutLong).Should(Equal(corev1.PodRunning)) g.Eventually(IntegrationPhase(t, ctx, ns, name), TestTimeoutShort).Should(Equal(v1.IntegrationPhaseRunning)) @@ -251,7 +243,12 @@ func TestHealthTrait(t *testing.T) { g.Expect(CreateTimerKamelet(t, ctx, operatorID, ns, source)()).To(Succeed()) g.Expect(CreateLogKamelet(t, ctx, operatorID, ns, sink)()).To(Succeed()) - g.Expect(KamelBindWithID(t, ctx, operatorID, ns, source, sink, "-p", "source.message=Magicstring!", "-p", "sink.loggerName=binding", "--annotation", "trait.camel.apache.org/health.enabled=true", "--annotation", "trait.camel.apache.org/jolokia.enabled=true", "--annotation", "trait.camel.apache.org/jolokia.use-ssl-client-authentication=false", "--annotation", "trait.camel.apache.org/jolokia.protocol=http", "--name", name).Execute()).To(Succeed()) + g.Expect(KamelBindWithID(t, ctx, operatorID, ns, source, sink, "-p", + "source.message=Magicstring!", "-p", "sink.loggerName=binding", + "--annotation", "trait.camel.apache.org/health.enabled=true", + "--annotation", "trait.camel.apache.org/jolokia.enabled=true", + "--annotation", "trait.camel.apache.org/jolokia.use-ssl-client-authentication=false", + "--annotation", "trait.camel.apache.org/jolokia.protocol=http", "--name", name).Execute()).To(Succeed()) g.Eventually(IntegrationPodPhase(t, ctx, ns, name), TestTimeoutLong).Should(Equal(corev1.PodRunning)) g.Eventually(IntegrationPhase(t, ctx, ns, name), TestTimeoutShort).Should(Equal(v1.IntegrationPhaseRunning)) @@ -414,6 +411,7 @@ func TestHealthTrait(t *testing.T) { name := RandomizedSuffixName("startup-probe-never-ready-route") g.Expect(KamelRunWithID(t, ctx, operatorID, ns, "files/NeverReady.java", "--name", name, + "-t", "health.enabled=true", "-t", "health.startup-probe-enabled=true", "-t", "health.startup-timeout=60").Execute()).To(Succeed()) g.Eventually(IntegrationPodPhase(t, ctx, ns, name), TestTimeoutMedium).Should(Equal(corev1.PodRunning)) @@ -463,6 +461,7 @@ func TestHealthTrait(t *testing.T) { name := RandomizedSuffixName("startup-probe-ready-route") g.Expect(KamelRunWithID(t, ctx, operatorID, ns, "files/Java.java", "--name", name, + "-t", "health.enabled=true", "-t", "health.startup-probe-enabled=true", "-t", "health.startup-timeout=60").Execute()).To(Succeed()) g.Eventually(IntegrationPodPhase(t, ctx, ns, name), TestTimeoutMedium).Should(Equal(corev1.PodRunning)) diff --git a/pkg/apis/camel/v1/trait/health.go b/pkg/apis/camel/v1/trait/health.go index a691afadfd..2d7e88870d 100644 --- a/pkg/apis/camel/v1/trait/health.go +++ b/pkg/apis/camel/v1/trait/health.go @@ -19,7 +19,7 @@ package trait // The health trait is responsible for configuring the health probes on the integration container. // -// NOTE: this trait is enabled by default. +// NOTE: this trait is disabled by default. // // +camel-k:trait=health. type HealthTrait struct { diff --git a/pkg/trait/health.go b/pkg/trait/health.go index 7c8d11132a..25d5401f97 100644 --- a/pkg/trait/health.go +++ b/pkg/trait/health.go @@ -71,7 +71,7 @@ func (t *healthTrait) Configure(e *Environment) (bool, *TraitCondition, error) { } } - return pointer.BoolDeref(t.Enabled, true), nil, nil + return pointer.BoolDeref(t.Enabled, false), nil, nil } func (t *healthTrait) Apply(e *Environment) error { diff --git a/pkg/trait/health_test.go b/pkg/trait/health_test.go index 571908f7ff..c9c306856d 100644 --- a/pkg/trait/health_test.go +++ b/pkg/trait/health_test.go @@ -119,28 +119,30 @@ func TestHealthTrait(t *testing.T) { assert.Equal(t, "/q/health/started", d.Spec.Template.Spec.Containers[0].StartupProbe.HTTPGet.Path) } -func TestConfigureHealthTraitDoesSucceed(t *testing.T) { +func TestConfigureHealthTraitDefault(t *testing.T) { ht, environment := createNominalHealthTrait(t) configured, condition, err := ht.Configure(environment) - assert.True(t, configured) + assert.False(t, configured) assert.Nil(t, err) assert.Nil(t, condition) } -func TestConfigureHealthTraitDisabled(t *testing.T) { - enabled := false +func TestConfigureHealthTraitEnabled(t *testing.T) { + enabled := true ht, environment := createNominalHealthTrait(t) ht.Enabled = &enabled configured, condition, err := ht.Configure(environment) - assert.False(t, configured) + assert.True(t, configured) assert.Nil(t, err) assert.Nil(t, condition) } func TestApplyHealthTraitDefault(t *testing.T) { + enabled := true ht, environment := createNominalHealthTrait(t) + ht.Enabled = &enabled configured, condition, err := ht.Configure(environment) assert.True(t, configured) assert.Nil(t, err) @@ -155,6 +157,7 @@ func TestApplyHealthTraitDefault(t *testing.T) { func TestApplyHealthTraitLivenessDefault(t *testing.T) { enabled := true ht, environment := createNominalHealthTrait(t) + ht.Enabled = &enabled ht.LivenessProbeEnabled = &enabled configured, condition, err := ht.Configure(environment) assert.True(t, configured) @@ -171,6 +174,7 @@ func TestApplyHealthTraitLivenessDefault(t *testing.T) { func TestApplyHealthTraitStartupDefault(t *testing.T) { enabled := true ht, environment := createNominalHealthTrait(t) + ht.Enabled = &enabled ht.StartupProbeEnabled = &enabled configured, condition, err := ht.Configure(environment) assert.True(t, configured) diff --git a/pkg/trait/trait_test.go b/pkg/trait/trait_test.go index 0b003abe8c..34f0ac5bc6 100644 --- a/pkg/trait/trait_test.go +++ b/pkg/trait/trait_test.go @@ -562,7 +562,7 @@ func TestExecutedTraitsCondition(t *testing.T) { v1.IntegrationConditionTraitInfo, corev1.ConditionTrue, "TraitConfiguration", - "Applied traits: camel,environment,logging,deployer,deployment,gc,container,security-context,mount,health,quarkus,jvm,owner", + "Applied traits: camel,environment,logging,deployer,deployment,gc,container,security-context,mount,quarkus,jvm,owner", ) assert.Contains(t, conditions, expectedCondition) }