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 2021/10/11 12:28:00 UTC
[GitHub] [apisix] leslie-tsang opened a new pull request #5200: chore: replace `install-ext-services-via-docker.sh` with docker-compose
leslie-tsang opened a new pull request #5200:
URL: https://github.com/apache/apisix/pull/5200
Ref to #5188
Optimization:
* Global sync exec becomes async exec will reduce some launch time (unready)
* Use healthcheck feature to solve startup sequence deps, this will do help for low pref machine to successfully launch ext service
* Now all CI Action share docker service
Signed-off-by: leslie <le...@icloud.com>
### 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. -->
### Pre-submission checklist:
* [x] 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/tree/master#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] leslie-tsang commented on a change in pull request #5200: chore: replace `install-ext-services-via-docker.sh` with docker-compose
Posted by GitBox <gi...@apache.org>.
leslie-tsang commented on a change in pull request #5200:
URL: https://github.com/apache/apisix/pull/5200#discussion_r726144560
##########
File path: ci/pod/nacos/service/Dockerfile
##########
@@ -0,0 +1,16 @@
+
Review comment:
BTW, I found it will cost 800m+ memory for this container. Is there a way to reduce the memory cost?
##########
File path: Makefile
##########
@@ -15,14 +15,45 @@
# limitations under the License.
#
+# Makefile basic env setting
+.DEFAULT_GOAL := help
+# add pipefail support for default shell
+SHELL := /bin/bash -o pipefail
+
+
+# Project basic setting
+project_name ?= apache-apisix
+project_version ?= latest
+project_compose_ci ?= ci/pod/docker-compose.yml
+project_release_name ?= $(project_name)-$(project_version)-src
+
+
+# Hyperconverged Infrastructure
+ENV_OS_NAME ?= $(shell uname -s | tr '[:upper:]' '[:lower:]')
+ENV_OS_ARCH ?= $(shell uname -m | tr '[:upper:]' '[:lower:]')
+ENV_APISIX ?= $(CURDIR)/bin/apisix
+ENV_GIT ?= git
+ENV_DOCKER ?= docker
+ENV_DOCKER_COMPOSE ?= docker-compose --project-directory $(CURDIR) -p $(project_name) -f $(project_compose_ci)
+ENV_NGINX ?= nginx -p $(CURDIR) -c $(CURDIR)/conf/nginx.conf
+
+
+# OSX archive `._` cache file
+ifeq ($(ENV_OS_NAME), darwin)
+ ENV_TAR ?= COPYFILE_DISABLE=1 tar
+else
+ ENV_TAR ?= tar
+endif
+
+
+# OLD VAR
INST_PREFIX ?= /usr
INST_LIBDIR ?= $(INST_PREFIX)/lib64/lua/5.1
INST_LUADIR ?= $(INST_PREFIX)/share/lua/5.1
INST_BINDIR ?= /usr/bin
INSTALL ?= install
-UNAME ?= $(shell uname)
-UNAME_MACHINE ?= $(shell uname -m)
OR_EXEC ?= $(shell which openresty || which nginx)
+LUAJIT_DIR ?= $(shell ${OR_EXEC} -V 2>&1 | grep prefix | grep -Eo 'prefix=(.*)/nginx\s+--' | grep -Eo '/.*/')luajit
Review comment:
Unused. can we remove it ?
##########
File path: ci/pod/nacos/service/Dockerfile
##########
@@ -0,0 +1,16 @@
+
Review comment:
@spacewander It may be better to use an apisix official docker image to replace it.
Can APISIX community build an official docker hub image ?
--
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] spacewander commented on a change in pull request #5200: chore: replace `install-ext-services-via-docker.sh` with docker-compose
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5200:
URL: https://github.com/apache/apisix/pull/5200#discussion_r726161448
##########
File path: Makefile
##########
@@ -15,14 +15,45 @@
# limitations under the License.
#
+# Makefile basic env setting
+.DEFAULT_GOAL := help
+# add pipefail support for default shell
+SHELL := /bin/bash -o pipefail
+
+
+# Project basic setting
+project_name ?= apache-apisix
+project_version ?= latest
+project_compose_ci ?= ci/pod/docker-compose.yml
+project_release_name ?= $(project_name)-$(project_version)-src
+
+
+# Hyperconverged Infrastructure
+ENV_OS_NAME ?= $(shell uname -s | tr '[:upper:]' '[:lower:]')
+ENV_OS_ARCH ?= $(shell uname -m | tr '[:upper:]' '[:lower:]')
+ENV_APISIX ?= $(CURDIR)/bin/apisix
+ENV_GIT ?= git
+ENV_DOCKER ?= docker
+ENV_DOCKER_COMPOSE ?= docker-compose --project-directory $(CURDIR) -p $(project_name) -f $(project_compose_ci)
+ENV_NGINX ?= nginx -p $(CURDIR) -c $(CURDIR)/conf/nginx.conf
+
+
+# OSX archive `._` cache file
+ifeq ($(ENV_OS_NAME), darwin)
+ ENV_TAR ?= COPYFILE_DISABLE=1 tar
+else
+ ENV_TAR ?= tar
+endif
+
+
+# OLD VAR
INST_PREFIX ?= /usr
INST_LIBDIR ?= $(INST_PREFIX)/lib64/lua/5.1
INST_LUADIR ?= $(INST_PREFIX)/share/lua/5.1
INST_BINDIR ?= /usr/bin
INSTALL ?= install
-UNAME ?= $(shell uname)
-UNAME_MACHINE ?= $(shell uname -m)
OR_EXEC ?= $(shell which openresty || which nginx)
+LUAJIT_DIR ?= $(shell ${OR_EXEC} -V 2>&1 | grep prefix | grep -Eo 'prefix=(.*)/nginx\s+--' | grep -Eo '/.*/')luajit
Review comment:
OK
--
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] spacewander commented on pull request #5200: chore: replace `install-ext-services-via-docker.sh` with docker-compose
Posted by GitBox <gi...@apache.org>.
spacewander commented on pull request #5200:
URL: https://github.com/apache/apisix/pull/5200#issuecomment-940078377
>
>
> > Could you split the change in Makefile to a separate PR?
>
> Sure. But this change will break this PR. Makefile PR need to be merge first. This PR is **not ready** to merge right now, further discuss and development in need. Can we keep it in one PR ?
Err. It is a headache to review such big PR. Especially the change in Makefile is fragmented.
--
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] spacewander commented on pull request #5200: chore: replace `install-ext-services-via-docker.sh` with docker-compose
Posted by GitBox <gi...@apache.org>.
spacewander commented on pull request #5200:
URL: https://github.com/apache/apisix/pull/5200#issuecomment-940079075
We can make a Makefile PR without the docker compose part, and merge it 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] spacewander commented on pull request #5200: chore: replace `install-ext-services-via-docker.sh` with docker-compose
Posted by GitBox <gi...@apache.org>.
spacewander commented on pull request #5200:
URL: https://github.com/apache/apisix/pull/5200#issuecomment-940039865
Could you split the change in Makefile to a separate PR?
--
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] leslie-tsang closed pull request #5200: chore: replace `install-ext-services-via-docker.sh` with docker-compose
Posted by GitBox <gi...@apache.org>.
leslie-tsang closed pull request #5200:
URL: https://github.com/apache/apisix/pull/5200
--
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] leslie-tsang commented on pull request #5200: chore: replace `install-ext-services-via-docker.sh` with docker-compose
Posted by GitBox <gi...@apache.org>.
leslie-tsang commented on pull request #5200:
URL: https://github.com/apache/apisix/pull/5200#issuecomment-940079421
> We can make a Makefile PR without the docker compose part, and merge it 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] spacewander merged pull request #5200: ci: replace `install-ext-services-via-docker.sh` with docker-compose
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #5200:
URL: https://github.com/apache/apisix/pull/5200
--
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] spacewander commented on a change in pull request #5200: chore: replace `install-ext-services-via-docker.sh` with docker-compose
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5200:
URL: https://github.com/apache/apisix/pull/5200#discussion_r726125928
##########
File path: ci/pod/nacos/healthcheck/nacos-server-healthcheck.sh
##########
@@ -0,0 +1,9 @@
+#!/bin/bash
+#set -ex
Review comment:
Why comment out 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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] leslie-tsang commented on pull request #5200: chore: replace `install-ext-services-via-docker.sh` with docker-compose
Posted by GitBox <gi...@apache.org>.
leslie-tsang commented on pull request #5200:
URL: https://github.com/apache/apisix/pull/5200#issuecomment-940080078
> > > Could you split the change in Makefile to a separate PR?
> >
> >
> > Sure. But this change will break this PR. Makefile PR need to be merge first. This PR is **not ready** to merge right now, further discuss and development in need. Can we keep it in one PR ?
>
> Err. It is a headache to review such big PR. Especially the change in Makefile is fragmented.
Make sense. I will spilt 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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] leslie-tsang commented on pull request #5200: chore: replace `install-ext-services-via-docker.sh` with docker-compose
Posted by GitBox <gi...@apache.org>.
leslie-tsang commented on pull request #5200:
URL: https://github.com/apache/apisix/pull/5200#issuecomment-939986453
@spacewander @tzssangglass @Yiyiyimu @membphis
Any feedback appricated. 🙏
--
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] leslie-tsang commented on pull request #5200: chore: replace `install-ext-services-via-docker.sh` with docker-compose
Posted by GitBox <gi...@apache.org>.
leslie-tsang commented on pull request #5200:
URL: https://github.com/apache/apisix/pull/5200#issuecomment-940050946
> Could you split the change in Makefile to a separate PR?
Sure. But this change will break this PR. Makefile PR need to be merge first.
This PR is **not ready** to merge right now, further discuss and development in need.
Can we keep it in one PR ?
--
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] leslie-tsang removed a comment on pull request #5200: chore: replace `install-ext-services-via-docker.sh` with docker-compose
Posted by GitBox <gi...@apache.org>.
leslie-tsang removed a comment on pull request #5200:
URL: https://github.com/apache/apisix/pull/5200#issuecomment-940079421
> We can make a Makefile PR without the docker compose part, and merge it 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] leslie-tsang commented on pull request #5200: chore: replace `install-ext-services-via-docker.sh` with docker-compose
Posted by GitBox <gi...@apache.org>.
leslie-tsang commented on pull request #5200:
URL: https://github.com/apache/apisix/pull/5200#issuecomment-941168461
@tokers Could you please review this PR ?
--
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] leslie-tsang commented on pull request #5200: chore: replace `install-ext-services-via-docker.sh` with docker-compose
Posted by GitBox <gi...@apache.org>.
leslie-tsang commented on pull request #5200:
URL: https://github.com/apache/apisix/pull/5200#issuecomment-941168461
@tokers Could you please review this PR ?
--
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] tzssangglass commented on a change in pull request #5200: ci: replace `install-ext-services-via-docker.sh` with docker-compose
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on a change in pull request #5200:
URL: https://github.com/apache/apisix/pull/5200#discussion_r727852007
##########
File path: ci/pod/nacos/service/Dockerfile
##########
@@ -0,0 +1,16 @@
+
Review comment:
Perhaps we need to adapt the nacos test cases and the nacos test service, we now have 7 nacos for testing, that's a lot.
--
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] spacewander merged pull request #5200: ci: replace `install-ext-services-via-docker.sh` with docker-compose
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #5200:
URL: https://github.com/apache/apisix/pull/5200
--
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] tzssangglass commented on a change in pull request #5200: ci: replace `install-ext-services-via-docker.sh` with docker-compose
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on a change in pull request #5200:
URL: https://github.com/apache/apisix/pull/5200#discussion_r727852007
##########
File path: ci/pod/nacos/service/Dockerfile
##########
@@ -0,0 +1,16 @@
+
Review comment:
Perhaps we need to adapt the nacos test cases and the nacos test service, we now have 7 nacos for testing, that's a lot.
--
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] leslie-tsang commented on a change in pull request #5200: chore: replace `install-ext-services-via-docker.sh` with docker-compose
Posted by GitBox <gi...@apache.org>.
leslie-tsang commented on a change in pull request #5200:
URL: https://github.com/apache/apisix/pull/5200#discussion_r726126989
##########
File path: ci/pod/nacos/healthcheck/nacos-server-healthcheck.sh
##########
@@ -0,0 +1,9 @@
+#!/bin/bash
+#set -ex
Review comment:
debug mode. will fix in next commit.
--
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