setup continuous integration #17
Conversation
webern
requested review from
samuelkarp,
arnaldo2792,
srgothi92,
jahkeup and
etungsten
webern
force-pushed
the
setup
branch
2 times, most recently
from
c63e358 to
ed61209
Compare
Continuous integration runs in two simultaneous jobs. One for make ci, which runs cargo build, test, fmt and clippy. The other, make image, builds the updater container image. Caching for make ci speeds up the build time from over 10 minutes to just a minute or two. cargo install cargo-deny took five minutes on its own before caching CARGO_HOME. Caching for the make image build required some hacks and ended up being slower that running with no caching (due to all the compressing and decompressing of docker image layers, CPU-bound), so no caching there.
That's neat, when I rebased over develop, the base branch was changed automatically. Edit: actually it was probably my deletion of the previous base branch that caused it. |
| path: updater/target | ||
| key: ${{ hashFiles('.github/cache-bust') }}-${{ hashFiles('updater/Cargo.lock') }} | ||
| restore-keys: | | ||
| ${{ hashFiles('.github/cache-bust') }}- |
There was a problem hiding this comment.
Is this matched as a prefix? Or should this say:
| ${{ hashFiles('.github/cache-bust') }}- | |
| ${{ hashFiles('.github/cache-bust') }}-${{ hashFiles('updater/Cargo.lock') }} |
There was a problem hiding this comment.
The syntax is correct. What it's doing is providing a fallback to a cache where the first part matches but the second part doesn't. So when the Cargo.lock changes, a cache miss will happen on the two-part key, but a cache hit will happen on the one-part-only key. Then since a cache miss happened, a new cache will be created on the full two part key at the end of the build.
| path: integ/target | ||
| key: ${{ hashFiles('.github/cache-bust') }}-${{ hashFiles('integ/Cargo.lock') }} | ||
| restore-keys: | | ||
| ${{ hashFiles('.github/cache-bust') }}- |
There was a problem hiding this comment.
Same question here (is this a prefix?).
| cd updater && cargo fmt -- --check | ||
| cd updater && cargo clippy --locked -- -D warnings | ||
| cd integ && cargo fmt -- --check | ||
| cd integ && cargo clippy --locked -- -D warnings |
There was a problem hiding this comment.
This seems to point to a Cargo workspace being a reasonable choice to manage the crates driven by this Makefile. Is that something that was considered and dismissed? Can you see if a workspace simplifies the developer's experience (and the Makefile's verbosity)?
There was a problem hiding this comment.
@tjkirch and I rejected a workspace because:
- workspaces can be kind of a pain sometimes.
- the updater project should be self-contained since that is the actual product, i.e. if you want to build and deploy this, you shouldn't have to build all the dependencies that only the integ crate needs.
It's not dissimilar to having tools and sources in Bottlerocket.
| cd integ && cargo clippy --locked -- -D warnings | ||
|
|
||
| .PHONY: ci # these are all of the checks (except for image) that we run for ci | ||
| ci: check-licenses lint build unit-tests |
There was a problem hiding this comment.
I'd have called this all, though the conventional names use it for a tighter scoped use case. Either that or have the CI builds run this (with some minor renaming):
# Run easy lints on projects
# Run build for projects
# Run checks for the projects
make lint build checkThere was a problem hiding this comment.
What I'm going for is an easy, single thing to remember that I can type locally to see if my ci run will pass before pushing.
make ci or make all would both be fine, but make lint build check would suck because I would have to open the CI yaml definition to remember what to do.
I used make ci for the same purpose in tough, so keeping consistent with that pattern would make me happy, but if we like all better, I can change it.
|
Merging based on Matt's request. |
Issue number:
Progress on #1
Base branch will change from
citodevelopafter merging #16.Description of changes:
Add
deny.tomlfiles like the ones we use in Bottlerocket to ensure we're aware of dependency licenses.Setup CI:
Testing done:
make ci && make imagelocally and in the actions runner.Terms of contribution:
By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.