diff --git a/.github/actions/ci-gate/action.yml b/.github/actions/ci-gate/action.yml new file mode 100644 index 0000000000..3866975826 --- /dev/null +++ b/.github/actions/ci-gate/action.yml @@ -0,0 +1,70 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright Open Network Fabric Authors + +# Enable a job on a deep run (push, merge queue, or dispatch) or when a pull +# request has a requested label. + +name: "CI gate" +description: "Decide whether a phased job runs on this event" + +inputs: + labels: + description: >- + Space-separated label suffixes; `ci:+` is implied. + required: true + always-labels: + description: >- + Label suffixes that enable every gate. + required: false + default: "merge-ready" + on-value: + description: "What to emit when the gate is on." + required: false + default: "true" + off-value: + description: "What to emit when the gate is off." + required: false + default: "false" + +outputs: + value: + description: "`on-value` if the gate is on, `off-value` if it is not." + value: "${{ steps.gate.outputs.value }}" + +runs: + using: "composite" + steps: + - id: "gate" + shell: "bash" + # Avoid expression interpolation in the script body. + env: + EVENT: "${{ github.event_name }}" + LABELS: "${{ toJSON(github.event.pull_request.labels.*.name) }}" + WANTED: "${{ inputs.labels }} ${{ inputs.always-labels }}" + ON_VALUE: "${{ inputs.on-value }}" + OFF_VALUE: "${{ inputs.off-value }}" + run: | + set -euo pipefail + # Split WANTED into suffixes without allowing shell glob expansion. + set -f + + on=false + case "${EVENT}" in + push | merge_group | workflow_dispatch) + on=true + ;; + *) + for suffix in ${WANTED}; do + if jq -e --arg l "ci:+${suffix}" \ + 'if type == "array" then index($l) != null else false end' \ + <<<"${LABELS:-[]}" >/dev/null; then + on=true + break + fi + done + ;; + esac + + if "${on}"; then value="${ON_VALUE}"; else value="${OFF_VALUE}"; fi + printf '%s -> %s\n' "${WANTED}" "${value}" + printf 'value=%s\n' "${value}" >>"${GITHUB_OUTPUT}" diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 68746b0322..9eab13c3c6 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -35,20 +35,12 @@ Production artifacts are produced via nix builds in a separate CI workflow. ### Main steps -1. Check code changes to determine which tests are required -2. Build and test across a matrix of nix targets and profiles: - - Nix targets: `tests.all`, `frr.dataplane`, `dataplane` - - Profiles: `debug`, `release` -3. Run `cargo deny` checks for license and security issues -4. Execute tests: - - Regular tests using `cargo nextest` (via `just test`) - - Shuttle tests (concurrent execution testing with `features=shuttle`) -5. Run `cargo clippy` for linting (via `just clippy`) -6. Build documentation with `rustdoc` (via `just docs`) -7. Run doctests (via `just doctest`) -8. Push container images to GHCR (for non-test targets) -9. Run VLAB/HLAB integration tests (virtual/hybrid lab environments) -10. Publish release artifacts and bump fabricator on tag pushes +1. Plan jobs from the event and `ci:+` labels +2. Run lint, debug checks, and debug coverage on pull requests +3. Run expensive profiles and specialized jobs on labeled or deep runs +4. Build and push containers required by VLAB/HLAB +5. Aggregate required results in the `Summary` job +6. Publish release artifacts and bump fabricator on tag pushes ### Manual dispatch options @@ -60,16 +52,56 @@ Production artifacts are produced via nix builds in a separate CI workflow. ### Pull Request label options +- `ci:+merge-ready` - Run everything the merge queue will run, so a failure + is found before queueing; HLAB remains excluded, because the merge queue does + not run it either. It tests this branch's head while the queue tests the + result of merging it, so a green run here can still fail in the queue if + `main` moved underneath it +- `ci:+test/all-profiles` - Add release and fuzz checks plus fuzz coverage +- `ci:+sanitize` - Run address and thread sanitizer tests +- `ci:+test-each` - Test each workspace package independently +- `ci:+miri` - Run Miri checks +- `ci:+wasm` - Run the WASM build check +- `ci:+concurrency` - Run Shuttle and Loom tests +- `ci:+cross` - Build all cross-platform containers +- `ci:+cross/full` - Also run the workspace test suite under qemu-user, on the + two aarch64 musl legs. Gated like every other job, so the merge queue and + `ci:+merge-ready` include it - `ci:+vlab` - Run VLAB tests on this PR - `ci:+hlab` - Run HLAB tests on this PR - `ci:+release` - Enable release tests for VLAB/HLAB on this PR -- `ci:-upgrade` - Disable upgrade tests on this PR +- `ci:-upgrade` - Disable upgrade tests on this PR. `ci:+merge-ready` + overrides it, because the merge queue has no labels to read and would + run the upgrade legs anyway; a `merge-ready` run that skipped them would + not be the preview it claims to be + +Labels are additive, and optional: a pull request needs none of them. +`ci:-upgrade` is the sole exception, subtracting a job that would otherwise run. + +Adding a label starts a **new** workflow run, and that run repeats the default +jobs as well as the ones the label enabled. This applies to _every_ label, not +only the `ci:` ones: the trigger cannot filter by name and the run in flight is +cancelled, so adding `bug` or `documentation` mid-run discards whatever it had +finished. Label first, or wait for the run to end. +GitHub cannot add a job to a run that already exists, so this is unavoidable +without teaching jobs to skip work an earlier run finished for the same commit. +Set the labels you expect to need when opening the pull request and the repeat +does not arise. + +Not labelling is also a reasonable choice. +The gated jobs are the ones judged unlikely to fail, and the merge queue runs +the full suite regardless, so a bad assumption costs a re-queue rather than a +bad merge. +When the merge queue does catch one of these, add the matching label and push +the fix, which keeps the check on the pull request from then on. +If those queue failures stop being rare, the phasing is worth revisiting. ### Job matrix -- Nix targets: `tests.all` (runs tests, lints, docs), `frr.dataplane` - and `dataplane` (build and push containers) -- Profiles: `debug`, `release` +- Checks: `debug` by default; `release` and `fuzz` on deep runs +- Coverage: `debug` by default; `fuzz` on deep runs +- Miri: required on deep runs; opt-in on pull requests with `ci:+miri` +- Containers: debug/release for dataplane and FRR; release for validator - VLAB configurations: spine-leaf fabric mode, L2VNI/L3VNI VPC modes, with gateway enabled @@ -77,6 +109,15 @@ Production artifacts are produced via nix builds in a separate CI workflow. - Container images pushed to GitHub Container Registry (GHCR) - Release containers published on tag pushes via `just push` +- Coverage reports from each `coverage/` job, kept for 7 days: + - `coverage-html-.tar.gz` - `llvm-cov` HTML report, including the + per-branch counts that Codecov does not render. Unpack and open + `html/index.html` + - `lcov-.info` - LCOV report with repository-relative paths, for + feeding to other coverage tooling + + Both upload unarchived, so they download as the named file rather than + wrapped in a zip. --- diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index fd460480a9..0afab3591d 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -10,7 +10,9 @@ name: "dev.yml" on: - pull_request: {} + pull_request: + # Removing a label must recompute the plan just as adding one does. + types: ["opened", "synchronize", "reopened", "labeled", "unlabeled"] push: branches: - "main" @@ -54,31 +56,92 @@ concurrency: permissions: contents: "read" +env: + CI_DEBUG_JUSTFILE: "${{ github.event_name == 'workflow_dispatch' && inputs.debug_justfile || false }}" + jobs: - check_changes: - name: "Deduce required tests from code changes" + plan: + name: "Plan which jobs to run" permissions: contents: "read" - pull-requests: "read" # dorny/paths-filter reads the PR's changed-file list via the API runs-on: "ubuntu-latest" outputs: - devfiles: "${{ steps.changes.outputs.devfiles }}" + container_profiles: "${{ steps.container-profiles.outputs.value }}" + profiles: "${{ steps.profiles.outputs.value }}" + concurrency: "${{ steps.concurrency.outputs.value }}" + cross: "${{ steps.cross.outputs.value }}" + cross_full: "${{ steps.cross-full.outputs.value }}" + miri: "${{ steps.miri.outputs.value }}" + sanitize: "${{ steps.sanitize.outputs.value }}" + test_each: "${{ steps.test-each.outputs.value }}" + wasm: "${{ steps.wasm.outputs.value }}" steps: - name: "Checkout" - if: "${{ !github.event.pull_request }}" uses: "actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1" # v7.0.1 with: persist-credentials: "false" - fetch-depth: "0" - - name: "Check code changes" - uses: "dorny/paths-filter@7b450fff21473bca461d4b92ce414b9d0420d706" # v4.0.2 - id: "changes" + + - id: "miri" + uses: &gate "./.github/actions/ci-gate" + with: + labels: "miri" + + - id: "sanitize" + uses: *gate + with: + labels: "sanitize" + + - id: "cross" + uses: *gate + with: + labels: "cross cross/full" + + - id: "cross-full" + uses: *gate with: - filters: | - devfiles: - - '!(README.md|LICENSE|NOTICE|.zed/**|.vscode/**|CLAUDE.md|.rules|development/**|testing.md|workspace-deps.svg|codebook.toml|.markdownlint.json|.gitattributes|.gitignore|.github/**)' - - '.github/workflows/dev.yml' - - '.github/actions/**' + labels: "cross/full" + + - id: "concurrency" + uses: *gate + with: + labels: "concurrency" + + - id: "test-each" + uses: *gate + with: + labels: "test-each" + + - id: "profiles" + uses: *gate + with: + labels: "test/all-profiles" + on-value: '["debug", "release", "fuzz"]' + off-value: '["debug"]' + + # Lab jobs require release images but not other release/fuzz checks. + - id: "container-profiles" + uses: *gate + with: + labels: "test/all-profiles" + # The build matrix has no fuzz container targets. + on-value: '["debug", "release"]' + # Pull-request lab labels require both debug and release images. + off-value: >- + ${{ + github.event_name == 'pull_request' + && ( + contains(github.event.pull_request.labels.*.name, 'ci:+vlab') + || contains(github.event.pull_request.labels.*.name, 'ci:+hlab') + ) + && '["debug", "release"]' + || '["debug"]' + }} + + - id: "wasm" + uses: *gate + with: + # Cross waits for WASM to limit runner load. + labels: "wasm cross cross/full" version: name: "Generate temp artifact version" @@ -108,53 +171,22 @@ jobs: echo "ref=${commit_sha}" >> "$GITHUB_OUTPUT" check: - if: >- - ${{ - needs.check_changes.outputs.devfiles == 'true' - || startsWith(github.event.ref, 'refs/tags/v') - || github.event_name == 'workflow_dispatch' - }} - name: "check/${{ matrix.build.name }}/${{ matrix.features || 'default' }}" + name: "check/${{ matrix.profile }}" runs-on: "lab" needs: - - check_changes + - plan permissions: &check-perms contents: "read" - # The lab runners have 10 cores. `jobs` is nix's --max-jobs (concurrent - # derivations) and `cores` is nix's --cores (NIX_BUILD_CORES, which crane - # turns into CARGO_BUILD_JOBS and enableParallelBuilding turns into - # `make -j`), so the cap on a job is their product. Every job below that - # reaches `nix build` sets `jobs=1 cores=8`: one derivation at a time, - # eight compile threads inside it, two cores left for everything else. - # A single cargo build of the workspace dominates these jobs, so - # serializing derivations costs us little and that build gets the 8. - # The exception is `cross`, which runs two matrix entries at once and - # so halves its `cores` to stay inside the same budget. - env: &check-env + env: &ci-env USER: "runner" - JUST_VARS: >- - jobs=1 - cores=8 - docker_sock=/run/docker/docker.sock - debug_justfile=${{ github.event_name == 'workflow_dispatch' && github.event.inputs.debug_justfile || false }} - profile=${{ matrix.build.profile }} - sanitize=${{ matrix.build.sanitize }} - instrument=${{ matrix.build.instrument }} - features=${{ matrix.features }} - oci_repo=ghcr.io + # The `just` action runs under `set -u`, so this stays defined. + JUST_VARS: "" strategy: fail-fast: false + # Keep one pull request from occupying the shared lab pool. + max-parallel: 1 matrix: - build: &default-build - - &debug-build - name: "debug" - profile: "debug" - sanitize: "" - instrument: "none" - - name: "release" - profile: "release" - sanitize: "" # TODO: enable cfi and safe-stack when possible - instrument: "none" + profile: "${{ fromJSON(needs.plan.outputs.profiles) }}" steps: - *checkout @@ -167,23 +199,25 @@ jobs: - name: "fmt" uses: &just "./.github/actions/just" with: - recipe: "fmt" - recipe_args: "--check" + recipe: "ci::check-fmt" - - name: "test" + - name: "clippy" uses: *just with: - recipe: "test" + recipe: "ci::check-clippy" + recipe_args: "${{ matrix.profile }}" - - name: "clippy" + - name: "test" uses: *just with: - recipe: "clippy" + recipe: "ci::check-test" + recipe_args: "${{ matrix.profile }}" - name: "doctest" uses: *just with: - recipe: "doctest" + recipe: "ci::check-doctest" + recipe_args: "${{ matrix.profile }}" - &verify-clean-tree name: "verify-clean-tree" @@ -209,18 +243,8 @@ jobs: limit-access-to-actor: true lint: - if: >- - ${{ - github.event_name == 'pull_request' - || github.event_name == 'merge_group' - || needs.check_changes.outputs.devfiles == 'true' - || startsWith(github.event.ref, 'refs/tags/v') - || github.event_name == 'workflow_dispatch' - }} name: "lint" runs-on: "lab" - needs: - - check_changes permissions: contents: "read" env: @@ -329,148 +353,193 @@ jobs: - *tmate build: - if: >- - ${{ - needs.check_changes.outputs.devfiles == 'true' - || startsWith(github.event.ref, 'refs/tags/v') - || github.event_name == 'workflow_dispatch' - }} - name: "${{matrix.nix-target}}/${{matrix.build.name}}" + name: "${{ matrix.nix-target }}/${{ matrix.profile }}" runs-on: lab needs: - - check_changes + - plan - version - lint permissions: contents: "read" packages: "write" # nix-setup logs into ghcr.io; this job pushes images via `just push-container` - env: - USER: "runner" - JUST_VARS: >- - debug_justfile=${{ github.event_name == 'workflow_dispatch' && github.event.inputs.debug_justfile || false }} + env: *ci-env strategy: fail-fast: false + max-parallel: 1 matrix: nix-target: - frr.dataplane - dataplane - validator - build: *default-build + # TODO: enable cfi and safe-stack on release when possible + profile: "${{ fromJSON(needs.plan.outputs.container_profiles) }}" exclude: - nix-target: validator - build: - name: "debug" + profile: debug steps: - *checkout - *nix-setup - name: "push container" - env: - VERSION: "${{ needs.version.outputs.version }}" - PROFILE: "${{ matrix.build.profile }}" - NIX_TARGET: "${{ matrix.nix-target }}" - SANITIZE: "${{ matrix.build.sanitize }}" - INSTRUMENT: "${{ matrix.build.instrument }}" - DEBUG_JUSTFILE: "${{ github.event_name == 'workflow_dispatch' && github.event.inputs.debug_justfile || false }}" - run: | - set -euo pipefail - if [ "${NIX_TARGET}" = "validator" ]; then - platform="wasm32-wasip1" - else - platform="x86-64-v3" - fi - base_args=( - jobs=1 - cores=8 - docker_sock=/run/docker/docker.sock - debug_justfile="${DEBUG_JUSTFILE}" - profile="${PROFILE}" - platform="${platform}" - sanitize="${SANITIZE}" - instrument="${INSTRUMENT}" - oci_repo=ghcr.io - ) - just "${base_args[@]}" push-container "${NIX_TARGET}" - just "${base_args[@]}" "version=${VERSION}-${PROFILE}" push-container "${NIX_TARGET}" + uses: *just + with: + recipe: "ci::push-container" + recipe_args: "${{ matrix.nix-target }} ${{ matrix.profile }} ${{ needs.version.outputs.version }}" - *verify-clean-tree - *tmate sanitize: - if: >- - ${{ - needs.check_changes.outputs.devfiles == 'true' - || startsWith(github.event.ref, 'refs/tags/v') - || github.event_name == 'workflow_dispatch' - }} - name: "sanitize/${{ matrix.build.profile }}/${{ matrix.build.sanitize }}" + if: "${{ needs.plan.outputs.sanitize == 'true' }}" + name: "sanitize/${{ matrix.sanitizer }}" runs-on: "lab" needs: - - check_changes + - plan - check permissions: *check-perms - env: *check-env + env: *ci-env strategy: fail-fast: false max-parallel: 1 matrix: - build: - - name: "thread" - profile: "fuzz" - sanitize: "thread" - instrument: "none" - - name: "address" - profile: "fuzz" - sanitize: "address" - instrument: "none" + sanitizer: + - thread + - address steps: - *checkout - *nix-setup - name: "test" uses: *just with: - recipe: "test" + recipe: "ci::sanitize" + recipe_args: "${{ matrix.sanitizer }}" - *tmate test_each: - if: >- - ${{ - needs.check_changes.outputs.devfiles == 'true' - || startsWith(github.event.ref, 'refs/tags/v') - || github.event_name == 'workflow_dispatch' - }} + if: "${{ needs.plan.outputs.test_each == 'true' }}" name: "test_each" runs-on: "lab" needs: - - check_changes + - plan - check permissions: *check-perms - env: *check-env + env: *ci-env + steps: + - *checkout + - *nix-setup + - name: "test-each" + uses: *just + with: + recipe: "ci::test-each" + - *tmate + + coverage: + name: "coverage/${{ matrix.profile }}" + runs-on: "lab" + # Start baseline coverage immediately; deep jobs wait for cheap checks first. + needs: + - plan + permissions: + contents: "read" + id-token: "write" # codecov-action authenticates by OIDC, not a stored token + env: *ci-env strategy: fail-fast: false max-parallel: 1 + # Fuzz provides optimized coverage while retaining safety checks. matrix: - build: - - *debug-build + profile: "${{ fromJSON(needs.plan.outputs.profiles) }}" + exclude: + - profile: release steps: - *checkout - *nix-setup - - name: "test-each" + + - name: "coverage-archive" uses: *just with: - recipe: "test-each" + recipe: "ci::coverage" + recipe_args: "${{ matrix.profile }}" + + # Preserve partial results and keep profiles separate in Codecov. + - name: "upload coverage to codecov" + if: "${{ always() }}" + uses: "codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f" # v7.0.0 + with: + files: "./target/coverage/lcov.info" + report_type: "coverage" + flags: "${{ matrix.profile }}" + disable_search: "true" + fail_ci_if_error: "false" + use_oidc: "true" + verbose: "true" + + - name: "upload test results to codecov" + if: "${{ always() }}" + uses: "codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f" # v7.0.0 + with: + files: "./target/nextest/default/junit.xml" + report_type: "test_results" + flags: "${{ matrix.profile }}" + disable_search: "true" + fail_ci_if_error: "false" + use_oidc: "true" + verbose: "true" + + # Publish branch coverage, which Codecov does not render. + - name: "pack coverage reports" + if: "${{ always() }}" + env: + PROFILE: "${{ matrix.profile }}" + run: | + set -euo pipefail + declare -r out="./target/coverage" + # gzip, not zstd: this step runs outside the nix shell. + if [ -d "${out}/html" ]; then + tar -C "${out}" -czf "${out}/coverage-html-${PROFILE}.tar.gz" html + else + echo "::warning::no HTML report to pack" + fi + # The artifact name is the file name, so qualify it by profile. + if [ -r "${out}/lcov.info" ]; then + cp -- "${out}/lcov.info" "${out}/lcov-${PROFILE}.info" + else + echo "::warning::no LCOV report to publish" + fi + + # Avoid wrapping the reports in another archive. + - name: "upload html coverage report" + if: "${{ always() }}" + uses: "actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a" # v7.0.1 + with: + path: "./target/coverage/coverage-html-${{ matrix.profile }}.tar.gz" + archive: "false" + if-no-files-found: "warn" + retention-days: "7" + + - name: "upload lcov coverage report" + if: "${{ always() }}" + uses: "actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a" # v7.0.1 + with: + path: "./target/coverage/lcov-${{ matrix.profile }}.info" + archive: "false" + if-no-files-found: "warn" + retention-days: "7" + + - name: "discard coverage intermediates" + if: "${{ always() }}" + run: | + set -euo pipefail + rm -rf ./target/coverage/extract ./target/coverage/profraw + - *tmate miri: - if: >- - ${{ - needs.check_changes.outputs.devfiles == 'true' - || startsWith(github.event.ref, 'refs/tags/v') - || github.event_name == 'workflow_dispatch' - }} + if: "${{ needs.plan.outputs.miri == 'true' }}" name: "check/miri/${{ matrix.cpu }}" runs-on: "lab" needs: - - check_changes + - plan + - check permissions: *check-perms env: USER: "runner" @@ -494,9 +563,7 @@ jobs: - name: "all packages" uses: *just env: - # miri never reaches `nix build`, so the root justfile's `jobs` and - # `cores` can't cap it; the module takes its own `cores` and hands - # it to cargo and to nextest. See the comment on `check-env`. + # Miri bypasses `nix build`, so cap it through its own module. JUST_VARS: >- miri::cores=8 miri::cpu=${{ matrix.cpu }} @@ -534,70 +601,38 @@ jobs: - *tmate wasm: - if: >- - ${{ - needs.check_changes.outputs.devfiles == 'true' - || startsWith(github.event.ref, 'refs/tags/v') - || github.event_name == 'workflow_dispatch' - }} - name: "${{ matrix.platform }}/${{ matrix.libc }}/${{ matrix.profile }}" + if: "${{ needs.plan.outputs.wasm == 'true' }}" + name: "wasm32-wasip1" runs-on: "lab" needs: - - check_changes + - plan + - check permissions: *check-perms - env: - USER: "runner" - strategy: - fail-fast: false - max-parallel: 1 - matrix: - include: - - platform: "wasm32-wasip1" - profile: "release" - libc: "none" - recipe: - name: "check" - args: "" + env: *ci-env steps: - *checkout - *nix-setup - - name: "${{ matrix.platform }}/${{ matrix.libc }}/${{ matrix.profile }}" + - name: "check" uses: *just - env: - JUST_VARS: >- - jobs=1 - cores=8 - platform=${{ matrix.platform }} - profile=${{ matrix.profile }} - libc=${{ matrix.libc }} - with: - recipe: "${{ matrix.recipe.name }}" - recipe_args: "${{ matrix.recipe.args }}" + with: + recipe: "ci::wasm" - *tmate cross: - if: >- - ${{ - github.event_name == 'pull_request' - && ( - contains(github.event.pull_request.labels.*.name, 'ci:+cross') - || contains(github.event.pull_request.labels.*.name, 'ci:+cross/full') - ) - || (github.event_name == 'push' || github.event_name == 'merge_group') - }} - name: "${{ matrix.recipe.name }}/${{ matrix.recipe.args }}/${{ matrix.platform }}/${{ matrix.libc }}" + if: "${{ needs.plan.outputs.cross == 'true' }}" + name: "cross/${{ matrix.target }}/${{ matrix.platform }}/${{ matrix.libc }}" runs-on: "lab" needs: - - check_changes + - plan - check - - miri - wasm permissions: *check-perms env: USER: "runner" + JUST_VARS: "" strategy: fail-fast: false - max-parallel: 2 + max-parallel: 1 matrix: platform: - "aarch64" @@ -605,108 +640,58 @@ jobs: libc: - "gnu" - "musl" - profile: - - "debug" - recipe: - - name: "build-container" - args: "dataplane" - - name: "build-container" - args: "frr.dataplane" + target: + - "dataplane" + - "frr.dataplane" steps: - *checkout - *nix-setup + # Emulate the two aarch64 musl dataplane builds before merge. - name: "test" if: >- ${{ - matrix.recipe.args == 'dataplane' + needs.plan.outputs.cross_full == 'true' + && matrix.target == 'dataplane' && matrix.libc == 'musl' - && github.event_name == 'pull_request' - && contains(github.event.pull_request.labels.*.name, 'ci:+cross/full') }} uses: *just - env: - # cores=4, not the 8 the rest of CI uses: this is the one job with - # `max-parallel: 2`, so two matrix entries that land on the same - # lab runner still fit inside its 10 cores. - JUST_VARS: >- - jobs=1 - cores=4 - platform=${{ matrix.platform }} - profile=${{ matrix.profile }} - libc=${{ matrix.libc }} - with: - recipe: "test" - - name: "${{ matrix.platform }}/${{ matrix.libc }}/${{ matrix.profile }}/${{ matrix.recipe.args }}" + with: + recipe: "ci::cross-test" + recipe_args: "${{ matrix.platform }} ${{ matrix.libc }}" + - name: "build-container" uses: *just - env: - JUST_VARS: >- - jobs=1 - cores=4 - platform=${{ matrix.platform }} - profile=${{ matrix.profile }} - libc=${{ matrix.libc }} - with: - recipe: "${{ matrix.recipe.name }}" - recipe_args: "${{ matrix.recipe.args }}" + with: + recipe: "ci::cross" + recipe_args: "${{ matrix.platform }} ${{ matrix.libc }} build-container ${{ matrix.target }}" - *tmate concurrency: - if: >- - ${{ - needs.check_changes.outputs.devfiles == 'true' - || startsWith(github.event.ref, 'refs/tags/v') - || github.event_name == 'workflow_dispatch' - }} + if: "${{ needs.plan.outputs.concurrency == 'true' }}" name: "concurrency" runs-on: "lab" needs: - - check_changes + - plan - check permissions: *check-perms - # This job doesn't use the `*check-env` anchor: that anchor's - # `JUST_VARS` references `matrix.build.*`, and this job has no - # matrix. Each step below sets `JUST_VARS` itself, inlining the - # jobs / cores / docker_sock / debug_justfile / oci_repo settings - # that the anchor would have provided. - env: - USER: "runner" + env: *ci-env steps: - *checkout - *nix-setup - name: "shuttle" - env: - JUST_VARS: >- - jobs=1 - cores=8 - docker_sock=/run/docker/docker.sock - debug_justfile=${{ github.event_name == 'workflow_dispatch' && github.event.inputs.debug_justfile || false }} - profile=fuzz - features=shuttle - oci_repo=ghcr.io uses: *just with: - recipe: "test" + recipe: "ci::shuttle" - name: "loom" - env: - JUST_VARS: >- - jobs=1 - cores=8 - docker_sock=/run/docker/docker.sock - debug_justfile=${{ github.event_name == 'workflow_dispatch' && github.event.inputs.debug_justfile || false }} - profile=fuzz - features=loom - oci_repo=ghcr.io uses: *just with: - recipe: "test" + recipe: "ci::loom" - *tmate vlab: - if: "${{ needs.check_changes.outputs.devfiles == 'true' || (startsWith(github.event.ref, 'refs/tags/v') || startsWith(github.ref, 'refs/tags/v')) && (github.event_name == 'push' || github.event_name == 'workflow_dispatch') }}" needs: - version - build - - check_changes + - plan name: "${{ matrix.hybrid && 'h' || 'v' }}-${{ matrix.upgradefrom && 'up' || '' }}${{ matrix.upgradefrom }}${{ matrix.upgradefrom && '-' || '' }}${{ matrix.mesh && 'mesh-' || '' }}${{ matrix.gateway && 'gw-' || '' }}${{ matrix.includeonie && 'onie-' || '' }}${{ matrix.buildmode }}-${{ matrix.vpcmode }}" @@ -716,13 +701,18 @@ jobs: # ci:+vlab is required to enable virtual lab tests on PR # ci:-upgrade disables upgrade tests on PR # hlab is disabled for main and merge_queue till we have gateway tests for it + # ci:+merge-ready mirrors the merge queue, which skips HLAB. skip: >- ${{ github.event_name == 'pull_request' && ( matrix.hybrid && !contains(github.event.pull_request.labels.*.name, 'ci:+hlab') - || !matrix.hybrid && !contains(github.event.pull_request.labels.*.name, 'ci:+vlab') - || matrix.upgradefrom != '' && contains(github.event.pull_request.labels.*.name, 'ci:-upgrade') + || !matrix.hybrid + && !contains(github.event.pull_request.labels.*.name, 'ci:+vlab') + && !contains(github.event.pull_request.labels.*.name, 'ci:+merge-ready') + || matrix.upgradefrom != '' + && contains(github.event.pull_request.labels.*.name, 'ci:-upgrade') + && !contains(github.event.pull_request.labels.*.name, 'ci:+merge-ready') ) || github.event_name == 'workflow_dispatch' @@ -788,81 +778,45 @@ jobs: name: "Summary" runs-on: "ubuntu-latest" needs: + - build - check + - concurrency + - coverage + - cross - lint + - miri + # Include prerequisites so dependent skips cannot hide their failures. + - plan - sanitize - - concurrency - - build - - vlab - test_each - - miri + - version + - vlab - wasm - # Run always so this job can aggregate results even when one of its - # dependencies failed or was skipped. - # - # Skipped dependency jobs are handled explicitly by the step-level - # conditions below, so a skipped "build" or "check" job does not cause - # "summary" itself to be skipped. + # Aggregate failures while allowing phased jobs to be skipped. if: ${{ always() }} steps: - - name: "Flag any check matrix failures" - if: ${{ needs.check.result != 'success' && needs.check.result != 'skipped' }} - run: | - echo '::error:: Some check job(s) failed' - exit 1 - - name: "Flag any lint failures" - if: ${{ needs.lint.result != 'success' && needs.lint.result != 'skipped' }} - run: | - echo '::error:: lint job failed' - exit 1 - - name: "Flag any concurrency job failures" - if: ${{ needs.concurrency.result != 'success' && needs.concurrency.result != 'skipped' }} - run: | - echo '::error:: concurrency job failed' - exit 1 - - name: "Flag any test_each matrix failures" - if: ${{ needs.test_each.result != 'success' && needs.test_each.result != 'skipped' }} - run: | - echo '::error:: Some test_each job(s) failed' - exit 1 - - name: "Flag any sanitize matrix failures" - if: ${{ needs.sanitize.result != 'success' && needs.sanitize.result != 'skipped' }} - run: | - echo '::error:: Some sanitize job(s) failed' - exit 1 - ## FIXME: Make Miri's only job (powerpc64) not "required" for the summary - ## to pass, because we face an issue (caching?), and it's blocking CI. - ## See https://github.com/githedgehog/dataplane/actions/runs/28030274103/job/82968910470?pr=1603 - ## Revert this once the issue is solved. - #- name: "Flag any miri matrix failures" - # if: ${{ needs.miri.result != 'success' && needs.miri.result != 'skipped' }} - # run: | - # echo '::error:: Some miri job(s) failed' - # exit 1 - - name: "Flag any build matrix failures" - if: ${{ needs.build.result != 'success' && needs.build.result != 'skipped' }} - run: | - echo '::error:: Some build job(s) failed' - exit 1 - - name: "Flag any wasm failures" - if: ${{ needs.wasm.result != 'success' && needs.wasm.result != 'skipped' }} - run: | - echo '::error:: Some wasm job(s) failed' - exit 1 - - name: "Flag any vlab matrix failures" - if: ${{ needs.vlab.result != 'success' && needs.vlab.result != 'skipped' }} + - name: "Flag any job failures" + env: + NEEDS: "${{ toJSON(needs) }}" run: | - echo '::error:: Some vlab job(s) failed' - exit 1 + set -euo pipefail + failed="$(jq -r 'to_entries[] + | select(.value.result != "success" and .value.result != "skipped") + | .key' <<<"${NEEDS}")" + status=0 + for job in ${failed}; do + echo "::error::${job} did not succeed" + status=1 + done + exit "${status}" publish: name: "Publish release artifacts" runs-on: lab if: startsWith(github.event.ref, 'refs/tags/v') && github.event_name == 'push' + # Gate releases on the aggregate result so every deep check remains blocking. needs: - - build - - sanitize - - vlab + - summary permissions: packages: write # docker/login-action + `just push` publish images to ghcr.io diff --git a/ci.just b/ci.just new file mode 100644 index 0000000000..1bc2fc076a --- /dev/null +++ b/ci.just @@ -0,0 +1,75 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright Open Network Fabric Authors + +set unstable := true +set shell := ["/usr/bin/env", "bash", "-euo", "pipefail", "-c"] +set script-interpreter := ["/usr/bin/env", "bash", "-euo", "pipefail"] + +# Reproducible entry points for `.github/workflows/dev.yml` jobs. Keep +# job-specific build settings here instead of duplicating workflow variables. + +debug_justfile := env("CI_DEBUG_JUSTFILE", "false") + +# Nix build budget for a 10-core lab runner. +jobs := "1" + +cores := "8" + +[private] +_lab := "jobs=" + jobs + " cores=" + cores + " docker_sock=/run/docker/docker.sock" + " oci_repo=ghcr.io" + " debug_justfile=" + debug_justfile + +[default] +[private] +@default: + just --list --justfile {{ justfile() }} + +# Separate recipes keep failures visible as individual workflow steps. +# `cargo fmt` is profile-independent, so `check-fmt` accepts no profile. +check-fmt: + just {{ _lab }} fmt --check + +check-test profile: + just {{ _lab }} profile={{ profile }} test + +check-clippy profile: + just {{ _lab }} profile={{ profile }} clippy + +check-doctest profile: + just {{ _lab }} profile={{ profile }} doctest + +sanitize san profile="fuzz": + just {{ _lab }} profile={{ profile }} sanitize={{ san }} test + +test-each profile="debug": + just {{ _lab }} profile={{ profile }} test-each + +coverage profile="debug": + just {{ _lab }} profile={{ profile }} instrument=coverage coverage-archive + +# Optimized fuzz builds let schedule explorers cover more interleavings. +shuttle profile="fuzz": + just {{ _lab }} profile={{ profile }} features=shuttle test + +loom profile="fuzz": + just {{ _lab }} profile={{ profile }} features=loom test + +wasm: + just {{ _lab }} platform=wasm32-wasip1 profile=release libc=none check + +cross platform libc +args: + just {{ _lab }} platform={{ platform }} libc={{ libc }} profile=debug {{ args }} + +# Bound individual tests when running them under qemu-user. +cross-test platform libc: + NEXTEST_PROFILE=cross-qemu just {{ _lab }} platform={{ platform }} libc={{ libc }} profile=debug test + +# Publish both content-derived and discoverable per-commit tags. +[script] +push-container target profile version: + declare platform="x86-64-v3" + if [ "{{ target }}" = "validator" ]; then + platform="wasm32-wasip1" + fi + declare -ra base=( {{ _lab }} profile="{{ profile }}" platform="${platform}" ) + just "${base[@]}" push-container "{{ target }}" + just "${base[@]}" version="{{ version }}-{{ profile }}" push-container "{{ target }}" diff --git a/codecov.yml b/codecov.yml index 77ff065878..5bbda8c1ac 100644 --- a/codecov.yml +++ b/codecov.yml @@ -1,10 +1,20 @@ -comment: false +comment: + # Show each cargo profile separately. + layout: "condensed_header, flags, condensed_files, condensed_footer" + behavior: default + require_changes: false + +# Fuzz runs only on deep or explicitly labeled runs. +flags: + debug: + carryforward: false + fuzz: + carryforward: true coverage: status: project: default: - # Our usage is intended for discussion and analysis in PRs - # we have no explicit coverage target at the moment. + # Coverage is informational until the project defines a target. informational: true patch: default: diff --git a/default.nix b/default.nix index 4aeeda88eb..acadbf858e 100644 --- a/default.nix +++ b/default.nix @@ -504,19 +504,27 @@ let profile = profile-tests'; args = { inherit pname cargoArtifacts; - buildPhaseCargoCommand = builtins.concatStringsSep " " ( - [ - "mkdir -p $out;" - "cargo" - "nextest" - "archive" - "--archive-file" - "$out/${pname}.tar.zst" - "--cargo-profile=${cargo-profile}" - ] - ++ (if package != null then [ "--package=${pname}" ] else [ ]) - ++ cargo-cmd-prefix-tests - ); + buildPhaseCargoCommand = + (builtins.concatStringsSep " " ( + [ + "mkdir -p $out;" + "cargo" + "nextest" + "archive" + "--archive-file" + "$out/${pname}.tar.zst" + "--cargo-profile=${cargo-profile}" + ] + ++ (if package != null then [ "--package=${pname}" ] else [ ]) + ++ cargo-cmd-prefix-tests + )) + # Record the remapped source root without changing normal archives. + + ( + if instrumentation == "coverage" then + "; echo -n '${src}' > $out/source-prefix" + else + "" + ); }; }; diff --git a/development/code/running-tests.md b/development/code/running-tests.md index f15e435f45..a8bac9880d 100644 --- a/development/code/running-tests.md +++ b/development/code/running-tests.md @@ -45,6 +45,33 @@ python3 -m http.server And then open a web-browser to [http://localhost:8000](http://localhost:8000) to view coverage data. +### Coverage from a nextest archive + +Use `just coverage` for incremental local coverage. To report on the Nix-built +[nextest archive] used by CI, run: + +```shell +just coverage-archive # the whole workspace +just coverage-archive nat # one package, as with `just test` +``` + +Additional arguments are forwarded to nextest. CI collects `debug` on every pull +request and adds `fuzz` on a deep run or behind the `ci:+test/all-profiles` +label; pick one with, for example, `just profile=fuzz coverage-archive`. +`release` is deliberately excluded from the coverage matrix: it strips the +debug assertions and overflow checks that make a coverage run worth reading, +and `fuzz` gives the same optimization while keeping them. + +Reports are written to `./target/coverage`: + +- `lcov.info` — repository-relative LCOV report +- `html/index.html` — browsable report with branch counts +- `coverage.profdata` — merged LLVM profile + +The first run requires a full instrumented build. Reports include workspace +sources only; host proc-macro crates and crates that produce no standalone code +may be absent rather than shown at 0%. + ## Fuzz testing (bolero) The dataplane project makes fairly extensive use of [fuzz testing](https://en.wikipedia.org/wiki/Fuzzing). @@ -137,5 +164,6 @@ than a real campaign. The two are complementary. [bolero]: https://github.com/camshaft/bolero [cargo llvm-cov]: https://github.com/taiki-e/cargo-llvm-cov?tab=readme-ov-file#cargo-llvm-cov [cargo profiles]: https://doc.rust-lang.org/cargo/reference/profiles.html +[nextest archive]: https://nexte.st/docs/ci-features/archiving/ [nextest profiles]: https://nexte.st/docs/configuration/#profiles [nextest]: https://nexte.st/ diff --git a/justfile b/justfile index d89fb58b65..93227c9f26 100644 --- a/justfile +++ b/justfile @@ -5,6 +5,7 @@ set unstable := true set shell := ["/usr/bin/env", "bash", "-euo", "pipefail", "-c"] set script-interpreter := ["/usr/bin/env", "bash", "-euo", "pipefail"] +mod ci mod miri # enable to debug just recipes @@ -492,6 +493,123 @@ coverage *args: cargo llvm-cov report --branch --codecov --output-path="${out}/codecov.json" cargo llvm-cov report --branch --summary-only +# Use Nix-built archives so local and CI coverage report the same binaries. +[script] +coverage-archive package="tests.all" *args: + {{ _just_debuggable_ }} + declare -r target="{{ if package == "tests.all" { "tests.all" } else { "tests.pkg." + package } }}" + just \ + jobs="{{jobs}}" \ + cores="{{cores}}" \ + debug_justfile="{{debug_justfile}}" \ + profile="{{profile}}" \ + libc="{{libc}}" \ + sanitize="{{sanitize}}" \ + features="{{features}}" \ + default_features="{{default_features}}" \ + platform="{{platform}}" \ + nightly="{{nightly}}" \ + instrument=coverage \ + build "${target}" + + declare -r root="$(pwd)" + declare -r out="${root}/target/coverage" + declare -r profraw="${out}/profraw" + declare -r extract="${out}/extract" + + rm -rf -- "${out}" + mkdir -p -- "${profraw}" "${extract}" + + # Make the count below see zero instead of a literal unmatched glob. + shopt -s nullglob + declare -ra archives=( "results/${target}"/*.tar.zst ) + shopt -u nullglob + if [ "${#archives[@]}" -ne 1 ]; then + >&2 echo "::error::expected exactly one archive in results/${target}, found ${#archives[@]}" + exit 1 + fi + declare -r archive="${archives[0]}" + + declare -r prefix_file="results/${target}/source-prefix" + if [ ! -r "${prefix_file}" ]; then + >&2 echo "::error::${prefix_file} is missing; the archive predates it, rebuild it" + exit 1 + fi + declare src_prefix + src_prefix="$(cat "${prefix_file}")" + declare -r src_prefix + + # Nextest changes cwd; `%m` also pools compatible profiles across tests. + export LLVM_PROFILE_FILE="${profraw}/cov-%m.profraw" + + # Report partial coverage before propagating a test failure. + declare -i test_status=0 + cargo nextest run \ + --archive-file "${archive}" \ + --extract-to "${extract}" \ + --workspace-remap "${root}" \ + {{ filter }} {{ args }} || test_status="$?" + + declare -r profraw_list="${out}/profraw.list" + find "${profraw}" -type f -name '*.profraw' > "${profraw_list}" + if [ ! -s "${profraw_list}" ]; then + >&2 echo "::error::no raw profiles were written; was ${archive} built with instrument=coverage?" + exit 1 + fi + llvm-profdata merge -sparse --input-files="${profraw_list}" -o "${out}/coverage.profdata" + + # Pass one primary object and filter reports to workspace sources. + declare target_dir + target_dir="$(jq -er '."rust-build-meta"."target-directory"' "${extract}/target/nextest/binaries-metadata.json")" + declare -r target_dir + declare -a objects=() + while IFS= read -r binary; do + if [ ! -x "${binary}" ]; then + >&2 echo "::error::${binary} is listed in the archive metadata but is not present" + exit 1 + fi + if [ "${#objects[@]}" -eq 0 ]; then + objects+=( "${binary}" ) + else + objects+=( -object "${binary}" ) + fi + done < <( + jq -er --arg prefix "${target_dir}/" --arg extract "${extract}/target/" \ + '."rust-binaries"[]."binary-path" | $extract + ltrimstr($prefix)' \ + "${extract}/target/nextest/binaries-metadata.json" + ) + + llvm-cov export \ + --format=lcov \ + --instr-profile="${out}/coverage.profdata" \ + "${objects[@]}" \ + "${src_prefix}" \ + | sed -e "s#^SF:${src_prefix}/#SF:#" > "${out}/lcov.info" + + # Codecov needs repository-relative paths; reject failed rewrites. + if grep -q '^SF:/' "${out}/lcov.info"; then + >&2 echo "::error::absolute paths survived the ${src_prefix} rewrite:" + >&2 grep -m5 '^SF:/' "${out}/lcov.info" + exit 1 + fi + + llvm-cov show \ + --format=html \ + --output-dir="${out}/html" \ + --show-branches=count \ + --instr-profile="${out}/coverage.profdata" \ + "${objects[@]}" \ + "${src_prefix}" + + llvm-cov report \ + --instr-profile="${out}/coverage.profdata" \ + "${objects[@]}" \ + "${src_prefix}" + + echo "lcov report: ${out}/lcov.info" + echo "html report: ${out}/html/index.html" + exit "${test_status}" + # Regenerate the dependency graph for the project [script] depgraph: diff --git a/nix/profiles.nix b/nix/profiles.nix index a3e24879ec..db0e0b6502 100644 --- a/nix/profiles.nix +++ b/nix/profiles.nix @@ -240,6 +240,7 @@ let instrument.coverage.NIX_CFLAGS_LINK = instrument.coverage.NIX_CFLAGS_COMPILE; instrument.coverage.RUSTFLAGS = [ "-Cinstrument-coverage" + "-Zcoverage-options=branch" ] ++ (map (flag: "-Clink-arg=${flag}") instrument.coverage.NIX_CFLAGS_LINK); combine-profiles =