Skip to content

new ci for skopeo#9

Closed
timcoding1988 wants to merge 3 commits into
mainfrom
ci/iterate
Closed

new ci for skopeo#9
timcoding1988 wants to merge 3 commits into
mainfrom
ci/iterate

Conversation

@timcoding1988
Copy link
Copy Markdown
Contributor

@timcoding1988 timcoding1988 commented May 28, 2026

follow up on #1

Mirrors validate, doccheck, cross, osx, test_skopeo, and ostree_rs_ext
from containers/skopeo's .cirrus.yml into a GH Actions workflow on
podman-io/skopeo-sandbox.

Signed-off-by: Tim Zhou <tizhou@redhat.com>
Signed-off-by: Tim Zhou <tizhou@redhat.com>
Signed-off-by: Tim Zhou <tizhou@redhat.com>
Copy link
Copy Markdown
Contributor

@mtrmac mtrmac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The version in #1 removed .cirrus.yml, check_cirrus_cron.yml, contrib/cirrus, this one leaves them around. Is that intentional?

ACK in general.

Comment thread .github/workflows/ci.yml
- name: Check all required jobs
run: |
if [[ "${{ contains(needs.*.result, 'failure') }}" == "true" ]] || \
[[ "${{ contains(needs.*.result, 'cancelled') }}" == "true" ]]; then
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking: I’d slightly prefer an allow-list of success states here.

Comment thread .github/workflows/ci.yml
runs-on: oracle-vm-4cpu-16gb-x86-64
timeout-minutes: 45
container:
image: quay.io/libpod/skopeo_cidev:${{ vars.IMAGE_TAG }}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Where does IMAGE_TAG come from now? Will we still get Renovate PRs for that?)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is currently no automation for building containers images, that needs to be re-added into https://github.com/podman-container-tools/automation/

For now hard code to the last known tag

Comment thread .github/workflows/ci.yml
cancel-in-progress: true

jobs:
path-filter:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I didn’t review this mechanism in detail.)

Comment thread Makefile
# Set this to a skopeo_cidev image (e.g. quay.io/libpod/skopeo_cidev:<tag>)
# when running those targets locally. If unset, the wrapper targets will fail
# loudly with an empty image reference.
SKOPEO_CIDEV_CONTAINER_FQIN ?=
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I guess this is related to the IMAGE_TAG question … but then I know about no-one using these containerized targets manually, they tend to be broken most of the time.)

Comment thread hack/get_ci_vm.sh
@@ -1,61 +0,0 @@
#!/usr/bin/env bash
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(This is happening consistently in the other repos? (I have ~never used the script and I don’t have an opinion on it existing.))

Comment thread .github/filters.yaml
- '**/*.go'
- '!**/*_test.go'
- '!systemtest/**'
- '!integration/**'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note I switched this in my podman work, in short this does not really work because any match == true so any go file changes triggers all and you do not exclude tests like that.

Also it cannot work without specific test matchers, i.e. in podman it was setup to only run sys tests when only a sys test file was changed.
If all tests are excluded you would skip them if someone only touches a test file which is not right,

Comment thread .github/filters.yaml
Comment on lines +16 to +17
# Production Go source. Tests and bats are excluded so a test-only diff
# does not force every "code" gated job to run.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we change tests, we definitely do want the test passing before we merge a PR.

(Overall, I’m not sure the path filter is worth the complexity here. There are not that many PRs in this repo, and it’s all fairly small. [It would definitely be useful in container-libs, with the 3 separate modules and many more test jobs.])

@timcoding1988
Copy link
Copy Markdown
Contributor Author

timcoding1988 commented Jun 1, 2026

close in favor of : podman-container-tools/skopeo#2883

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants