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/11/14 14:52:10 UTC

[GitHub] [apisix-ingress-controller] zou8944 opened a new pull request, #1460: Draft: make: dev-env

zou8944 opened a new pull request, #1460:
URL: https://github.com/apache/apisix-ingress-controller/pull/1460

   <!-- Please answer these questions before submitting a pull request -->
   
   ### Type of change:
   
   <!-- Please delete options that are not relevant. -->
   
   - [ ] Bugfix
   - [v] New feature provided
   - [ ] Improve performance
   - [ ] Backport patches
   
   ### What this PR does / why we need it:
   <!--- Why is this change required? What problem does it solve? -->
   <!--- If it fixes an open issue, please link to the issue here. -->
   this is for issue: https://github.com/apache/apisix-ingress-controller/issues/1146
   
   ### Pre-submission checklist:
   
   <!--
   Please follow the requirements:
   1. Use Draft if the PR is not ready to be reviewed
   2. Test is required for the feat/fix PR, unless you have a good reason
   3. Doc is required for the feat PR
   4. Use a new commit to resolve review instead of `push -f`
   5. Use "request review" to notify the reviewer once you have resolved the review
   -->
   
   * [ ] Did you explain what problem does this PR solve? Or what new features have been added?
   * [ ] Have you added corresponding test cases?
   * [ ] Have you modified the corresponding document?
   * [ ] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix-ingress-controller#community) first**
   


-- 
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] github-actions[bot] closed pull request #1460: make: dev-env

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #1460: make: dev-env
URL: https://github.com/apache/apisix-ingress-controller/pull/1460


-- 
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] Chever-John commented on a diff in pull request #1460: make: dev-env

Posted by "Chever-John (via GitHub)" <gi...@apache.org>.
Chever-John commented on code in PR #1460:
URL: https://github.com/apache/apisix-ingress-controller/pull/1460#discussion_r1160626545


##########
Makefile:
##########
@@ -325,3 +324,19 @@ install-gateway-api-local:
 .PHONY: uninstall-gateway-api
 uninstall-gateway-api:
 	kubectl delete -f $(GATEWAY_API_CRDS_LOCAL_PATH)
+
+### dev-env:			  Launch development environment
+.PHONY: dev-env
+dev-env: kind-up pack-image
+	helm repo add apisix https://charts.apiseven.com
+	helm repo update
+	helm install apisix apisix/apisix \
+		--set gateway.type=NodePort \
+		--set ingress-controller.enabled=true \
+		--namespace ingress-apisix \
+		--create-namespace
+	kubectl set image deployment/apisix-ingress-controller ingress-controller=$(REGISTRY)/apache/apisix-ingress-controller:dev --namespace ingress-apisix
+

Review Comment:
   Hi, can u try this?
   
   ```makefile
   ### dev-env:			  Launch development environment
   .PHONY: dev-env
   dev-env: kind-up pack-image
   	-@helm repo add apisix https://charts.apiseven.com 2>&1 | grep -q "Error: repository name (apisix) already exists" && echo "APISIX chart exists, skipping helm repo add"; true
   	@helm repo update
   	@helm list -n ingress-apisix | grep -q apisix && echo "APISIX already exists, skipping installation" || \
   	helm install apisix apisix/apisix \
   		--set gateway.type=NodePort \
   		--set ingress-controller.enabled=true \
   		--namespace ingress-apisix \
   		--create-namespace
   	kubectl set image deployment/apisix-ingress-controller ingress-controller=$(REGISTRY)/apache/apisix-ingress-controller:dev --namespace ingress-apisix
   ```



-- 
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] aynp commented on a diff in pull request #1460: make: dev-env

Posted by "aynp (via GitHub)" <gi...@apache.org>.
aynp commented on code in PR #1460:
URL: https://github.com/apache/apisix-ingress-controller/pull/1460#discussion_r1161343050


##########
Makefile:
##########
@@ -325,3 +324,19 @@ install-gateway-api-local:
 .PHONY: uninstall-gateway-api
 uninstall-gateway-api:
 	kubectl delete -f $(GATEWAY_API_CRDS_LOCAL_PATH)
