You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2022/12/02 04:57:31 UTC

[GitHub] [camel-k] tadayosi commented on a diff in pull request #3871: fix(service): Create k8s service when knative-service trait is disabled

tadayosi commented on code in PR #3871:
URL: https://github.com/apache/camel-k/pull/3871#discussion_r1037785200


##########
e2e/global/knative/knative_test.go:
##########
@@ -107,6 +107,20 @@ func TestKnative(t *testing.T) {
 
 			Expect(Kamel("delete", "--all", "-n", ns).Execute()).To(Succeed())
 		})
+
+		t.Run("Knative-service disabled", func(t *testing.T) {
+			Expect(KamelRunWithID(operatorID, ns, "files/http_out.groovy", "-t", "knative-service.enabled=false").Execute()).To(Succeed())
+			Eventually(IntegrationPodPhase(ns, "http-out"), TestTimeoutLong).Should(Equal(v1.PodRunning))
+			Eventually(Service(ns, "http-out"), TestTimeoutShort).ShouldNot(BeNil())
+			Expect(Kamel("delete", "--all", "-n", ns).Execute()).To(Succeed())
+		})
+
+		t.Run("Knative-service priority", func(t *testing.T) {
+			Expect(KamelRunWithID(operatorID, ns, "files/http_out.groovy").Execute()).To(Succeed())
+			Eventually(IntegrationPodPhase(ns, "http-out"), TestTimeoutLong).Should(Equal(v1.PodRunning))
+			Eventually(KnativeService(ns, "http-out"), TestTimeoutShort).ShouldNot(BeNil())

Review Comment:
   In contract, here you can also check that the k8s service is not created.



##########
e2e/global/knative/knative_test.go:
##########
@@ -107,6 +107,20 @@ func TestKnative(t *testing.T) {
 
 			Expect(Kamel("delete", "--all", "-n", ns).Execute()).To(Succeed())
 		})
+
+		t.Run("Knative-service disabled", func(t *testing.T) {
+			Expect(KamelRunWithID(operatorID, ns, "files/http_out.groovy", "-t", "knative-service.enabled=false").Execute()).To(Succeed())
+			Eventually(IntegrationPodPhase(ns, "http-out"), TestTimeoutLong).Should(Equal(v1.PodRunning))
+			Eventually(Service(ns, "http-out"), TestTimeoutShort).ShouldNot(BeNil())

Review Comment:
   Maybe you can also check that Knative service is not created.



##########
pkg/trait/service.go:
##########
@@ -46,7 +46,7 @@ func newServiceTrait() Trait {
 // IsAllowedInProfile overrides default.
 func (t *serviceTrait) IsAllowedInProfile(profile v1.TraitProfile) bool {
 	return profile.Equal(v1.TraitProfileKubernetes) ||
-		profile.Equal(v1.TraitProfileOpenShift)
+		profile.Equal(v1.TraitProfileOpenShift) || profile.Equal(v1.TraitProfileKnative)

Review Comment:
   By default `IsAllowedInProfile()` allows every profile, so instead of adding another profile you can simply remove this override.



-- 
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