workflows: Run stable branches' L4LB workflows on a schedule #25080
Conversation
pchaigno
force-pushed
the
pr/pchaigno/scheduled-run-stable-branch-l4lb-workflow
branch
from
676dca8
to
2c5580a
Compare
When using the schedule trigger, github.sha points to the latest commit of the default branch [1]. When running the scheduled workflows of stable branches, that is however not what we want to test. Instead, we need to pick the last commit of the corresponding stable branch. We can get that very easily via the GitHub API. 1 - https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#schedule Fixes: 416f6d9 ("workflows: Run stable branches' workflows on a schedule") Signed-off-by: Paul Chaignon <paul@cilium.io>
pchaigno
force-pushed
the
pr/pchaigno/scheduled-run-stable-branch-l4lb-workflow
branch
from
2c5580a
to
fac0e99
Compare
| @@ -145,6 +145,10 @@ jobs: | |||
| ${{ github.event.issue.pull_request.url || github.event.pull_request.url }}) | |||
| SHA=$(echo "$PR_API_JSON" | jq -r ".head.sha") | |||
| OWNER=$(echo "$PR_API_JSON" | jq -r ".number") | |||
| elif [ "${{ github.event_name }}" = "schedule" ]; then | |||
| curl https://api.github.com/repos/cilium/cilium/branches/v1.11 > branch.json | |||
There was a problem hiding this comment.
Is this intermediate branch.json file really needed? It seems you can just pipe curl to jq. Or, if pipefail is not set and we care about curl's errors, and want to stop right away, rather than set $SHA to an empty value, you can use a variable, like it's done in a block above.
There was a problem hiding this comment.
Does it matter? It's not as if we care about disk usage, leftover files, or even speed in that context 🤔
There was a problem hiding this comment.
It's not critical (I didn't mark it as "changes requested" after all), it just looks cleaner to me ("best practices", even if there are no practical drawbacks right now).
In this particular case, if it runs in some ephemeral container that gets deleted in the end, and no one else can interfere with this file, of course, we don't care about leftover files.
| elif [ "${{ github.event_name }}" = "schedule" ]; then | ||
| curl https://api.github.com/repos/cilium/cilium/branches/v1.11 > branch.json | ||
| SHA=$(jq -r '.commit.sha' branch.json) | ||
| OWNER=v1.11 |
There was a problem hiding this comment.
I suggest setting $OWNER first and using it as part of the URL to make the copy-pasted code more uniform and reduce the chance of copy-paste mistakes.
|
The code is reviewed. The nits would be good to have, but not worth going through those 25 files again and retesting 😅 Merging. |
pchaigno
deleted the
pr/pchaigno/scheduled-run-stable-branch-l4lb-workflow
branch
First commit adds the scheduled runs for a few L4LB, which was missed in #24991. Second commit fixes the workflows in case of scheduled runs on stable branches to select the proper SHA. See commits for details.
I've tested the new code in #25082. We can't actually test the
scheduletrigger until we merge unfortunately.Fixes: #24991.