You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/03/02 08:49:44 UTC

[GitHub] [apisix-ingress-controller] Belyenochi opened a new pull request #896: feat: support custom registry for e2e test

Belyenochi opened a new pull request #896:
URL: https://github.com/apache/apisix-ingress-controller/pull/896


   ### Type of change:
   
   <!-- Please delete options that are not relevant. -->
   
   - [ ] Bugfix
   - [ ] New feature provided
   - [ ] Improve performance
   - [ ] Backport patches
   
   ### What this PR does / why we need it:
   Solved [#880](https://github.com/apache/apisix-ingress-controller/issues/880)
   
   


-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [apisix-ingress-controller] tao12345666333 commented on a change in pull request #896: feat: support custom registry for e2e test

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on a change in pull request #896:
URL: https://github.com/apache/apisix-ingress-controller/pull/896#discussion_r820424984



##########
File path: Makefile
##########
@@ -63,52 +63,57 @@ lint:
 unit-test:
 	go test -cover -coverprofile=coverage.txt ./...
 
-### e2e-test:             Run e2e test cases (kind is required)
+### e2e-test:             Run e2e test cases (in existing clusters directly)
 .PHONY: e2e-test
-e2e-test: ginkgo-check push-images-to-kind
+e2e-test: ginkgo-check push-images
 	kubectl apply -k $(PWD)/samples/deploy/crd
 	cd test/e2e \
 		&& go mod download \
+		&& export REGISTRY=$(REGISTRY)

Review comment:
       ```suggestion
   		&& export REGISTRY=$(REGISTRY) \
   ```
   
   need to be modified here.




-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [apisix-ingress-controller] lingsamuel commented on a change in pull request #896: feat: support custom registry for e2e test

Posted by GitBox <gi...@apache.org>.
lingsamuel commented on a change in pull request #896:
URL: https://github.com/apache/apisix-ingress-controller/pull/896#discussion_r818239373



##########
File path: Makefile
##########
@@ -19,6 +19,7 @@ default: help
 VERSION ?= 1.4.0
 RELEASE_SRC = apache-apisix-ingress-controller-${VERSION}-src
 LOCAL_REGISTRY="localhost:5000"
+CUSTOM_REGISTRY ?= ""

Review comment:
       Instead of using a new environment variable, can we change the old one to a more generic name? The new variable does not seem necessary.




-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #896: feat: support custom registry for e2e test

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on pull request #896:
URL: https://github.com/apache/apisix-ingress-controller/pull/896#issuecomment-1056675076


   Thanks! it's on my list. 
   
   I will review it ASAP.


-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [apisix-ingress-controller] lingsamuel merged pull request #896: feat: support custom registry for e2e test

Posted by GitBox <gi...@apache.org>.
lingsamuel merged pull request #896:
URL: https://github.com/apache/apisix-ingress-controller/pull/896


   


-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [apisix-ingress-controller] Belyenochi commented on a change in pull request #896: feat: support custom registry for e2e test

Posted by GitBox <gi...@apache.org>.
Belyenochi commented on a change in pull request #896:
URL: https://github.com/apache/apisix-ingress-controller/pull/896#discussion_r820348411



##########
File path: Makefile
##########
@@ -19,6 +19,7 @@ default: help
 VERSION ?= 1.4.0
 RELEASE_SRC = apache-apisix-ingress-controller-${VERSION}-src
 LOCAL_REGISTRY="localhost:5000"
+CUSTOM_REGISTRY ?= ""

Review comment:
       > IMO, it would be better to move the kind-up from the prerequisite of push-images to the e2e-test. Since the users may indeed have a registry at localhost:5000. Users can then run the e2e like this: build the images, export the REGISTRY env var, run `make push-images`, and then execute ginkgo directly. Or we could provide a new entry such as `e2e-test-local` just like the e2e-test but without kind-up.
   
   @lingsamuel Thanks for the suggestion, e2e-test-local has been added.




-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [apisix-ingress-controller] Belyenochi commented on a change in pull request #896: feat: support custom registry for e2e test

Posted by GitBox <gi...@apache.org>.
Belyenochi commented on a change in pull request #896:
URL: https://github.com/apache/apisix-ingress-controller/pull/896#discussion_r820500910



##########
File path: Makefile
##########
@@ -63,52 +63,57 @@ lint:
 unit-test:
 	go test -cover -coverprofile=coverage.txt ./...
 
-### e2e-test:             Run e2e test cases (kind is required)
+### e2e-test:             Run e2e test cases (in existing clusters directly)
 .PHONY: e2e-test
-e2e-test: ginkgo-check push-images-to-kind
+e2e-test: ginkgo-check push-images
 	kubectl apply -k $(PWD)/samples/deploy/crd
 	cd test/e2e \
 		&& go mod download \
+		&& export REGISTRY=$(REGISTRY)

Review comment:
       Done




-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [apisix-ingress-controller] lingsamuel commented on pull request #896: feat: support custom registry for e2e test

Posted by GitBox <gi...@apache.org>.
lingsamuel commented on pull request #896:
URL: https://github.com/apache/apisix-ingress-controller/pull/896#issuecomment-1061284593


   Thanks!


-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [apisix-ingress-controller] codecov-commenter commented on pull request #896: feat: support custom registry for e2e test

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #896:
URL: https://github.com/apache/apisix-ingress-controller/pull/896#issuecomment-1059981814


   # [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/896?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#896](https://codecov.io/gh/apache/apisix-ingress-controller/pull/896?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7173ed6) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/e2475cf8573f9f1efd6956c0e4e6eab2142b8061?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e2475cf) will **increase** coverage by `0.00%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-ingress-controller/pull/896/graphs/tree.svg?width=650&height=150&src=pr&token=WPLQXPY3V0&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/apisix-ingress-controller/pull/896?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@           Coverage Diff           @@
   ##           master     #896   +/-   ##
   =======================================
     Coverage   32.01%   32.02%           
   =======================================
     Files          70       71    +1     
     Lines        7749     7772   +23     
   =======================================
   + Hits         2481     2489    +8     
   - Misses       4993     5009   +16     
   + Partials      275      274    -1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/896?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [pkg/apisix/cluster.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/896/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2FwaXNpeC9jbHVzdGVyLmdv) | `31.88% <0.00%> (-0.70%)` | :arrow_down: |
   | [pkg/ingress/endpoint.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/896/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2luZ3Jlc3MvZW5kcG9pbnQuZ28=) | `0.00% <0.00%> (ø)` | |
   | [pkg/ingress/apisix\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/896/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2luZ3Jlc3MvYXBpc2l4X3JvdXRlLmdv) | `0.00% <0.00%> (ø)` | |
   | [pkg/apisix/noop.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/896/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2FwaXNpeC9ub29wLmdv) | `0.00% <0.00%> (ø)` | |
   | [pkg/apisix/stream\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/896/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2FwaXNpeC9zdHJlYW1fcm91dGUuZ28=) | `36.42% <0.00%> (+0.12%)` | :arrow_up: |
   | [pkg/kube/translation/apisix\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/896/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2t1YmUvdHJhbnNsYXRpb24vYXBpc2l4X3JvdXRlLmdv) | `22.43% <0.00%> (+0.60%)` | :arrow_up: |
   | [pkg/ingress/pod.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/896/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2luZ3Jlc3MvcG9kLmdv) | `37.34% <0.00%> (+2.34%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/896?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/896?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e2475cf...7173ed6](https://codecov.io/gh/apache/apisix-ingress-controller/pull/896?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [apisix-ingress-controller] Belyenochi commented on a change in pull request #896: feat: support custom registry for e2e test

Posted by GitBox <gi...@apache.org>.
Belyenochi commented on a change in pull request #896:
URL: https://github.com/apache/apisix-ingress-controller/pull/896#discussion_r820221275



##########
File path: Makefile
##########
@@ -19,6 +19,7 @@ default: help
 VERSION ?= 1.4.0
 RELEASE_SRC = apache-apisix-ingress-controller-${VERSION}-src
 LOCAL_REGISTRY="localhost:5000"
+CUSTOM_REGISTRY ?= ""

Review comment:
       Thanks for review @tao12345666333 @lingsamuel, I've tried renaming REGISTRY, here I'm worried if the kind-up part is needed, I think we should check if REGISTRY points to kind and then run the judgment?
   




-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [apisix-ingress-controller] tao12345666333 commented on a change in pull request #896: feat: support custom registry for e2e test

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on a change in pull request #896:
URL: https://github.com/apache/apisix-ingress-controller/pull/896#discussion_r818245059



##########
File path: Makefile
##########
@@ -19,6 +19,7 @@ default: help
 VERSION ?= 1.4.0
 RELEASE_SRC = apache-apisix-ingress-controller-${VERSION}-src
 LOCAL_REGISTRY="localhost:5000"
+CUSTOM_REGISTRY ?= ""

Review comment:
       +1




-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [apisix-ingress-controller] lingsamuel commented on a change in pull request #896: feat: support custom registry for e2e test

Posted by GitBox <gi...@apache.org>.
lingsamuel commented on a change in pull request #896:
URL: https://github.com/apache/apisix-ingress-controller/pull/896#discussion_r820224408



##########
File path: Makefile
##########
@@ -19,6 +19,7 @@ default: help
 VERSION ?= 1.4.0
 RELEASE_SRC = apache-apisix-ingress-controller-${VERSION}-src
 LOCAL_REGISTRY="localhost:5000"
+CUSTOM_REGISTRY ?= ""

Review comment:
       IMO, it would be better to move the kind-up from the prerequisite of push-images to the e2e-test. Since the users may indeed have a registry at localhost:5000.
   Users can then run the e2e like this: build the images, export the REGISTRY env var, run `make push-images`, and then execute ginkgo directly.
   Or we could provide a new entry such as `e2e-test-local` just like the e2e-test but without kind-up.




-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [apisix-ingress-controller] Belyenochi commented on a change in pull request #896: feat: support custom registry for e2e test

Posted by GitBox <gi...@apache.org>.
Belyenochi commented on a change in pull request #896:
URL: https://github.com/apache/apisix-ingress-controller/pull/896#discussion_r820348411



##########
File path: Makefile
##########
@@ -19,6 +19,7 @@ default: help
 VERSION ?= 1.4.0
 RELEASE_SRC = apache-apisix-ingress-controller-${VERSION}-src
 LOCAL_REGISTRY="localhost:5000"
+CUSTOM_REGISTRY ?= ""

Review comment:
       > IMO, it would be better to move the kind-up from the prerequisite of push-images to the e2e-test. Since the users may indeed have a registry at localhost:5000. Users can then run the e2e like this: build the images, export the REGISTRY env var, run `make push-images`, and then execute ginkgo directly. Or we could provide a new entry such as `e2e-test-local` just like the e2e-test but without kind-up.
   
   @lingsamuel Thanks for the suggestion, e2e-test-local has been re-added.




-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org