fix(leaderelection): survive malformed API error bodies and lock annotations - #2707
fix(leaderelection): survive malformed API error bodies and lock annotations#2707nkbeast wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: nkbeast The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
|
|
Welcome @nkbeast! |
…tations try_acquire_or_renew() parsed the raw error body with json.loads and indexing straight into it, on the assumption that whatever came back is a Kubernetes Status object. Anything sitting in front of the API server (in an ingress, load balancer or proxy) happily answers with an HTML error page, an empty payload or some other non-JSON body, and ApiException.body can also be None. Each of those raised out of the election loop and took the whole leader election down, which is the one failure mode this code exists to prevent - a controller that stops renewing its lease without ever calling onstopped_leading leaves the workload in limbo until an operator notices. Treat an unparsable or missing error body as 'not a 404' and retry on the next period, in both the sync and the aio elector (the aio one also crashed on an empty body through an assert). The same class of problem existed on the read path of the ConfigMap lock: a corrupted leader-election annotation raised out of get() and killed the elector. Treat a non-JSON annotation like a missing one so the next update rewrites a clean record. Signed-off-by: NK <nk@localhost.localdomain> Signed-off-by: NK <92711184+nkbeast@users.noreply.github.com>
d301654 to
ce274dc
Compare
What this fixes
try_acquire_or_renew()parsed the raw API error body withjson.loadsand indexed straight into it:That assumes whatever came back is a Kubernetes Status object. Anything sitting in front of the API server — an ingress, load balancer or proxy — answers with an HTML error page, an empty payload or some other non-JSON body on a bad day, and
ApiException.bodycan also beNone. Each of those raises out of the election loop (JSONDecodeError/TypeError), and in the aio elector an empty body dies even earlier on anassert.Leader election dying like this is the one failure mode the code exists to prevent: the controller stops renewing its lease, never calls
onstopped_leading, and the workload sits in limbo instead of failing over. Reproduced locally with a lock returning a 502 HTML body —JSONDecodeErrorbefore this change, cleanFalseretry after.The fix
get(). It is now treated like a missing annotation so the next update rewrites a clean record.Tests
Four regression tests added: HTML error body,
Nonebody, the clean-404 create path (to prove that behaviour is untouched), and the aio elector. Full suite green locally:436 passed, 28 skippedwith the same command CI uses.