+
+### dev-env:			  Launch development environment
+.PHONY: dev-env
+dev-env: kind-up pack-image
+	helm repo add apisix https://charts.apiseven.com
+	helm repo update
+	helm install apisix apisix/apisix \
+		--set gateway.type=NodePort \
+		--set ingress-controller.enabled=true \
+		--namespace ingress-apisix \
+		--create-namespace
+	kubectl set image deployment/apisix-ingress-controller ingress-controller=$(REGISTRY)/apache/apisix-ingress-controller:dev --namespace ingress-apisix
+

Review Comment:
   I guess newer versions of helm automatically detect if the repository exists and skips over without throwing any error. 
   Also if errors are to be handled, maybe it would be cleaner to be a script file just like `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] Chever-John commented on a diff in pull request #1460: make: dev-env

Posted by "Chever-John (via GitHub)" <gi...@apache.org>.
Chever-John commented on code in PR #1460:
URL: https://github.com/apache/apisix-ingress-controller/pull/1460#discussion_r1161364824


##########
Makefile:
##########
@@ -325,3 +324,19 @@ install-gateway-api-local:
 .PHONY: uninstall-gateway-api
 uninstall-gateway-api:
 	kubectl delete -f $(GATEWAY_API_CRDS_LOCAL_PATH)
+
+### dev-env:			  Launch development environment
+.PHONY: dev-env
+dev-env: kind-up pack-image
+	helm repo add apisix https://charts.apiseven.com
+	helm repo update
+	helm install apisix apisix/apisix \
+		--set gateway.type=NodePort \
+		--set ingress-controller.enabled=true \
+		--namespace ingress-apisix \
+		--create-namespace
+	kubectl set image deployment/apisix-ingress-controller ingress-controller=$(REGISTRY)/apache/apisix-ingress-controller:dev --namespace ingress-apisix
+

Review Comment:
   Have you tried it? Don't try to guess. You should try:)
   
   Of course, it is my habit to make mistakes in this place. 
   
   Perhaps we need @tao12345666333 's pieces of advice.



-- 
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] zou8944 commented on a diff in pull request #1460: make: dev-env

Posted by "zou8944 (via GitHub)" <gi...@apache.org>.
zou8944 commented on code in PR #1460:
URL: https://github.com/apache/apisix-ingress-controller/pull/1460#discussion_r1167789061


##########
Makefile:
##########
@@ -325,3 +324,19 @@ install-gateway-api-local:
 .PHONY: uninstall-gateway-api
 uninstall-gateway-api:
 	kubectl delete -f $(GATEWAY_API_CRDS_LOCAL_PATH)
+
+### dev-env:			  Launch development environment
+.PHONY: dev-env
+dev-env: kind-up pack-image
+	helm repo add apisix https://charts.apiseven.com
+	helm repo update
+	helm install apisix apisix/apisix \
+		--set gateway.type=NodePort \
+		--set ingress-controller.enabled=true \
+		--namespace ingress-apisix \
+		--create-namespace
+	kubectl set image deployment/apisix-ingress-controller ingress-controller=$(REGISTRY)/apache/apisix-ingress-controller:dev --namespace ingress-apisix
+

Review Comment:
   https://github.com/apache/apisix-ingress-controller/pull/1460/commits/bd8a97be1163f2dbceabdfd94abfa1894f43a95f



-- 
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] Chever-John commented on a diff in pull request #1460: make: dev-env

Posted by "Chever-John (via GitHub)" <gi...@apache.org>.
Chever-John commented on code in PR #1460:
URL: https://github.com/apache/apisix-ingress-controller/pull/1460#discussion_r1160626545


##########
Makefile:
##########
@@ -325,3 +324,19 @@ install-gateway-api-local:
 .PHONY: uninstall-gateway-api
 uninstall-gateway-api:
 	kubectl delete -f $(GATEWAY_API_CRDS_LOCAL_PATH)
+
+### dev-env:			  Launch development environment
+.PHONY: dev-env
+dev-env: kind-up pack-image
+	helm repo add apisix https://charts.apiseven.com
+	helm repo update
+	helm install apisix apisix/apisix \
+		--set gateway.type=NodePort \
+		--set ingress-controller.enabled=true \
+		--namespace ingress-apisix \
+		--create-namespace
+	kubectl set image deployment/apisix-ingress-controller ingress-controller=$(REGISTRY)/apache/apisix-ingress-controller:dev --namespace ingress-apisix
+

Review Comment:
   Hi, can we try this?
   
   ```makefile
   ### dev-env:			  Launch development environment
   .PHONY: dev-env
   dev-env: kind-up pack-image
   	-@helm repo add apisix https://charts.apiseven.com 2>&1 | grep -q "Error: repository name (apisix) already exists" && echo "APISIX chart exists, skipping helm repo add"; true
   	@helm repo update
   	@helm list -n ingress-apisix | grep -q apisix && echo "APISIX already exists, skipping installation" || \
   	helm install apisix apisix/apisix \
   		--set gateway.type=NodePort \
   		--set ingress-controller.enabled=true \
   		--namespace ingress-apisix \
   		--create-namespace
   	kubectl set image deployment/apisix-ingress-controller ingress-controller=$(REGISTRY)/apache/apisix-ingress-controller:dev --namespace ingress-apisix
   ```



-- 
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 diff in pull request #1460: make: dev-env

Posted by "tao12345666333 (via GitHub)" <gi...@apache.org>.
tao12345666333 commented on code in PR #1460:
URL: https://github.com/apache/apisix-ingress-controller/pull/1460#discussion_r1162444917


##########
Makefile:
##########
@@ -325,3 +324,19 @@ install-gateway-api-local:
 .PHONY: uninstall-gateway-api
 uninstall-gateway-api:
 	kubectl delete -f $(GATEWAY_API_CRDS_LOCAL_PATH)
+
+### dev-env:			  Launch development environment
+.PHONY: dev-env
+dev-env: kind-up pack-image
+	helm repo add apisix https://charts.apiseven.com
+	helm repo update
+	helm install apisix apisix/apisix \
+		--set gateway.type=NodePort \
+		--set ingress-controller.enabled=true \
+		--namespace ingress-apisix \
+		--create-namespace
+	kubectl set image deployment/apisix-ingress-controller ingress-controller=$(REGISTRY)/apache/apisix-ingress-controller:dev --namespace ingress-apisix
+

Review Comment:
   We don't need to do any additional processing for 'helm repo add'. Even if the repository already exists, it won't affect normal logic.
   
   ```
   ➜  ~ helm repo add apisix https://charts.apiseven.com
   "apisix" already exists with the same configuration, skipping
   ➜  ~ echo $?
   0
   
   ```
   
   But the judgment logic of `helm list` is very good, unless we use ` helm upgrade` directly. Using `helm list` to pre-judge can be safer, and I agree.
   
   @zou8944 PTAL, 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] zou8944 commented on pull request #1460: make: dev-env

Posted by "zou8944 (via GitHub)" <gi...@apache.org>.
zou8944 commented on PR #1460:
URL: https://github.com/apache/apisix-ingress-controller/pull/1460#issuecomment-1496830760

   @tao12345666333 sorry for late responding, conflict resolved


-- 
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] Chever-John commented on a diff in pull request #1460: make: dev-env

Posted by "Chever-John (via GitHub)" <gi...@apache.org>.
Chever-John commented on code in PR #1460:
URL: https://github.com/apache/apisix-ingress-controller/pull/1460#discussion_r1186958059


##########
Makefile:
##########
@@ -325,3 +324,13 @@ install-gateway-api-local:
 .PHONY: uninstall-gateway-api
 uninstall-gateway-api:
 	kubectl delete -f $(GATEWAY_API_CRDS_LOCAL_PATH)
+
+### dev-env:			  Launch development environment
+.PHONY: dev-env
+dev-env: kind-up pack-image
+	./utils/install-apisix-with-helm.sh
+	kubectl set image deployment/apisix-ingress-controller ingress-controller=$(REGISTRY)/apache/apisix-ingress-controller:dev --namespace ingress-apisix

Review Comment:
   change it to 
   
   `kubectl set image deployment/apisix-ingress-controller ingress-controller=$(REGISTRY)/apisix-ingress-controller:$(IMAGE_TAG) --namespace ingress-apisix`



-- 
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 #1460: make: dev-env

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #1460:
URL: https://github.com/apache/apisix-ingress-controller/pull/1460#issuecomment-1501302308

   ## [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1460?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 [#1460](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1460?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cc8ebd6) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/38710e71d9dc11a9962198c8927912c44c92a54c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (38710e7) will **not change** coverage.
   > The diff coverage is `n/a`.
   
   > :exclamation: Current head cc8ebd6 differs from pull request most recent head 9d05bcf. Consider uploading reports for the commit 9d05bcf to get more accurate results
   
   ```diff
   @@           Coverage Diff           @@
   ##           master    #1460   +/-   ##
   =======================================
     Coverage   39.72%   39.72%           
   =======================================
     Files          92       92           
     Lines        8065     8065           
   =======================================
     Hits         3204     3204           
     Misses       4457     4457           
     Partials      404      404           
   ```
   
   
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?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] Chever-John commented on a diff in pull request #1460: make: dev-env

Posted by "Chever-John (via GitHub)" <gi...@apache.org>.
Chever-John commented on code in PR #1460:
URL: https://github.com/apache/apisix-ingress-controller/pull/1460#discussion_r1160626545


##########
Makefile:
##########
@@ -325,3 +324,19 @@ install-gateway-api-local:
 .PHONY: uninstall-gateway-api
 uninstall-gateway-api:
 	kubectl delete -f $(GATEWAY_API_CRDS_LOCAL_PATH)
+
+### dev-env:			  Launch development environment
+.PHONY: dev-env
+dev-env: kind-up pack-image
+	helm repo add apisix https://charts.apiseven.com
+	helm repo update
+	helm install apisix apisix/apisix \
+		--set gateway.type=NodePort \
+		--set ingress-controller.enabled=true \
+		--namespace ingress-apisix \
+		--create-namespace
+	kubectl set image deployment/apisix-ingress-controller ingress-controller=$(REGISTRY)/apache/apisix-ingress-controller:dev --namespace ingress-apisix
+

Review Comment:
   Hi, can u try this?
   
   ```makefile
   ### dev-env:			  Launch development environment
   .PHONY: dev-env
   dev-env: kind-up pack-image
   	-helm repo add apisix https://charts.apiseven.com 2>&1 | grep -q "Error: repository name (apisix) already exists" && echo "Apisix already exists, skipping installation" || \
   		(helm repo update && \
   		 helm list -n ingress-apisix | grep -q apisix && echo "Apisix already exists, skipping installation" || \
   		 helm install apisix apisix/apisix \
   			--set gateway.type=NodePort \
   			--set ingress-controller.enabled=true \
   			--namespace ingress-apisix \
   			--create-namespace && \
   		 kubectl set image deployment/apisix-ingress-controller ingress-controller=$(REGISTRY)/apache/apisix-ingress-controller:dev --namespace ingress-apisix)
   
   ```



-- 
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] AlinsRan commented on a diff in pull request #1460: make: dev-env

Posted by "AlinsRan (via GitHub)" <gi...@apache.org>.
AlinsRan commented on code in PR #1460:
URL: https://github.com/apache/apisix-ingress-controller/pull/1460#discussion_r1163533251


##########
Makefile:
##########
@@ -325,3 +324,19 @@ install-gateway-api-local:
 .PHONY: uninstall-gateway-api
 uninstall-gateway-api:
 	kubectl delete -f $(GATEWAY_API_CRDS_LOCAL_PATH)
+
+### dev-env:			  Launch development environment
+.PHONY: dev-env
+dev-env: kind-up pack-image
+	helm repo add apisix https://charts.apiseven.com
+	helm repo update
+	helm install apisix apisix/apisix \
+		--set gateway.type=NodePort \
+		--set ingress-controller.enabled=true \
+		--namespace ingress-apisix \
+		--create-namespace
+	kubectl set image deployment/apisix-ingress-controller ingress-controller=$(REGISTRY)/apache/apisix-ingress-controller:dev --namespace ingress-apisix
+

Review Comment:
   Perhaps we don't need to verify the release, just upgrade directly. 
   ```shell
   helm upgrade --install apisix apisix/apisix \
   		--set gateway.type=NodePort \
   		--set ingress-controller.enabled=true \
   		--namespace ingress-apisix \
   		--create-namespace
   ```



-- 
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] github-actions[bot] commented on pull request #1460: make: dev-env

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #1460:
URL: https://github.com/apache/apisix-ingress-controller/pull/1460#issuecomment-1672402766

   This pull request/issue has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.


-- 
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 #1460: make: dev-env

Posted by "tao12345666333 (via GitHub)" <gi...@apache.org>.
tao12345666333 commented on PR #1460:
URL: https://github.com/apache/apisix-ingress-controller/pull/1460#issuecomment-1453203631

   Please resolve conflicts. 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] Chever-John commented on a diff in pull request #1460: make: dev-env

Posted by "Chever-John (via GitHub)" <gi...@apache.org>.
Chever-John commented on code in PR #1460:
URL: https://github.com/apache/apisix-ingress-controller/pull/1460#discussion_r1186958059


##########
Makefile:
##########
@@ -325,3 +324,13 @@ install-gateway-api-local:
 .PHONY: uninstall-gateway-api
 uninstall-gateway-api:
 	kubectl delete -f $(GATEWAY_API_CRDS_LOCAL_PATH)
+
+### dev-env:			  Launch development environment
+.PHONY: dev-env
+dev-env: kind-up pack-image
+	./utils/install-apisix-with-helm.sh
+	kubectl set image deployment/apisix-ingress-controller ingress-controller=$(REGISTRY)/apache/apisix-ingress-controller:dev --namespace ingress-apisix

Review Comment:
   change it to 
   
   `kubectl set image deployment/apisix-ingress-controller ingress-controller=$(REGISTRY)/apisix-ingress-controller:dev --namespace ingress-apisix`



-- 
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] zou8944 commented on pull request #1460: make: dev-env

Posted by GitBox <gi...@apache.org>.
zou8944 commented on PR #1460:
URL: https://github.com/apache/apisix-ingress-controller/pull/1460#issuecomment-1367709927

   Sorry for late response, draft status removed.
   
   I keeped it as draft status, as nobody answer me in this issue https://github.com/apache/apisix-ingress-controller/issues/1146. could you give me some advise?


-- 
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] Chever-John commented on a diff in pull request #1460: make: dev-env

Posted by "Chever-John (via GitHub)" <gi...@apache.org>.
Chever-John commented on code in PR #1460:
URL: https://github.com/apache/apisix-ingress-controller/pull/1460#discussion_r1161364824


##########
Makefile:
##########
@@ -325,3 +324,19 @@ install-gateway-api-local:
 .PHONY: uninstall-gateway-api
 uninstall-gateway-api:
 	kubectl delete -f $(GATEWAY_API_CRDS_LOCAL_PATH)
+
+### dev-env:			  Launch development environment
+.PHONY: dev-env
+dev-env: kind-up pack-image
+	helm repo add apisix https://charts.apiseven.com
+	helm repo update
+	helm install apisix apisix/apisix \
+		--set gateway.type=NodePort \
+		--set ingress-controller.enabled=true \
+		--namespace ingress-apisix \
+		--create-namespace
+	kubectl set image deployment/apisix-ingress-controller ingress-controller=$(REGISTRY)/apache/apisix-ingress-controller:dev --namespace ingress-apisix
+

Review Comment:
   Of course, it is my habit to make mistakes in this place. 
   
   Perhaps we need @tao12345666333 's pieces of advice.



-- 
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] aynp commented on a diff in pull request #1460: make: dev-env

Posted by "aynp (via GitHub)" <gi...@apache.org>.
aynp commented on code in PR #1460:
URL: https://github.com/apache/apisix-ingress-controller/pull/1460#discussion_r1161343050


##########
Makefile:
##########
@@ -325,3 +324,19 @@ install-gateway-api-local:
 .PHONY: uninstall-gateway-api
 uninstall-gateway-api:
 	kubectl delete -f $(GATEWAY_API_CRDS_LOCAL_PATH)
+
+### dev-env:			  Launch development environment
+.PHONY: dev-env
+dev-env: kind-up pack-image
+	helm repo add apisix https://charts.apiseven.com
+	helm repo update
+	helm install apisix apisix/apisix \
+		--set gateway.type=NodePort \
+		--set ingress-controller.enabled=true \
+		--namespace ingress-apisix \
+		--create-namespace
+	kubectl set image deployment/apisix-ingress-controller ingress-controller=$(REGISTRY)/apache/apisix-ingress-controller:dev --namespace ingress-apisix
+

Review Comment:
   I guess newer versions of helm automatically detect if the repository exists and skips over without throwing any error. 
   Also if errors are to be handled, maybe it would be cleaner to have a script file just like `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] tao12345666333 commented on pull request #1460: Draft: make: dev-env

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

   Are you still working on this PR? I see the status of draft in the title


