CI: Use Docker for CI #46
Conversation
|
#47 fails as expected. Failing CI run: https://github.com/cps-org/cps-config/actions/runs/8227179114/job/22494683293. |
There was a problem hiding this comment.
I like how this will keep me from breaking things again!
Since the project is quite small now, I'm OK with taking a step back on ccache support to improve test coverage. If this PR merges, @lunacd, can you make an issue to revisit compile caching support?
.github/workflows/test.yml
Outdated
| @@ -0,0 +1,52 @@ | |||
| # This is a basic workflow to help you get started with Actions | |||
There was a problem hiding this comment.
Can we delete these sorts of comments? I think they're intended to help the initial author of a workflow, but aren't supposed to stay in the document.
It would be helpful for GitHub Actions newcomers to have comments linking any or all of:
- a useful tutorial
- an onboarding guide
- a cookbook
- reference documentation
...so those comments seem great if you have ideas there.
There was a problem hiding this comment.
The current workflow yml file doesn't conform to .editorconfig in this repo. I was thinking about making a separate PR to sort out those stylistic stuffs and comments. But it seems that the current change is significant enough for git to treat it not as a rename but as an entirely new file. I guess it makes sense add those changes to this PR.
|
I'm not as familiar with github actions (particularly complex ones) as I am with gitlab pipelines, so this may not be possible (or easily possible), but one of the hopes I had for using docker was that we could decouple of the building of the base images from our push and pull request workflow. The Images could be built on a timer, or when a user manually updates them, and then we just re-use those images. I'm concerned that as we add more OSes and tests that the CI time is going to get really long. |
Not sure if I followed you correctly, are you saying that we publish to Please correct me if I'm wrong, but I don't think that would have any performance benefit over the workflow in this PR. Docker builds are cached by GH Actions by layer. If we don't change build dependencies, for example, Further, the benefit of this is that it automatically detects when a layer is changed, e.g. adding/removing dependencies, and correctly invalidates the cache. If we publish the image, a PR that requires a new dependency, or one that updates the image from Ubuntu 22.04 to 24.04, is guaranteed to fail.
They are parallel so they probably wouldn't be too long. My concern is using too much cache to a point of GHA stopping us from doing so. But you are right, and I would propose:
Or we can have one primary distro we are testing multiple flavors on. All the others can just be baseline gcc. I'm flexible and this can be whichever people feel best about. |
I googled a bit and found this issue: moby/buildkit#1512. The issue isn't directly related to this, but from the comments, it seems that |
|
Ah, I'm demonstrating what I don't understand! It look like it's doing exactly what I hoped for then, this requires a bit more manual intervention on GitLab, so I was expecting more intervention on GitHub as well. Please disregard my comments, this all looks good from my point of view. |
**Changes** 1. Under `tests/docker`, added two dockerfiles that builds docker images for testing cps-config. The two dockerfiles are for ubuntu and rockylinux currently. More platforms can be added later. 2. In GitHub Actions workflow, switched from building and testing directly within the workflow to building and using docker images. 3. Partly to demonstrate the benefits of this approach, added testing for rockylinux within CI pipeline. 4. Because the CI pipeline is no longer testing ubuntu specifically, rename the pipeline to "test", expecting that in the future, more workflows, like "lint", would be added. **Benefits** 1. Developers would be better able to reproduce CI results without repetitively pushing and triggering GitHub Actions. 2. A single GitHub Actions workflow file would be able to run tests on multiple distributions seamlessly, where all the platform-specific behaviors are handled within dockerfiles for each distribution. 3. In the future, other CI workflows, like running clang-tidy on the codebase, might be interesting. With docker, we will be able to reuse platform-specific setups and have one workflow file per type of check. 4. Docker builds can be cached. This could lead to much faster CI pipeline builds. **Limitation and future work** 1. It is not entirely clear to me whether docker build cache includes ccache mounted with type=cache. Intuitively, it should, but I couldn't find official docs that say so. It that is not cached, this would be a step back from the previous CI setup. In that case, a future PR that caches ccache artifacts would be needed. 2. Currently building the dockerfile also builds cps-config. This might not be necessary for some workflows. It might be interesting to do that as part of the workflow rather than the image build process. 3. It would be nice to document local workflows to test cps-config with those dockerfiles, even if it's "go look at what is being used in test.yml". But that would fit into `docs/dev.md` which is currently WIP in cps-org#42. I will follow up with a docs PR once `docs/dev.md` is merged.
|
Changes:
|
Changes
tests/docker, added two dockerfiles that builds docker images for testing cps-config. The two dockerfiles are for ubuntu and rockylinux currently. More platforms can be added later.Benefits
Limitation and future work
docs/dev.mdwhich is currently WIP in Add a developer guide document #42. I will follow up with a docs PR oncedocs/dev.mdis merged.Testing
I have tested in my own repository, but it's after all just an approximation. Thus, I have created #47 to test that this CI pipeline correctly fails when there is a test failure.