You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "lfabriko (via GitHub)" <gi...@apache.org> on 2023/04/12 09:32:42 UTC
[GitHub] [camel-k] lfabriko opened a new pull request, #4245: Add Kameletbinding test to olm_upgrade_tes
lfabriko opened a new pull request, #4245:
URL: https://github.com/apache/camel-k/pull/4245
<!-- Description -->
<!--
Enter your extended release note in the below block. If the PR requires
additional action from users switching to the new release, include the string
"action required". If no release note is required, write "NONE".
You can (optionally) mark this PR with labels "kind/bug" or "kind/feature" to make sure
the text is added to the right section of the release notes.
-->
**Release Note**
```release-note
NONE
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on pull request #4245: Add Kameletbinding test to olm_upgrade_tes
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #4245:
URL: https://github.com/apache/camel-k/pull/4245#issuecomment-1534238602
> May I ask, as branches `main` and `release-1.10.x` diverge in location of `olm_upgrade_test.go` and names of used functions (`C/clusterServiceVersion`), do I understand correctly I need to prepare another MR for branch `release-1.10.x`?
Yes, very likely.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] lfabriko commented on pull request #4245: Add Kameletbinding test to olm_upgrade_tes
Posted by "lfabriko (via GitHub)" <gi...@apache.org>.
lfabriko commented on PR #4245:
URL: https://github.com/apache/camel-k/pull/4245#issuecomment-1504976962
Sorry, it was by mistake.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on a diff in pull request #4245: Add Kameletbinding test to olm_upgrade_tes
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on code in PR #4245:
URL: https://github.com/apache/camel-k/pull/4245#discussion_r1163933550
##########
e2e/install/olm/olm_upgrade_test.go:
##########
@@ -196,40 +201,49 @@ func TestOLMAutomaticUpgrade(t *testing.T) {
// Check the Integration runs correctly
Eventually(IntegrationPodPhase(ns, name), TestTimeoutLong).Should(Equal(corev1.PodRunning))
+ Eventually(IntegrationPodPhase(ns, kbindName), TestTimeoutLong).Should(Equal(corev1.PodRunning))
+
Eventually(IntegrationConditionStatus(ns, name, v1.IntegrationConditionReady), TestTimeoutMedium).
Should(Equal(corev1.ConditionTrue))
+ Eventually(IntegrationConditionStatus(ns, kbindName, v1.IntegrationConditionReady), TestTimeoutMedium).Should(Equal(corev1.ConditionTrue))
// Check the Integration version has been upgraded
Eventually(IntegrationVersion(ns, name)).Should(ContainSubstring(newIPVersionPrefix))
+ Eventually(IntegrationVersion(ns, kbindName)).Should(ContainSubstring(newIPVersionPrefix))
// Check the previous kit is not garbage collected (skip Build - present in case of respin)
prevCSVVersionMajorMinorPatch := fmt.Sprintf("%d.%d.%d",
prevCSVVersion.Version.Major, prevCSVVersion.Version.Minor, prevCSVVersion.Version.Patch)
- Eventually(Kits(ns, KitWithVersion(prevCSVVersionMajorMinorPatch))).Should(HaveLen(1))
+ Eventually(Kits(ns, KitWithVersion(prevCSVVersionMajorMinorPatch))).Should(HaveLen(2))
// Check a new kit is created with the current version
Eventually(Kits(ns, KitWithVersion(defaults.Version)),
- TestTimeoutMedium).Should(HaveLen(1))
+ TestTimeoutMedium).Should(HaveLen(2))
// Check the new kit is ready
Eventually(Kits(ns, KitWithVersion(defaults.Version), KitWithPhase(v1.IntegrationKitPhaseReady)),
TestTimeoutMedium).Should(HaveLen(1))
- kit := Kits(ns, KitWithVersion(defaults.Version))()[0]
+ kit := Kits(ns, KitWithVersion(defaults.Version), KitWithLabels(map[string]string{"camel.apache.org/created.by.name": name}))()[0]
+ kitKbind := Kits(ns, KitWithVersion(defaults.Version), KitWithLabels(map[string]string{"camel.apache.org/created.by.name": kbindName}))()[0]
// Check the Integration uses the new kit
Eventually(IntegrationKit(ns, name), TestTimeoutMedium).Should(Equal(kit.Name))
+ Eventually(IntegrationKit(ns, kbindName), TestTimeoutMedium).Should(Equal(kitKbind.Name))
// Check the Integration Pod uses the new image
Eventually(IntegrationPodImage(ns, name)).Should(Equal(kit.Status.Image))
+ Eventually(IntegrationPodImage(ns, kbindName)).Should(Equal(kitKbind.Status.Image))
// Check the Integration runs correctly
Eventually(IntegrationPodPhase(ns, name)).Should(Equal(corev1.PodRunning))
+ Eventually(IntegrationPodPhase(ns, kbindName)).Should(Equal(corev1.PodRunning))
Eventually(IntegrationConditionStatus(ns, name, v1.IntegrationConditionReady), TestTimeoutLong).
Should(Equal(corev1.ConditionTrue))
-
- // Clean up
- Expect(Kamel("delete", "--all", "-n", ns).Execute()).To(Succeed())
- Expect(Kamel("uninstall", "-n", ns).Execute()).To(Succeed())
- // Clean up cluster-wide resources that are not removed by OLM
- Expect(Kamel("uninstall", "--all", "-n", ns, "--olm=false").Execute()).To(Succeed())
+ Eventually(IntegrationConditionStatus(ns, kbindName, v1.IntegrationConditionReady), TestTimeoutLong).
+ Should(Equal(corev1.ConditionTrue))
})
+ // Clean up
+ Expect(Kamel("delete", "--all", "-n", ns).Execute()).To(Succeed())
+ Expect(Kamel("uninstall", "-n", ns).Execute()).To(Succeed())
Review Comment:
Uninstall is not required. Better remove it.
##########
e2e/install/olm/olm_upgrade_test.go:
##########
@@ -196,40 +201,49 @@ func TestOLMAutomaticUpgrade(t *testing.T) {
// Check the Integration runs correctly
Eventually(IntegrationPodPhase(ns, name), TestTimeoutLong).Should(Equal(corev1.PodRunning))
+ Eventually(IntegrationPodPhase(ns, kbindName), TestTimeoutLong).Should(Equal(corev1.PodRunning))
+
Eventually(IntegrationConditionStatus(ns, name, v1.IntegrationConditionReady), TestTimeoutMedium).
Should(Equal(corev1.ConditionTrue))
+ Eventually(IntegrationConditionStatus(ns, kbindName, v1.IntegrationConditionReady), TestTimeoutMedium).Should(Equal(corev1.ConditionTrue))
// Check the Integration version has been upgraded
Eventually(IntegrationVersion(ns, name)).Should(ContainSubstring(newIPVersionPrefix))
+ Eventually(IntegrationVersion(ns, kbindName)).Should(ContainSubstring(newIPVersionPrefix))
// Check the previous kit is not garbage collected (skip Build - present in case of respin)
prevCSVVersionMajorMinorPatch := fmt.Sprintf("%d.%d.%d",
prevCSVVersion.Version.Major, prevCSVVersion.Version.Minor, prevCSVVersion.Version.Patch)
- Eventually(Kits(ns, KitWithVersion(prevCSVVersionMajorMinorPatch))).Should(HaveLen(1))
+ Eventually(Kits(ns, KitWithVersion(prevCSVVersionMajorMinorPatch))).Should(HaveLen(2))
// Check a new kit is created with the current version
Eventually(Kits(ns, KitWithVersion(defaults.Version)),
- TestTimeoutMedium).Should(HaveLen(1))
+ TestTimeoutMedium).Should(HaveLen(2))
// Check the new kit is ready
Eventually(Kits(ns, KitWithVersion(defaults.Version), KitWithPhase(v1.IntegrationKitPhaseReady)),
TestTimeoutMedium).Should(HaveLen(1))
- kit := Kits(ns, KitWithVersion(defaults.Version))()[0]
+ kit := Kits(ns, KitWithVersion(defaults.Version), KitWithLabels(map[string]string{"camel.apache.org/created.by.name": name}))()[0]
+ kitKbind := Kits(ns, KitWithVersion(defaults.Version), KitWithLabels(map[string]string{"camel.apache.org/created.by.name": kbindName}))()[0]
// Check the Integration uses the new kit
Eventually(IntegrationKit(ns, name), TestTimeoutMedium).Should(Equal(kit.Name))
+ Eventually(IntegrationKit(ns, kbindName), TestTimeoutMedium).Should(Equal(kitKbind.Name))
// Check the Integration Pod uses the new image
Eventually(IntegrationPodImage(ns, name)).Should(Equal(kit.Status.Image))
+ Eventually(IntegrationPodImage(ns, kbindName)).Should(Equal(kitKbind.Status.Image))
// Check the Integration runs correctly
Eventually(IntegrationPodPhase(ns, name)).Should(Equal(corev1.PodRunning))
+ Eventually(IntegrationPodPhase(ns, kbindName)).Should(Equal(corev1.PodRunning))
Eventually(IntegrationConditionStatus(ns, name, v1.IntegrationConditionReady), TestTimeoutLong).
Should(Equal(corev1.ConditionTrue))
-
- // Clean up
- Expect(Kamel("delete", "--all", "-n", ns).Execute()).To(Succeed())
- Expect(Kamel("uninstall", "-n", ns).Execute()).To(Succeed())
- // Clean up cluster-wide resources that are not removed by OLM
- Expect(Kamel("uninstall", "--all", "-n", ns, "--olm=false").Execute()).To(Succeed())
+ Eventually(IntegrationConditionStatus(ns, kbindName, v1.IntegrationConditionReady), TestTimeoutLong).
+ Should(Equal(corev1.ConditionTrue))
})
+ // Clean up
+ Expect(Kamel("delete", "--all", "-n", ns).Execute()).To(Succeed())
+ Expect(Kamel("uninstall", "-n", ns).Execute()).To(Succeed())
+ // Clean up cluster-wide resources that are not removed by OLM
+ Expect(Kamel("uninstall", "--all", "-n", ns, "--olm=false").Execute()).To(Succeed())
Review Comment:
This neither
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] lfabriko commented on pull request #4245: Add Kameletbinding test to olm_upgrade_tes
Posted by "lfabriko (via GitHub)" <gi...@apache.org>.
lfabriko commented on PR #4245:
URL: https://github.com/apache/camel-k/pull/4245#issuecomment-1534228348
May I ask, as branches `main` and `release-1.10.x` diverge in location of `olm_upgrade_test.go` and names of used functions (`C/clusterServiceVersion`), do I understand correctly I need to prepare another MR for branch `release-1.10.x`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on a diff in pull request #4245: Add Kameletbinding test to olm_upgrade_tes
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on code in PR #4245:
URL: https://github.com/apache/camel-k/pull/4245#discussion_r1163934727
##########
e2e/install/olm/olm_upgrade_test.go:
##########
@@ -196,40 +201,49 @@ func TestOLMAutomaticUpgrade(t *testing.T) {
// Check the Integration runs correctly
Eventually(IntegrationPodPhase(ns, name), TestTimeoutLong).Should(Equal(corev1.PodRunning))
+ Eventually(IntegrationPodPhase(ns, kbindName), TestTimeoutLong).Should(Equal(corev1.PodRunning))
+
Eventually(IntegrationConditionStatus(ns, name, v1.IntegrationConditionReady), TestTimeoutMedium).
Should(Equal(corev1.ConditionTrue))
+ Eventually(IntegrationConditionStatus(ns, kbindName, v1.IntegrationConditionReady), TestTimeoutMedium).Should(Equal(corev1.ConditionTrue))
// Check the Integration version has been upgraded
Eventually(IntegrationVersion(ns, name)).Should(ContainSubstring(newIPVersionPrefix))
+ Eventually(IntegrationVersion(ns, kbindName)).Should(ContainSubstring(newIPVersionPrefix))
Review Comment:
I think it would be nice to check some condition on the KameletBinding as well. We should more interested in it than in the Integration underneath
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] lfabriko commented on pull request #4245: Add Kameletbinding test to olm_upgrade_tes
Posted by "lfabriko (via GitHub)" <gi...@apache.org>.
lfabriko commented on PR #4245:
URL: https://github.com/apache/camel-k/pull/4245#issuecomment-1521696206
It seems the `Creating` phase is completed too quickly - prior to the check is even run... I tried removing the check...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on pull request #4245: Add Kameletbinding test to olm_upgrade_tes
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #4245:
URL: https://github.com/apache/camel-k/pull/4245#issuecomment-1521395711
It seems we have some errors to check:
```
olm_upgrade_test.go:207:
Timed out after 900.001s.
Expected
<v1alpha1.KameletBindingPhase>: Ready
to equal
<v1alpha1.KameletBindingPhase>: Creating
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] lfabriko commented on pull request #4245: Add Kameletbinding test to olm_upgrade_tes
Posted by "lfabriko (via GitHub)" <gi...@apache.org>.
lfabriko commented on PR #4245:
URL: https://github.com/apache/camel-k/pull/4245#issuecomment-1534604774
MR to `release-1.10.x`: https://github.com/apache/camel-k/pull/4323
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] lfabriko closed pull request #4245: Add Kameletbinding test to olm_upgrade_tes
Posted by "lfabriko (via GitHub)" <gi...@apache.org>.
lfabriko closed pull request #4245: Add Kameletbinding test to olm_upgrade_tes
URL: https://github.com/apache/camel-k/pull/4245
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] lfabriko commented on a diff in pull request #4245: Add Kameletbinding test to olm_upgrade_tes
Posted by "lfabriko (via GitHub)" <gi...@apache.org>.
lfabriko commented on code in PR #4245:
URL: https://github.com/apache/camel-k/pull/4245#discussion_r1164036824
##########
e2e/install/olm/olm_upgrade_test.go:
##########
@@ -196,40 +201,49 @@ func TestOLMAutomaticUpgrade(t *testing.T) {
// Check the Integration runs correctly
Eventually(IntegrationPodPhase(ns, name), TestTimeoutLong).Should(Equal(corev1.PodRunning))
+ Eventually(IntegrationPodPhase(ns, kbindName), TestTimeoutLong).Should(Equal(corev1.PodRunning))
+
Eventually(IntegrationConditionStatus(ns, name, v1.IntegrationConditionReady), TestTimeoutMedium).
Should(Equal(corev1.ConditionTrue))
+ Eventually(IntegrationConditionStatus(ns, kbindName, v1.IntegrationConditionReady), TestTimeoutMedium).Should(Equal(corev1.ConditionTrue))
// Check the Integration version has been upgraded
Eventually(IntegrationVersion(ns, name)).Should(ContainSubstring(newIPVersionPrefix))
+ Eventually(IntegrationVersion(ns, kbindName)).Should(ContainSubstring(newIPVersionPrefix))
Review Comment:
I'm not sure if could be left some checks of underlying integration (also I noticed original checks of integration from L198 and L199 are repeated in the end)...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] lfabriko commented on pull request #4245: Add Kameletbinding test to olm_upgrade_tes
Posted by "lfabriko (via GitHub)" <gi...@apache.org>.
lfabriko commented on PR #4245:
URL: https://github.com/apache/camel-k/pull/4245#issuecomment-1704968574
As the file moved to other location, I would find it clearer to close this MR in favor of new one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] lfabriko commented on pull request #4245: Add Kameletbinding test to olm_upgrade_tes
Posted by "lfabriko (via GitHub)" <gi...@apache.org>.
lfabriko commented on PR #4245:
URL: https://github.com/apache/camel-k/pull/4245#issuecomment-1698618870
@claudio4j Yes, please don't close it yet, I should rework it due to chages in naming.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] claudio4j commented on pull request #4245: Add Kameletbinding test to olm_upgrade_tes
Posted by "claudio4j (via GitHub)" <gi...@apache.org>.
claudio4j commented on PR #4245:
URL: https://github.com/apache/camel-k/pull/4245#issuecomment-1697830019
Is there going to be some work on this PR or can we close it ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org