You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "gansheer (via GitHub)" <gi...@apache.org> on 2023/10/24 07:53:11 UTC

[PR] chore(ci): Check kind registry configuration for kustomize [camel-k]

gansheer opened a new pull request, #4849:
URL: https://github.com/apache/camel-k/pull/4849

   Add check for kind action registry configuration from KEP-1755.
   
   
   **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


Re: [PR] fix(core): Access configmap from another namespace for KEP-1755 [camel-k]

Posted by "gansheer (via GitHub)" <gi...@apache.org>.
gansheer commented on code in PR #4849:
URL: https://github.com/apache/camel-k/pull/4849#discussion_r1369963334


##########
e2e/install/kustomize/operator_test.go:
##########
@@ -69,6 +69,13 @@ func TestOperatorBasic(t *testing.T) {
 		Expect(operatorPod.Spec.Containers[0].SecurityContext.AllowPrivilegeEscalation).To(Equal(kubernetes.DefaultOperatorSecurityContext().AllowPrivilegeEscalation))
 
 		Eventually(Platform(ns)).ShouldNot(BeNil())
+		registry := os.Getenv("KIND_REGISTRY")
+		if registry != "" {
+			platform := Platform(ns)()
+			Expect(platform.Spec.Build.Registry).ShouldNot(BeNil())
+			Expect(platform.Spec.Build.Registry.Address).To(Equal("kind-registry:5000"))

Review Comment:
   :face_with_spiral_eyes: good catch



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


Re: [PR] fix(core): Access configmap from another namespace for KEP-1755 [camel-k]

Posted by "gansheer (via GitHub)" <gi...@apache.org>.
gansheer commented on PR #4849:
URL: https://github.com/apache/camel-k/pull/4849#issuecomment-1790909734

   @squakez @oscerd could one of you please run again the failing tests ? :pray: 


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


Re: [PR] fix(core): Access configmap from another namespace for KEP-1755 [camel-k]

Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #4849:
URL: https://github.com/apache/camel-k/pull/4849#issuecomment-1781169526

   OLM checks should be fixed now. You can rebase please.


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


Re: [PR] fix(core): Access configmap from another namespace for KEP-1755 [camel-k]

Posted by "gansheer (via GitHub)" <gi...@apache.org>.
gansheer commented on PR #4849:
URL: https://github.com/apache/camel-k/pull/4849#issuecomment-1792091514

   Yes please.


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


Re: [PR] fix(core): Access configmap from another namespace for KEP-1755 [camel-k]

Posted by "gansheer (via GitHub)" <gi...@apache.org>.
gansheer commented on PR #4849:
URL: https://github.com/apache/camel-k/pull/4849#issuecomment-1781325272

   > OLM checks should be fixed now. You can rebase please.
   
   Done, but I still need to fix some configuration to really test KEP-1755.


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


Re: [PR] fix(core): Access configmap from another namespace for KEP-1755 [camel-k]

Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #4849:
URL: https://github.com/apache/camel-k/pull/4849#issuecomment-1792079611

   All green. Is it ready to merge?


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


Re: [PR] chore(ci): Check kind registry configuration for kustomize [camel-k]

Posted by "gansheer (via GitHub)" <gi...@apache.org>.
gansheer commented on PR #4849:
URL: https://github.com/apache/camel-k/pull/4849#issuecomment-1776712294

   @squakez @oscerd If you can fire the `install/non-olm` e2e workflow that would be nice.


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


Re: [PR] fix(core): Access configmap from another namespace for KEP-1755 [camel-k]

Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez merged PR #4849:
URL: https://github.com/apache/camel-k/pull/4849


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


Re: [PR] chore(ci): Check kind registry configuration for kustomize [camel-k]

Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on code in PR #4849:
URL: https://github.com/apache/camel-k/pull/4849#discussion_r1369767838


##########
e2e/install/kustomize/operator_test.go:
##########
@@ -69,6 +69,10 @@ func TestOperatorBasic(t *testing.T) {
 		Expect(operatorPod.Spec.Containers[0].SecurityContext.AllowPrivilegeEscalation).To(Equal(kubernetes.DefaultOperatorSecurityContext().AllowPrivilegeEscalation))
 
 		Eventually(Platform(ns)).ShouldNot(BeNil())
+		platform := Platform(ns)()
+		Expect(platform.Spec.Build.Registry).ShouldNot(BeNil())
+		Expect(platform.Spec.Build.Registry.Address).To(Equal("kind-registry:5000"))

Review Comment:
   I think we have an environment variable controlling that value, `KIND_REGISTRY`. I'd say that, for portability reasons, we should that instead of a constant.



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


Re: [PR] chore(ci): Check kind registry configuration for kustomize [camel-k]

Posted by "gansheer (via GitHub)" <gi...@apache.org>.
gansheer commented on code in PR #4849:
URL: https://github.com/apache/camel-k/pull/4849#discussion_r1369771333


##########
e2e/install/kustomize/operator_test.go:
##########
@@ -69,6 +69,10 @@ func TestOperatorBasic(t *testing.T) {
 		Expect(operatorPod.Spec.Containers[0].SecurityContext.AllowPrivilegeEscalation).To(Equal(kubernetes.DefaultOperatorSecurityContext().AllowPrivilegeEscalation))
 
 		Eventually(Platform(ns)).ShouldNot(BeNil())
+		platform := Platform(ns)()
+		Expect(platform.Spec.Build.Registry).ShouldNot(BeNil())
+		Expect(platform.Spec.Build.Registry.Address).To(Equal("kind-registry:5000"))

Review Comment:
   Will do, for now I am just trying to check if KEP-1755 feature really works and something is actually extracted from `local-registry-hosting` configmap.



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


Re: [PR] fix(core): Access configmap from another namespace for KEP-1755 [camel-k]

Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on code in PR #4849:
URL: https://github.com/apache/camel-k/pull/4849#discussion_r1369954503


##########
e2e/install/kustomize/operator_test.go:
##########
@@ -69,6 +69,13 @@ func TestOperatorBasic(t *testing.T) {
 		Expect(operatorPod.Spec.Containers[0].SecurityContext.AllowPrivilegeEscalation).To(Equal(kubernetes.DefaultOperatorSecurityContext().AllowPrivilegeEscalation))
 
 		Eventually(Platform(ns)).ShouldNot(BeNil())
+		registry := os.Getenv("KIND_REGISTRY")
+		if registry != "" {
+			platform := Platform(ns)()
+			Expect(platform.Spec.Build.Registry).ShouldNot(BeNil())
+			Expect(platform.Spec.Build.Registry.Address).To(Equal("kind-registry:5000"))

Review Comment:
   You're not using the `registry` variable in the assertion



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