Browse code

ci: re-enable firewalld jobs

Commit 4e567e16 added firewalld to the test matrix for various CI jobs
(namely unit, integration and integration-cli).

Commit 2807c0c2 reverted that commit as it was putting too much load on
GHA cache, and thus it was returning 429 more frequently, so builds had
a greater chance of spending time building everything from scratch. This
was slowing down our CI even more than what it was before.

This new commit re-adds firewalld to the test matrix of unit,
integration and integration-cli jobs. Unlike 4e567e16, not all
combinations of OS, storage and 'mode' will be tested. Instead,
firewalld jobs will run only on ubuntu-22.04, and with the containerd
snapshotter.

Also, the revert commit mistakenly reverted a fix that was originally
intended for commit 8883db20, but was actually 'fixed up' in the wrong
commit. Let's re-revert that too.

Signed-off-by: Albin Kerouanton <albinker@gmail.com>

Albin Kerouanton authored on 2024/10/25 23:11:21
Showing 2 changed files
... ...
@@ -32,10 +32,47 @@ env:
32 32
   SETUP_BUILDKIT_IMAGE: moby/buildkit:latest
33 33
 
34 34
 jobs:
35
+
36
+  unit-prepare:
37
+    runs-on: ubuntu-20.04
38
+    timeout-minutes: 10 # guardrails timeout for the whole job
39
+    continue-on-error: ${{ github.event_name != 'pull_request' }}
40
+    outputs:
41
+      includes: ${{ steps.set.outputs.includes }}
42
+    steps:
43
+      -
44
+        name: Create matrix includes
45
+        id: set
46
+        uses: actions/github-script@v7
47
+        with:
48
+          script: |
49
+            let includes = [
50
+              { mode: '' },
51
+              { mode: 'rootless' },
52
+              { mode: 'systemd' },
53
+            ];
54
+            if ("${{ inputs.storage }}" == "snapshotter") {
55
+              includes.push({ mode: 'firewalld' });
56
+            }
57
+            await core.group(`Set matrix`, async () => {
58
+              core.info(`matrix: ${JSON.stringify(includes)}`);
59
+              core.setOutput('includes', JSON.stringify(includes));
60
+            });
61
+      -
62
+        name: Show matrix
63
+        run: |
64
+          echo ${{ steps.set.outputs.includes }}
65
+
35 66
   unit:
36 67
     runs-on: ubuntu-20.04
37 68
     timeout-minutes: 120 # guardrails timeout for the whole job
38 69
     continue-on-error: ${{ github.event_name != 'pull_request' }}
70
+    needs:
71
+      - unit-prepare
72
+    strategy:
73
+      fail-fast: false
74
+      matrix:
75
+        include: ${{ fromJson(needs.unit-prepare.outputs.includes) }}
39 76
     steps:
40 77
       -
41 78
         name: Checkout
... ...
@@ -44,6 +81,15 @@ jobs:
44 44
         name: Set up runner
45 45
         uses: ./.github/actions/setup-runner
46 46
       -
47
+        name: Prepare
48
+        run: |
49
+          CACHE_DEV_SCOPE=dev
50
+          if [[ "${{ matrix.mode }}" == *"firewalld"* ]]; then
51
+            echo "DOCKER_FIREWALLD=true" >> $GITHUB_ENV
52
+            CACHE_DEV_SCOPE="${CACHE_DEV_SCOPE}firewalld"
53
+          fi
54
+          echo "CACHE_DEV_SCOPE=${CACHE_DEV_SCOPE}" >> $GITHUB_ENV
55
+      -
47 56
         name: Set up Docker Buildx
48 57
         uses: docker/setup-buildx-action@v3
49 58
         with:
... ...
@@ -83,7 +129,7 @@ jobs:
83 83
         if: always()
84 84
         uses: actions/upload-artifact@v4
85 85
         with:
