Skip to content

Commit 1a2c876

Browse files
IvanHunterslexfrei
IvanHunters
authored and
lexfrei
committed
fix(opensearch-operator): bound every call to a cluster's HTTP API
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]>
1 parent 08bbed3 commit 1a2c876

2 files changed

Lines changed: 807 additions & 3 deletions

File tree

‎packages/system/opensearch-operator/images/opensearch-operator/Dockerfile‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,24 @@
2525
# upstream as opensearch-project/opensearch-k8s-operator#1585, fixed by #1586;
2626
# drop it on the first release carrying that.
2727
#
28+
# patches/timeout-cluster-api-calls.diff bounds the calls the operator makes to
29+
# a cluster's HTTP API. Upstream builds its own http.Transport, which carries no
30+
# dial, handshake or response-header timeout; it passes context.Background() to
31+
# every call it makes on its own behalf; the calls that take a context from the
32+
# caller are handed one with no deadline on it; and no http.Client is
33+
# configured, so there is no timeout there either. A cluster that accepts a
34+
# connection and then says nothing therefore holds the call for as long as the
35+
# process lives, and with one worker per controller that one call stops every
36+
# OpenSearchCluster from being reconciled. The deletion that then waits out its
37+
# whole termination timeout is the visible end of it. The patch bounds a request
38+
# on the RoundTripper, the one level that also reaches the product check
39+
# opensearch-go runs for itself before a client's first request, and bounds the
40+
# call again where the call site can be reached, because the library retries a
41+
# request up to three more times on 502, 503 and 504 and the transport's
42+
# deadline starts over on each. Tracked as cozystack/cozystack#4337;
43+
# it belongs upstream, there is no upstream issue for it yet, and the patch goes
44+
# away with the first release that carries a fix.
45+
#
2846
# patches/trace-stalled-component-reconcilers.diff names the component a
2947
# reconcile is stuck in. The cluster controller runs nine component reconcilers
3048
# in sequence and none of them logs unconditionally when it is entered, so a
@@ -37,9 +55,12 @@
3755
# It reports rather than repairs: drop it once the stall itself is understood
3856
# and fixed.
3957
#
40-
# The patches are applied in the order the glob expands, which is alphabetical,
41-
# and skip-unusable-drain-exclusion builds on restart-on-permanently-yellow. A
42-
# rename that reorders them fails the build rather than producing a wrong binary.
58+
# The patches are applied in the order the glob expands, which is alphabetical.
59+
# skip-unusable-drain-exclusion builds on restart-on-permanently-yellow, and
60+
# timeout-cluster-api-calls rewrites os_client.go call sites that
61+
# skip-unusable-drain-exclusion both adds and quotes as context, so it has to
62+
# come after it. A rename that reorders them fails the build rather than
63+
# producing a wrong binary.
4364
FROM golang:1.26@sha256:079e59808d2d252516e27e3f3a9c003740dee7f75e55aa71528766d52bcfc16a AS builder
4465

4566
# Keep in step with the chart version pulled by `make update`: the chart's

0 commit comments

Comments
 (0)