From 4869913519e0b7bb12b4ba1ef6b7ce8aabb53825 Mon Sep 17 00:00:00 2001 From: Erik Olof Gunnar Andersson Date: Mon, 23 Dec 2019 01:47:58 -0800 Subject: [PATCH] Improve sink recordset creation Reduced the number of calls we need to make when creating records using the sink by better using the create/update recordset api. This also fixes a bug where the sink could trigger a race condition in the worker causing it to throw a BadAction exception. Partial-Bug: #1768618 Change-Id: Iaf21ec59755375d3c3bc043b16a1b14aa991475e --- .../notification_handler/sample.py | 8 ++--- designate/notification_handler/base.py | 35 ++++++++++--------- .../test_notification_handler/test_nova.py | 18 +++++----- 3 files changed, 30 insertions(+), 31 deletions(-) diff --git a/contrib/designate-ext-samplehandler/designate_ext_samplehandler/notification_handler/sample.py b/contrib/designate-ext-samplehandler/designate_ext_samplehandler/notification_handler/sample.py index 60ce1ae5f..56df3b55e 100644 --- a/contrib/designate-ext-samplehandler/designate_ext_samplehandler/notification_handler/sample.py +++ b/contrib/designate-ext-samplehandler/designate_ext_samplehandler/notification_handler/sample.py @@ -78,8 +78,6 @@ class SampleHandler(NotificationHandler): 'data': fixed_ip['address'], } - recordset = self._find_or_create_recordset(context, - **recordset_values) - - self.central_api.create_record(context, zone_id, recordset['id'], - Record(**record_values)) + self._create_or_update_recordset( + context, [Record(**record_values)], **recordset_values + ) diff --git a/designate/notification_handler/base.py b/designate/notification_handler/base.py index e78192f43..07862b9b5 100644 --- a/designate/notification_handler/base.py +++ b/designate/notification_handler/base.py @@ -25,6 +25,7 @@ from designate import exceptions from designate.central import rpcapi as central_rpcapi from designate.context import DesignateContext from designate.objects import Record +from designate.objects import RecordList from designate.objects import RecordSet from designate.plugin import ExtensionPlugin @@ -64,28 +65,35 @@ class NotificationHandler(ExtensionPlugin): context = DesignateContext.get_admin_context(all_tenants=True) return self.central_api.get_zone(context, zone_id) - def _find_or_create_recordset(self, context, zone_id, name, type, - ttl=None): + def _create_or_update_recordset(self, context, records, zone_id, name, + type, ttl=None): name = name.encode('idna').decode('utf-8') try: - # Attempt to create an empty recordset + # Attempt to create a new recordset. values = { 'name': name, 'type': type, 'ttl': ttl, } + recordset = RecordSet(**values) + recordset.records = RecordList(objects=records) recordset = self.central_api.create_recordset( - context, zone_id, RecordSet(**values)) - + context, zone_id, recordset + ) except exceptions.DuplicateRecordSet: - # Fetch the existing recordset + # Fetch and update the existing recordset. recordset = self.central_api.find_recordset(context, { 'zone_id': zone_id, 'name': name, 'type': type, }) - + for record in records: + recordset.records.append(record) + recordset = self.central_api.update_recordset( + context, recordset + ) + LOG.debug('Creating record in %s / %s', zone_id, recordset['id']) return recordset @@ -164,9 +172,6 @@ class BaseAddressHandler(NotificationHandler): 'name': fmt % event_data, 'type': 'A' if addr['version'] == 4 else 'AAAA'} - recordset = self._find_or_create_recordset( - context, **recordset_values) - record_values = { 'data': addr['address'], 'managed': True, @@ -175,13 +180,9 @@ class BaseAddressHandler(NotificationHandler): 'managed_resource_type': resource_type, 'managed_resource_id': resource_id } - - LOG.debug('Creating record in %s / %s with values %r', - zone['id'], recordset['id'], record_values) - self.central_api.create_record(context, - zone['id'], - recordset['id'], - Record(**record_values)) + self._create_or_update_recordset( + context, [Record(**record_values)], **recordset_values + ) def _delete(self, zone_id, resource_id=None, resource_type='instance', criterion=None): diff --git a/designate/tests/test_notification_handler/test_nova.py b/designate/tests/test_notification_handler/test_nova.py index 33a9b0561..d91cc5abf 100644 --- a/designate/tests/test_notification_handler/test_nova.py +++ b/designate/tests/test_notification_handler/test_nova.py @@ -146,8 +146,8 @@ class NovaFixedHandlerTest(TestCase, NotificationHandlerMixin): formatv6=['%(label)s.example.com.'], group='handler:nova_fixed') fixture = self.get_notification_fixture('nova', event_type) - with mock.patch.object(self.plugin, '_find_or_create_recordset')\ - as finder: + with mock.patch.object( + self.plugin, '_create_or_update_recordset') as finder: with mock.patch.object(self.plugin.central_api, 'create_record'): finder.return_value = {'id': 'fakeid'} @@ -155,7 +155,7 @@ class NovaFixedHandlerTest(TestCase, NotificationHandlerMixin): self.admin_context.to_dict(), event_type, fixture['payload']) finder.assert_called_once_with( - mock.ANY, type='A', zone_id=self.zone_id, + mock.ANY, mock.ANY, type='A', zone_id=self.zone_id, name='private.example.com.') def test_formatv4(self): @@ -163,8 +163,8 @@ class NovaFixedHandlerTest(TestCase, NotificationHandlerMixin): self.config(formatv4=['%(label)s-v4.example.com.'], group='handler:nova_fixed') fixture = self.get_notification_fixture('nova', event_type) - with mock.patch.object(self.plugin, '_find_or_create_recordset')\ - as finder: + with mock.patch.object( + self.plugin, '_create_or_update_recordset') as finder: with mock.patch.object(self.plugin.central_api, 'create_record'): finder.return_value = {'id': 'fakeid'} @@ -172,7 +172,7 @@ class NovaFixedHandlerTest(TestCase, NotificationHandlerMixin): self.admin_context.to_dict(), event_type, fixture['payload']) finder.assert_called_once_with( - mock.ANY, type='A', zone_id=self.zone_id, + mock.ANY, mock.ANY, type='A', zone_id=self.zone_id, name='private-v4.example.com.') def test_formatv6(self): @@ -180,8 +180,8 @@ class NovaFixedHandlerTest(TestCase, NotificationHandlerMixin): self.config(formatv6=['%(label)s-v6.example.com.'], group='handler:nova_fixed') fixture = self.get_notification_fixture('nova', event_type) - with mock.patch.object(self.plugin, '_find_or_create_recordset')\ - as finder: + with mock.patch.object( + self.plugin, '_create_or_update_recordset') as finder: with mock.patch.object(self.plugin.central_api, 'create_record'): finder.return_value = {'id': 'fakeid'} @@ -189,5 +189,5 @@ class NovaFixedHandlerTest(TestCase, NotificationHandlerMixin): self.admin_context.to_dict(), event_type, fixture['payload_v6']) finder.assert_called_once_with( - mock.ANY, type='AAAA', zone_id=self.zone_id, + mock.ANY, mock.ANY, type='AAAA', zone_id=self.zone_id, name='private-v6.example.com.')