86
-          name: test-reports-unit-${{ inputs.storage }}
86
+          name: test-reports-unit-${{ inputs.storage }}-${{ matrix.mode }}
87 87
           path: /tmp/reports/*
88 88
           retention-days: 1
89 89
 
... ...
@@ -105,7 +151,7 @@ jobs:
105 105
         name: Download reports
106 106
         uses: actions/download-artifact@v4
107 107
         with:
108
-          name: test-reports-unit-${{ inputs.storage }}
108
+          pattern: test-reports-unit-${{ inputs.storage }}-*
109 109
           path: /tmp/reports
110 110
       -
111 111
         name: Install teststat
... ...
@@ -205,21 +251,51 @@ jobs:
205 205
         env:
206 206
           TEST_SKIP_INTEGRATION_CLI: 1
207 207
 
208
+  integration-prepare:
209
+    runs-on: ubuntu-20.04
210
+    timeout-minutes: 10 # guardrails timeout for the whole job
211
+    continue-on-error: ${{ github.event_name != 'pull_request' }}
212
+    outputs:
213
+      includes: ${{ steps.set.outputs.includes }}
214
+    steps:
215
+      -
216
+        name: Create matrix includes
217
+        id: set
218
+        uses: actions/github-script@v7
219
+        with:
220
+          script: |
221
+            let includes = [
222
+              { os: 'ubuntu-20.04', mode: '' },
223
+              { os: 'ubuntu-20.04', mode: 'rootless' },
224
+              { os: 'ubuntu-20.04', mode: 'systemd' },
225
+              { os: 'ubuntu-22.04', mode: '' },
226
+              { os: 'ubuntu-22.04', mode: 'rootless' },
227
+              { os: 'ubuntu-22.04', mode: 'systemd' },
228
+              // { os: 'ubuntu-20.04', mode: 'rootless-systemd' }, // FIXME: https://github.com/moby/moby/issues/44084
229
+              // { os: 'ubuntu-22.04', mode: 'rootless-systemd' }, // FIXME: https://github.com/moby/moby/issues/44084
230
+            ];
231
+            if ("${{ inputs.storage }}" == "snapshotter") {
232
+              includes.push({ os: 'ubuntu-22.04', mode: 'firewalld' });
233
+            }
234
+            await core.group(`Set matrix`, async () => {
235
+              core.info(`matrix: ${JSON.stringify(includes)}`);
236
+              core.setOutput('includes', JSON.stringify(includes));
237
+            });
238
+      -
239
+        name: Show matrix
240
+        run: |
241
+          echo ${{ steps.set.outputs.includes }}
242
+
208 243
   integration:
209 244
     runs-on: ${{ matrix.os }}
210 245
     timeout-minutes: 120 # guardrails timeout for the whole job
211 246
     continue-on-error: ${{ github.event_name != 'pull_request' }}
247
+    needs:
248
+      - integration-prepare
212 249
     strategy:
213 250
       fail-fast: false
214 251
       matrix:
215
-        os:
216
-          - ubuntu-20.04
217
-          - ubuntu-22.04
218
-        mode:
219
-          - ""
220
-          - rootless
221
-          - systemd
222
-          #- rootless-systemd FIXME: https://github.com/moby/moby/issues/44084
252
+        include: ${{ fromJson(needs.integration-prepare.outputs.includes) }}
223 253
     steps:
224 254
       -
225 255
         name: Checkout
... ...
@@ -241,6 +317,10 @@ jobs:
241 241
             echo "SYSTEMD=true" >> $GITHUB_ENV
242 242
             CACHE_DEV_SCOPE="${CACHE_DEV_SCOPE}systemd"
243 243
           fi
244
+          if [[ "${{ matrix.mode }}" == *"firewalld"* ]]; then
245
+            echo "DOCKER_FIREWALLD=true" >> $GITHUB_ENV
246
+            CACHE_DEV_SCOPE="${CACHE_DEV_SCOPE}firewalld"
247
+          fi
244 248
           echo "CACHE_DEV_SCOPE=${CACHE_DEV_SCOPE}" >> $GITHUB_ENV
245 249
       -
246 250
         name: Set up Docker Buildx
... ...
@@ -338,7 +418,7 @@ jobs:
338 338
     timeout-minutes: 120 # guardrails timeout for the whole job
339 339
     continue-on-error: ${{ github.event_name != 'pull_request' }}
340 340
     outputs:
341
-      matrix: ${{ steps.tests.outputs.matrix }}
341
+      matrix: ${{ steps.set.outputs.matrix }}
342 342
     steps:
343 343
       -
344 344
         name: Checkout
... ...
@@ -354,7 +434,7 @@ jobs:
354 354
         run:
355 355
           go install github.com/crazy-max/gotestlist/cmd/gotestlist@${{ env.GOTESTLIST_VERSION }}
356 356
       -
357
-        name: Create matrix
357
+        name: Create test matrix
358 358
         id: tests
359 359
         working-directory: ./integration-cli
360 360
         run: |
... ...
@@ -366,9 +446,43 @@ jobs:
366 366
           matrix="$(gotestlist -d ${{ env.ITG_CLI_MATRIX_SIZE }} -o "./..." -o "DockerSwarmSuite" -o "DockerNetworkSuite|DockerExternalVolumeSuite" ./...)"
367 367
           echo "matrix=$matrix" >> $GITHUB_OUTPUT
368 368
       -
369
-        name: Show matrix
369
+        name: Create gha matrix
370
+        id: set
371
+        uses: actions/github-script@v7
372
+        with:
373
+          script: |
374
+            let matrix = {
375
+              test: ${{ steps.tests.outputs.matrix }},
376
+              include: [],
377
+            };
378
+            // For some reasons, GHA doesn't combine a dynamically defined
379
+            // 'include' with other matrix variables that aren't part of the
380
+            // include items.
381
+            // Moreover, since the goal is to run only relevant tests with
382
+            // firewalld enabled to minimize the number of CI jobs, we
383
+            // statically define the list of test suites that we want to run.
384
+            if ("${{ inputs.storage }}" == "snapshotter") {
385
+              matrix.include.push({
386
+                'mode': 'firewalld',
387
+                'test': 'DockerCLINetworkSuite|DockerCLIPortSuite|DockerDaemonSuite'
388
+              });
389
+              matrix.include.push({
390
+                'mode': 'firewalld',
391
+                'test': 'DockerSwarmSuite'
392
+              });
393
+              matrix.include.push({
394
+                'mode': 'firewalld',
395
+                'test': 'DockerNetworkSuite'
396
+              });
397
+            }
398
+            await core.group(`Set matrix`, async () => {
399
+              core.info(`matrix: ${JSON.stringify(matrix)}`);
400
+              core.setOutput('matrix', JSON.stringify(matrix));
401
+            });
402
+      -
403
+        name: Show final gha matrix
370 404
         run: |
371
-          echo ${{ steps.tests.outputs.matrix }}
405
+          echo ${{ steps.set.outputs.matrix }}
372 406
 
373 407
   integration-cli:
374 408
     runs-on: ubuntu-20.04
... ...
@@ -378,8 +492,7 @@ jobs:
378 378
       - integration-cli-prepare
379 379
     strategy:
380 380
       fail-fast: false
381
-      matrix:
382
-        test: ${{ fromJson(needs.integration-cli-prepare.outputs.matrix) }}
381
+      matrix: ${{ fromJson(needs.integration-cli-prepare.outputs.matrix) }}
383 382
     steps:
384 383
       -
385 384
         name: Checkout
... ...
@@ -391,6 +504,15 @@ jobs:
391 391
         name: Set up tracing
392 392
         uses: ./.github/actions/setup-tracing
393 393
       -
394
+        name: Prepare
395
+        run: |
396
+          CACHE_DEV_SCOPE=dev
397
+          if [[ "${{ matrix.mode }}" == *"firewalld"* ]]; then
398
+            echo "DOCKER_FIREWALLD=true" >> $GITHUB_ENV
399
+            CACHE_DEV_SCOPE="${CACHE_DEV_SCOPE}firewalld"
400
+          fi
401
+          echo "CACHE_DEV_SCOPE=${CACHE_DEV_SCOPE}" >> $GITHUB_ENV
402
+      -
394 403
         name: Set up Docker Buildx
395 404
         uses: docker/setup-buildx-action@v3
396 405
         with:
... ...
@@ -59,7 +59,7 @@ fi
59 59
 # Allow connections coming from the host (through eth0). This is needed to
60 60
 # access the daemon port (independently of which port is used), or run a
61 61
 # 'remote' Delve session, etc...
62
-if [ ${DOCKER_FIREWALLD:-} = "true" ]; then
62
+if [ "${DOCKER_FIREWALLD:-}" = "true" ]; then
63 63
 	cat > /etc/firewalld/zones/trusted.xml << EOF
64 64
 <?xml version="1.0" encoding="utf-8"?>
65 65
 <zone target="ACCEPT">
... ...
@@ -76,7 +76,7 @@ env > /etc/docker-entrypoint-env
76 76
 cat > /etc/systemd/system/docker-entrypoint.target << EOF
77 77
 [Unit]
78 78
 Description=the target for docker-entrypoint.service
79
-Requires=docker-entrypoint.service systemd-logind.service systemd-user-sessions.service $([ ${DOCKER_FIREWALLD:-} = "true" ] && echo firewalld.service)
79
+Requires=docker-entrypoint.service systemd-logind.service systemd-user-sessions.service $([ "${DOCKER_FIREWALLD:-}" = "true" ] && echo firewalld.service)
80 80
 EOF
81 81
 
82 82
 quoted_args="$(printf " %q" "${@}")"