Recheck annotations when reading leader object on 409 (#3174)

There are cases when we may send the same PATCH request more than one time to K8s API server and it could happen that the first request actually successfully updated the target and we cancelled while waiting for a response. The second PATCH request in this case will fail due to resource_version mismatch.

So far our strategy for update_leader() method was - re-read the object and repeat the request with the new resource_version. However, we can avoid the update by comparing annotations on the read object with annotations that we wanted to set.
This commit is contained in:
Alexander Kukushkin
2024-10-02 15:06:12 +02:00
committed by GitHub
parent e91e6b5484
commit 8e46086335
2 changed files with 15 additions and 6 deletions
+8 -3
View File
@@ -1215,9 +1215,6 @@ class Kubernetes(AbstractDCS):
self._kinds.set(self.leader_path, kind)
if not retry.ensure_deadline(0.5):
return False
kind_annotations = kind and kind.metadata.annotations or EMPTY_DICT
kind_resource_version = kind and kind.metadata.resource_version
@@ -1225,6 +1222,14 @@ class Kubernetes(AbstractDCS):
if kind and (kind_annotations.get(self._LEADER) != self._name or kind_resource_version == resource_version):
return False
# We can get 409 because we do at least one retry, and the first update might have succeeded,
# therefore we will check if annotations on the read object match expectations.
if all(kind_annotations.get(k) == v for k, v in annotations.items()):
return True
if not retry.ensure_deadline(0.5):
return False
return bool(_run_and_handle_exceptions(self._patch_or_create, self.leader_path, annotations,
kind_resource_version, ips=ips, retry=_retry))
+7 -3
View File
@@ -422,14 +422,18 @@ class TestKubernetesEndpoints(BaseTestKubernetes):
self.assertFalse(self.k.update_leader(cluster, '123'))
mock_patch.side_effect = RetryFailedError('')
self.assertRaises(KubernetesError, self.k.update_leader, cluster, '123')
mock_patch.side_effect = k8s_client.rest.ApiException(409, '')
mock_patch.side_effect = [k8s_client.rest.ApiException(409, ''),
k8s_client.rest.ApiException(409, ''), mock_namespaced_kind()]
mock_read.return_value.metadata.resource_version = '2'
with patch('time.time', Mock(side_effect=[0, 0, 100, 200, 0, 0, 0, 0, 0, 100, 200])):
self.assertFalse(self.k.update_leader(cluster, '123'))
self.assertFalse(self.k.update_leader(cluster, '123'))
mock_patch.side_effect = k8s_client.rest.ApiException(409, '')
self.assertFalse(self.k.update_leader(cluster, '123'))
mock_patch.side_effect = [k8s_client.rest.ApiException(409, ''), mock_namespaced_kind()]
mock_read.return_value.metadata.resource_version = '2'
self.assertIsNotNone(self.k._update_leader_with_retry({}, '1', []))
self.assertTrue(self.k._update_leader_with_retry({}, '1', []))
mock_patch.side_effect = [k8s_client.rest.ApiException(409, ''), mock_namespaced_kind()]
self.assertIsNotNone(self.k._update_leader_with_retry({'foo': 'bar'}, '1', []))
mock_patch.side_effect = k8s_client.rest.ApiException(409, '')
mock_read.side_effect = RetryFailedError('')
self.assertRaises(KubernetesError, self.k.update_leader, cluster, '123')