-- 
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] zou8944 commented on a diff in pull request #1460: make: dev-env

Posted by "zou8944 (via GitHub)" <gi...@apache.org>.
zou8944 commented on code in PR #1460:
URL: https://github.com/apache/apisix-ingress-controller/pull/1460#discussion_r1166248206


##########
Makefile:
##########
@@ -325,3 +324,19 @@ install-gateway-api-local:
 .PHONY: uninstall-gateway-api
 uninstall-gateway-api:
 	kubectl delete -f $(GATEWAY_API_CRDS_LOCAL_PATH)
+
+### dev-env:			  Launch development environment
+.PHONY: dev-env
+dev-env: kind-up pack-image
+	helm repo add apisix https://charts.apiseven.com
+	helm repo update
+	helm install apisix apisix/apisix \
+		--set gateway.type=NodePort \
+		--set ingress-controller.enabled=true \
+		--namespace ingress-apisix \
+		--create-namespace
+	kubectl set image deployment/apisix-ingress-controller ingress-controller=$(REGISTRY)/apache/apisix-ingress-controller:dev --namespace ingress-apisix
+

Review Comment:
   Thanks for all your suggestions, I will try is later. 



-- 
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] zou8944 commented on a diff in pull request #1460: make: dev-env

Posted by "zou8944 (via GitHub)" <gi...@apache.org>.
zou8944 commented on code in PR #1460:
URL: https://github.com/apache/apisix-ingress-controller/pull/1460#discussion_r1167786982


