Skip to content
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: support devcontainer #11765

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

bzp2010
Copy link
Contributor

@bzp2010 bzp2010 commented Nov 21, 2024

Description

Add devcontainer support to the project, which is expected to cover Linux, Windows (WSL2), macOS OSs, and the amd64, arm64 CPU ISAs, i.e. the environments used by most developers.

Basically, the container provides an isolated environment from the host, the runtime and toolset required for APISIX development are built in the container build, they will be available out of the box and developers no longer need to build and install them on the host. Just start dev container, wait a few moments, and it's ready to go.

Significantly simplify the parts that may be unfamiliar to developers and just focus on writing Lua code and test cases.

In addition to this, etcd is ready, maintained by docker-compose, and accessible on the local loopback.

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@bzp2010
Copy link
Contributor Author

bzp2010 commented Nov 21, 2024

I have tested this on Windows WSL2 + Docker Desktop and macOS + OrbStack, where macOS is an arm64 based m4 chip.
Developers don't need to make any individual adjustments to any of the above environments, they will automatically do the right thing. (e.g. Installs the arm64-based binaries)

However, I don't have a desktop system running Linux, which would require someone else to help test, but that should not be a problem.

@bzp2010
Copy link
Contributor Author

bzp2010 commented Nov 21, 2024

The error in the CI is not related to this PR, it also fails on other PRs, will investigate it later.

UPDATE

I have fixed the issue in #11769.

@bzp2010 bzp2010 force-pushed the bzp/feat-devcontainer branch from 5533908 to 44bc599 Compare November 21, 2024 12:43
@bzp2010 bzp2010 marked this pull request as ready for review November 22, 2024 04:48
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. enhancement New feature or request labels Nov 22, 2024
@bzp2010 bzp2010 requested a review from SkyeYoung November 22, 2024 06:19
@SkyeYoung
Copy link
Member

🆒 I will help with testing using Manjaro KDE Plasma(amd64) + Docker Engine.

@SkyeYoung
Copy link
Member

SkyeYoung commented Nov 22, 2024

Tested in Manjaro KDE Plasma (amd64) + Docker Engine:

image image

But I encountered some problems during the test:

image image

Error Log

error.log

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants