feat(keeper): hand leadership over before restarting the leader

[melancholictheory]

Why

When a Keeper leader pod stops, the ensemble has no leader until the election finishes. #346 measured that window at 5 to 6 seconds, with ClickHouse logging session expirations and INSERT p95 going from about 150 ms to 6.3 s while it lasted.

The operator's own rolling update is only one way a leader pod stops. The descheduler, the cluster autoscaler, VPA and a node drain all terminate pods without the operator in the loop, so a handover implemented in the reconcile loop covers a fraction of the cases (raised by @mu-soliman below).

What

The Keeper container gets a preStop hook that asks the server to hand Raft leadership to a peer before it stops:

lifecycle:
  preStop:
    httpGet:
      path: /api/v1/commands?command=ydld
      port: 9123

It goes through the http_control port the readiness probe already uses, via Keeper's HTTP commands API, which executes four letter words from the command query parameter. That keeps it shell-free, so it works on the distroless images too, and it runs on every graceful termination regardless of who initiated it.

Keeper serves the commands API from 26.3 (checked on 26.3.22.7, 26.5.7.64, 26.6.3.62 and 26.7.5.10) and not on 25.8.32.4, which the compat matrix still covers. On the older servers the request returns 404, the kubelet records a FailedPreStopHook event, and the container stops exactly as before, so the cost there is one warning event per termination.

The hook is deliberately not gated on status.version. Keeper has no version probe; that value comes from mntr on running replicas, so it is empty when the StatefulSets are created and arrives afterwards. A template that depends on it changes after startup and rolls every new cluster once, which is what the first push of this PR did in e2e. A unit test now pins that templateContainer output does not vary with status.version.

No CRD change. terminationGracePeriodSeconds is already exposed on podTemplate; lifecycle is not exposed on containerTemplate and this PR does not add it, since the default needs no override to work on distroless. If someone needs to disable or replace the hook, exposing lifecycle is a small follow-up.

One cost to be aware of: the kubelet bounds a preStop hook by the pod's grace period, so a server that has stopped answering on http_control entirely holds termination open for up to terminationGracePeriodSeconds (30 s by default) before SIGTERM goes out. A healthy server answers in milliseconds, and a server that answers with an error does not delay anything, only a hung one does. The readiness probe already depends on the same port, so a hung server is already failing readiness by then.

Measured on a 3-node Keeper 26.7.5.10

• ydld over HTTP, then mntr reports follower after 0.085 s
• ydld over HTTP followed by an immediate SIGTERM, which is what the kubelet does right after the hook returns: a surviving node reports itself leader after 0.04 s
• plain SIGTERM with no hook, on a fresh trio: new leader after 1.36 s
• on a lone replica the command returns 200 and the node stays standalone, so single-node clusters are unaffected

The response body is Sent yield leadership request to leader. even when the command lands on a follower, so nothing in the hook depends on it.

Testing

Unit tests on templateContainer: the hook is present with the expected path, port and scheme, and the output is identical for an empty, a 25.8 and a 26.7 status.version.

Related Issues

Fixes #346

[melancholictheory]

The first push failed e2e-test (2) and the operator-upgrade compat job on the UpdateOrder invariant (replica 0 updated before 1): I had also reordered the rollout to restart followers before the leader, and the invariant pins highest-id-first for Keeper.

Dropped that part. The change is now only the ydld handover before a leader restart; the order is untouched. Leadership can move more than once per rollout as a result, but each move is a transfer rather than an election, so the leaderless window this issue is about is still gone. Diff is smaller for it.

[mu-soliman]

The PR handles the leader yield problem during shut down initiated by the operator only. However multiple native kubernetes components can gracefully initiate a pod shut down. Examples are :

• Descheduler for multiple reasons - depending on the plugin and policy
• Cluster autoscaler during compaction and node shutdown
• Vertical pod autoscaler - depending on kubernetes version and the operation
• Kubelet - applying taints, node cordoning, etc.

So the solution to the problem should be a generic solution that handles all graceful shutdowns of a pod, ideally using a native kuberntes construct.

I suggest using the container's Lifecycle.PreStop, executing a bash script that triggers leader yield then waits until it completes.

Currently our CRD does not allow for setting the Lifecycle.PreStop and terminationGracePeriodSeconds, so I suggest that we should modify the CRD to allow setting the container's Lifecycle.PreStop and terminationGracePeriodSeconds, with default that executes the suggested bash script and reasonable graceful termination period. We need to allow users who run distroless containers to override these default because their images don't contain bash.

[melancholictheory]

You are right, and I measured a couple of things before changing course.

The operator-side handover only covers restarts the operator itself issues. A descheduler eviction or a node drain never goes through the reconcile loop, so that version of the PR left the window open for exactly the cases you list. Dropped it.

What replaced it is a preStop hook, but as httpGet rather than a shell script. Keeper's http_control port serves four letter words at /api/v1/commands?command=ydld (the command comes from the query parameter, the path segment is ignored), and the operator already opens that port for the readiness probe. So the hook needs no bash, no nc, and works on the distroless images. The kubelet keeps the query string in httpGet.path (url.Parse in pkg/probe/http), so it is a plain HTTPGetAction.

On timing: the kubelet sends SIGTERM as soon as the hook returns, so I checked whether the transfer survives that. On a 3-node 26.7.5.10, ydld over HTTP followed by an immediate SIGTERM gives the ensemble a new leader in 0.04 s; a plain SIGTERM with no hook takes 1.36 s on the same trio. The transfer request is already in flight by the time the response is sent, and the outgoing leader finishes it during shutdown.

