[viliakov]

Fixes #126

elasticsearch restore now deletes the sts_k8s_logs data stream instead of rolling it over, and names the data stream in the _restore request. Orphaned backing indices are still deleted as plain indices.

• Snapshots that contain the backing indices but not the data stream are rejected before scale-down.
• After cleanup the indices are listed again, and the restore fails before _restore if STS indices reappeared.
• Separate commit: scale-down waited on the workload label, which no pod carries, so it returned while pods were terminating. It now waits on each workload's spec.selector. This applies to every datastore.

Validation: go test ./... and golangci-lint v2.15.0 pass, with regression tests for the three cases in #126. Not yet run against a real Elasticsearch; the acceptance run on sts-882-nonha is still to do.

🤖 Generated with Claude Code

[ai-collaboration-app]

Reviewed at f1c786b.

Verdict: the Elasticsearch changes fix all three #126 cases (checked against a local Elasticsearch 8.19.16), but the scale-down commit 25b8f86 has one blocker.

Blocker B1: a scale-down timeout can leave workloads at 0 replicas for good

• Cause: the pod wait also counts finished pods (internal/orchestration/scale/scale.go:175). An evicted (Failed) receiver or e2es pod lingers after scale-to-0, so the 120 s wait times out.
• Effect: the lock is released while replicas stay at 0 (scale.go:117). A retry overwrites pre-restore-replicas with "0" (internal/clients/k8s/client.go:279, :302), so scale-up restores 0 replicas without a warning. Before this PR the wait matched no ES, VictoriaMetrics or ClickHouse pods, so this path was unreachable.
• Fix: the pod wait needs to ignore finished pods (Succeeded/Failed), while still counting terminating ones. On a wait failure, either keep the lock and print the --finalize-only hint, or don't overwrite an existing annotation when the workload is already at 0. scale_test.go:126 says terminal pods "should be ignored", but it passes only because it deletes them.

Suggestions

• S1: the reappearance guard (cmd/elasticsearch/restore.go:273) rarely catches an active writer; _restore (:166) then fails with a bare name-collision error, or succeeds with the writer's new index orphaned outside the stream.
• S2: after the restore, check that sts_k8s_logs exists and that no .ds-sts_k8s_logs-* index sits outside it (restore.go:166).
• S3: only the guard error prints the --finalize-only hint; data-stream delete, index delete and _restore failures leave workloads scaled down without it. Wrapping the error at restore.go:144 covers all of them.
• S4: indicesPattern is not applied to backing indices once the stream is named: the restore brings all of them back (restore.go:316) and the delete removes all live ones (restore.go:249). The chart pattern selects all of them anyway, so documenting the limit is enough.
• S5: test gaps: no scale-down timeout/retry test, nothing asserts that restoreTargets runs before scale-down, and the mock RestoreSnapshot (restore_test.go:91) never models the restored stream.
• S6: helm-charts-internal stable/suse-observability/scripts/restore-elasticsearch-snapshot.sh:23 still rolls over and still ships; worth mentioning as a follow-up.

The sts-882-nonha acceptance run is still pending.

[viliakov]

Review addressed in two new commits (the earlier ones are unchanged).

B1 (70ef3c4): the pod wait now ignores Succeeded/Failed pods and still counts terminating ones. A scale-down retry no longer overwrites pre-restore-replicas when the workload is already at 0. The new tests fail on the previous code (the wait times out after 120s, and scale-up restores 0).

S1–S3, S5 (91d0b91):

• After the restore completes, restore and check-and-finalize fail before scale-up unless sts_k8s_logs exists and no .ds-sts_k8s_logs-* index is outside it.
• Every error after scale-down now carries the --finalize-only hint. A failing _restore points at writers not matched by scaleDownLabelSelector.
• The snapshot check runs in prepareRestoreTargets, which is tested to reject before any cluster access. The mock now models what _restore brings back.

S4: documented in the README.

S6 (not in this PR): stable/suse-observability/scripts/restore-elasticsearch-snapshot.sh is still rendered by configmap-backup-restore-scripts.yaml and still rolls over. Follow-up in helm-charts-internal.

The acceptance run on sts-882-nonha is still pending.

