Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion .github/actions/setup-integration-test-env/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,22 @@ runs:
# `.tool-versions` is the single source of truth for the Viceroy pin.
run: echo "viceroy-version=$(grep '^viceroy ' .tool-versions | awk '{print $2}')" >> "$GITHUB_OUTPUT"

- name: Remove runner-image Rust toolchain
# rust-cache hashes every installed toolchain into its key, so the image's
# preinstalled stable would rotate the key on each runner-image rollout.
shell: bash
run: if command -v rustup >/dev/null; then rustup toolchain uninstall stable; fi

- name: Set up Rust toolchain
uses: actions-rust-lang/setup-rust-toolchain@v1
with:
toolchain: ${{ steps.rust-version.outputs.rust-version }}
target: wasm32-wasip1
cache-shared-key: cargo-${{ runner.os }}
# Restore the axum job's main cache: it builds the same axum debug and
# Fastly release WASM artifacts. Integration tests run on PRs only, and
# PR caches are scoped to their ref and only evict main's shared caches.
cache-shared-key: cargo-axum-${{ runner.os }}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🌱 seedling: Most of the integration jobs' Rust compile isn't in cargo-axum

cargo-axum covers prepare-artifacts' axum debug build (277 crates) and Fastly release WASM build (255). The rest of the workflow's compiles aren't in any main cache. Counts are from this PR's run:

  • integration tests and Fastly EC lifecycle run cargo test --manifest-path crates/trusted-server-integration-tests/Cargo.toml --target x86_64-unknown-linux-gnu (376 crates). test-axum never builds that crate. At most 326 of those crate versions appear in test-axum's explicit-target CLI build, and feature differences will cut that further. test-parity compiles the same 376 crates, but without --target, so they land in target/debug instead. The explicit --target dates from Add integration testing with testcontainers and Playwright #442, when .cargo/config.toml set a global wasm32 build target. Add trusted-server-adapter-axum native dev server (PR 16) #643 removed that setting.
  • generate-integration-viceroy-configs.sh builds 228 crates into crates/trusted-server-integration-tests/target, which rust-cache doesn't save.
  • The Cloudflare worker-build --release (198 crates) isn't built by any main job. test-cloudflare only runs cargo check.

Possible follow-up: drop --target from the two cargo test steps, give this action a cache-key input, and have those two jobs restore cargo-parity-${{ runner.os }}. None of this is a regression from the shared key, so it can wait.

cache-save-if: ${{ github.ref == 'refs/heads/main' }}
Comment on lines +65 to +69

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔧 wrench: The save gate is open on workflow_dispatch

Integration Tests also runs on workflow_dispatch (integration-tests.yml:11), so "Integration tests run on PRs only" doesn't hold. A dispatch from main makes this expression true in all four jobs that use the action, and they save under the key test-axum uses. rust-cache never saves over an exact key match ("Cache up-to-date."), so the first job to save owns cargo-axum until the key changes. If a dispatched job saves first, for example in the ~9 minutes before test-axum finishes after a Cargo.lock change lands on main, test-axum restores that partial cache and rebuilds its dependencies on every run. That's the failure #1258 fixes. The integration tests job, for instance, only compiles the integration-tests crate (376 crates in this PR's run) and none of test-axum's axum, bench, or release WASM builds.

All 18 dispatches so far ran on feature branches, so this hasn't happened yet. The action only needs to restore, so a constant closes it:

Suggested change
# Restore the axum job's main cache: it builds the same axum debug and
# Fastly release WASM artifacts. Integration tests run on PRs only, and
# PR caches are scoped to their ref and only evict main's shared caches.
cache-shared-key: cargo-axum-${{ runner.os }}
cache-save-if: ${{ github.ref == 'refs/heads/main' }}
# Restore the axum job's main cache: it builds the same axum debug and
# Fastly release WASM artifacts. Restore only: test-axum owns this key,
# and rust-cache never replaces an exact-match key, so a partial cache
# saved by a workflow_dispatch run on main would stick.
cache-shared-key: cargo-axum-${{ runner.os }}
cache-save-if: "false"

Checked in a scratch copy: the YAML parses to cache-save-if: 'false', which fails rust-cache's save === "true" test, and actionlint passes on integration-tests.yml.


- name: Cache Viceroy binary
if: ${{ inputs.install-viceroy == 'true' }}
Expand Down
10 changes: 9 additions & 1 deletion .github/workflows/format.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,21 @@ jobs:
run: echo "rust-version=$(grep '^rust ' .tool-versions | awk '{print $2}')" >> $GITHUB_OUTPUT
shell: bash

- name: Remove runner-image Rust toolchain
# rust-cache hashes every installed toolchain into its key, so the image's
# preinstalled stable would rotate the key on each runner-image rollout.
shell: bash
run: if command -v rustup >/dev/null; then rustup toolchain uninstall stable; fi

- name: Set up rust toolchain
uses: actions-rust-lang/setup-rust-toolchain@v1
with:
toolchain: ${{ steps.rust-version.outputs.rust-version }}
target: wasm32-wasip1,wasm32-unknown-unknown
components: "clippy, rustfmt"
cache-shared-key: cargo-${{ runner.os }}
cache-shared-key: cargo-clippy-${{ runner.os }}
# PR caches are scoped to their ref and only evict main's shared caches.
cache-save-if: ${{ github.ref == 'refs/heads/main' }}

- name: Run cargo fmt
uses: actions-rust-lang/rustfmt@v1
Expand Down
58 changes: 53 additions & 5 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,20 @@ jobs:
run: echo "node-version=$(grep '^nodejs ' .tool-versions | awk '{print $2}')" >> $GITHUB_OUTPUT
shell: bash

- name: Remove runner-image Rust toolchain

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ refactor: One shared Rust setup action instead of nine copies

This PR adds the same uninstall step and the same cache-save-if line and comment in nine places: seven jobs here, format.yml, and the integration action. Each sits next to a "Retrieve Rust version" step that every job already repeats. A new Rust job that skips the uninstall gets a key that changes with each runner image. One that skips the save gate starts saving PR caches again. Neither fails visibly. A local composite action would hold all three in one place. Sketch:

# .github/actions/setup-rust/action.yml
name: Set up pinned Rust
description: Install the .tool-versions Rust toolchain with a per-job cache that only main saves.
inputs:
  cache-key:
    description: Per-job cache key prefix.
    required: true
  target:
    description: Comma-separated extra targets.
    default: ""
  components:
    description: Comma-separated extra components.
    default: ""
  save-cache:
    description: Set to "false" for restore-only callers.
    default: "true"
runs:
  using: composite
  steps:
    - id: rust-version
      shell: bash
      run: echo "rust-version=$(awk '$1 == "rust" { print $2 }' .tool-versions)" >> "$GITHUB_OUTPUT"
    - name: Remove runner-image Rust toolchain
      # rust-cache hashes every installed toolchain into its key.
      shell: bash
      run: if command -v rustup >/dev/null; then rustup toolchain uninstall stable; fi
    - uses: actions-rust-lang/setup-rust-toolchain@v1
      with:
        toolchain: ${{ steps.rust-version.outputs.rust-version }}
        target: ${{ inputs.target }}
        components: ${{ inputs.components }}
        cache-shared-key: ${{ inputs.cache-key }}-${{ runner.os }}
        cache-save-if: ${{ inputs.save-cache == 'true' && github.ref == 'refs/heads/main' }}

Each job would then use uses: ./.github/actions/setup-rust with cache-key: cargo-fastly and so on, and the integration action would pass save-cache: "false". Optional for this PR.

# rust-cache hashes every installed toolchain into its key, so the image's
# preinstalled stable would rotate the key on each runner-image rollout.
shell: bash
run: if command -v rustup >/dev/null; then rustup toolchain uninstall stable; fi

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 thinking: Removing the default toolchain lets rustup reinstall stable silently

After this step rustup's default still points at stable, which is no longer installed. setup-rust-toolchain only sets a directory override for the checkout, and the runner's rustup (1.29.1) auto-installs a missing active toolchain. So a future step that runs cargo or rustc outside the checkout, from $HOME or $RUNNER_TEMP for example, would quietly download the current stable (1.99.0 today) and build with it instead of the pinned 1.95.0. Nothing does that today: none of the 12 jobs that run this step logged an auto-install in this PR's run. The cache key wouldn't move either, because rust-cache saves under the key it computed at restore.

