Skip to content

refactor(ops): remove legacy monitoring collector environment aliases - #29947

Merged
lancy merged 1 commit into
mainfrom
refactor/retire-monitoring-env-aliases
Aug 27, 2026
Merged

refactor(ops): remove legacy monitoring collector environment aliases#29947
lancy merged 1 commit into
mainfrom
refactor/retire-monitoring-env-aliases

Conversation

@lancy

@lancy lancy commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Summary

  • remove the production readers for VM0_WORKSPACE_IMAGE_CACHE_DIR, VM0_MONITORING_TEXTFILE_DIR, and VM0_RUNNERS_DIR from the two monitoring collectors
  • resolve only the corresponding OKOU_* paths while preserving the exact fixed defaults for unset and empty canonical values
  • remove migration-only alias resolution, conflict handling, source diagnostics, and legacy constants
  • keep retired spellings only in bounded entry-point tests proving legacy-only values cannot redirect and conflicting legacy values cannot override canonical paths or create diagnostics

Closes #29945

Predecessor: #29064 / #29067

Parent EPIC: #28914

Implementation baseline

  • fetched origin/main immediately before the exact-key and open-PR inventory, then branched from clean current main@5c46336786140483bda4208015e478fece4a6fec
  • committed reviewed implementation head efb4fca401393ff0a6e50500a576ac32e423c91c
  • refreshed the target immediately before push at origin/main@aad55f424b8a4b72c2b06e25335e06de53e10c7a; the only intervening commit was refactor(db): canonicalize built-in provider writes #29938, whose 54 changed files have no overlap or functional dependency with this change
  • merge base remains 5c46336786140483bda4208015e478fece4a6fec; no speculative main update was performed solely for ordering

The pre-edit exact-key inventory found production readers only in:

  • ansible/files/vm0-monitoring-collect.sh
  • ansible/files/vm0-runner-status-collect.py

All other occurrences were in their two focused entry-point tests. After this change, neither production collector contains any retired key or migration source/conflict diagnostic; every remaining retired spelling is inside a bounded negative assertion in one of those tests.

Fully paginated open-PR audit

The mandatory pre-edit audit covered all 26 open PRs and 474/474 declared/fetched changed files, with zero pagination mismatch and zero overlap against the four scoped files plus ansible/playbooks/provision-monitoring.yml and .github/actions/provision/action.yml.

The immediately pre-push refresh covered all 25 then-open PRs and 420/420 declared/fetched files, again with zero mismatch and zero overlap. Every changed-file page was fetched with per_page=100; each fetched total was checked against GitHub's changed_files count.

PR Audited head Files fetched/declared Scoped/protected overlap
#25722 2aed1f0de2a54db881ca266151c1362ea6c9dd62 185/185 none
#27177 ea90ade7071d0b712cc3264b781bca14c5a066b1 5/5 none
#28110 431d74acf5364b5c3f27812af9b73e646eaf9157 2/2 none
#28549 e6c5f00572fbd1f6c70a35be1e231a8e4a4fd69d 16/16 none
#28665 1724554da2d5eb3b3a19eab50dd8b6eea7a66e21 4/4 none
#28666 ccb634ac6344b59be2cb19dce31c17f19427c3f5 3/3 none
#28667 1b8ced20fa060e9b5ba0327dc8d50b5c21e6a314 2/2 none
#28668 5097513d3241d6125abc14f6a7391cf8e95dbcb2 2/2 none
#28669 c177613f8004bd66039e90ec82395e871acc2bc7 2/2 none
#28670 2f5dd568c6522bdffee4819b7e708906419fc3c7 2/2 none
#28736 35f50a81405220c726494075214b0db1a2abec66 2/2 none
#29068 d93f0fe028d0f0c96c0a5337a47054eb1f04490b 8/8 none
#29120 3a63d7329cb751dbe32941ef42168941c6b7476a 2/2 none
#29355 036894436fc3157e8c23b8c8efccb8800514f5ea 1/1 none
#29357 3068fb80ba75be6a8d4cf44870ca63c3ae6dcdcf 1/1 none
#29575 9b839d4678c6e2f641c7a90209313fdcf22e5c59 3/3 none
#29722 c3c505ab5efc894e2093dd59730d6c1dc0ec0de5 2/2 none
#29752 7ed31f2a66197f148e723fd989bdb87901202356 16/16 none
#29757 35005696ea376a5b7b79c9f217d0474f98262956 53/53 none
#29778 9aac2f3ad773fd11708235f06c5c3123d281f7fd 2/2 none
#29875 ccd4cabddccd3170afd7aa4ded90c3b71b9162ee 26/26 none
#29901 110d83d7382624946cc89e9fa5ad20fda18a216a 14/14 none
#29939 9ff8a81105390a87626d15cba87c5d863def1aca 34/34 none
#29941 8a13e7d4f971c8318acbbd294030b18a99740e7c 5/5 none
#29943 7143c03c63ffa7101bf32cba614bd2f4bf8a871e 28/28 none
Total 420/420 none

