openstack/designate
Code Issues Proposed changes

Improve sink recordset creation

Browse Source 
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
This commit is contained in:
Erik Olof Gunnar Andersson 2019-12-23 01:47:58 -08:00
parent 0050549441
commit 4869913519
3 changed files with 30 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
contrib/designate-ext-samplehandler/designate_ext_samplehandler/notification_handler/sample.py
View File

@ -78,8 +78,6 @@ class SampleHandler(NotificationHandler):
'data': fixed_ip['address'], 'data': fixed_ip['address'],
} }
recordset = self._find_or_create_recordset(context, self._create_or_update_recordset(
**recordset_values) context, [Record(**record_values)], **recordset_values
)
self.central_api.create_record(context, zone_id, recordset['id'],
Record(**record_values))

35
designate/notification_handler/base.py
View File

@ -25,6 +25,7 @@ from designate import exceptions
from designate.central import rpcapi as central_rpcapi from designate.central import rpcapi as central_rpcapi
from designate.context import DesignateContext from designate.context import DesignateContext
from designate.objects import Record from designate.objects import Record
from designate.objects import RecordList
from designate.objects import RecordSet from designate.objects import RecordSet
from designate.plugin import ExtensionPlugin from designate.plugin import ExtensionPlugin
@ -64,28 +65,35 @@ class NotificationHandler(ExtensionPlugin):
context = DesignateContext.get_admin_context(all_tenants=True) context = DesignateContext.get_admin_context(all_tenants=True)
return self.central_api.get_zone(context, zone_id) return self.central_api.get_zone(context, zone_id)
def _find_or_create_recordset(self, context, zone_id, name, type, def _create_or_update_recordset(self, context, records, zone_id, name,
ttl=None): type, ttl=None):
name = name.encode('idna').decode('utf-8') name = name.encode('idna').decode('utf-8')
try: try:
# Attempt to create an empty recordset # Attempt to create a new recordset.
values = { values = {
'name': name, 'name': name,
'type': type, 'type': type,
'ttl': ttl, 'ttl': ttl,
} }
recordset = RecordSet(**values)
recordset.records = RecordList(objects=records)
recordset = self.central_api.create_recordset( recordset = self.central_api.create_recordset(
context, zone_id, RecordSet(**values)) context, zone_id, recordset
)
except exceptions.DuplicateRecordSet: except exceptions.DuplicateRecordSet:
# Fetch the existing recordset # Fetch and update the existing recordset.
recordset = self.central_api.find_recordset(context, { recordset = self.central_api.find_recordset(context, {
'zone_id': zone_id, 'zone_id': zone_id,
'name': name, 'name': name,
'type': type, '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 return recordset
@ -164,9 +172,6 @@ class BaseAddressHandler(NotificationHandler):
'name': fmt % event_data, 'name': fmt % event_data,
'type': 'A' if addr['version'] == 4 else 'AAAA'} 'type': 'A' if addr['version'] == 4 else 'AAAA'}
recordset = self._find_or_create_recordset(
context, **recordset_values)
record_values = { record_values = {
'data': addr['address'], 'data': addr['address'],
'managed': True, 'managed': True,
@ -175,13 +180,9 @@ class BaseAddressHandler(NotificationHandler):
'managed_resource_type': resource_type, 'managed_resource_type': resource_type,
'managed_resource_id': resource_id 'managed_resource_id': resource_id
} }
self._create_or_update_recordset(
LOG.debug('Creating record in %s / %s with values %r', context, [Record(**record_values)], **recordset_values
zone['id'], recordset['id'], record_values) )
self.central_api.create_record(context,
zone['id'],
recordset['id'],
Record(**record_values))
def _delete(self, zone_id, resource_id=None, resource_type='instance', def _delete(self, zone_id, resource_id=None, resource_type='instance',
criterion=None): criterion=None):

18
designate/tests/test_notification_handler/test_nova.py
View File

@ -146,8 +146,8 @@ class NovaFixedHandlerTest(TestCase, NotificationHandlerMixin):
formatv6=['%(label)s.example.com.'], formatv6=['%(label)s.example.com.'],
group='handler:nova_fixed') group='handler:nova_fixed')
fixture = self.get_notification_fixture('nova', event_type) fixture = self.get_notification_fixture('nova', event_type)
with mock.patch.object(self.plugin, '_find_or_create_recordset')\ with mock.patch.object(
as finder: self.plugin, '_create_or_update_recordset') as finder:
with mock.patch.object(self.plugin.central_api, with mock.patch.object(self.plugin.central_api,
'create_record'): 'create_record'):
finder.return_value = {'id': 'fakeid'} finder.return_value = {'id': 'fakeid'}
@ -155,7 +155,7 @@ class NovaFixedHandlerTest(TestCase, NotificationHandlerMixin):
self.admin_context.to_dict(), self.admin_context.to_dict(),
event_type, fixture['payload']) event_type, fixture['payload'])
finder.assert_called_once_with( 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.') name='private.example.com.')
def test_formatv4(self): def test_formatv4(self):
@ -163,8 +163,8 @@ class NovaFixedHandlerTest(TestCase, NotificationHandlerMixin):
self.config(formatv4=['%(label)s-v4.example.com.'], self.config(formatv4=['%(label)s-v4.example.com.'],
group='handler:nova_fixed') group='handler:nova_fixed')
fixture = self.get_notification_fixture('nova', event_type) fixture = self.get_notification_fixture('nova', event_type)
with mock.patch.object(self.plugin, '_find_or_create_recordset')\ with mock.patch.object(
as finder: self.plugin, '_create_or_update_recordset') as finder:
with mock.patch.object(self.plugin.central_api, with mock.patch.object(self.plugin.central_api,
'create_record'): 'create_record'):
finder.return_value = {'id': 'fakeid'} finder.return_value = {'id': 'fakeid'}
@ -172,7 +172,7 @@ class NovaFixedHandlerTest(TestCase, NotificationHandlerMixin):
self.admin_context.to_dict(), self.admin_context.to_dict(),
event_type, fixture['payload']) event_type, fixture['payload'])
finder.assert_called_once_with( 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.') name='private-v4.example.com.')
def test_formatv6(self): def test_formatv6(self):
@ -180,8 +180,8 @@ class NovaFixedHandlerTest(TestCase, NotificationHandlerMixin):
self.config(formatv6=['%(label)s-v6.example.com.'], self.config(formatv6=['%(label)s-v6.example.com.'],
group='handler:nova_fixed') group='handler:nova_fixed')
fixture = self.get_notification_fixture('nova', event_type) fixture = self.get_notification_fixture('nova', event_type)
with mock.patch.object(self.plugin, '_find_or_create_recordset')\ with mock.patch.object(
as finder: self.plugin, '_create_or_update_recordset') as finder:
with mock.patch.object(self.plugin.central_api, with mock.patch.object(self.plugin.central_api,
'create_record'): 'create_record'):
finder.return_value = {'id': 'fakeid'} finder.return_value = {'id': 'fakeid'}
@ -189,5 +189,5 @@ class NovaFixedHandlerTest(TestCase, NotificationHandlerMixin):
self.admin_context.to_dict(), self.admin_context.to_dict(),
event_type, fixture['payload_v6']) event_type, fixture['payload_v6'])
finder.assert_called_once_with( 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.') name='private-v6.example.com.')