Remove the orphaned v5/v6/v7 search indexes

[jonathangreen]

Description

Stacked on top of #3523. Cleans up the orphaned circulation-works-v5, -v6 and -v7 OpenSearch indexes that earlier schema revisions left behind in the cluster.

When the search schema migrates to a new revision, the reindex flow creates the new index and re-points the read/write aliases at it, but it never deletes the index it replaced — so every retired revision leaves an orphaned {base}-v{n} index sitting in the cluster, consuming shards and disk. Nothing in the codebase has ever removed them. #3523 removes the v5–v7 revision code; this PR removes the leftover indexes.

This adds:

• SearchService.index_remove(name) — deletes an index if it exists and reports whether it did. The service had no delete-index capability before.
• remove_search_indices(service, versions, *, log) helper — deletes the named old versions, with a safety guard that never deletes an index a read or write alias still points at, so it cannot drop the live index even if asked to. It's idempotent (delete-if-exists) and returns the names it removed.
• A one-shot startup task (startup_tasks/2026_06_30_remove_old_search_indexes.py) that calls the helper inline for versions [5, 6, 7] on the next boot.

Removing a handful of indexes is a fast metadata operation, so this runs inline in the startup task rather than via a background job. If OpenSearch is briefly unavailable the helper raises, the startup runner declines to record the task as done, and it simply retries on the next boot.

Important

Do not merge until the release containing #3523 has shipped and production is fully on v8. Like #3523 this is "release 2" work: the deletion is only safe once no production index is on v5/v6/v7. The helper is additionally defensive — it skips any index an alias still points at — but the merge gate is the real guarantee. Kept in draft until then.

Motivation and Context

Removing the v5–v7 revision modules (#3523) stops the code from referencing those schemas, but the actual indexes remain in the cluster forever because the migration flow only swaps aliases and never drops the old index. This reclaims that cluster state (shards, disk) as the final step of retiring those revisions.

How Has This Been Tested?

New tests, all passing under the docker tox environment:

• tests/manager/search/test_service.py::TestService::test_index_remove — index_remove deletes a real index and is a no-op (returns False) when it's absent.
• tests/manager/search/test_service.py::TestRemoveSearchIndices — three cases against the real OpenSearch fixture: the happy path (old indexes removed, the live index preserved), the alias guard (an index a read alias points at is skipped), and a no-op when the indexes don't exist.

Also ran the startup-task and initialization suites to confirm the new task file is discovered and the boot wiring is unaffected.

Checklist

• I have updated the documentation accordingly.
• All new and existing tests passed.

[claude]

Claude finished @jonathangreen's task in 2m 58s —— View job

---

Code Review

• Read the changed files and PR diff
• Review service.py changes (index_remove, remove_search_indices)
• Review startup task
• Review tests and mock changes
• Post review findings

Summary

This is a clean, well-scoped, and well-tested change. The index_remove/remove_search_indices split is sensible, the alias-safety guard is correct, and the helper is idempotent so a partial failure is safe to retry. I traced the runtime concern from the earlier review rounds and it is now fully resolved: remove_search_indices is a plain synchronous helper (no try/except OpenSearchException of its own), so the whole call — including the read_pointer()/write_pointer() guard lookups — runs inside the startup runner's try block in scripts/startup.py:212-221. Any OpenSearchException (connection/transport error included) propagates up, the task is not recorded as RUN, and it retries on the next boot — exactly as the PR description claims. The Greptile P1 "pointer lookups outside the retry scope" comment refers to a prior Celery-task implementation and no longer applies. The P2 TOCTOU window is also handled via ignore=[404] on indices.delete (service.py:369).

I verified the index-naming convention matches end to end (remove_search_indices builds f"{base}-v{version}", which is exactly what SearchSchemaRevision.name_for_index produces and what SearchPointer._parse_version parses), confirmed both concrete SearchService subclasses implement the new abstract index_remove, and confirmed the three real-OpenSearch test cases exercise the happy path, the alias guard, and the no-op path. Not adding a dedicated test for the new run() function is consistent with the repo's convention — the other startup_tasks/*.py files have no companion tests either, and the underlying remove_search_indices logic is covered directly.

No actionable issues found. The merge gate in the PR description (hold until the release containing #3523 is fully on v8 in production) remains the right call.
· chore/cleanup-old-search-indexes

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.44%. Comparing base (1558880) to head (dedf921).

Additional details and impacted files

@@                            Coverage Diff                            @@
##           chore/remove-old-search-schema-revisions    #3524   +/-   ##
=========================================================================
  Coverage                                     93.44%   93.44%           
=========================================================================
  Files                                           509      509           
  Lines                                         46464    46486   +22     
  Branches                                       6343     6347    +4     
=========================================================================
+ Hits                                          43418    43441   +23     
+ Misses                                         1969     1968    -1     
  Partials                                       1077     1077           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[greptile-apps]

Greptile Summary

This PR adds the final cleanup step for retiring the v5–v7 OpenSearch schema revisions: it deletes the orphaned circulation-works-v5/v6/v7 indexes that old reindex migrations left behind when swapping aliases without dropping the replaced index.

• SearchService.index_remove(name) — new abstract method + SearchServiceOpensearch1 implementation that deletes an index if it exists, with ignore=[404] on the delete call to handle the TOCTOU window gracefully.
• remove_search_indices(service, versions, *, log) — standalone helper that builds a protected set from the live read/write aliases before touching anything, ensuring the active index can never be deleted; idempotent (delete-if-exists) and returns the names of what was removed.
• startup_tasks/2026_06_30_remove_old_search_indexes.py — one-shot boot task that invokes the helper inline for versions [5, 6, 7]; any OpenSearchException propagates to the startup runner so the task retries on the next boot rather than being silently marked done.

Confidence Score: 5/5

Safe to merge once production is fully on v8; the alias-protection guard provides an in-code safeguard against accidentally dropping the live index.

The alias-guard is correct and tested across three cases. The TOCTOU window in index_remove is already handled with ignore=[404]. Error propagation from pointer lookups is intentional and matches the retry-on-next-boot contract.

No files require special attention; the merge gate (production fully on v8) is the only dependency.

Important Files Changed

Filename	Overview
src/palace/manager/search/service.py	Adds index_remove abstract method + concrete implementation, and standalone remove_search_indices helper with alias-protection guard; TOCTOU window already handled with ignore=[404]
startup_tasks/2026_06_30_remove_old_search_indexes.py	One-shot startup task that runs remove_search_indices inline for versions 5/6/7; any OpenSearch exception propagates to the runner for retry on next boot
tests/manager/search/test_service.py	New TestRemoveSearchIndices suite covers happy path, alias-guard skip, and no-op; uses real OpenSearch fixture for integration coverage
tests/mocks/search.py	Adds _created_indices tracking to SearchServiceFake and implements index_remove to keep the fake consistent with the updated abstract interface

_{Reviews (4): Last reviewed commit: "Address review feedback on the search in..." | Re-trigger Greptile}

[jonathangreen]

This is ready for review, but I'll keep it in draft until its ready to be merged

[tdilauro]

This looks good! 🏁🔥