Skip to content

Commit 51e150b

Browse files
Copilotfabricebrito
andcommitted
Add retry logic for Kubernetes 410 (Gone) errors
Co-authored-by: fabricebrito <1178901+fabricebrito@users.noreply.github.com>
1 parent 16b8eab commit 51e150b

3 files changed

Lines changed: 73 additions & 1 deletion

File tree

calrissian/retry.py

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,18 @@ def _is_4xx(exc) -> bool:
1818
return False
1919

2020

21+
def _is_retryable_4xx(exc) -> bool:
22+
"""
23+
Check if a 4xx error is retryable.
24+
410 (Gone) errors are retryable because they indicate the watch resource version is too old.
25+
"""
26+
status = getattr(exc, "status", None)
27+
try:
28+
return int(status) == 410
29+
except (TypeError, ValueError):
30+
return False
31+
32+
2133
def retry_exponential_if_exception_type(exc_type, logger):
2234
"""
2335
Decorator function that returns the tenacity @retry decorator with our commonly-used config
@@ -27,8 +39,9 @@ def retry_exponential_if_exception_type(exc_type, logger):
2739
"""
2840
retry_on_type = retry_if_exception_type(exc_type)
2941
retry_not_4xx = retry_if_exception(lambda e: not _is_4xx(e))
42+
retry_on_410 = retry_if_exception(_is_retryable_4xx)
3043

31-
return retry(retry=retry_on_type & retry_not_4xx,
44+
return retry(retry=retry_on_type & (retry_not_4xx | retry_on_410),
3245
wait=wait_exponential(multiplier=RetryParameters.MULTIPLIER, min=RetryParameters.MIN, max=RetryParameters.MAX),
3346
stop=stop_after_attempt(RetryParameters.ATTEMPTS),
3447
before_sleep=before_sleep_log(logger, logging.DEBUG),

tests/test_k8s.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,29 @@ def test_wait_raises_exception_when_state_is_unexpected(self, mock_watch, mock_g
196196
with self.assertRaisesRegex(CalrissianJobException, 'Unexpected pod container status'):
197197
kc.wait_for_completion()
198198

199+
@patch('calrissian.k8s.watch', autospec=True)
200+
@patch('calrissian.k8s.KubernetesClient._extract_cpu_memory_requests')
201+
def test_wait_raises_410_error_when_retries_disabled(self, mock_cpu_memory, mock_watch, mock_get_namespace, mock_client):
202+
"""Test that 410 errors are raised when retries are disabled (RETRY_ATTEMPTS=0)"""
203+
mock_cpu_memory.return_value = ('1', '1Mi')
204+
205+
mock_pod = create_autospec(V1Pod)
206+
mock_pod.status.container_statuses[0].state = Mock(running=None, waiting=None, terminated=Mock(exit_code=0))
207+
208+
# Stream raises 410
209+
mock_watch.Watch.return_value.stream.side_effect = ApiException(status=410, reason="Expired: too old resource version")
210+
mock_watch.Watch.return_value.stop = Mock()
211+
212+
kc = KubernetesClient()
213+
kc._set_pod(mock_pod)
214+
215+
# With RETRY_ATTEMPTS=0, the 410 should be raised immediately
216+
with self.assertRaises(ApiException) as context:
217+
kc.wait_for_completion()
218+
219+
# The 410 error should be raised (retries disabled in test env)
220+
self.assertEqual(context.exception.status, 410)
221+
199222
def test_raises_on_set_second_pod(self, mock_get_namespace, mock_client):
200223
kc = KubernetesClient()
201224
kc._set_pod(Mock())

tests/test_retry.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,3 +252,39 @@ def wrapped():
252252
wrapped()
253253

254254
self.assertEqual(self.mock.call_count, mock_retry_parameters.ATTEMPTS)
255+
256+
257+
@patch("calrissian.retry.RetryParameters")
258+
def test_retry_on_410_gone_error(self, mock_retry_parameters):
259+
"""410 (Gone) IS 4xx but SHOULD retry due to special handling."""
260+
self.setup_mock_retry_parameters(mock_retry_parameters)
261+
self.mock.side_effect = FakeApiException(410, "Expired: too old resource version")
262+
263+
@retry_exponential_if_exception_type(FakeApiException, self.logger)
264+
def wrapped():
265+
return self.mock()
266+
267+
with self.assertRaisesRegex(FakeApiException, "Expired"):
268+
wrapped()
269+
270+
# Should retry because 410 is a retryable 4xx
271+
self.assertEqual(self.mock.call_count, mock_retry_parameters.ATTEMPTS)
272+
273+
274+
@patch("calrissian.retry.RetryParameters")
275+
def test_retry_on_410_eventually_succeeds(self, mock_retry_parameters):
276+
"""410 should retry and eventually succeed."""
277+
self.setup_mock_retry_parameters(mock_retry_parameters)
278+
self.mock.side_effect = [
279+
FakeApiException(410, "Expired: too old resource version"),
280+
FakeApiException(410, "Expired: too old resource version"),
281+
"ok",
282+
]
283+
284+
@retry_exponential_if_exception_type(FakeApiException, self.logger)
285+
def wrapped():
286+
return self.mock()
287+
288+
result = wrapped()
289+
self.assertEqual(result, "ok")
290+
self.assertEqual(self.mock.call_count, 3)

0 commit comments

Comments
 (0)