Skip to content

Commit 0d98a99

Browse files
kubernetes-prow[bot]kubernetes-prow[bot]
authored
Merge pull request #2707 from nkbeast/leaderelection-survive-malformed-payloads
fix(leaderelection): survive malformed API error bodies and lock annotations
2 parents e24ea6b + ce274dc commit 0d98a99

6 files changed

Lines changed: 163 additions & 9 deletions

File tree

‎kubernetes/aio/leaderelection/leaderelection.py‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -145,11 +145,22 @@ async def try_acquire_or_renew(self) -> bool:
145145

146146
# A lock is not created with that name, try to create one
147147
if not lock_status:
148-
assert (
148+
# The error body comes straight from the API server, but anything
149+
# sitting in front of it (ingress, load balancer, proxy) can answer
150+
# with an HTML page, an empty payload or some other non-JSON body.
151+
# Only a clean 404 means the lock is absent and may be created;
152+
# everything else is retried on the next period instead of taking
153+
# the whole leader election down.
154+
error_code = None
155+
if (
149156
isinstance(old_election_record, ApiException)
150157
and old_election_record.body is not None
151-
)
152-
if json.loads(old_election_record.body)["code"] != HTTPStatus.NOT_FOUND:
158+
):
159+
try:
160+
error_code = json.loads(old_election_record.body)["code"]
161+
except (ValueError, TypeError, KeyError, AttributeError):
162+
error_code = None
163+
if error_code != HTTPStatus.NOT_FOUND:
153164
logger.error(
154165
"Error retrieving resource lock %s as %s",
155166
self.election_config.lock.name,

‎kubernetes/aio/leaderelection/leaderelection_test.py‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,5 +365,38 @@ async def update(
365365
self.lock.release()
366366

367367

368+
def test_acquire_survives_non_json_error_body(self):
369+
"""A proxy answering with an HTML error page must not kill the elector."""
370+
371+
class GatewayErrorLock:
372+
def __init__(self):
373+
self.name = "lock"
374+
self.namespace = "ns"
375+
self.identity = "candidate"
376+
377+
async def get(self, name, namespace):
378+
return False, ApiException(
379+
status=502,
380+
reason="Bad Gateway",
381+
body="<html><body>502 Bad Gateway</body></html>",
382+
)
383+
384+
async def create(self, name, namespace, election_record):
385+
return False
386+
387+
config = electionconfig.Config(
388+
lock=GatewayErrorLock(),
389+
lease_duration=4,
390+
renew_deadline=3,
391+
retry_period=1,
392+
onstarted_leading=lambda: None,
393+
onstopped_leading=lambda: None,
394+
)
395+
396+
elector = leaderelection.LeaderElection(config)
397+
result = asyncio.run(elector.try_acquire_or_renew())
398+
self.assertFalse(result)
399+
400+
368401
if __name__ == "__main__":
369402
unittest.main()

‎kubernetes/aio/leaderelection/resourcelock/configmaplock.py‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,26 @@ async def get(
8080
self.configmap_reference = api_response
8181
return True, None
8282

83-
lock_record = self.get_lock_object(
84-
json.loads(annotations[self.leader_electionrecord_annotationkey])
85-
)
83+
# A corrupted annotation must not take the elector down: treat it
84+
# like a missing one so the next update rewrites a clean record.
85+
try:
86+
annotation_record = json.loads(
87+
annotations[self.leader_electionrecord_annotationkey]
88+
)
89+
except ValueError:
90+
logger.warning(
91+
"Leader election annotation on ConfigMap %s/%s is not valid "
92+
"JSON; treating the lock as unheld",
93+
name,
94+
namespace,
95+
)
96+
api_response.metadata.annotations = {
97+
self.leader_electionrecord_annotationkey: ""
98+
}
99+
self.configmap_reference = api_response
100+
return True, None
101+
102+
lock_record = self.get_lock_object(annotation_record)
86103

87104
self.configmap_reference = api_response
88105
return True, lock_record

‎kubernetes/leaderelection/leaderelection.py‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,17 @@ def try_acquire_or_renew(self):
130130

131131
# A lock is not created with that name, try to create one
132132
if not lock_status:
133-
if json.loads(old_election_record.body)[
134-
'code'] != HTTPStatus.NOT_FOUND:
133+
# The error body comes straight from the API server, but anything
134+
# sitting in front of it (ingress, load balancer, proxy) can answer
135+
# with an HTML page, an empty payload or some other non-JSON body.
136+
# Only a clean 404 means the lock is absent and may be created;
137+
# everything else is retried on the next period instead of taking
138+
# the whole leader election down.
139+
try:
140+
error_code = json.loads(old_election_record.body)['code']
141+
except (ValueError, TypeError, KeyError, AttributeError):
142+
error_code = None
143+
if error_code != HTTPStatus.NOT_FOUND:
135144
logger.info(
136145
"Error retrieving resource lock {} as {}".format(
137146
self.election_config.lock.name,

‎kubernetes/leaderelection/leaderelection_test.py‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,5 +321,76 @@ def update(self, name, namespace, updated_record):
321321
self.lock.release()
322322

323323

324+
def test_acquire_survives_non_json_error_body(self):
325+
"""A proxy answering with an HTML error page must not kill the elector."""
326+
class GatewayErrorLock:
327+
def __init__(self):
328+
self.name = "lock"
329+
self.namespace = "ns"
330+
self.identity = "candidate"
331+
332+
def get(self, name, namespace):
333+
return False, ApiException(
334+
status=502, reason="Bad Gateway",
335+
body="<html><body>502 Bad Gateway</body></html>")
336+
337+
config = electionconfig.Config(
338+
lock=GatewayErrorLock(), lease_duration=4, renew_deadline=3,
339+
retry_period=1, onstarted_leading=lambda: None,
340+
onstopped_leading=lambda: None)
341+
342+
result = leaderelection.LeaderElection(config).try_acquire_or_renew()
343+
self.assertFalse(result)
344+
345+
def test_acquire_survives_empty_error_body(self):
346+
class EmptyErrorLock:
347+
def __init__(self):
348+
self.name = "lock"
349+
self.namespace = "ns"
350+
self.identity = "candidate"
351+
352+
def get(self, name, namespace):
353+
return False, ApiException(status=500, reason="Server Error",
354+
body=None)
355+
356+
config = electionconfig.Config(
357+
lock=EmptyErrorLock(), lease_duration=4, renew_deadline=3,
358+
retry_period=1, onstarted_leading=lambda: None,
359+
onstopped_leading=lambda: None)
360+
361+
result = leaderelection.LeaderElection(config).try_acquire_or_renew()
362+
self.assertFalse(result)
363+
364+
def test_acquire_still_creates_on_clean_404(self):
365+
class NotFoundLock:
366+
def __init__(self):
367+
self.name = "lock"
368+
self.namespace = "ns"
369+
self.identity = "candidate"
370+
self.created = False
371+
372+
def get(self, name, namespace):
373+
if self.created:
374+
return True, LeaderElectionRecord(
375+
"candidate", "4", "now", "now")
376+
return False, ApiException(
377+
status=404, reason="Not Found",
378+
body=json.dumps({'code': 404}))
379+
380+
def create(self, name, namespace, election_record):
381+
self.created = True
382+
return True
383+
384+
def update(self, name, namespace, updated_record):
385+
return True
386+
387+
config = electionconfig.Config(
388+
lock=NotFoundLock(), lease_duration=4, renew_deadline=3,
389+
retry_period=1, onstarted_leading=lambda: None,
390+
onstopped_leading=lambda: None)
391+
392+
self.assertTrue(leaderelection.LeaderElection(config).try_acquire_or_renew())
393+
394+
324395
if __name__ == '__main__':
325396
unittest.main()

‎kubernetes/leaderelection/resourcelock/configmaplock.py‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,20 @@ def get(self, name, namespace):
6464
self.configmap_reference = api_response
6565
return True, None
6666

67-
lock_record = self.get_lock_object(json.loads(annotations[self.leader_electionrecord_annotationkey]))
67+
# A corrupted annotation must not take the elector down: treat it
68+
# like a missing one so the next update rewrites a clean record.
69+
try:
70+
annotation_record = json.loads(
71+
annotations[self.leader_electionrecord_annotationkey])
72+
except ValueError:
73+
logger.warning(
74+
"Leader election annotation on ConfigMap {}/{} is not valid "
75+
"JSON; treating the lock as unheld".format(name, namespace))
76+
api_response.metadata.annotations = {self.leader_electionrecord_annotationkey: ''}
77+
self.configmap_reference = api_response
78+
return True, None
79+
80+
lock_record = self.get_lock_object(annotation_record)
6881

6982
self.configmap_reference = api_response
7083
return True, lock_record

0 commit comments

Comments
 (0)