ci: split and use matrix to test containerd backed image store#46572
ci: split and use matrix to test containerd backed image store#46572thaJeztah merged 3 commits intomoby:masterfrom
Conversation
3efd07c to
889384c
Compare
83a7d30 to
2f4a634
Compare
|
just asking; does this approach allow for a separated status badge to be created for snapshotter? basically if we want to enable this on master it'd be good if we could see that "regular" master is green (but it's fine to have the snapshotter variant fail for now) |
No it's grouped under the same |
|
Yeah, the idea was that if we run it on master (so that we can always track the state of the containerd integration, and not just on PR's in that area), we don't want master to be "failing" app the time, so .. looking for a way to show master as "green" (but containerd integration still needing work) |
In this case we can use |
2f4a634 to
eafa888
Compare
ecf9c13 to
cbd6b54
Compare
|
I think the Windows artifacts all need to have their matrix tags added, as it looks like in the last run, the two build jobs might have stomped on each other's artifacts, leading to both the snapshotter and graphdriver integration test runs not finding any build artifacts. Alternatively, perhaps the failure cause is that the build job has been split across the image-store test split, but really should only happen once, and be consumed by both test streams. (Probably true for integration-test-prepare too, but I'm not sure it, or even the build job, can be unsplit like that, I didn't look that closely, just noticed the Windows graphdriver failures and assumed from there.,) I did notice that the artefact |
Thanks! I was not near my computer and just let the workflow run tbh 😅 Edit: I missed a |
cbd6b54 to
241fd20
Compare
.github/workflows/windows-2019.yml
Outdated
| script: | | ||
| let matrix = ['graphdriver']; | ||
| if ("${{ contains(github.event.pull_request.labels.*.name, 'containerd-integration') || github.event_name != 'pull_request' }}" == "true") { | ||
| matrix.push('snapshotter'); | ||
| } | ||
| await core.group(`Set matrix`, async () => { | ||
| core.info(`matrix: ${JSON.stringify(matrix)}`); | ||
| core.setOutput('matrix', JSON.stringify(matrix)); | ||
| }); |
241fd20 to
46513e5
Compare
46513e5 to
12906c9
Compare
|
Rebased in case you still want this. |
Oh we do want this |
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
12906c9 to
e1bacd1
Compare
rumpl
left a comment
There was a problem hiding this comment.
LGTM, my eyes are bleeding from so many yaml :D
thaJeztah
left a comment
There was a problem hiding this comment.
"reviewed" this PR after others already did
✅ it has spaces and indentation
✅ it's yaml
✅ CI runs
I'd say: LGTM let's get this in and we'll see if the world burns
|
^^ we should look at overlap in unit tests etc in a follow-up though; could someone create a ticket for that? (perhaps summarising parts to look into?) |

follow-up #46402 (review)
- What I did
Alternative to #46402 using a matrix and set required env vars to test containerd backed image store.
- How I did it
Sets a
storagematrix conditionally based oncontainerd-integrationGitHub label being set.- How to verify it
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)