-
Notifications
You must be signed in to change notification settings - Fork 68
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
Improve IPDK cli and ci build/run with buildx, multiarch, artefacts and push to registries #102
Conversation
First stab at implementing multi platform docker images! Please review + need a lot to test and figure out. One of the things to figure out is how to get buildx support on github actions 😉 |
d82d673
to
d60a7de
Compare
Won't this take a really long time to run because of the Qemu emulation? |
Hahaha, yes, thats why I didn't use --platform linux/arm64 (for example) to start with ;-) but it fails anyway as buildx needs to be installed on the github actions ubuntu image.... I need to find out how exactly to cross compile together with buildx platforms, but I have an idea, will work on it tomorrow! |
5b52df0
to
8712575
Compare
Looks like the arm64 requests are taking too long. I think I'll try to setup the OCI GitHub runner today to get that part working and we can do arm64 builds there. |
Yeah, those aarch64 builds are taking too long, but what is worse is that some commands take more then the maximum intermediate response time of github so the runner thinks its dead... Hopefully the Arm runners will help here!!! |
Is it also possible to add docker hub or github registry credentials to the org ? Then we can push the build result there so people don't need to build containers themselves anymore if they don't want to... |
Yes, this is next on my list today, I pushed out a few PRs fixing a few nit-pick things in the meantime. Will work on Dockerhub account and ability to push there next. |
840e8de
to
aa35aad
Compare
aa62aae
to
a9e9cc5
Compare
Added concurrency to prevent several checks for the same PR at the same time, the last push will be run by Github Actions. If more are running they are cancelled. Added Artefact saving, tagging and prepared DockerHub saving and Github Container Registry saving. Will let it rest until after the weekend! Please review! |
a9e9cc5
to
075c23d
Compare
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.
Do you want to separate out the platform support from this PR? It seems as if you're making enhancements to the CI which are not related to the arm platform anymore. Also, I think we should just the OCI arm runners for arm builds. I am going to work on getting that up and running at some point today.
dbfc254
to
a3afa07
Compare
a3afa07
to
ed96411
Compare
Ok, I think everything is implemented to support several solutions of building IPDK containers. Support is available for base distribution selection, single/multi architecture image builds, tags & label adding, save to artefacts and push to registries. All those options also work for local builds and Github CI builds but with the caveat that the local docker image store only supports single architecture images! (that's a docker limitation!!!) The three remaining questions are:
@mestery and others, what would you think???? |
I think we should, but I don't think we can do it using docker buildx with Qemu on the default runners GitHub Actions uses because it's too slow. Ideally, we'd use something like OCI and have our own native arm64 runners.
My thinking is that IPDK should not be building P4-OVS each time, and that we should move to focus on the OVS repository and get proper CI which builds P4-OVS and stores it somewhere. The IPDK build will then pull the latest version of P4-OVS. This means the IPDK build would be really fast, and only people modifying P4-OVS would require the hour long build we currently have.
I think we'd just integrate arm64 runners on OCI by making them runners.
|
@mestery I will change the PR then to use a
Agree, I think we should split a "once a day build" for p4c as well. That shouldn't be a big problem as the (container) build examples already exist for Ubuntu and Fedora and can easily be merged with the IPDK build just like p4-ovs. |
@stolsma Is this ready for review and merge? |
ed96411
to
b3e0d03
Compare
@mestery If the last run checks out ok it is ready for review and merge!!! Changed the action so OCI runner support can be added later by changing |
…nd push to registries Implements large parts of ipdk-io#101 Expanded IPDK CLI commands and options with: - Support `ipdk config [key]=[value]` for setting key=value in ~/.ipdk/ipdk.env - Support `ipdk build --platform [docker buildx supported platforms]` for local and multiarch builds - Support `ipdk build --tags [taglist] --labels [labellist] --push --export [filename]` - Support `ipdk start --platform [docker buildx supported platform]` - Support `ipdk tag [tag]` for local image builds - Support `ipdk export [filename]` for local image builds - Support `ipdk push [tag]` for local image builds - check for support of `docker buildx` on host system Example: `ipdk build --no-cache --platform linux/arm64` builds a ARM64 based ipdk image if run on a linux/amd-64 based host system `ipdk start --platform linux/arm64` starts a ARM64 based image on a linux/amd-64 based host system by using qemu emulation Expanded the Github `makefile.yml` action file with: - concurrency option to prevent running multiple build actions for the same PR or push. - install of qemu runtimes - install of buildx - (for now disabled) login to dockerhub - login to Github Container Registry (ghcr) - create build metadata (tags and labels) - use ipdk cli build command - push the created multiarch container images to the registries when run from push - upload created multiarch container images as artefacts when the action is run from PR - Currently only building containers for one architecture platform (linux/amd-64) Signed-off-by: stolsma <github@tolsma.net>
b3e0d03
to
26db22d
Compare
This is great! I'll merge this one once it passes (and I fix @rst0git's comments), and then this afternoon I'll create an OCI account and try to get an arm64 runner working there! |
A more reliable way of getting docker versions. Signed-off-by: Kyle Mestery <mestery@mestery.com>
If it's not enabled by default on the GitHub runner, try to enable. Signed-off-by: Kyle Mestery <mestery@mestery.com>
e716e8a
to
60ad5be
Compare
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.
LGTM, thanks @stolsma!
docker_experimental="$(docker version --format='{{.Server.Experimental}}')" | ||
if [[ "$docker_experimental" != 'true' ]]; then | ||
echo "docker experimental flag not enabled: Set with 'export DOCKER_CLI_EXPERIMENTAL=enabled'" >&2 | ||
return 1 |
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.
I only check and do not install because in the build code I fall back to the old docker build command if the host doesnt comply to the new docker buildx functionalities... By trying to reconfigure the host you use sudo so the cli will crash instead of using the old command if the user is not allowed to use sudo but is allowed to use docker. Prefer not change the host system but warn....
Just had a 110mph gust around the house... We are still save but trees are coming down in the neighbourhood 😔
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.
The problem was that your original code was not working, and in fact, buildx wasn't enabled on the GitHub runners, and things failed. However, re-enabling it here makes things work ok, so this is what I merged. Feel free to submit a followup which fixes this is if you want.
Implements large parts of #101
Expanded IPDK CLI commands and options with:
ipdk config [key]=[value]
for setting key=value in ~/.ipdk/ipdk.envipdk build --platform [docker buildx supported platforms]
for local and multiarch buildsipdk build --tags [taglist] --labels [labellist] --push --export [filename]
ipdk start --platform [docker buildx supported platform]
ipdk tag [tag]
for local image buildsipdk export [filename]
for local image buildsipdk push [tag]
for local image buildsdocker buildx
on host systemExample:
ipdk build --no-cache --platform linux/arm64
builds a ARM64 based ipdk image if run on a linux/amd-64 based host systemipdk start --platform linux/arm64
starts a ARM64 based image on a linux/amd-64 based host system by using qemu emulationExpanded the Github
makefile.yml
action file with:Signed-off-by: stolsma github@tolsma.net