From 391150f0e141fef63b0c1c97db8dcae5522260a6 Mon Sep 17 00:00:00 2001 From: Jamil Date: Mon, 11 Mar 2024 19:06:19 -0700 Subject: [PATCH] chore(ci): Fix new issues in cd.yml (#4085) Fixes some issues encountered after the merge of #4049 - Fix performance tests to only run using base_ref and head_ref to avoid dependence on `main` - Fixes some typos - Prevents a catch-22 condition where breaking compatibility meant we wouldn't be able to deploy production --- .github/actions/setup-rust/action.yml | 2 +- .github/workflows/_build_artifacts.yml | 10 ++- .github/workflows/_integration_tests.yml | 41 ----------- .github/workflows/_tauri.yml | 2 +- .github/workflows/cd.yml | 3 +- .github/workflows/ci.yml | 92 +++++++++++++++--------- .github/workflows/hotfix.yml | 16 ++++- scripts/tests/perf/results.js | 36 ++++++---- 8 files changed, 106 insertions(+), 96 deletions(-) diff --git a/.github/actions/setup-rust/action.yml b/.github/actions/setup-rust/action.yml index 1c269f24c..ca8a83888 100644 --- a/.github/actions/setup-rust/action.yml +++ b/.github/actions/setup-rust/action.yml @@ -56,7 +56,7 @@ runs: shell: bash # Start sccache - - if: ${{ inputs.sccache_backend == 'sccache' }} + - if: ${{ inputs.cache_backend == 'sccache' }} name: Start sccache run: $SCCACHE_PATH --start-server shell: bash diff --git a/.github/workflows/_build_artifacts.yml b/.github/workflows/_build_artifacts.yml index 3ecf52e3f..fa88912ab 100644 --- a/.github/workflows/_build_artifacts.yml +++ b/.github/workflows/_build_artifacts.yml @@ -31,8 +31,6 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} with: - prerelease: true - include-pre-releases: true version: ${{ env.VERSION }} - name: Delete stale artifacts env: @@ -125,6 +123,9 @@ jobs: ${{ steps.login.outputs.registry }}/firezone/${{ matrix.image_name }}:${{ env.CACHE_TAG }} data-plane: + # Runs the job after update-release-draft, regardless of the outcome + if: ${{ always() }} + needs: update-release-draft name: ${{ matrix.name.image_name }}-${{ matrix.arch.shortname }} runs-on: ubuntu-22.04 defaults: @@ -163,6 +164,8 @@ jobs: - uses: ./.github/actions/setup-rust with: targets: ${{ matrix.arch.target }} + # Cross doesn't support scccache without a lot of work + cache_backend: github - uses: taiki-e/install-action@v2 with: tool: cross @@ -184,7 +187,7 @@ jobs: # Used for Docker images cp target/${{ matrix.arch.target }}/${{ inputs.profile }}/${{ matrix.name.package }} ${{ matrix.name.package }} - name: Upload Release Assets - if: ${{ inputs.profile == 'release' && contains(fromJSON('["client", "gateway"]'), matrix.name.artifact) }} + if: ${{ inputs.profile == 'release' && (matrix.name.image_name == 'gateway' || matrix.name.image_name == 'client') }} env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | @@ -270,6 +273,7 @@ jobs: merge-docker-artifacts: name: merge-${{ matrix.image_name }} needs: data-plane + if: ${{ always() }} runs-on: ubuntu-22.04 strategy: fail-fast: false diff --git a/.github/workflows/_integration_tests.yml b/.github/workflows/_integration_tests.yml index be878cf63..2a1b9f05f 100644 --- a/.github/workflows/_integration_tests.yml +++ b/.github/workflows/_integration_tests.yml @@ -51,49 +51,8 @@ on: required: false type: string default: ${{ github.sha }} - snownet_image: - required: false - type: string - default: 'us-east1-docker.pkg.dev/firezone-staging/firezone/snownet-tests' - snownet_tag: - required: false - type: string - default: ${{ github.sha }} jobs: - snownet-integration-tests: - name: snownet-integration-tests-${{ matrix.name }} - runs-on: ubuntu-22.04 - permissions: - contents: read - id-token: write - pull-requests: write - env: - RELAY_IMAGE: ${{ inputs.relay_image }} - RELAY_TAG: ${{ inputs.relay_tag }} - SNOWNET_IMAGE: ${{ inputs.snownet_image }} - SNOWNET_TAG: ${{ inputs.snownet_tag }} - strategy: - fail-fast: false - matrix: - include: - - file: docker-compose.lan.yml - name: lan - - file: docker-compose.wan-hp.yml - name: wan-hp - - file: docker-compose.wan-relay.yml - name: wan-relay - steps: - - uses: actions/checkout@v4 - - uses: ./.github/actions/gcp-docker-login - id: login - with: - project: firezone-staging - - name: Run ${{ matrix.file }} test - run: | - sudo sysctl -w vm.overcommit_memory=1 - timeout 600 docker compose -f rust/snownet-tests/${{ matrix.file }} up --exit-code-from dialer --abort-on-container-exit - integration-tests: name: integration-tests-${{ matrix.test }} runs-on: ubuntu-22.04 diff --git a/.github/workflows/_tauri.yml b/.github/workflows/_tauri.yml index 62ad2a747..ecc2a58e1 100644 --- a/.github/workflows/_tauri.yml +++ b/.github/workflows/_tauri.yml @@ -85,7 +85,7 @@ jobs: with: name: ${{ matrix.binary-dest-path }} path: | - ${{ matrix.artifacts }} + ${{ join(matrix.artifacts, '\n') }} # Only for builds on main - name: Upload Release Assets if: ${{ inputs.release_tag }} diff --git a/.github/workflows/cd.yml b/.github/workflows/cd.yml index cca038cbf..79ef706fc 100644 --- a/.github/workflows/cd.yml +++ b/.github/workflows/cd.yml @@ -28,14 +28,13 @@ jobs: deploy-staging: if: ${{ github.event_name == 'workflow_dispatch' && inputs.deploy-staging }} runs-on: ubuntu-22.04 - secrets: inherit permissions: contents: write # Cancel old workflow runs if new code is pushed concurrency: group: "staging-deploy-${{ github.workflow }}-${{ github.ref }}" cancel-in-progress: false - needs: integration-tests + needs: ci env: TF_CLOUD_ORGANIZATION: "firezone" TF_API_TOKEN: "${{ secrets.TF_API_TOKEN }}" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f5a2d933e..58ba2420a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -50,7 +50,43 @@ jobs: needs: build-artifacts secrets: inherit + snownet-tests: + needs: build-artifacts + if: ${{ github.event_name == 'pull_request' || github.event_name == 'merge_group' }} + name: snownet-tests-${{ matrix.name }} + runs-on: ubuntu-22.04 + permissions: + contents: read + id-token: write + pull-requests: write + env: + RELAY_IMAGE: 'us-east1-docker.pkg.dev/firezone-staging/firezone/relay' + RELAY_TAG: ${{ github.sha }} + SNOWNET_IMAGE: 'us-east1-docker.pkg.dev/firezone-staging/firezone/snownet-tests' + SNOWNET_TAG: ${{ github.sha }} + strategy: + fail-fast: false + matrix: + name: + - lan + - wan-hp + - wan-relay + steps: + - uses: actions/checkout@v4 + - uses: ./.github/actions/gcp-docker-login + id: login + with: + project: firezone-staging + - name: Run docker-compose.${{ matrix.name }}.yml test + run: | + sudo sysctl -w vm.overcommit_memory=1 + timeout 600 docker compose -f rust/snownet-tests/docker-compose.${{ matrix.name }}.yml up --exit-code-from dialer --abort-on-container-exit + compatibility-tests: + # Don't run compatibility tests when called from cd.yml on `main` because + # it'll be red if there was a breaking change we're tring to publish, + # and the publish workflow checks for main to be green. + if: ${{ github.event_name == 'pull_request' || github.event_name == 'merge_group' }} uses: ./.github/workflows/_integration_tests.yml needs: build-artifacts secrets: inherit @@ -62,6 +98,8 @@ jobs: # client_tag: "latest" perf-tests: + # Only the debug images have perf tooling + if: ${{ github.event_name == 'pull_request' }} name: perf-tests-${{ matrix.test_name }} needs: build-artifacts runs-on: ubuntu-22.04 @@ -70,15 +108,18 @@ jobs: id-token: write pull-requests: write env: - API_TAG: ${{ github.sha }} - WEB_TAG: ${{ github.sha }} - GATEWAY_TAG: ${{ github.sha }} - CLIENT_TAG: ${{ github.sha }} - RELAY_TAG: ${{ github.sha }} - ELIXIR_TAG: ${{ github.sha }} + API_TAG: ${{ matrix.sha }} + WEB_TAG: ${{ matrix.sha }} + GATEWAY_TAG: ${{ matrix.sha }} + CLIENT_TAG: ${{ matrix.sha }} + RELAY_TAG: ${{ matrix.sha }} + ELIXIR_TAG: ${{ matrix.sha }} strategy: fail-fast: false matrix: + sha: + - ${{ github.sha }} + - ${{ github.event.pull_request.base.sha }} test_name: - direct-tcp-client2server - direct-tcp-server2client @@ -90,6 +131,8 @@ jobs: - relayed-udp-server2client steps: - uses: actions/checkout@v4 + with: + ref: ${{ matrix.sha }} - uses: ./.github/actions/gcp-docker-login id: login with: @@ -110,7 +153,6 @@ jobs: docker compose up -d relay docker compose up -d gateway docker compose up -d client - - name: 'Performance test: ${{ matrix.test_name }}' timeout-minutes: 5 env: @@ -120,7 +162,7 @@ jobs: uses: actions/upload-artifact@v4 with: overwrite: true - name: ${{ matrix.test_name }}-${{ github.ref_name == 'main' && 'main' || github.sha }}-iperf3results + name: ${{ matrix.test_name }}-${{ matrix.sha }}-iperf3results path: ./${{ matrix.test_name }}.json - name: Show Client logs if: "!cancelled()" @@ -154,41 +196,25 @@ jobs: pull-requests: write steps: - uses: actions/checkout@v4 - - name: Download PR performance test results + - name: Download base ref performance test results + uses: actions/download-artifact@v4 + with: + pattern: '*-${{ github.event.pull_request.base.sha }}-iperf3results' + merge-multiple: true + path: ./${{ github.event.pull_request.base.sha }} + - name: Download head ref performance test results uses: actions/download-artifact@v4 with: pattern: '*-${{ github.sha }}-iperf3results' merge-multiple: true - path: ./ - - name: Get last Continous Delivery workflow run Id - uses: actions/github-script@v7 - id: get_last_cd_run - with: - github-token: ${{ secrets.GITHUB_TOKEN }} - script: | - const { data } = await github.rest.actions.listWorkflowRuns({ - owner: context.repo.owner, - repo: context.repo.repo, - workflow_id: "cd.yml", - status: 'success', - per_page: 1 - }); - return data.workflow_runs[0].id; - - name: Download main branch performance test results - uses: actions/download-artifact@v4 - with: - pattern: '*-main-iperf3results' - merge-multiple: true - github-token: ${{ secrets.GITHUB_TOKEN }} - run-id: ${{ steps.get_last_cd_run.outputs.result }} - path: ./main + path: ./${{ github.sha }} - name: Update PR with results uses: actions/github-script@v7 with: github-token: ${{ secrets.GITHUB_TOKEN }} script: | const { script } = require('./scripts/tests/perf/results.js'); - script(github, context, [ + script(github, context, '${{ github.event.pull_request.base.sha }}', '${{ github.sha }}', [ 'direct-tcp-client2server', 'direct-tcp-server2client', 'direct-udp-client2server', diff --git a/.github/workflows/hotfix.yml b/.github/workflows/hotfix.yml index b3f31270f..d2fb35a46 100644 --- a/.github/workflows/hotfix.yml +++ b/.github/workflows/hotfix.yml @@ -14,8 +14,22 @@ concurrency: cancel-in-progress: false jobs: + # This is *not* run in CI on main in order to allow + # breaking changes to be merged as administrator and have the + # resulting CI green on main. + # So run them here. + compatibility-tests: + uses: ./.github/workflows/_integration_tests.yml + secrets: inherit + with: + gateway_image: "ghcr.io/firezone/gateway" + gateway_tag: "latest" + # FIXME: Uncomment this after the next release -- the + # client will be published then. + # client_tag: "latest" + deploy-production: - needs: integration-tests + needs: compatibility-tests secrets: inherit uses: ./.github/workflows/_deploy_production.yml with: diff --git a/scripts/tests/perf/results.js b/scripts/tests/perf/results.js index e334564a4..e93b06787 100644 --- a/scripts/tests/perf/results.js +++ b/scripts/tests/perf/results.js @@ -1,8 +1,8 @@ const fs = require("fs"); const path = require("path"); -function getDiffPercents(main, current) { - let diff = -1 * (100 - current / (main / 100)); +function getDiffPercents(base, head) { + let diff = -1 * (100 - head / (base / 100)); if (diff > 0) { return "+" + diff.toFixed(0) + "%"; @@ -35,7 +35,13 @@ function humanFileSize(bytes, dp = 1) { return bytes.toFixed(dp) + " " + units[u]; } -exports.script = async function (github, context, test_names) { +exports.script = async function ( + github, + context, + base_sha, + head_sha, + test_names +) { let output = `### Performance Test Results `; @@ -52,12 +58,14 @@ exports.script = async function (github, context, test_names) { `; for (const test_name of test_names) { - // 1. Read the current results - const results = JSON.parse(fs.readFileSync(test_name + ".json")).end; + // 1. Read the head ref results + const results = JSON.parse( + fs.readFileSync(path.join(head_sha, test_name + ".json")) + ).end; - // 2. Read the main results - const results_main = JSON.parse( - fs.readFileSync(path.join("main", test_name + ".json")) + // 2. Read the base ref results + const results_base = JSON.parse( + fs.readFileSync(path.join(base_sha, test_name + ".json")) ).end; if (test_name.includes("tcp")) { @@ -65,7 +73,7 @@ exports.script = async function (github, context, test_names) { humanFileSize(results.sum_received.bits_per_second) + " (" + getDiffPercents( - results_main.sum_received.bits_per_second, + results_base.sum_received.bits_per_second, results.sum_received.bits_per_second ) + ")"; @@ -73,7 +81,7 @@ exports.script = async function (github, context, test_names) { humanFileSize(results.sum_sent.bits_per_second) + " (" + getDiffPercents( - results_main.sum_sent.bits_per_second, + results_base.sum_sent.bits_per_second, results.sum_sent.bits_per_second ) + ")"; @@ -81,7 +89,7 @@ exports.script = async function (github, context, test_names) { results.sum_sent.retransmits + " (" + getDiffPercents( - results_main.sum_sent.retransmits, + results_base.sum_sent.retransmits, results.sum_sent.retransmits ) + ")"; @@ -92,20 +100,20 @@ exports.script = async function (github, context, test_names) { humanFileSize(results.sum.bits_per_second) + " (" + getDiffPercents( - results_main.sum.bits_per_second, + results_base.sum.bits_per_second, results.sum.bits_per_second ) + ")"; const udp_sum_jitter_ms = results.sum.jitter_ms.toFixed(2) + "ms (" + - getDiffPercents(results_main.sum.jitter_ms, results.sum.jitter_ms) + + getDiffPercents(results_base.sum.jitter_ms, results.sum.jitter_ms) + ")"; const udp_sum_lost_percent = results.sum.lost_percent.toFixed(2) + "% (" + getDiffPercents( - results_main.sum.lost_percent, + results_base.sum.lost_percent, results.sum.lost_percent ) + ")";