diff --git a/nbxsync/tests/utils/test_duplicate_recovery.py b/nbxsync/tests/utils/test_duplicate_recovery.py new file mode 100644 index 00000000..9d14c628 --- /dev/null +++ b/nbxsync/tests/utils/test_duplicate_recovery.py @@ -0,0 +1,40 @@ +"""Tests for recovering from a concurrent create in Zabbix. + +Two workers can create the same hostgroup/proxy/template between the lookup and +the create call. Recovering from that race must not swallow genuine validation +errors, so only Zabbix's -32602 'already exists' response is treated as a race. +""" + +from django.test import TestCase + +from nbxsync.utils.sync.syncbase import ZabbixSyncBase + + +class _ApiError(Exception): + def __init__(self, message, code=None, data=''): + super().__init__(f'{message} {data}'.strip()) + if code is not None: + self.code = code + self.data = data + self.message = message + + +class DuplicateErrorDetectionTestCase(TestCase): + def test_duplicate_params_error_is_recognised(self): + err = _ApiError('Invalid params.', code=-32602, data='Host group "Linux servers" already exists.') + + self.assertTrue(ZabbixSyncBase._is_duplicate_error(err)) + + def test_other_invalid_params_error_is_not_a_duplicate(self): + err = _ApiError('Invalid params.', code=-32602, data='Incorrect value for field "name": cannot be empty.') + + self.assertFalse(ZabbixSyncBase._is_duplicate_error(err)) + + def test_internal_error_is_not_a_duplicate(self): + err = _ApiError('Internal error.', code=-32500, data='already exists somewhere') + + self.assertFalse(ZabbixSyncBase._is_duplicate_error(err)) + + def test_plain_exception_falls_back_to_the_message(self): + self.assertTrue(ZabbixSyncBase._is_duplicate_error(Exception('Host group already exists'))) + self.assertFalse(ZabbixSyncBase._is_duplicate_error(Exception('Connection refused'))) diff --git a/nbxsync/utils/sync/syncbase.py b/nbxsync/utils/sync/syncbase.py index 723a6d1f..6cb3e5ff 100644 --- a/nbxsync/utils/sync/syncbase.py +++ b/nbxsync/utils/sync/syncbase.py @@ -79,17 +79,42 @@ def sync(self): self.obj.update_sync_info(success=True) def try_create(self): + params = self.get_create_params() + if not params: + return None try: - # print('Create params:') - # print(self.get_create_params()) - result = self.api_object().create(**self.get_create_params()) - # print('Zabbix result: ') - # print(result) + result = self.api_object().create(**params) return result.get(self.result_key(), [None])[0] except Exception as err: + # Race condition: another concurrent job may have created the + # object between our find_by_name() check and this create(). + # Recover only when Zabbix rejected the parameters (-32602) and + # exactly one object with our name now exists, so a genuine + # validation failure is still reported instead of silently + # adopting an unrelated object. + if self._is_duplicate_error(err): + found_by_name = self.find_by_name() + if len(found_by_name) == 1: + logger.debug(f'{self.__class__.__name__} already existed (race), reusing ID') + return found_by_name[0].get(self.id_field.split('.')[-1]) msg = f'{self.__class__.__name__} creation failed: {err}' raise RuntimeError(msg) + @staticmethod + def _is_duplicate_error(err) -> bool: + """Whether a Zabbix API error reports an already-existing object. + + zabbix_utils raises APIRequestError with the JSON-RPC payload attached. + Duplicates come back as "Invalid params" (-32602), whose data carries + the specific reason, so both the code and the reason are checked. + """ + code = getattr(err, 'code', None) + if code is not None and int(code) != -32602: + return False + detail = ' '.join(str(part) for part in (getattr(err, 'data', ''), getattr(err, 'message', ''), err) if part) + return 'already exists' in detail.lower() + + def find_by_name(self): name_key = 'name' if self.name_field: