Skip to content

Commit ad9e5dd

Browse files
authored
test(integration): catch on-disk storage format breaks pre-merge (#4003)
1 parent 38d58ca commit ad9e5dd

10 files changed

Lines changed: 1886 additions & 10 deletions

File tree

.config/nextest.toml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,12 @@
1515
# specific language governing permissions and limitations
1616
# under the License.
1717

18+
# Floor for the flags CI and scripts/ci/storage-compat.sh pass: --no-tests
19+
# (0.9.75), --run-ignored only (0.9.76) and --ignore-default-filter (0.9.77).
20+
# nextest refuses to run below it, and setup-rust-with-cache replaces a
21+
# cache-restored binary that fails `cargo nextest show-config version`.
22+
nextest-version = { required = "0.9.77" }
23+
1824
[[profile.default.overrides]]
1925
# This is a solution (or actually a workaround) for the problem that nextest does not support
2026
# #[serial] macro which shall enforce sequential execution of the test case.

.github/actions/rust/pre-merge/action.yml

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ description: Rust pre-merge testing and linting github iggy actions
2020

2121
inputs:
2222
task:
23-
description: "Task to run (check, check-msrv, fmt, clippy, sort, machete, doctest, verify-publish, test-1, test-2, test-3, miri)"
23+
description: "Task to run (check, check-msrv, fmt, clippy, sort, machete, doctest, verify-publish, test-1, test-2, test-3, test-storage-compat, miri)"
2424
required: true
2525
component:
2626
description: "Component name (for context)"
@@ -84,19 +84,22 @@ runs:
8484
# a subset of crates changed.
8585
# Safety: cargo check/clippy run on the full workspace separately, catching all
8686
# compilation errors. This only scopes test BUILD and EXECUTION.
87+
# test-storage-compat is excluded from the DAG and coverage setup below:
88+
# storage-compat.sh builds two explicit binaries and drives its own nextest
89+
# run, and reads none of the /tmp plan files or the llvm-cov environment.
8790
- name: Fetch base branch for DAG analysis
88-
if: startsWith(inputs.task, 'test-')
91+
if: startsWith(inputs.task, 'test-') && inputs.task != 'test-storage-compat'
8992
run: git fetch origin master --depth=1 2>/dev/null || true
9093
shell: bash
9194

9295
- name: Install cargo-rail
93-
if: startsWith(inputs.task, 'test-')
96+
if: startsWith(inputs.task, 'test-') && inputs.task != 'test-storage-compat'
9497
uses: taiki-e/install-action@v2
9598
with:
9699
tool: cargo-rail
97100

98101
- name: Compute affected crates (cargo-rail)
99-
if: startsWith(inputs.task, 'test-')
102+
if: startsWith(inputs.task, 'test-') && inputs.task != 'test-storage-compat'
100103
run: |
101104
METADATA_JSON=$(cargo metadata --format-version 1 --no-deps 2>/dev/null || echo "{}")
102105
TOTAL_CRATES=$(echo "$METADATA_JSON" | jq '.workspace_members | length' 2>/dev/null || echo "?")
@@ -202,18 +205,25 @@ runs:
202205
shell: bash
203206

204207
- name: Install cargo-llvm-cov
205-
if: startsWith(inputs.task, 'test-')
208+
if: startsWith(inputs.task, 'test-') && inputs.task != 'test-storage-compat'
206209
uses: taiki-e/install-action@v2
207210
with:
208211
tool: cargo-llvm-cov
209212

210213
- name: Build and test with coverage
211-
if: startsWith(inputs.task, 'test-')
214+
# test-storage-compat drives its own nextest invocation from
215+
# scripts/ci/storage-compat.sh. Without this exclusion it would also run
216+
# here with an empty PARTITION_FLAG, i.e. a fourth *unpartitioned* copy
217+
# of the whole suite on top of test-1/2/3.
218+
if: startsWith(inputs.task, 'test-') && inputs.task != 'test-storage-compat'
212219
run: |
213220
# Parse partition index from task name (test-1 -> hash:1/3, test-2 -> hash:2/3, ...).
214221
# TEST_PARTITIONS must match the number of test-N tasks in
215222
# .github/config/components.yml. Cluster bootstrap makes each test
216223
# CPU-heavy, so partitions stay small.
224+
# The regex is numeric-only on purpose: non-numeric test-* tasks
225+
# (test-storage-compat) opt out of partitioning rather than producing
226+
# an out-of-range hash:N/3.
217227
TEST_PARTITIONS=3
218228
TASK="${{ inputs.task }}"
219229
PARTITION_FLAG=""
@@ -383,6 +393,17 @@ runs:
383393
ls -la codecov.json
384394
shell: bash
385395

396+
# On-disk format backwards compatibility. The script builds an iggy-server
397+
# from the master tip under the checked-out merge ref (origin/master off a
398+
# pull_request run) and one from HEAD, seeds a data directory with the
399+
# former and reads it back with the latter. It does its own shallow fetch
400+
# of the baseline commit and exits non-zero if it cannot resolve one, so
401+
# the default depth-1 checkout is sufficient here.
402+
- name: Storage format backwards compatibility
403+
if: inputs.task == 'test-storage-compat'
404+
run: ./scripts/ci/storage-compat.sh
405+
shell: bash
406+
386407
# Miri (UB detector) on the unsafe-heavy crates that don't pull tokio /
387408
# compio. Pinned nightly so MIRIFLAGS behavior is stable across CI runs;
388409
# bump the date quarterly. Tree-borrows is the future-default aliasing

.github/actions/utils/setup-rust-with-cache/action.yml

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,9 +125,12 @@ runs:
125125
- name: Install cargo-nextest
126126
if: runner.os == 'Linux' && inputs.install-nextest == 'true'
127127
run: |
128-
if command -v cargo-nextest &> /dev/null; then
128+
# A binary restored from the cargo cache can predate the floor in
129+
# .config/nextest.toml. `show-config version` exits non-zero below it,
130+
# and on a nextest too old to know the subcommand at all, so both fall
131+
# through to a fresh install instead of aborting the lane later.
132+
if command -v cargo-nextest &> /dev/null && cargo nextest show-config version; then
129133
echo "cargo-nextest already installed"
130-
cargo nextest --version
131134
exit 0
132135
fi
133136

.github/config/components.yml

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,43 @@ components:
135135
- "core/message_bus/**"
136136
- "core/partitions/**"
137137

138+
# On-disk format backwards compatibility. Boots a server built from the
139+
# master tip the PR merges onto, seeds a data directory, then swaps in a
140+
# HEAD-built binary and asserts it reads everything back. Paths are the
141+
# crates that own the on-disk representation plus the boot path that reads
142+
# it back; a change to any of them can silently break rollforward for
143+
# existing deployments, which no other check covers. Matching is literal
144+
# glob with no crate-graph expansion, so every crate has to be spelled out:
145+
# `core/server/**` does not cover `core/server_common/**`.
146+
# `breaking:storage` on the PR skips it (see .github/workflows/_test.yml).
147+
rust-storage-compat:
148+
depends_on:
149+
- "rust-workspace"
150+
- "rust-configs" # partition/metadata on-disk schema knobs and their defaults
151+
- "ci-infrastructure" # action.yml/workflow edits should re-run the check
152+
paths:
153+
- "core/server/**"
154+
- "core/server_common/**" # per-message segment record + segment readers/writers
155+
- "core/binary_protocol/**"
156+
- "core/common/**"
157+
- "core/consensus/**" # VsrState is the superblock payload
158+
- "core/journal/**"
159+
- "core/metadata/**"
160+
- "core/partitions/**"
161+
- "core/shard/**" # drives the shutdown flush and boot-time partition recovery
162+
- "scripts/ci/storage-compat.sh"
163+
# The check itself, so weakening it re-runs it. Scoped to the test, the
164+
# module wiring that keeps it compiled in, the crate manifest and the
165+
# harness it drives rather than `core/integration/**`, which would fire
166+
# this lane on every unrelated integration test change.
167+
- "core/integration/Cargo.toml"
168+
- "core/integration/tests/mod.rs"
169+
- "core/integration/tests/data_integrity/mod.rs"
170+
- "core/integration/tests/data_integrity/storage_compat.rs"
171+
- "core/integration/src/harness/**"
172+
tasks:
173+
- "test-storage-compat"
174+
138175
# Standalone simulation tool, does NOT affect server binary or foreign SDKs.
139176
# Split from rust-cluster to avoid triggering SDK tests on simulator-only changes.
140177
rust-simulator:

.github/workflows/_test.yml

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ on:
3333

3434
permissions:
3535
contents: read
36+
issues: read # listLabelsOnIssue, for the breaking:storage escape hatch
3637
security-events: write
3738
pull-requests: write
3839

@@ -55,15 +56,62 @@ jobs:
5556
run: echo "No changes detected, skipping tests"
5657

5758
# Rust
59+
# `breaking:storage` is the documented escape hatch for a deliberate
60+
# on-disk format change, which the compat check cannot pass by
61+
# construction. The label has to be read live: pre-merge.yml does not
62+
# subscribe to `labeled` (pr-triage-apply.yml writes S-* labels, so every
63+
# triage write would re-run the whole gate), and a re-run replays the
64+
# webhook-frozen `pull_request.labels` from the last push. The operator
65+
# flow is "check fails, apply label, re-run", which the frozen payload
66+
# cannot see. Paginated so a PR with more than 100 labels still resolves.
67+
- name: Resolve breaking:storage escape hatch
68+
id: storage_hatch
69+
if: >-
70+
startsWith(inputs.component, 'rust') &&
71+
inputs.task == 'test-storage-compat' &&
72+
github.event.pull_request.number
73+
uses: actions/github-script@v9
74+
with:
75+
script: |
76+
let skip = false;
77+
try {
78+
const labels = await github.paginate(
79+
github.rest.issues.listLabelsOnIssue,
80+
{
81+
owner: context.repo.owner,
82+
repo: context.repo.repo,
83+
issue_number: context.payload.pull_request.number,
84+
per_page: 100,
85+
},
86+
);
87+
skip = labels.some(l => l.name === 'breaking:storage');
88+
} catch (e) {
89+
// Fail toward running. A rate limit or a narrowed token must not
90+
// wave an on-disk format break through as a green check.
91+
core.warning(`listLabelsOnIssue failed (${e.status ?? e.code ?? 'network'}), running the check`);
92+
}
93+
core.info(`breaking:storage escape hatch: skip=${skip}`);
94+
core.setOutput('skip', String(skip));
95+
5896
- name: Run Rust task
59-
if: startsWith(inputs.component, 'rust')
97+
# Skipping the step (not the job) keeps the leg green so the pre-merge
98+
# status roll-up stays reportable instead of pending.
99+
if: >-
100+
startsWith(inputs.component, 'rust') &&
101+
steps.storage_hatch.outputs.skip != 'true'
60102
uses: ./.github/actions/rust/pre-merge
61103
with:
62104
task: ${{ inputs.task }}
63105
component: ${{ inputs.component }}
64106

65107
- name: Upload coverage to Codecov
66-
if: startsWith(inputs.component, 'rust') && startsWith(inputs.task, 'test-')
108+
# test-storage-compat runs its own nextest invocation without llvm-cov,
109+
# so it emits no codecov.json. Uploading an empty report under the
110+
# `rust` flag would read as a coverage drop on the PR.
111+
if: >-
112+
startsWith(inputs.component, 'rust') &&
113+
startsWith(inputs.task, 'test-') &&
114+
inputs.task != 'test-storage-compat'
67115
uses: codecov/codecov-action@v7.0.0
68116
with:
69117
token: ${{ secrets.CODECOV_TOKEN }}

.github/workflows/pre-merge.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ concurrency:
3131

3232
permissions:
3333
contents: read
34+
# listLabelsOnIssue in _test.yml, for the breaking:storage escape hatch
35+
issues: read
3436
security-events: write
3537
pull-requests: write
3638

core/integration/src/harness/handle/server.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -726,6 +726,17 @@ impl ServerHandle {
726726
self.test_transport = Some(transport);
727727
}
728728

729+
/// Point the next `start()` at a different server binary.
730+
///
731+
/// `start()` re-reads `config.executable_path` on every call, so a test
732+
/// can boot one build, then restart the SAME data directory under another
733+
/// one. `None` restores the cargo-built binary of the crate under test,
734+
/// which is why this takes an explicit `Option` rather than
735+
/// `impl Into<String>`.
736+
pub fn set_executable_path(&mut self, path: Option<String>) {
737+
self.config.executable_path = path;
738+
}
739+
729740
/// Configure MCP server for this iggy server.
730741
pub fn set_mcp_config(&mut self, config: McpConfig) {
731742
self.mcp = Some(McpHandle::with_server_id(

core/integration/tests/data_integrity/mod.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,8 @@ mod verify_cluster_replica_data_identical;
3939
// Auto-commit offset replication is inherently a multi-node (VSR) property: the
4040
// backup only holds the offset if the poll's auto-commit rode consensus.
4141
mod verify_auto_commit_offset_replicates;
42+
43+
// On-disk format compatibility across a binary swap. `#[ignore]`d: it needs a
44+
// baseline `iggy-server` built from the merge base, which only
45+
// `scripts/ci/storage-compat.sh` provides.
46+
mod storage_compat;

0 commit comments

Comments
 (0)