ci: replace Cirrus CI with GitHub Actions#1
Conversation
02f998c to
6088e5c
Compare
6088e5c to
9608a60
Compare
Luap99
left a comment
There was a problem hiding this comment.
Thanks this is a good start, mostly comments about the unnecessary dependency installs?
I have not deeply looked at the actual test setup/runs
Kir is suggesting to use hack/ci over contrib/ci, I don't mind either way but we should likely all agree on the same for all repos for consistency podman-container-tools/podman-sandbox#8
| - uses: actions/checkout@v6 | ||
| - uses: dorny/paths-filter@v3 |
There was a problem hiding this comment.
please pin all actions with its commit sha
| ############################################################################### | ||
| # Dependency installation | ||
| ############################################################################### |
There was a problem hiding this comment.
The feels wrong, what is actually missing here? All deps must be backed into the VM images and should not be installed at runtime.
| if [[ "$VARIANT" == "sequoia" ]]; then | ||
| case $OS_RELEASE_ID in | ||
| fedora) sudo dnf install -y openssl-devel capnproto ;; | ||
| debian) sudo apt-get install -y libssl-dev capnproto ;; | ||
| esac | ||
| curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --default-toolchain stable | ||
| source "$HOME/.cargo/env" | ||
| git clone --depth=1 https://github.com/ueno/podman-sequoia.git /tmp/podman-sequoia | ||
| cargo build --release --manifest-path /tmp/podman-sequoia/Cargo.toml | ||
| case $OS_RELEASE_ID in | ||
| fedora) sudo cp /tmp/podman-sequoia/target/release/libpodman_sequoia.so /usr/lib64/ ;; | ||
| debian) sudo cp /tmp/podman-sequoia/target/release/libpodman_sequoia.so /usr/lib/x86_64-linux-gnu/ ;; | ||
| esac | ||
| sudo ldconfig |
There was a problem hiding this comment.
why is this needed? podman-sequoia should be packaged, we should not runtime compile it all the time as it wasted test time and can fail suddenly
| # N/B: The prow merge-bot (tide) is sensitized to this exact name, DO NOT CHANGE IT. | ||
| # Ref: https://github.com/openshift/release/pull/49820 |
There was a problem hiding this comment.
This comment can be dropped since we do not use the bot but it can highlight github merge protection settings.
|
oh btw I think it would make sense if you push the go fix commit on the main repo, no need to wait for this PR here. |
opened a PR: podman-container-tools/container-libs#874 |
fc09a2d to
c47e364
Compare
|
github seems on fire right now |
4af5768 to
97147ae
Compare
| permissions: | ||
| pull-requests: read |
There was a problem hiding this comment.
The PR read permission should be applied from the permissions: read-all, so this is redundant, no?
| persist-credentials: false | ||
| - uses: lima-vm/lima-actions/setup@55627e31b78637bf254a8b2a14da8ea7d12564e5 # v1.1.0 | ||
| with: | ||
| version: v2.1.1 |
There was a problem hiding this comment.
Perhaps it would be better if the version was specified as a constant similarly to as it is done for go-version?
| image: ${{ steps.filter.outputs.image }} | ||
| steps: | ||
| - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 | ||
| - uses: dorny/paths-filter@6852f92c20ea7fd3b0c25de3b5112db3a98da050 # v3 |
There was a problem hiding this comment.
why is this pinned to v3, latest version seem to v4.0.1 dorny/paths-filter@fbd0ab8
a83fe52 to
fd95acc
Compare
kolyshkin
left a comment
There was a problem hiding this comment.
Left a few nits.
Too bad file renames can't be shown here. Perhaps it's better to start with a commit which renames all contrib/cirrus to hack/ci (similar to podman-container-tools/podman-sandbox#8) and then do the migration, so the reviewers will see the exact changes to these files.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
fd95acc to
a9927ac
Compare
No description provided.