mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-06 20:13:43 +09:00
Aleksei Sviridkin
92d2eede52
## What this is
`Refs #4259`. A diagnosis of the opensearch suite timeout plus the one
in-repo change it warrants: making the failing step say which of its two
candidates actually happened. It does **not** claim to fix the root
cause — see "What this does not do".
## What the run actually showed
The `verify-statefulset-rolled-onto-the-new-mount-plan` assert erred
with exactly two field failures, both under `status`: `(currentRevision
== updateRevision): false` and `updatedReplicas: field not found`. No
volume field appears in the error list, and `readyReplicas: 1` shows as
an unchanged line, so the pod was Ready at the deadline and nothing had
replaced it. Per e2e-testing.md ("Reading a failure diff: the actual
side is a projection"), the real cause is the error line, not the diff —
and a Chainsaw diff projects the live object onto the expected fields,
so the three `http-ca` / `http-cert` / `http-key` lines showing in the
diff are the expected tree, not proof the volumes are absent. Read that
way, the volume assertions matched: the three volumes were present on
the live template. The "volumes missing" reading is the projection trap
the doc warns against.
## Which candidate
Candidate (b): the template was rewritten onto the subPath mount plan,
and the operator has not restarted the pod onto it. This is consistent
with the source, traced against opensearch-k8s-operator v2.8.0 as
vendored:
- The chart renders the CR correctly — `generate` dropped, `secret` and
`caSecret` named — proven by
`packages/apps/opensearch/tests/tls_upgrade_test.yaml`.
- Given that CR, `pkg/reconcilers/tls.go` `handleHttp` appends exactly
the three volumes the step asserts (`http-ca` from
`opensearch-test.http-ca`, `http-key` and `http-cert` from
`opensearch-test-http-server`), and `pkg/reconcilers/cluster.go`
`reconcileNodeStatefulSet` writes that template onto the StatefulSet.
Three branches return ahead of that write — no node-pool config hash,
the `opensearch.role` recreate, and parallel recovery — but the
parallel-recovery gate is `ReadyReplicas < replicas-1`, unreachable at
one replica, so the write happens. The reconcilers that run ahead of the
cluster reconciler do not stop it either: the TLS reconciler runs first
and returns the admin-certificate path's result, empty in the ordinary
case, and `securityconfig.Reconcile` short-circuits to nil when the
security-config checksum is unchanged, which the move does not change.
So the operator does write the volumes.
- The nodePool StatefulSet is `OnDelete` (`pkg/builders/cluster.go` sets
`UpdateStrategy.Type: OnDelete` alongside `PodManagementPolicy:
OrderedReady`), so the StatefulSet controller never deletes the pod to
apply a template change: `podManagementPolicy` orders creation and
deletion during scaling, `updateStrategy` decides the update. The roll
is the operator's rolling-restart reconciler's to drive
(`pkg/reconcilers/rollingRestart.go`) — it deletes the pod itself. It
needs `status.Initialized`, it needs `ReadyReplicas == replicas` (a
Ready pod is a precondition, not the blocker), then it opens a client to
the OpenSearch API and calls `CheckClusterStatusForRestart`, which holds
the restart while the cluster is red; on yellow the gate now enables
shard allocation, waits for every data node to join, waits for shard
activity to settle, then proceeds (#4317 deleted the old
`.opensearch-observability` carve-out, and this suite builds the
operator with that patch). Any of those gates can sit indefinitely.
`status.currentRevision` never advances by itself under `OnDelete`; the
operator patches it once `updatedReplicas == replicas`, so the second
assert's own condition cannot go true unless that reconciler ran to the
end.
- The pod's startup probe is a TCP check on 9200
(`builders/cluster.go`), so "connection refused" is OpenSearch not
listening, not a probe-scheme mismatch. The plaintext `GET / HTTP/1.1`
from `curl/8.5.0` on the TLS listener is neither the startup/liveness
probe (TCP) nor the readiness probe (`curl -k https://localhost:9200`,
localhost, HTTPS) — it is external traffic to the LoadBalancer address,
a red herring for the stall.
## What this changes
The step now runs two asserts instead of one. The first waits for the
operator to rewrite the template onto the three volumes, on a
reconcile-scale budget (6m) — a failure here is the operator not writing
them (candidate a). The second waits for the roll to complete onto that
template, on the pod-lifetime budget (12m) — a failure here with the
first green is the roll not completing (candidate b): the operator never
restarting the pod, which the rolling-restart reconciler holds while
`CheckClusterStatusForRestart` reports the cluster red — on yellow the
gate enables shard allocation, waits for every data node to join, waits
for shard activity to settle, then proceeds — or a replacement that was
deleted but never came Ready. The second assert carries the three volume
projections and `observedGeneration == metadata.generation` as well, so
the roll is pinned to the template the first assert checked rather than
to whatever revision is current when the status converges. The suite
`catch` also captures the StatefulSet as YAML and as a describe, and the
operator's own log, because `updateRevision`, `currentRevision`,
`generation` and `updatedReplicas` are what separate the two halves —
describe alone omits them — and the restart decision is logged in the
operator, not in the cluster pods. The reconciler logs `Starting rolling
restart of the StatefulSet` (`rollingRestart.go:155`) once its early
gates (`Initialized`, ready-replica, update-pending) pass; it builds its
OpenSearch client just before, at `:138`, and that client pings the
cluster — so a run whose log lacks `Starting rolling restart` either had
an earlier gate hold it silently or had the client ping fail loudly, not
only the former, while that line present followed by a `Couldn't proceed
with rolling restart ... because <reason>` line names a health-stage
hold (the health gate is `CheckClusterStatusForRestart`, and it is not
the last: `PreparePodForDelete` follows it). The operator-log `tail` was
raised so it spans the whole assert window rather than a third of it.
## What this does not do
It does not fix the stall; #4317 does, and is already merged. The run
that captured this (`Run E2E tests`, job 104576486900) shows the
rolling-restart reconciler holding on health: the manager logged
`Starting rolling restart of the StatefulSet opensearch-test-nodes`
(`rollingRestart.go:155`, printed only after the `Initialized` and
ready-replica gates passed) and then, every ten seconds to the deadline,
`Couldn't proceed with rolling restart ... waiting for health to be
green` (`os_data_service.go:180`), which is reached only after
`GetFlatClusterSettings` returned 2xx (`os_client.go:224` makes anything
else an error). So the operator reached the cluster and the cluster
answered; health held the restart. #4259 established why, and the CA
move plays no part: on a single data node the bootstrap node leaving is
audited into a fresh `security-auditlog-*` index with one replica, that
replica has nowhere to go, the cluster stays yellow for good, and the
v2.8.0 restart gate opens only on green (or on a yellow
`.opensearch-observability`) so it never opens. #4317 backports
opensearch-project/opensearch-k8s-operator#1569 to fix that gate; this
PR is only the diagnostic split that names which half of the roll a
failure is. The split content itself is not the failure: on `49f3f059f`
both asserts passed, the second after 1m47s, and this change touches
only the step's comments and the operator-log tail, not the assertions.
With this change in place, a future occurrence names the half, and the
catch carries the roll's own fields and the operator log, so the
crust-gather can be read against a specific claim rather than an
ambiguous one.
### Release note
```release-note
NONE
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Tests**
* Improved OpenSearch StatefulSet failure diagnostics by capturing YAML,
detailed resource descriptions, and the latest operator pod logs.
* Expanded rolling-restart verification to check volume projections,
template updates, rollout readiness, observed and metadata generations,
and revision consistency.
* Separated template validation from rollout completion checks with
dedicated six- and twelve-minute time limits.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->