Two details on the CRD side. terminationGracePeriodSeconds is already exposed on podTemplate, so that half needs nothing. lifecycle is not exposed on containerTemplate, and I have not added it here: with the HTTP variant the default works everywhere the endpoint exists, so there is nothing to override for distroless users. It is a small follow-up if someone wants to disable the hook.

The one gate is the Keeper version: the commands API is there from 26.3 and missing on 25.8, which the compat matrix still includes, so the hook is only added when status.version is 26.3 or later.

One thing I want to be upfront about: the kubelet bounds a hook by the grace period, so a Keeper that has stopped answering on http_control altogether delays its own termination by up to terminationGracePeriodSeconds. A server that answers with an error does not, only a hung one. I have called that out in the docs note.

[melancholictheory]

The previous push failed e2e-test (3) on should manage the cluster NetworkPolicies, and it was a real regression rather than a flake: the last connectivity probe got nc: bad address for keeper-0-0 because that pod was being recreated at that moment. The operator had started a rolling update of a Keeper cluster it had created seconds earlier.

The cause was the version gate. Keeper has no version probe; status.version is derived from mntr on running replicas, so it is empty when the StatefulSets are first created and gets filled once the pods report in. A container template that depends on it therefore changes after startup, the StatefulSet hash moves, and every new cluster is rolled once. The same would have happened on every image upgrade, one extra rollout after the new version was observed.

Dropped the gate: the hook is always set. On Keeper older than 26.3 the request returns 404, the kubelet records FailedPreStopHook, and the container stops as before, so the cost there is one warning event per termination. A unit test now pins that templateContainer output does not vary with status.version, so this cannot come back quietly.

[mu-soliman]

The timing in your test is not always guaranteed. The current leader keeps acting as leader for whatever was already in its log/pipeline or election_timeout_upper_bound_ms passes, which defaults to 2 seconds. So in a cluster with heavy load the results you saw are not guaranteed to hold true.

This is why I suggested a bash script to make sure that the new leader election finished before we let k8s send a sigterm.

[melancholictheory]

You are right, and I have numbers now rather than an idle-cluster measurement.

I read the NuRaft path first. ydld calls yield_leadership() in its non-immediate form: the leader sets write_paused_, nominates the highest-priority responsive peer, and only steps down in handle_append_entries_resp once that peer's matched_idx equals next_slot() - 1, or when reelection_timer_ expires, which Keeper sets to election_timeout_upper_bound_ms (2 s by default). Graceful shutdown itself does no handover, shutdownRaftServer just calls raft_instance->shutdown(). So the transfer is only as fast as the successor's catch-up, exactly as you describe.

Then a 3-node 26.7.5.10 under write load from a Go go-zookeeper/zk client, two rounds each. Time is until a surviving node reports itself leader over mntr.

load	A: httpGet ydld, SIGTERM at once	B: plain SIGTERM	C: ydld, wait for follower, then SIGTERM
512 B sets, 16 writers, ~25k ops/s	0.019 s, 0.018 s	2.02 s, 1.35 s	0.049 s, 0.019 s (waited 78 ms, 58 ms)
64 KB sets, 32 writers	1.49 s, 0.020 s	1.84 s, 9.05 s	0.23 s, 0.023 s (waited 113 ms, 67 ms)

Under the heavy load the old leader still answered leader about 15 ms after the hook returned in both rounds, and in one of the two the SIGTERM won the race and the ensemble fell back to a timeout election, 1.49 s, which is the case you predicted. Waiting for the handover costs 60 to 110 ms and keeps the switch under a quarter of a second in every run. The 9 s figure for a plain SIGTERM under load is worth keeping in mind as the status quo.

So the hook has to wait, which means exec. The shape I would go with, given what the image contains (bash and nc, no curl):

h() { exec 3<>/dev/tcp/127.0.0.1/9123; printf 'GET %s HTTP/1.0\r\n\r\n' "$1" >&3; cat <&3; exec 3<&-; }
h '/api/v1/commands?command=ydld' >/dev/null 2>&1
for i in $(seq 1 100); do h /ready 2>/dev/null | grep -q '"role":"leader"' || exit 0; sleep 0.1; done

It talks to http_control rather than the client port, because that one stays plain HTTP when tls.required is set, and it polls /ready, which exists on every supported version, so the wait works even where ydld itself 404s. Capped at 10 s, well inside the default grace period.

The trade-off is the one you named: on a distroless image bash is missing, the exec fails at once, the kubelet records FailedPreStopHook and the pod stops as it does today. The httpGet variant currently in the PR is the mirror image, best effort everywhere, guaranteed nowhere. Exposing lifecycle on containerTemplate so distroless users can supply their own hook is a small follow-up, about 14 KB on each CRD.

Longer term the right place for the wait is Keeper itself. A blocking form of the command on the HTTP API would make the shell-free hook correct on any image, and that is a ClickHouse issue rather than an operator one.

I will hold the PR in its current state until a maintainer weighs in on which shape they want, exec with the distroless caveat, or the best-effort httpGet, rather than rewrite it a fourth time on my own.

[melancholictheory]

Closing in favour of #351, which is the exec-with-wait shape from the measurements above, plus the containerTemplate.lifecycle override for distroless images and a deploy test fix. No point keeping two PRs open for the same issue.

The load measurements and the NuRaft notes in the thread above stay relevant to #351, so I will leave them here rather than repeat them.