##########
Makefile:
##########
@@ -325,3 +324,19 @@ install-gateway-api-local:
 .PHONY: uninstall-gateway-api
 uninstall-gateway-api:
 	kubectl delete -f $(GATEWAY_API_CRDS_LOCAL_PATH)
+
+### dev-env:			  Launch development environment
+.PHONY: dev-env
+dev-env: kind-up pack-image
+	helm repo add apisix https://charts.apiseven.com
+	helm repo update
+	helm install apisix apisix/apisix \
+		--set gateway.type=NodePort \
+		--set ingress-controller.enabled=true \
+		--namespace ingress-apisix \
+		--create-namespace
+	kubectl set image deployment/apisix-ingress-controller ingress-controller=$(REGISTRY)/apache/apisix-ingress-controller:dev --namespace ingress-apisix
+

Review Comment:
   @Chever-John @tao12345666333 I update the code to skip installation if apisix already exists, and to keep Makefile clean, I moved the installation code to separated shell script `utils/install-apisix-with-helm.sh`, please review. 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] zou8944 commented on a diff in pull request #1460: make: dev-env

Posted by "zou8944 (via GitHub)" <gi...@apache.org>.
zou8944 commented on code in PR #1460:
URL: https://github.com/apache/apisix-ingress-controller/pull/1460#discussion_r1177859117


