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

contrib/designate-ext-samplehandler/designate_ext_samplehandler/notification_handler/sample.py

@@ -78,8 +78,6 @@ def process_notification(self, context, event_type, payload):
                 '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
+            )

designate/notification_handler/base.py

@@ -25,6 +25,7 @@
 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 @@ def get_zone(self, zone_id):
         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 @@ def _create(self, addresses, extra, zone_id, resource_type=None,
                     '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 @@ def _create(self, addresses, extra, zone_id, resource_type=None,
                     '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):

designate/tests/test_notification_handler/test_nova.py

@@ -146,48 +146,48 @@ def test_label_in_format_v4_v6(self):
                     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'}
                     self.plugin.process_notification(
                         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):
         event_type = 'compute.instance.create.end'
         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'}
                     self.plugin.process_notification(
                         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):
         event_type = 'compute.instance.create.end'
         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'}
                     self.plugin.process_notification(
                         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.')