[Backport release-1.6] fix(opensearch-operator): let a permanently yellow cluster restart - #5074

…nish

The yellow restart gate carried in the previous patch lets a permanently
yellow cluster through, and with three or more nodes packages/apps/opensearch
turns drainDataNodes on. The restart then reaches PreparePodForDelete, which
excludes the candidate from allocation before it can know whether the drain
will finish. On the cluster that made the gate wait it never finishes: a shard
on that node has nowhere else to go, which is the same condition holding the
cluster yellow. The exclusion is withdrawn only after a pod is really deleted,
so a data node stays out of allocation for as long as the restart is stuck,
shedding its shards onto the rest of the cluster and buying nothing.

Ask the allocator before excluding anything, and only while nothing is moving,
so a drain that is making progress costs nothing. A started shard answering
can_move_to_other_node: no means the drain cannot finish, and then no exclusion
is written and one left by an earlier pass is withdrawn. The two-data-node path
keeps its behaviour, because it never waits for a full drain.

Checking after the fact instead would flap: the next reconcile, ten seconds
later, would exclude the node again.

Upstream as opensearch-project/opensearch-k8s-operator#1585, fixed there by
#1586; the patch goes away with the first release carrying it.

Backport: the Dockerfile header keeps release-1.6's trace paragraph,
placed between the new ones as on main.

Assisted-by: LLM
Signed-off-by: IvanHunters <[email protected]>
(cherry picked from commit 28195d3)
Signed-off-by: Myasnikov Daniil <[email protected]>

… build

The patches under images/opensearch-operator/patches carry their own tests and
nothing runs them. The build compiles main.go, and go-unit-tests in the root
Makefile covers this repository's own Go tree, so a test file that had stopped
compiling still produced an image:

    printf 'func Broken() { this is not go }' >> restart_gate_test.go
    go build -a -o manager main.go   # exit 0

Run go test over the two package trees the patches touch before building the
binary. Both run without a cluster, and the stage takes about eight minutes on
an arm64 builder, most of it the module download the build needs anyway.

Assisted-by: LLM
Signed-off-by: IvanHunters <[email protected]>
(cherry picked from commit 000e2e3)
Signed-off-by: Myasnikov Daniil <[email protected]>

Upstream gives the operator's calls to a cluster no deadline of any kind. The
transport it builds carries no dial, handshake or response-header timeout, no
http.Client is configured, the calls the client makes on its own behalf pass
context.Background(), and the ones that take a context from the caller are
handed one with no deadline on it. A cluster that accepts a connection and then
says nothing holds such a call for as long as the process lives, and with one
worker per controller that single call stops every OpenSearchCluster from being
reconciled. The deletion that then waits out its whole termination timeout is
the visible end of it.

The bound goes on the RoundTripper, the one level every request passes through,
the ones opensearch-go builds for itself included: before a client's first
request the library runs a product check that carries a context.Background() of
its own and decodes the body of the answer, and a client is built per reconcile,
so that check runs on every one. The deadline cannot be released when RoundTrip
returns, because the body goes back to the caller still open; it rides on the
body and is released by Close, the way http.Client applies its own Timeout.

That bounds one attempt. opensearch-go retries a request up to three more times
on 502, 503 and 504 and the transport's deadline starts over on each, so the
calls carry a deadline a second time for the series: through requestContext
where the call decodes the answer before it returns, and through performRequest
where the still-open body goes back to the caller. The dial and the TLS
handshake keep a shorter bound of their own, so an unreachable node fails in a
third of the budget with an error that names what went wrong.

Each of those bounds has a test that fails when it is removed.

Signed-off-by: IvanHunters <[email protected]>
(cherry picked from commit 1a2c876)
Signed-off-by: Myasnikov Daniil <[email protected]>

The timeout tests drive a listener from one goroutine and inspect what it
accepted from another. The plain run passes an unsynchronised access there, so
nothing in this repository would catch one. The builder image carries cgo and
gcc, and the package images are built natively on x86-64 runners, so the
detector is supported where this runs.

Signed-off-by: IvanHunters <[email protected]>
(cherry picked from commit 48058dc)
Signed-off-by: Myasnikov Daniil <[email protected]>