Repository navigation
CI Validation Infra: fixes for WSLC test prereqs, Windows log collection, LXC bridge NAT, and rate limits - #939
Conversation
…m getting removed
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Improves validation-matrix reliability, networking setup, artifact collection, and test reproducibility.
Changes:
- Adds configurable backend startup staggering.
- Fixes Windows log collection and WSLC fixtures.
- Configures LXC bridge NAT and netfilter prerequisites.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/Validation.Tests.Matrix.Job.yml |
Adds delays, longer timeouts, and unconditional log uploads. |
.github/copilot-instructions.md |
Documents new CI conventions. |
docs/ci-validation-infrastructure.md |
Updates CI infrastructure guidance. |
scripts/ci/prepare-linux-host.sh |
Configures LXC bridge NAT and netfilter. |
scripts/ci/resolve-validation-test-matrix.mjs |
Validates and assigns startup delays. |
scripts/ci/validation-test-matrix.json |
Configures WSLC staggering. |
tests/scripts/run_ci_backend_tests.ps1 |
Redirects Windows temporary output for collection. |
tests/scripts/run_ci_backend_tests.sh |
Clarifies mandatory LXC execution. |
tests/scripts/run_wslc_all_tests.ps1 |
Creates the filesystem test fixture. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (2)
scripts/ci/prepare-linux-host.sh:151
- This idempotence check uses the bridge address (for example,
10.0.3.1/24) butiptables -Scanonicalizes it to the network (10.0.3.0/24), so the grep misses an existing equivalent rule and appends another MASQUERADE rule on every preparation. Thegrep -qpipeline can also report false underpipefailifiptablesreceives SIGPIPE. Resolve the actual connected subnet and useiptables -Cto test the exact rule instead.
# Match on the source subnet rather than the rule text: lxc-net's own rule
# and ours are equivalent however they are spelled.
if sudo iptables -t nat -S POSTROUTING 2>/dev/null |
grep -q -- "-s ${subnet%%/*}"; then
docs/ci-validation-infrastructure.md:182
- The workflow sleeps only after checkout and
actions/download-artifactin every job, so this is not before the first network step and those artifact downloads remain unstaggered. Either move the delay ahead of those steps or describe the narrower behavior accurately.
The resolver puts the offset on every matrix entry as
`startup_delay_seconds` — `0` where no stagger applies — and the job sleeps
that long before its first network step. Job timeout is a flat 180 minutes,
with plenty of room for any wait you'd reasonably configure.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (2)
scripts/ci/prepare-linux-host.sh:151
- This existence check is not idempotent.
ip addrreturns the bridge address (for example10.0.3.1/24), whileiptables -Sprints the canonical network (10.0.3.0/24); stripping the prefix searches for-s 10.0.3.1, so an existing lxc-net rule is missed and another MASQUERADE rule is appended on every preparation. Query the exact rule throughiptables -C, which performs the same CIDR normalization as insertion.
if sudo iptables -t nat -S POSTROUTING 2>/dev/null |
grep -q -- "-s ${subnet%%/*}"; then
.github/workflows/Validation.Tests.Matrix.Job.yml:60
- These offsets are measured from each runner's independently varying allocation time, not from a common epoch. If the zero-delay runner becomes ready late while a higher-offset runner is allocated promptly, prerequisite downloads can still overlap or even start in reverse order, so the advertised 0/45/90/135-second staggering is not guaranteed and may not prevent the rate-limit burst. Coordinate affected jobs against shared absolute start deadlines or serialize them; otherwise this needs to be documented as best-effort rather than fixed spacing.
$seconds = ${{ matrix.startup_delay_seconds }}
Write-Host "Waiting $seconds second(s) before downloading prerequisites."
Start-Sleep -Seconds $seconds
| - name: Delayed start | ||
| if: matrix.startup_delay_seconds > 0 | ||
| shell: pwsh | ||
| run: | | ||
| # Spreads network-heavy setup across the pool's shared egress address | ||
| # instead of issuing every job's downloads at once. Configured by | ||
| # backendDelayedStart in scripts/ci/validation-test-matrix.json. | ||
| $seconds = ${{ matrix.startup_delay_seconds }} | ||
| Write-Host "Waiting $seconds second(s) before downloading prerequisites." | ||
| Start-Sleep -Seconds $seconds |
There was a problem hiding this comment.
thought: I guess you're trying to find the sweet spot here with timing? Question though, I forgot to ask. Do we get emails about failures anywhere?
There was a problem hiding this comment.
No, not currently. Great idea though. I'll add it to the list :)
| path: | | ||
| ${{ runner.temp }}/mxc-ci.log | ||
| ${{ runner.temp }}/mxc-wpc-tests/logs | ||
| ${{ runner.temp }}/WinProcessContainer-Tests.results.* | ||
| ${{ runner.temp }}/mxc_concurrent_oneshot | ||
| ${{ runner.temp }} | ||
| !${{ runner.temp }}/**/*.vhd | ||
| !${{ runner.temp }}/**/*.vhdx | ||
| !${{ runner.temp }}/**/*.tar | ||
| !${{ runner.temp }}/**/*.msi | ||
| !${{ runner.temp }}/**/*.msix | ||
| !${{ runner.temp }}/**/*.appx | ||
| !${{ runner.temp }}/**/*.zip | ||
| !${{ runner.temp }}/**/*.exe | ||
| !${{ runner.temp }}/**/*.dll |
There was a problem hiding this comment.
question: Why use the whole temp here again? perhaps I'm misunderstanding the comment above but I'm still not sure why we wouldn't want to specify a specific folder inside type or file.
There was a problem hiding this comment.
Making it a catch-all for now since T1 tests create a ton of different logs in a couple folders. I'm willing to narrow the scope of it in the future.
| # Skip if the config references a tar file that doesn't exist locally. | ||
| # Read the chain defensively: this suite inherits Set-StrictMode -Version | ||
| # Latest from run_ci_backend_tests.ps1, under which touching a missing | ||
| # property is a terminating error, and most configs have no wslc.imageTarPath. | ||
| # Skip if the config references a tar file that doesn't exist locally | ||
| $configJson = Get-Content $configPath -Raw | ConvertFrom-Json | ||
| $tarPath = Get-JsonProperty (Get-JsonProperty (Get-JsonProperty $configJson 'experimental') 'wslc') 'imageTarPath' |
There was a problem hiding this comment.
issue: Soham Das (@SohamDas2021) are we good with this removal?
There was a problem hiding this comment.
I added it during my prior PR. Strict mode was unnecessary and I've stripped it back out.
📖 Description
A batch of validation-matrix (E2E) CI fixes. Four independent threads, all
confined to the matrix workflow, its resolver, and the CI-facing test entry
points.
1. LXC host preparation — container networking.
lxc-netskips its firewall setup when it believes another manager owns theruleset, leaving a bridge that hands out DHCP leases the container cannot
actually use.
prepare-linux-host.shnow enables bridge netfilter(a hard prerequisite — MXC refuses to report success
for a policy it cannot enforce without it) and ensures the bridge's MASQUERADE
rule exists.
2. Log collection — artifacts were arriving nearly empty.
$env:TEMPis not$RUNNER_TEMP. The Windows suites write scratch trees,transcripts, and results files to the former;
upload-artifactreads thelatter, so almost nothing was collected.
run_ci_backend_tests.ps1now pointsTEMP/TMPat$RUNNER_TEMPbeforedispatching.
This is a step towards the goal of having an internal dashboard where scheduled tests and logs are easily navigable.
3.
backendDelayedStart— staggering network-heavy job starts.Every runner in a 1ES pool shares one egress address, so a backend that pulls a
large runtime or several container images concentrates that traffic into a
burst when its jobs start together, drawing rate limiting and stalled
downloads. The matrix config .json file now accepts:
The resolver validates it and emits
startup_delay_secondson every matrixentry; each job sleeps that long before its first network step. In this example,
the first WSLC job starts immediately, the second WSLC job starts in 45 seconds, the third
in 90 seconds, and so on.
4. WSLC read-only mount test — missing fixture.
Removed Strict mode from the main dispatch script. Also added test.txt to the environment since a few tests required it, but I removed it during the merge conflict since it became obvious it was very recently fixed.
Limitations
the pool, so keep stagger times short until a better long-term solution is reached.
process-container ACL/UI failures, which are product-side.
🔗 References
Related product-side work for remaining LXC failures: #896.
Documentation:
docs/ci-validation-infrastructure.mdis updated for all of theabove (new "Log collection" section,
backendDelayedStartreference, andmatching runbook entries).
🔍 Validation
node scripts/ci/resolve-validation-test-matrix.mjs --plan <pr|nightly|weekly|enabled>— all four exit 0.startup_delay_seconds: 0.$TEMPby a child process land in$RUNNER_TEMP.✅ Checklist
Cargo.lock, thedependency-feed-checkcheck passes (see docs/pull-requests.md)📋 Issue Type
Microsoft Reviewers: Open in CodeFlow