ci: split and use matrix to test containerd backed image store #46572

crazy-max · 2023-09-30T01:15:32Z

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 storage matrix conditionally based on containerd-integration GitHub label being set.

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

.github/workflows/.windows.yml

thaJeztah · 2023-09-30T09:12:06Z

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)

crazy-max · 2023-09-30T09:28:29Z

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 test workflow compared to original PR. Didn't catch we wanted also a split within the UI.

thaJeztah · 2023-09-30T09:38:06Z

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)

crazy-max · 2023-09-30T09:50:23Z

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 continue-on-error when built on a branch and not on PR so the run is green.

crazy-max · 2023-09-30T10:12:33Z

Updated so now it splits based on storage input:

And added continue-on-error so it would not fail the workflow on non-PR if snapshotter is used.

TBBle · 2023-09-30T11:50:47Z

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 windows-2022-unit-reports was generated twice, so I suspect both things are true.

crazy-max · 2023-09-30T13:12:50Z

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 windows-2022-unit-reports was generated twice, so I suspect both things are true.

Thanks! I was not near my computer and just let the workflow run tbh 😅
I'm taking a look 👀

Edit: I missed a needs in integration-cli that's why this job could not download the built binaries 🙈

rumpl · 2023-09-30T13:43:27Z

.github/workflows/windows-2019.yml

+          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));
+            });


crazy-max · 2023-10-13T10:00:05Z

Rebased in case you still want this.

rumpl · 2023-10-13T10:08:02Z

Rebased in case you still want this.

Oh we do want this

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>

rumpl

LGTM, my eyes are bleeding from so many yaml :D

laurazard

LGTM

thaJeztah

"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

thaJeztah · 2023-10-16T11:08:48Z

^^ 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?)

crazy-max added area/testing containerd-integration Issues and PRs related to containerd integration labels Sep 30, 2023

crazy-max mentioned this pull request Sep 30, 2023

ci: Add conditional CI for the containerd backed image store #46402

Closed

crazy-max force-pushed the ci-snapshotter branch 2 times, most recently from 3efd07c to 889384c Compare September 30, 2023 01:33

TBBle reviewed Sep 30, 2023

View reviewed changes

.github/workflows/.windows.yml Outdated Show resolved Hide resolved

crazy-max commented Sep 30, 2023

View reviewed changes

.github/workflows/.windows.yml Show resolved Hide resolved

crazy-max commented Sep 30, 2023

View reviewed changes

.github/workflows/.windows.yml Outdated Show resolved Hide resolved

crazy-max force-pushed the ci-snapshotter branch 3 times, most recently from 83a7d30 to 2f4a634 Compare September 30, 2023 02:44

crazy-max force-pushed the ci-snapshotter branch from 2f4a634 to eafa888 Compare September 30, 2023 10:06

crazy-max force-pushed the ci-snapshotter branch 5 times, most recently from ecf9c13 to cbd6b54 Compare September 30, 2023 11:03

crazy-max force-pushed the ci-snapshotter branch from cbd6b54 to 241fd20 Compare September 30, 2023 13:17

crazy-max changed the title ~~ci: matrix to test containerd backed image store~~ ci: split and use matrix to test containerd backed image store Sep 30, 2023

rumpl reviewed Sep 30, 2023

View reviewed changes

crazy-max force-pushed the ci-snapshotter branch from 241fd20 to 46513e5 Compare September 30, 2023 15:16

crazy-max marked this pull request as ready for review October 3, 2023 17:37

crazy-max removed the containerd-integration Issues and PRs related to containerd integration label Oct 13, 2023

crazy-max force-pushed the ci-snapshotter branch from 46513e5 to 12906c9 Compare October 13, 2023 09:56

crazy-max added 3 commits October 13, 2023 16:38

ci: split tests in a reusable workflow

03decbc

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>

ci: use matrix to test containerd backed image store

7089222

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>

ci: continue-on-error on non-PR with snapshotter enabled

e1bacd1

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>

crazy-max force-pushed the ci-snapshotter branch from 12906c9 to e1bacd1 Compare October 13, 2023 14:42

rumpl approved these changes Oct 16, 2023

View reviewed changes

vvoland approved these changes Oct 16, 2023

View reviewed changes

laurazard approved these changes Oct 16, 2023

View reviewed changes

thaJeztah approved these changes Oct 16, 2023

View reviewed changes

thaJeztah merged commit 5c3d0fb into moby:master Oct 16, 2023
104 checks passed

TBBle mentioned this pull request Oct 16, 2023

c8d: Just enough Windows support to run the test suite #46539

Merged

4 tasks

crazy-max deleted the ci-snapshotter branch October 16, 2023 12:28

thaJeztah added this to the 25.0.0 milestone Oct 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci: split and use matrix to test containerd backed image store #46572

ci: split and use matrix to test containerd backed image store #46572

crazy-max commented Sep 30, 2023

thaJeztah commented Sep 30, 2023

crazy-max commented Sep 30, 2023

thaJeztah commented Sep 30, 2023

crazy-max commented Sep 30, 2023

crazy-max commented Sep 30, 2023

TBBle commented Sep 30, 2023 •

edited

Loading

crazy-max commented Sep 30, 2023 •

edited

Loading

rumpl Sep 30, 2023

crazy-max commented Oct 13, 2023

rumpl commented Oct 13, 2023

rumpl left a comment

laurazard left a comment

thaJeztah left a comment

thaJeztah commented Oct 16, 2023

ci: split and use matrix to test containerd backed image store #46572

ci: split and use matrix to test containerd backed image store #46572

Conversation

crazy-max commented Sep 30, 2023

thaJeztah commented Sep 30, 2023

crazy-max commented Sep 30, 2023

thaJeztah commented Sep 30, 2023

crazy-max commented Sep 30, 2023

crazy-max commented Sep 30, 2023

TBBle commented Sep 30, 2023 • edited Loading

crazy-max commented Sep 30, 2023 • edited Loading

rumpl Sep 30, 2023

Choose a reason for hiding this comment

crazy-max commented Oct 13, 2023

rumpl commented Oct 13, 2023

rumpl left a comment

Choose a reason for hiding this comment

laurazard left a comment

Choose a reason for hiding this comment

thaJeztah left a comment

Choose a reason for hiding this comment

thaJeztah commented Oct 16, 2023

TBBle commented Sep 30, 2023 •

edited

Loading

crazy-max commented Sep 30, 2023 •

edited

Loading