Behavior and scope

Canonical value Retired value Result
unset or empty unset or non-empty exact existing fixed default
non-empty unset, equal, or different canonical value

The diff is limited to the two collectors and their two focused tests. Collector filenames, systemd unit/timer names, vm0_* Prometheus metrics and labels, cadence, permissions, symlink handling, partial-status behavior, temporary-file patterns, atomic replacement, and default filesystem layout are unchanged. Provisioning, systemd, Alloy, SSH, secrets, deployment variables, Runner binaries, host.env, and unrelated environment contracts are unchanged.

Fallbacks

  • no legacy runtime fallback remains in the changed collectors
  • this merge changes repository source only and does not claim production completion
  • if a later provisioned canonical-only collector needs rollback, re-provision the exact recorded dual-reader Ansible tree at b095c5abb22e1d1dd5ab8194ccb0eae9ae5affdc before relying on any legacy-only external override, as recorded in refactor(ops): remove legacy monitoring collector environment aliases #29945

Verification

  • PYTHONPATH=<isolated-ruff-0.16.0> python3 -m ruff format --check ansible/files/vm0-runner-status-collect.py
  • npx --yes shellcheck@4.1.0 .github/scripts/tests/vm0-monitoring-collect-test.sh .github/scripts/tests/vm0-runner-status-collect-test.sh
  • npx --yes shellcheck@4.1.0 --exclude=SC2004 ansible/files/vm0-monitoring-collect.sh (SC2004 remains limited to pre-existing arithmetic-array expressions outside this change)
  • PYTHONPATH=<isolated-ruff-0.16.0> python3 -m ruff check ansible/files/vm0-runner-status-collect.py
  • bash -n ansible/files/vm0-monitoring-collect.sh .github/scripts/tests/vm0-monitoring-collect-test.sh .github/scripts/tests/vm0-runner-status-collect-test.sh
  • PYTHONPYCACHEPREFIX=<isolated-temp-dir> python3 -m py_compile ansible/files/vm0-runner-status-collect.py
  • bash .github/scripts/tests/vm0-monitoring-collect-test.sh
  • bash .github/scripts/tests/vm0-runner-status-collect-test.sh
  • git diff --check

No full local Vitest suite or development server was run. Protected PR CI is authoritative for repository-wide validation.

@github-actions

Copy link
Copy Markdown
Contributor

API preview: https://pr-29947-api.vm6.ai

@codecov

codecov Bot commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@lancy

lancy commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

LGTM

PR_AUTO_REVIEW_COMMIT: efb4fca
PR_AUTO_REVIEW_VERDICT: LGTM

Summary

This focused cleanup removes the three retired monitoring path readers, all legacy/dual/conflict resolution, and migration-only diagnostics while retaining the existing fixed defaults for unset or empty canonical values. The two real collector entry-point suites now use OKOU_* for normal behavior and keep only bounded checks that a retired environment value cannot redirect filesystem reads/writes, override a canonical value, or surface in diagnostics.

