CI: various small improvements#934
Conversation
|
Once this is reviewed I drop the old validate test names from the branch protection so this can be merged |
0627a21 to
da1ee9a
Compare
|
LGTM. |
mtrmac
left a comment
mtrmac
left a comment
There was a problem hiding this comment.
Thanks, that’s some nice cleanups.
| ./hack/ci/audit.log | ||
| ./hack/ci/df.log | ||
| if-no-files-found: ignore | ||
| needs: [path-filter, validate] |
There was a problem hiding this comment.
path-filter is unnecessary here.
There was a problem hiding this comment.
true but it feels most consistent, path-filter is fast and if it ever fails no point continuing
There was a problem hiding this comment.
Fair enough, makes for a simpler CI job graph.
| required: false | ||
| type: number | ||
|
|
||
| permissions: {} |
There was a problem hiding this comment.
Hum. This does seem to work, in that the token for the `… / lima” job has only "Metadata: read" action.
Do we need the permissions: read-all that we use ~everywhere else, then?
Actually, even the actions/upload-artifact works fine with the “metadata: read”-only token. How??! I must be missing something very basic.
https://github.com/actions/checkout#recommended-permissions suggests
permissions:
contents: read
… yet seems to work without that. Maybe because the repo is public [and the action is using the anonymous rate limits? Perhaps not the latter part].
There was a problem hiding this comment.
honestly I have not yet understood that either. we run with permissions: {} in podman and buildah without issues so far, but thanks for the link yeah I guess we should go with contents: read
There was a problem hiding this comment.
I would expect that the token automatically gets some permissions related to the job (e.g. access to the PR being proposed, and to the action’s artifacts) but I can’t find it written down in the workflow syntax documentation.
| driver: overlay-transient | ||
| name: "Storage: ${{ matrix.distro }} ${{ matrix.driver }}" | ||
| steps: | ||
| - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 |
There was a problem hiding this comment.
I now see a pretty unpleasant-looking digest-only dependency PR in #939 . Would using comments with a precise version numbers, not just the major, help here? (I didn’t yet test that.)
There was a problem hiding this comment.
oh weird, so yes in podman we use full version which works https://github.com/podman-container-tools/podman/pull/29019/changes
I add a commit to fix this here
Renovate seems to create better PRs if we have the full version as comment afterwards. podman-container-tools#934 (comment) Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Do it like in podman and buildah and use a extra lima.yml to deduplicate the yml file. Also while at it touch up the task names, the name is set like the arguments to hack/ci/ci.yml like in podman so we can just copy paste this to reproduce any CI failure locally with that. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Mainly so we trigger it from the main ci.yml and work it in the task dependencies so we do not have to have extra tasks in the merge protection settings. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
This should safe some test resources if basic validation fails already. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Because merge protection gates on Total Success we should have a job to ensure the yaml really has the right deps for it to work correctly. Also add a check to ensure all tasks are pinned with a version comment. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
The default github timeout is 6 hours which is way to long. In case there is ever a hang abort after 10 minutes as all these jobs should be quite fast. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
All the jobs should be much faster than this so we do not needs such long timeouts. In case of hangs it is better to abort early. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
This action has write access to PRs so we should guard against a compromised tag and only update explicitly via renovate PRs. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
The tests do not run in parallel so they do not really benefit from that much cpus. Safe some CNCF resources by using smaller runners. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
While the runs seems to work without any permission set, likely because we clone public repos the recommended permission is contents: read. We also do not need the other ones. https://github.com/actions/checkout#recommended-permissions Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Renovate seems to create better PRs if we have the full version as comment afterwards. podman-container-tools#934 (comment) Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Split filter list out in a extra file like podman has. Then define a new filter for things that should trigger all tests and then the ones that only need to trigger storage, image, common. Then rework the copnditions to use the new filters, also swap github.event_name == 'push' with github.event_name != 'pull_request'. While it should not matter right now I could see us adding a nightly trigger via cron and then we would still want to run all tests. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
|
(I have dropped the “git-validate”, “lint”, “codespell” and “go-vendor” status checks from the required list on I thought we might need updating the “Total Success” status check name as well, but it seems to match “ci / Total Success” here.) |
No the first part is ignored in the naming list on the checks page, it is just how this works for whatever reason. |
Renovate seems to create better PRs if we have the full version as comment afterwards. podman-container-tools#934 (comment) Signed-off-by: Paul Holzinger <pholzing@redhat.com> (cherry picked from commit c557c3a) Signed-off-by: Paul Holzinger <pholzing@redhat.com>
3 participants
see commits