##########
utils/install-apisix-with-helm.sh:
##########
@@ -0,0 +1,31 @@
+#!/usr/bin/env bash
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+helm list -n ingress-apisix | grep -wq apisix
+if [ $? -ne 0 ]; then
+  echo "installing apisix"
+  helm repo add apisix https://charts.apiseven.com

Review Comment:
   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] github-actions[bot] commented on pull request #1460: make: dev-env

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #1460:
URL: https://github.com/apache/apisix-ingress-controller/pull/1460#issuecomment-1449179917

   This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 30 days if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the dev@apisix.apache.org list. Thank you for your contributions.


-- 
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 diff in pull request #1460: make: dev-env

Posted by "tao12345666333 (via GitHub)" <gi...@apache.org>.
tao12345666333 commented on code in PR #1460:
URL: https://github.com/apache/apisix-ingress-controller/pull/1460#discussion_r1169565735


##########
utils/install-apisix-with-helm.sh:
##########
@@ -0,0 +1,31 @@
+#!/usr/bin/env bash
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+helm list -n ingress-apisix | grep -wq apisix
+if [ $? -ne 0 ]; then
+  echo "installing apisix"
+  helm repo add apisix https://charts.apiseven.com

Review Comment:
   We need to add the bitnami repo also. Please add the following command
   
   ```
   helm repo add bitnami https://charts.bitnami.com/bitnami
   ```



-- 
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] Chever-John commented on a diff in pull request #1460: make: dev-env

Posted by "Chever-John (via GitHub)" <gi...@apache.org>.
Chever-John commented on code in PR #1460:
URL: https://github.com/apache/apisix-ingress-controller/pull/1460#discussion_r1190110068


##########
Makefile:
##########
@@ -325,3 +324,13 @@ install-gateway-api-local:
 .PHONY: uninstall-gateway-api
 uninstall-gateway-api:
 	kubectl delete -f $(GATEWAY_API_CRDS_LOCAL_PATH)
+
+### dev-env:			  Launch development environment
+.PHONY: dev-env
+dev-env: kind-up pack-image
+	./utils/install-apisix-with-helm.sh
+	kubectl set image deployment/apisix-ingress-controller ingress-controller=$(REGISTRY)/apache/apisix-ingress-controller:dev --namespace ingress-apisix

Review Comment:
   ![image](https://github.com/apache/apisix-ingress-controller/assets/43690894/07d8ad53-345b-4f5c-80ca-95ed632ae903)
   
   Hi, anything else problems 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] Chever-John commented on a diff in pull request #1460: make: dev-env

Posted by "Chever-John (via GitHub)" <gi...@apache.org>.
Chever-John commented on code in PR #1460:
URL: https://github.com/apache/apisix-ingress-controller/pull/1460#discussion_r1160626545


