Skip to content

feat: bump APISIX Dashboard to 2.10#254

Merged
tokers merged 11 commits into
apache:masterfrom
bzp2010:feat-bump-dashboard-2100
Dec 21, 2021
Merged

feat: bump APISIX Dashboard to 2.10#254
tokers merged 11 commits into
apache:masterfrom
bzp2010:feat-bump-dashboard-2100

Conversation

@bzp2010
Copy link
Copy Markdown
Contributor

@bzp2010 bzp2010 commented Dec 16, 2021

Upgrade the latest version of APISIX Dashbaord 2.10 while supporting multi-architecture build and push.

@bzp2010 bzp2010 self-assigned this Dec 16, 2021
@bzp2010 bzp2010 requested a review from nic-chen December 16, 2021 07:56
@bzp2010
Copy link
Copy Markdown
Contributor Author

bzp2010 commented Dec 16, 2021

Hi, all. PTAL, however, it should be noted that these modifications cannot be tested by CI, and CI can only be triggered by push, so please look at the modifications carefully. Thanks!

@bzp2010 bzp2010 changed the title feat: bump APISIX Dashboard 2.10 feat: bump APISIX Dashboard to 2.10 Dec 16, 2021
@bzp2010
Copy link
Copy Markdown
Contributor Author

bzp2010 commented Dec 16, 2021

Please don't merge for now, I can't get the local build test to pass correctly.

@bzp2010 bzp2010 force-pushed the feat-bump-dashboard-2100 branch from f678f1e to 6a24a39 Compare December 17, 2021 02:31
@gxthrj
Copy link
Copy Markdown
Contributor

gxthrj commented Dec 17, 2021

LGTM, Can we merge this feature now?

@bzp2010
Copy link
Copy Markdown
Contributor Author

bzp2010 commented Dec 17, 2021

Hi,@gxthrj.

Docker buildx used in this PR uses the QEMU analog ARM instruction mode to compile the GO program and NodeJS, which will result in serious performance issues, I am working on it (using cross-compilation and fixing --platform).

It still needs to be waited for a while, and about it will be prepared to merge it tomorrow.

@bzp2010
Copy link
Copy Markdown
Contributor Author

bzp2010 commented Dec 19, 2021

Update

I've done a local test of multi-architecture packaging and it completes in about 10 minutes or so [demo], outputting linux/amd64 and linux/arm64 images.

Other

cc @gxthrj @nic-chen @starsz I'm sorry for making some more changes after the review, I fixed the build taking too long, please take a look again.

@bzp2010 bzp2010 requested review from gxthrj and starsz December 19, 2021 11:05
@bzp2010 bzp2010 requested a review from nic-chen December 19, 2021 11:05
@bzp2010 bzp2010 marked this pull request as draft December 19, 2021 11:38
Comment thread Makefile
push-multiarch-dashbaord:
@$(call func_echo_status, "$@ -> [ Start ]")
$(ENV_DOCKER) buildx build --push \
-t $(APISIX_DASHBOARD_IMAGE_NAME):$(APISIX_DASHBOARD_VERSION) \
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to add multiarch to image name.

@bzp2010 bzp2010 marked this pull request as ready for review December 19, 2021 16:35
Comment thread .github/workflows/dashboard-docker-test.yaml
@bzp2010 bzp2010 requested a review from tokers December 20, 2021 02:53
Comment thread dashboard/Dockerfile
RUN if [ "$ENABLE_PROXY" = "true" ] ; then go env -w GOPROXY=https://goproxy.io,direct ; fi \
&& go env -w GO111MODULE=on \
&& CGO_ENABLED=0 ./api/build.sh
&& CGO_ENABLED=0 GOOS=${TARGETOS} GOARCH=${TARGETARCH} ./api/build.sh
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could put GOOS and GOARCH into build.sh later.

Comment thread dashboard/Dockerfile
RUN if [ "$ENABLE_PROXY" = "true" ] ; then go env -w GOPROXY=https://goproxy.io,direct ; fi \
&& go env -w GO111MODULE=on \
&& CGO_ENABLED=0 ./api/build.sh
&& CGO_ENABLED=0 GOOS=${TARGETOS} GOARCH=${TARGETARCH} ./api/build.sh
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could put GOOS and GOARCH into build.sh later.

@tokers tokers merged commit 63acd13 into apache:master Dec 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants