Repository navigation
sre-lab: harden guided scenario operations - #48
Merged
Merged
Conversation
Close iterative review findings across alert polling, recovery state, Storage RBAC propagation, capture retries, cleanup evidence, and operator guidance. Add regression coverage for transient Azure failures and incomplete manual setup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Hardens the SRE Agent event lab’s guided scenario scripts and operator workflows, focusing on making scenario execution/capture/teardown more resilient to transient Azure failures and more explicit/safe on abnormal exits.
Changes:
- Improves
run-scenario.shresilience: distinguishes “alert never fired” from “could not query alerts”, captures error evidence, adds S3 RBAC data-plane propagation gating, and ensures aborted runs are recorded as failed. - Prevents “successful capture” (
conclusion) from being downgraded by later capture attempts; adds CLI support to query capture status. - Strengthens operator guidance and safety checks around Agent setup evidence, RBAC cleanup evidence, and manual recovery commands; expands test coverage accordingly.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| monitor/sre-agent-event-lab/scripts/tests/test_lab_state.py | Adds regression coverage for refusing to downgrade an already-successful capture conclusion. |
| monitor/sre-agent-event-lab/scripts/tests/test_lab_scripts.py | Adds extensive scenario-script behavior tests for transient alert failures, abort recording, invalid JSON handling, and S3 propagation gating. |
| monitor/sre-agent-event-lab/scripts/tests/test_lab_guides.py | Asserts new/updated guide content for alert budgets, role assignment setup, and S3 propagation + manual recovery. |
| monitor/sre-agent-event-lab/scripts/tests/test_doctor.py | Adds coverage for missing/invalid assignment evidence and case-insensitive ARM IDs. |
| monitor/sre-agent-event-lab/scripts/tests/test_common.py | Verifies default alert resolution budget updates are reflected in scripts. |
| monitor/sre-agent-event-lab/scripts/tests/test_cleanup_external.py | Adds teardown safety test for empty assignment IDs recorded with principals; strengthens guide references in errors/docs. |
| monitor/sre-agent-event-lab/scripts/tests/lab_script_harness.py | Extends the harness to simulate more failure modes (jq failures, loadgen failures, transient AZ errors, etc.). |
| monitor/sre-agent-event-lab/scripts/run-scenario.sh | Implements resilient alert polling, explicit failed recording on abnormal exits, and S3 RBAC propagation gating. |
| monitor/sre-agent-event-lab/scripts/query-evidence.sh | Gates evidence output directory creation on required deployment outputs being present. |
| monitor/sre-agent-event-lab/scripts/lab_state.py | Prevents downgrading a successful capture, and adds capture-status CLI command. |
| monitor/sre-agent-event-lab/scripts/doctor.sh | Improves actionable error messaging and validates recorded Monitoring Contributor assignment evidence. |
| monitor/sre-agent-event-lab/scripts/common.sh | Centralizes a lowercase() helper and updates default alert resolution budget. |
| monitor/sre-agent-event-lab/scripts/cleanup-external.sh | Uses shared lowercase() and improves safety/diagnostics for evidence validation and cleanup. |
| monitor/sre-agent-event-lab/scripts/capture-scenario.sh | Adds refusal to re-capture once a conclusion exists, improves evidence validation and retry guidance. |
| monitor/sre-agent-event-lab/README.md | Documents the “cancelled azd down” recovery step requiring evidence refresh with new assignment IDs. |
| monitor/sre-agent-event-lab/guides/05-results.md | Documents that a concluded capture cannot be collected again within the same run attempt. |
| monitor/sre-agent-event-lab/guides/04-scenario-s3.md | Documents the RBAC propagation wait, failure mode, and a copyable manual restore command. |
| monitor/sre-agent-event-lab/guides/02-scenario-s1.md | Updates alert resolution budget documentation (25 minutes) and explains stateful log alert behavior. |
| monitor/sre-agent-event-lab/guides/01-agent-setup.md | Adds explicit role creation commands, captures assignment IDs, and validates agent-setup.json completeness. |
Suppressed comments (1)
monitor/sre-agent-event-lab/scripts/run-scenario.sh:222
- The EXIT trap currently constructs a generic
run aborted with status ...reason wheneverOUTCOME_RECORDEDis still 0. If a specific failure reason was already known (but the earliermark-failedwrite failed), this loses the more actionable reason and can later record the generic one instead. Use the previously captured reason when available.
if [[ "${OUTCOME_RECORDED}" -eq 0 ]]; then
failure_reason="run aborted with status ${original_status}"
if [[ "${recovery_status}" -ne 0 ]]; then
failure_reason="${failure_reason}; automatic recovery failed"
fi
if ! record_failed_run "${failure_reason}"; then
echo "CRITICAL: could not record the aborted ${SCENARIO} run as failed." >&2
fi
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Keep the intended scenario failure reason pending until mark-failed succeeds, so an EXIT-trap retry cannot replace actionable alert or propagation diagnostics with a generic abort status. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.
Suppressed comments (1)
monitor/sre-agent-event-lab/scripts/tests/lab_script_harness.py:367
- In the
python3stub,mark_failed_failures(the decrementing counter) is not honored, while the.venv/bin/pythonstub supports it. Becausemake_lab()supportsvenv_present=False, tests that rely onmark_failed_failureswill behave differently (or silently stop simulating retries) when the virtualenv is absent.
*lab_state.py)
if [[ "$*" == *" mark-failed "* && -f "${{state}}/mark_failed_fails" ]]; then
exit 3
fi
exec "{REAL_PYTHON}" "$@"
;;
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Review cycles
Verification
python3 -m pytest scripts infra -q— 587 passedapp/.venv/bin/python -m pytest app -q— 10 passedinfra/*.biceptemplates built successfullyNotes
No Azure resources were created for this review-only hardening pass.