[ai-collaboration-app]

Re-reviewed at 91d0b91.

B1: resolved. Finished pods no longer block the wait (internal/orchestration/scale/scale.go:178), and a retry keeps the recorded replicas (internal/clients/k8s/client.go:280, :307). An evicted pod no longer causes a timeout. After a timeout from a pod stuck terminating, a retry and scale-up now restore the original count. The new tests fail on f1c786b.

• S1: fixed, via S2. With a continuous writer on a local Elasticsearch 8.19.16, every run that left logs outside the stream failed the new check, and the collision error now points at unmatched writers (cmd/elasticsearch/restore.go:179).
• S2: fixed (cmd/elasticsearch/check_and_finalize.go:475).
• S3: fixed (restore.go:140, check_and_finalize.go:467).
• S4: fixed (README.md:349).
• S5: fixed.
• S6: deferred to helm-charts-internal; I found no tracking issue there yet.

New, should fix before merge: verification can run over a dead port-forward. verifyAndFinalize uses the original esClient (check_and_finalize.go:431, :443), while polling replaces a broken port-forward because "a broken one never recovers" (:121). If the port-forward drops during a long restore, polling recovers. Verification then fails with a connection error, and the workloads stay scaled down after a successful restore. verify-backup counts that as a failure and retries, which deletes the restored indices. Run the verification through the reconnecting client or a fresh port-forward.

The sts-882-nonha acceptance run is still pending.

[viliakov]

Verification over a dead port-forward fixed in the new commit. The post-restore checks now run through the reconnecting client and retry transient request failures (3 attempts). A missing data stream or orphaned backing indices are still reported straight away. Tests cover the retry and the dropped-client reconnect.

S6: no tracking issue in helm-charts-internal yet. I'll leave creating it to the team.

[ai-collaboration-app]

Re-reviewed at 8171f9d.

Dead port-forward finding: fixed. Both verification paths go through the reconnecting client (cmd/elasticsearch/check_and_finalize.go:465, :477). Only the two read-only requests are retried (:511), so a missing stream or stray backing indices are reported after one call, and nothing is deleted or scaled during a retry. On a local Elasticsearch 8.19.16, a dead port-forward was dropped after one failure and reconnects were attempted. The run then failed with the hint, and the workloads stayed scaled down. B1 and S1–S5 are unaffected.

Optional:

• When verification fails on a request error, the hint at :529 points to --finalize-only, which skips the check. check-and-finalize --operation-id <snapshot> would run it again.
• The new tests cover dropping the client and retrying separately (check_and_finalize_test.go:545, restore_test.go:660). None runs a retry through reconnectingHealthClient.

Correction to my earlier reviews: #126's "Done when" list does not require the sts-882-nonha run. It is at most an optional extra step in this PR's validation plan.

[viliakov]

Acceptance run on sts-882-nonha (ES 8.19.16), CLI built from 8171f9d. Snapshot sts-backup-20261110-1006-bjuuqup2tpa2leyygndaza, data_streams: [sts_k8s_logs], 10 backing indices. Reference count for a window before the snapshot: 1,097,887 (stable).

(a) Ingesting target with a write index outside the snapshot (rolled over first, creating -000011)

• Log shows "Deleting datastream 'sts_k8s_logs'" and the restore completing, then workloads scaled up and the lock released.
• Data stream lists all 10 snapshot backing indices; -000011 is gone, no orphans.
• Window count: 1,097,887.

(b) Target in the #126 bug state

• v0.11.3 on the same snapshot left a data stream with only a new write index, 10 orphaned backing indices, and a window count of 0.
• This branch on top of that: data stream deleted, restore completed, 10 indices in the stream, no orphans, count 1,097,887.

Notes:

• The scale-down pod wait printed no "Waiting for N pod(s)" ticks in these runs, so this run shows no wait ticks. The receiver's spec.selector does match its pod, and no pod carries the restore label.
• The post-restore verification passed both times. The reappearance guard never fired, since nothing wrote during cleanup. Their failure paths are covered by unit tests only.
• e2es and receiver are back to 1/1 afterwards, with no restore annotations left.

[craffit]

The restore can finalize before every backing index in the restored logs data stream is ready.