##########
Makefile:
##########
@@ -325,3 +324,19 @@ install-gateway-api-local:
 .PHONY: uninstall-gateway-api
 uninstall-gateway-api:
 	kubectl delete -f $(GATEWAY_API_CRDS_LOCAL_PATH)
+
+### dev-env:			  Launch development environment
+.PHONY: dev-env
+dev-env: kind-up pack-image
+	helm repo add apisix https://charts.apiseven.com
+	helm repo update
+	helm install apisix apisix/apisix \
+		--set gateway.type=NodePort \
+		--set ingress-controller.enabled=true \
+		--namespace ingress-apisix \
+		--create-namespace
+	kubectl set image deployment/apisix-ingress-controller ingress-controller=$(REGISTRY)/apache/apisix-ingress-controller:dev --namespace ingress-apisix
+

Review Comment:
   Hi, can u try this?
   
   ```makefile
   
   ### dev-env:			  Launch development environment
   .PHONY: dev-env
   dev-env: kind-up pack-image
   	-@helm repo add apisix https://charts.apiseven.com 2>&1 | grep -q "Error: repository name (apisix) already exists" && echo "APISIX chart exists, skipping helm repo add"
   	@helm repo update
   	helm list -n ingress-apisix | grep -q apisix && echo "APISIX already exists, skipping installation" || \
   	helm install apisix apisix/apisix \
   		--set gateway.type=NodePort \
   		--set ingress-controller.enabled=true \
   		--namespace ingress-apisix \
   		--create-namespace
   	kubectl set image deployment/apisix-ingress-controller ingress-controller=$(REGISTRY)/apache/apisix-ingress-controller:dev --namespace ingress-apisix
   
   
   ```



-- 
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] github-actions[bot] commented on pull request #1460: make: dev-env

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #1460:
URL: https://github.com/apache/apisix-ingress-controller/pull/1460#issuecomment-1627917262

   This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 30 days if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the dev@apisix.apache.org list. Thank you for your contributions.


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