[Feature] Configure devcontainer.json

PeterJanRoes commented 1 year ago

Describe the solution you'd like Configure a /.devcontainer/devcontainer.json so any contributor can start developing right away with all required tools installed. This also ensures a consistent development environment across all the contributors that use the devcontainer.

Because the current development environment relies on a hardcoded 172.18.0.0/16 subnet for virtual IPs provided by MetalLB, it is expected that some tweaks are needed to get the environment working when the subnet is not known in advance. To find out the effective subnet the command docker network inspect kind can be used.

PeterJanRoes commented 1 year ago

I have created a basic devcontainer. The container works well. I can execute make cluster, make install, make net and make test successfully. The following things are still open:

Are there any other VS Code extensions that are valuable to add to the devcontainer? I can add those as well.
Currently it seems not possible to make cluster-delete and make cluster in succession. The command make cluster will hang on the "Creating control plane" phase. A work-around is to make cluster-delete, rebuild the devcontainer and then make cluster in the fresh container. I will look into it more to make this more stable.
I changed code to dynamically determine the Kind network CIDR and use that in the MetalLB services and tests. The changes should still work when developing outside of a devcontainer, but I cannot test this as the original approach only works on a Linux development machine and I am using Windows/WSL/Docker Desktop/VS Code. I figure these changes are an improvement for other developers on Linux as well if they already have other docker networks configured before Kind is deployed. @mmontes11, can you test the changes outside the devcontainer?
It looks like the performance is okay. On my machine the tests take around the same time as in the CI. However, I do notice that when executing make test for the first time I often hit a timeout of 60s, possibly because I am on a slower internet connection right now and it needs to pull images. The second time make test succeeds (the images are already pulled). I can also increase this timeout to 120s for instance, but I am not sure whether the 60s is configured on purpose. Can I increase the timeout?

mmontes11 commented 1 year ago

I have started looking into it, please bear with me as I'm not too familiar with devcontainer. To give you early feedback, this is amazing, thanks for the hard word on this one, this is a massive contribution 🦭 ❤️

Are there any other VS Code extensions that are valuable to add to the devcontainer? I can add those as well.

These are the ones I actually use in my local vscode, for a moment I was wondering whether I was in my local vscode or in the devcontainer. I think it's fine to keep this ones in the initial version.

will hang on the "Creating control plane" phase

It actually worked for me:

vscode ➜ /workspaces/mariadb-operator (configure-devcontainer) $ make cluster-delete
GOBIN=/workspaces/mariadb-operator/bin go install sigs.k8s.io/kind@v0.20.0
/workspaces/mariadb-operator/bin/kind delete cluster --name mdb
Deleting cluster "mdb" ...
Deleted nodes: ["mdb-control-plane"]
vscode ➜ /workspaces/mariadb-operator (configure-devcontainer) $ make cluster
/workspaces/mariadb-operator/bin/kind create cluster --name mdb --image kindest/node:v1.27.3
Creating cluster "mdb" ...
 ✓ Ensuring node image (kindest/node:v1.27.3) 🖼
 ✓ Preparing nodes 📦  
 ✓ Writing configuration 📜 
 ✓ Starting control-plane 🕹️ 
 ✓ Installing CNI 🔌 
 ✓ Installing StorageClass 💾 
Set kubectl context to "kind-mdb"
You can now use your cluster with:

kubectl cluster-info --context kind-mdb

Thanks for using kind! 😊
vscode ➜ /workspaces/mariadb-operator (configure-devcontainer) $ ^C

In any case, this shouldn't be a blocker for merging the PR

can you test the changes outside the devcontainer?

The CI tests passed, so this works flawlessly. I've tested on my Ubuntu 22.04 and works as well. Well done 🚀

Can I increase the timeout?

Yes, please proceed. The reason behind these 60s is not consuming too much CI time in case that something hangs, but increasing it to enable testing locally makes total sense.

I've noticed that the CI lint is failing:

pkg/docker/network.go:40:17: Error return value of `json.Unmarshal` is not checked (errcheck)
                json.Unmarshal([]byte(output), &networks)
                              ^
make: *** [make/dev.mk:15: lint] Error 1

This should be easy to fix, feel free to add //nolint if you think it doesn't really worth it 👍🏻 .

On my side, I will create a development guide documenting this. It's a bit of a shame that Metallb doesn't work well with Windows/Mac, see:

https://kind.sigs.k8s.io/docs/user/loadbalancer/

Anyway, this is a very nice contributor experience and we should be covered to onboard Windows/Mac contributors. Not only that, probably there will be potential contributors that are not willing to change their /etc/hosts or they just prefer to have a ephemeral devcontainer environment.

Again, thanks for the awesome work 🙇🏻

PeterJanRoes commented 1 year ago

I fixed the lint issue. Also, I cleaned up the commits a bit.

In any case, this shouldn't be a blocker for merging the PR

I agree. It probably is a quirk with the WSL/Docker Desktop setup. Rebuilding the container is not that much of a deal. Maybe I run into a fix in the future.

Yes, please proceed. The reason behind these 60s is not consuming too much CI time in case that something hangs, but increasing it to enable testing locally makes total sense.

I did not encounter this timeout anymore. I decided to leave it at 60s. If I run into it again in the future I will open a new pull request.

Anyway, this is a very nice contributor experience and we should be covered to onboard Windows/Mac contributors. Not only that, probably there will be potential contributors that are not willing to change their /etc/hosts or they just prefer to have a ephemeral devcontainer environment.

I can even imagine that after getting used to this dev container it could get to be the primary way to develop on the operator because it allows you as a maintainer to fully control the toolchain every developer is using. I myself try to work as much as possible in dev containers for my own projects. Finally, I also noticed that GitHub provides codespaces. I have not yet tested this but it seems that the dev container setup could be used for that as well.

I marked the pull request as ready for review.

mmontes11 commented 1 year ago

Hey @PeterJanRoes!

Just merged the PR! Thanks a lot for this massive contribution, as we discussed, this can be a game changer for onboarding new contributors, will write some documentation now:

https://github.com/mariadb-operator/mariadb-operator/issues/144

I did not encounter this timeout anymore. I decided to leave it at 60s. If I run into it again in the future I will open a new pull request.

Absolutely, feel free to do so.

Finally, I also noticed that GitHub provides codespaces. I have not yet tested this but it seems that the dev container setup could be used for that as well.

Yeah, it's a bit of a shame that the free tier is limited, Luckly enough the devcontainer.json standard opens the door to other alternatives:

https://github.com/loft-sh/devpod

mariadb-operator / mariadb-operator

[Feature] Configure devcontainer.json #195