Reproduced in rust:1.95.0-slim: with the default toolchain uninstalled, cd /tmp && cargo --version printed "the missing active toolchain ... has been auto-installed" and ran cargo 1.99.0. Unsetting the default in the same step turns that into an error ("rustup could not choose a version of cargo to run ... no default is configured"). In the same container, the in-repo override and rustup toolchain list (rust-cache's input) were unchanged:

if command -v rustup >/dev/null; then rustup toolchain uninstall stable && rustup default none; fi

This would go in all nine copies of the step.


- name: Set up Rust toolchain
uses: actions-rust-lang/setup-rust-toolchain@v1
with:
toolchain: ${{ steps.rust-version.outputs.rust-version }}
target: wasm32-wasip1
cache-shared-key: cargo-${{ runner.os }}
cache-shared-key: cargo-fastly-${{ runner.os }}
# PR caches are scoped to their ref and only evict main's shared caches.
cache-save-if: ${{ github.ref == 'refs/heads/main' }}

- name: Cache Viceroy binary
id: cache-viceroy
Expand Down Expand Up @@ -82,14 +90,22 @@ jobs:
run: echo "rust-version=$(grep '^rust ' .tool-versions | awk '{print $2}')" >> $GITHUB_OUTPUT
shell: bash

- name: Remove runner-image Rust toolchain
# rust-cache hashes every installed toolchain into its key, so the image's
# preinstalled stable would rotate the key on each runner-image rollout.
shell: bash
run: if command -v rustup >/dev/null; then rustup toolchain uninstall stable; fi

- name: Set up Rust toolchain
uses: actions-rust-lang/setup-rust-toolchain@v1
with:
toolchain: ${{ steps.rust-version.outputs.rust-version }}
# wasm32-wasip1 is required by the "Verify Fastly WASM release
# build" step below; the axum build and tests are native.
target: wasm32-wasip1
cache-shared-key: cargo-${{ runner.os }}
cache-shared-key: cargo-axum-${{ runner.os }}
# PR caches are scoped to their ref and only evict main's shared caches.
cache-save-if: ${{ github.ref == 'refs/heads/main' }}

- name: Read pinned Node.js version for CLI fixtures
id: node-version
Expand Down Expand Up @@ -136,12 +152,20 @@ jobs:
run: echo "rust-version=$(grep '^rust ' .tool-versions | awk '{print $2}')" >> $GITHUB_OUTPUT
shell: bash

- name: Remove runner-image Rust toolchain
# rust-cache hashes every installed toolchain into its key, so the image's
# preinstalled stable would rotate the key on each runner-image rollout.
shell: bash
run: if command -v rustup >/dev/null; then rustup toolchain uninstall stable; fi

- name: Set up Rust toolchain (native + wasm32-unknown-unknown)
uses: actions-rust-lang/setup-rust-toolchain@v1
with:
toolchain: ${{ steps.rust-version.outputs.rust-version }}
target: wasm32-unknown-unknown
cache-shared-key: cargo-${{ runner.os }}
cache-shared-key: cargo-cloudflare-${{ runner.os }}
# PR caches are scoped to their ref and only evict main's shared caches.
cache-save-if: ${{ github.ref == 'refs/heads/main' }}

- name: Check Cloudflare adapter (native host)
run: cargo check -p trusted-server-adapter-cloudflare
Expand All @@ -163,12 +187,20 @@ jobs:
run: echo "rust-version=$(grep '^rust ' .tool-versions | awk '{print $2}')" >> $GITHUB_OUTPUT
shell: bash

- name: Remove runner-image Rust toolchain
# rust-cache hashes every installed toolchain into its key, so the image's
# preinstalled stable would rotate the key on each runner-image rollout.
shell: bash
run: if command -v rustup >/dev/null; then rustup toolchain uninstall stable; fi

- name: Set up Rust toolchain (native + wasm32-wasip1)
uses: actions-rust-lang/setup-rust-toolchain@v1
with:
toolchain: ${{ steps.rust-version.outputs.rust-version }}
target: wasm32-wasip1
cache-shared-key: cargo-${{ runner.os }}
cache-shared-key: cargo-spin-${{ runner.os }}
# PR caches are scoped to their ref and only evict main's shared caches.
cache-save-if: ${{ github.ref == 'refs/heads/main' }}

- name: Check Spin adapter (native host)
run: cargo check -p trusted-server-adapter-spin
Expand Down Expand Up @@ -202,12 +234,20 @@ jobs:
run: echo "rust-version=$(grep '^rust ' .tool-versions | awk '{print $2}')" >> $GITHUB_OUTPUT
shell: bash

- name: Remove runner-image Rust toolchain
# rust-cache hashes every installed toolchain into its key, so the image's
# preinstalled stable would rotate the key on each runner-image rollout.
shell: bash
run: if command -v rustup >/dev/null; then rustup toolchain uninstall stable; fi

- name: Set up Rust toolchain
uses: actions-rust-lang/setup-rust-toolchain@v1
with:
toolchain: ${{ steps.rust-version.outputs.rust-version }}
components: clippy, rustfmt
cache-shared-key: cargo-${{ runner.os }}
cache-shared-key: cargo-parity-${{ runner.os }}
# PR caches are scoped to their ref and only evict main's shared caches.
cache-save-if: ${{ github.ref == 'refs/heads/main' }}

- name: Format check (parity test crate)
run: cargo fmt --manifest-path crates/trusted-server-integration-tests/Cargo.toml -- --check
Expand All @@ -233,12 +273,20 @@ jobs:
run: echo "rust-version=$(grep '^rust ' .tool-versions | awk '{print $2}')" >> $GITHUB_OUTPUT
shell: bash

- name: Remove runner-image Rust toolchain
# rust-cache hashes every installed toolchain into its key, so the image's
# preinstalled stable would rotate the key on each runner-image rollout.
shell: bash
run: if command -v rustup >/dev/null; then rustup toolchain uninstall stable; fi

- name: Set up Rust toolchain
uses: actions-rust-lang/setup-rust-toolchain@v1
with:
toolchain: ${{ steps.rust-version.outputs.rust-version }}
components: "clippy, rustfmt"
cache-shared-key: cargo-cli-${{ runner.os }}
# PR caches are scoped to their ref and only evict main's shared caches.
cache-save-if: ${{ github.ref == 'refs/heads/main' }}

- name: Read pinned Node.js version for CLI fixtures
id: node-version
Expand Down
Loading