-
Notifications
You must be signed in to change notification settings - Fork 92
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: use docker to deploy fpm #55
Conversation
fix: add proxy for testing fix: `ARG` before `FROM` test: add test for all branches test: add comments fix: remove docker system prune for testing fix: apisix output path fix: Update package-apisix-openresty.yml fix: apisix-dashboard output path fix: apisix-openresty path fix: bugs fix: bugs fix: bugs fix: openresty path fix: Update package-apisix-openresty.yml fix: Update Dockerfile.apisix.rpm fix: Update Makefile fix: dashboard bash script fix: dashboard bash script name fix: Makefile dashboard copy path fix: Dashboard and openresty output path fix: add docker system prune fix: remove comments and proxy fix: remove test for all branches
fdfabf6
to
d73bfed
Compare
dockerfiles/Dockerfile.apisix.rpm
Outdated
@@ -1,6 +1,6 @@ | |||
ARG image_base="centos" | |||
ARG image_tag="7" | |||
ARG checkout_v="2.2" | |||
ARG checkout_v="2.6" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We always require to pass a checkout_v. Could we assign it to an empty string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes.
Makefile
Outdated
--config-files usr/lib/systemd/system/apisix.service | ||
rm -rf ${PWD}/build | ||
docker build -t apache/apisix-packaged:$(version) \ | ||
--build-arg VERSION=$(version) \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please fix the indentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK.
Makefile
Outdated
--build-arg ITERATION=$(iteration) \ | ||
--build-arg PACKAGE_VERSION=$(version) \ | ||
-f ./dockerfiles/Dockerfile.package.apisix . | ||
docker run -d --name output --net="host" apache/apisix-packaged:$(version) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docker run --rm?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK
### build fpm for packaging: | ||
.PHONY: build-fpm | ||
build-fpm: | ||
docker build -t api7/fpm - < ./dockerfiles/Dockerfile.fpm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use -f
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if use -f
, we need to pass the context
which include files that are not necessary for building an image. This will results in a larger build context and larger image size.
@@ -22,7 +22,7 @@ jobs: | |||
|
|||
- name: build apisix-openresty rpm | |||
run: | | |||
make package type=rpm app=apisix-openresty version=${BUILD_APISIX_OR_VERSION} | |||
make package type=rpm app=apisix-openresty version=${BUILD_APISIX_OR_VERSION} checkout=${BUILD_APISIX_OR_VERSION} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
apisix-openresty uses 'checkout'?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the reminder. I will remove it.
Fix: checkout for apisix
Fix: #20
What this PR does: