Add check for dev containers
This PR adds a CI heck for DevContainer builds as a double check if a DevContainer builds.
Maybe, it can even be used as basis to do more build checks - based on the build dev container.
Definition of Done
The PR shall be merged only if all items mentioned in CONTRIBUTING.md have been followed. In case an item is not applicable as described, please provide a short explanation in the description.
- [x] All steps in CONTRIBUTING.md have been handled
Reviewed and added some comments.
When we are talking about fast builds, would it be possible to run the workflow only if one of the files changed? Maybe we can take over the checks in the documentation workflow.
@koppor: You could take a look at the documentation_changes job inside the documentation workflow and copy and adapt to something similar like the following:
...
- uses: dorny/[email protected]
id: filter
with:
filters: |
devconainer:
- '.devcontainer/**'
...
However, after the changes through this PR we need to adapt the self-service of Eclipse to make the status check required.
@koppor: You could take a look at the
documentation_changesjob inside the documentation workflow and copy and adapt to something similar like the following:
I used the path filtering directly (as outlined at https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#onpushpull_requestpull_request_targetpathspaths-ignore). Then, this action does not run at all if no changes were made. I think, this is even more efficient.
However, I am not sure how this relates to "required" actions to pass. Needs to be tried out... - In case it works, this would also be helpful for automatic dependency updates including an auto merge (future work).
@koppor: You could take a look at the
documentation_changesjob inside the documentation workflow and copy and adapt to something similar like the following:I used the path filtering directly (as outlined at https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#onpushpull_requestpull_request_targetpathspaths-ignore). Then, this action does not run at all if no changes were made. I think, this is even more efficient.
However, I am not sure how this relates to "required" actions to pass. Needs to be tried out... - In case it works, this would also be helpful for automatic dependency updates including an auto merge (future work).
We cannot use the path filtering directly if we mark this job as required status check later. Then the user is accidentally blocked from merging, because a workflow trigger condition that is skipped (due to no changes were made to the specified files) is marked as a pending job and would prevent the user from merging the PR. This is why you must transfer a workflow condition to a job condition. A skipped job condition would not mark a status check as pending even if the workflow itself is marked as required status check. The suggested Github action (in the documentation workflow) executes the path filter as a job condition. Then merges are not blocked.
This is also mentioned in your posted link (please have a look into the "note" box, where Github doc says explicitly "Note: You should not use path filtering to skip workflow runs if the workflow has been configured to be required.")
I parallelized the check for base and the "real" image, because the "real" image needs an image from the registry (and does not depend on the build base image).
Refs https://github.com/eclipse-ankaios/ankaios/issues/126 (because an enhancement of this workflow will publish a release of the DevContainer upon successful build and probably also update the Dockerfile automatically).
I currently don't have the time to continue here. Sorry for the efforts caused!
Sorry to hear that 🙁 Still, thanks a lot for the idea! We will take it over in #89
I found some time to continue.
I made following changes:
- Check for changes included in the jobs itself
- Added caching for docker layers (improves build speed by approx one minute)
- Added another job for a chained build: In case the base image changes, the final image is build on the current version of the base image in the repository.
I just wonder whether we can also use this action to build the base image for the dev container and how long this would take for building an amd64 and arm64 image using public Github Action runners? Probably the build would have to be started manually and then a parameter for the tag needs to be passed. First the check should run and finally the base image needs to be pushed. Then the tag name probably needs to be entered manually in the derived Dockerfile using the base image.
@windsource Sure. We can pair up, since I do not have your docker registry credentials. Someone with Ankaios registry push permissions should join the team.
Versioning is hard. We all know. Maybe using development or dev as "moving" tag? If you intend to release manually, for sure, fixed tags are OK. We could even include GitVersion to automatically set a meaningful SemVer of the Docker containers.
See line 114 (https://github.com/eclipse-ankaios/ankaios/pull/116/files#diff-d81ffa0ff90993ddb0df7ff9678e69ffa70438cace8f1a34804f3d03de55586eR114) that I even changed the reference of the next Dockerfile to use the previously build one.
Since this is a hobby contribution of me, I would really appreciate it if I could discuss with someone than guess what the architects of Ankaios thought with their containers and especially the container versioning, interpret that, build a separate CI/CD pipeline, push to my own Docker profile, propose something, ask to put credentials somewhere and then incorporate feedback etc. -- It's OK for me if this is the only way, then I'll do as soon I have time.
@windsource arm64 is IMHO only possible with BuildJet easily: https://buildjet.com/for-github-actions/docs/about/pricing#arm. I successfully run that in another project. (More background at https://github.com/actions/runner-images/issues/5631#issuecomment-1517056407)
@windsource
Probably the build would have to be started manually and then a parameter for the tag needs to be passed. First the check should run and finally the base image needs to be pushed
Currently, the build is done on each change of the image. And @inf17101 even proposed to make the check mandatory: https://github.com/eclipse-ankaios/ankaios/pull/116#issuecomment-1850259501.
One can also trigger builds if a tag is set. I don't know about your versioning policy in detail. I would version the dev containers along with the releases of Ankaios. Thus, at each "tag" of Ankaois, a bulid and push is triggered.
I am happy to contribute, but I need more guidance about your wishes and also credentials for the Docker registry.
Hi @koppor, sure, let's discuss the usage of containers in development and CI for Ankaios. So current we have the base container which serves as base of the dev container during development and also for CI builds. So the base container needs to contain all the tools that are also used for CI builds. Currently we build the base container manually. So before a PR with a change in the base container can be merged, the base container needs to be build manually by a committer and pushed to ghcr. In this step we also increase the version number of the container. Reproducible builds are super important for me. For that reason we do not use latest or dev as label. In order to also support Mac with Apple Silicon we create multi-arch container for amd64 and arm64. As those builds take a long time (one arch always needs to be emulated) we usually use Mac with Apple Silicon for the build as Rosetta does the emulation very efficient. So although this process is manually we expect to find build problems with the container very fast, as every committer usually uses new versions from main quite often and the CI build with every run. Having that said I am thinking of the benefit of automatic check for building the base container. You did quite some work for the new workflow but that needs to be maintained as well. As mentioned before the manual build and push step for the base container is not optimal. But on the other hand we also do not change the base container that often. So currently we can live we that. What's you opinion?
So currently we can live we that. What's you opinion?
OK. I understand that as vote for a close.