Review Criteria

Source Core item Result Evidence
REVIEW.md Correctness and security of changed conditionals, defaults, and filesystem routing Pass ansible/files/vm0-monitoring-collect.sh uses quoted ${OKOU_*:-fixed-default} expansion; ansible/files/vm0-runner-status-collect.py uses os.environ.get(...) or fixed-default. Unset and empty canonical values preserve the prior defaults, while retired values are never read.
REVIEW.md Interface changes remain documented and focused Pass The PR documents the environment-contract contraction, exact implementation base/head, all-host removal evidence, rollback source, and full open-PR audit. Only the two collectors and two focused tests change.
REVIEW.md A refactor retains coverage for existing behavior and changed boundaries Pass Both production entry points are exercised against real temporary filesystem state; existing metric, label, symlink, partial-status, permission, atomic-replacement, and cadence assertions remain.
docs/bad-smell.md Avoid fallback slop, defensive branches, suppressions, and hidden configuration behavior Pass The diff deletes the resolver branches and source/conflict diagnostics, adds no suppression or recovery branch, and keeps only the pre-existing fixed defaults that are the official collector configuration.
docs/testing.md Prefer integration entry points, real infrastructure, and behavioral assertions Pass Both shell suites invoke the actual Bash/Python collectors and use real files/directories; there are no internal mocks, fake timers, or implementation-call assertions.
docs/fallback.md Remove a retired fallback only after its rollout gate is evidenced, deleting the reader and its compatibility tests together Pass #29945 records the three-host dual-reader floor, zero official/drop-in overrides, and exact rollback tree b095c5abb22e1d1dd5ab8194ccb0eae9ae5affdc. The legacy branches, constants, source/conflict diagnostics, and alias-matrix compatibility cases are deleted together.
docs/fallback.md Do not retain generic tombstone tests; keep only a fail-closed boundary where the negative outcome is product behavior Pass The retained retired-key cases are bounded to the collector process boundary and prove an injected legacy path cannot redirect privileged filesystem reads/writes, override canonical output, or create diagnostics. They do not preserve a legacy route/parser or acceptance path.
docs/deployment-compatibility.md Contract compatibility only after the independently deployed old surface and rollback window are safely bounded Pass The linked removal gate proves every active host is on the dual-reader floor and records byte-exact rollback. This PR changes repository source only and does not provision or claim production completion.
docs/runner-host-configuration.md Keep runner host configuration and restart/rollback contracts separate from collector cleanup Pass No Runner binary, /etc/vm0-runner/host.env, runner service, tuning key, drain/restart workflow, or deployed host configuration changes; the modified environment contract belongs only to the two collector processes.

Findings

Critical (P0)

  • None.

High Priority (P1)

  • None.

Testing

  • Coverage: Adequate. Canonical-only, unset, empty, fixed-default, retired-only, and canonical-with-retired-conflict behavior is exercised through both collector entry points, alongside the preserved collector behavior suites.
  • Conventions: Compliant. Tests use real temporary filesystem state and behavior assertions with no internal mocking or timing shortcuts.
  • Static checks: Ruff 0.16.0 format and lint, ShellCheck 0.11.0 through shellcheck@4.1.0, Bash syntax, Python compilation, both focused collector suites, and git diff --check passed locally.
  • Protected repository CI is running and remains authoritative; no full local Vitest suite or development server was run.

@lancy
lancy added this pull request to the merge queue Aug 27, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 27, 2026
@lancy
lancy added this pull request to the merge queue Aug 27, 2026
Merged via the queue into main with commit 89246eb Aug 27, 2026
98 checks passed
@github-project-automation github-project-automation Bot moved this from Backlog to Done in VM0 Kanban Aug 27, 2026
@github-actions
github-actions Bot deleted the refactor/retire-monitoring-env-aliases branch August 28, 2026 05:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

refactor(ops): remove legacy monitoring collector environment aliases

1 participant