Support using non unix timestamp serial.

RFC1982 allows to have different formats including:
    - Simple increment of any number
    - Unixtime
    - Date with counter format YYYYMMDDNN
This changes allow to use any supported serial format and fix some pep8 errors for related code.
Serial will be increased to current timestamp if current timestamp between zone created_at and current timestamp
Serial will be increased to +1 if current timestamp less or bigger then zone created_at or bigger then current timestamp

Depends-On: https://review.opendev.org/c/openstack/designate-tempest-plugin/+/915069
Closes-bug: #2053022
Change-Id: I52f7834c423ed3fdbfe2441780f917db658a2bcb
Signed-off-by: Mikhail Samoylov <mikhailsamoiloff@gmail.com>

api-ref/source/dns-api-v2-zone.inc

@@ -40,6 +40,7 @@ Request
    - name: zone_name
    - email: zone_email
    - ttl: zone_ttl
+   - serial: zone_serial
    - description: zone_description
    - masters: zone_masters
    - type: zone_type
@@ -321,6 +322,7 @@ Request
    - x-auth-sudo-project-id: x-auth-sudo-project-id
    - zone_id: path_zone_id
    - email: zone_email_update
+   - serial: zone_serial
    - ttl: zone_ttl
    - description: zone_description
 

designate/api/v2/controllers/zones/__init__.py

@@ -168,7 +168,8 @@ class ZonesController(rest.RestController):
 
             # Update and persist the resource
 
-            increment_serial = zone.type == 'PRIMARY'
+            increment_serial = (zone.type == 'PRIMARY' and
+                                'serial' not in zone.obj_get_changes())
             zone = self.central_api.update_zone(
                 context, zone, increment_serial=increment_serial)
 

designate/central/service.py

@@ -721,11 +721,24 @@ class Service(service.RPCService):
         pool = self.storage.get_pool(elevated_context, pool_id)
         return pool.ns_records
 
+    def _generate_record_serial(self, zone, increment_serial):
+        if not increment_serial:
+            return zone.serial
+        else:
+            return zone.serial + 1
+
     @rpc.expected_exceptions()
     @transaction
     @lock.synchronized_zone()
     def increment_zone_serial(self, context, zone):
-        zone.serial = self.storage.increment_serial(context, zone.id)
+        if zone.created_at.timestamp() < zone.serial < timeutils.utcnow_ts():
+            zone.serial = self.storage.increment_serial(
+                context, zone.id, timeutils.utcnow_ts()
+            )
+        else:
+            zone.serial = self.storage.increment_serial(
+                context, zone.id, zone.serial + 1
+            )
         self._update_soa(context, zone)
         return zone.serial
 
@@ -1512,10 +1525,9 @@ class Service(service.RPCService):
             for record in recordset.records:
                 record.action = 'CREATE'
                 record.status = 'PENDING'
-                if not increment_serial:
-                    record.serial = zone.serial
-                else:
-                    record.serial = timeutils.utcnow_ts()
+                record.serial = self._generate_record_serial(
+                    zone, increment_serial
+                )
 
         new_recordset = self.storage.create_recordset(context, zone.id,
                                                       recordset)
@@ -1739,10 +1751,9 @@ class Service(service.RPCService):
                     continue
                 record.action = 'UPDATE'
                 record.status = 'PENDING'
-                if not increment_serial:
-                    record.serial = zone.serial
-                else:
-                    record.serial = timeutils.utcnow_ts()
+                record.serial = self._generate_record_serial(
+                    zone, increment_serial
+                )
 
             # Ensure the tenant has enough zone record quotas to
             # create new records
@@ -1827,10 +1838,9 @@ class Service(service.RPCService):
             for record in recordset.records:
                 record.action = 'DELETE'
                 record.status = 'PENDING'
-                if not increment_serial:
-                    record.serial = zone.serial
-                else:
-                    record.serial = timeutils.utcnow_ts()
+                record.serial = self._generate_record_serial(
+                    zone, increment_serial
+                )
 
         # Update the recordset's action/status and then delete it
         self.storage.update_recordset(context, recordset)

designate/objects/adapters/api_v2/zone.py

@@ -38,7 +38,9 @@ class ZoneAPIv2Adapter(base.APIv2Adapter):
             "ttl": {
                 'read_only': False
             },
-            "serial": {},
+            "serial": {
+                'read_only': False
+            },
             "shared": {},
             "status": {},
             "action": {},

designate/objects/zone.py

@@ -97,6 +97,22 @@ class Zone(base.DesignateObject, base.DictObjectMixin,
         errors = ValidationErrorList()
 
         if self.type == 'PRIMARY':
+            if 'serial' in self.obj_what_changed():
+                if self.obj_get_original_value('serial') > self.serial:
+                    e = ValidationError()
+                    e.path = ['type']
+                    e.validator = 'minimum'
+                    e.validator_value = ['serial']
+                    e.message = (
+                        "serial %d is less than previously set serial of %d" %
+                        (
+                            self.serial,
+                            self.obj_get_original_value('serial')
+                        )
+                    )
+                    errors.append(e)
+                else:
+                    self.serial -= 1  # increment zone has to be called later
             if self.obj_attr_is_set('masters') and len(self.masters) != 0:
                 e = ValidationError()
                 e.path = ['type']
@@ -123,7 +139,7 @@ class Zone(base.DesignateObject, base.DictObjectMixin,
                     e.message = "'masters' is a required property"
                     errors.append(e)
 
-                for i in ['email', 'ttl']:
+                for i in ['email', 'ttl', 'serial']:
                     if i in self.obj_what_changed():
                         e = ValidationError()
                         e.path = ['type']

designate/storage/sqlalchemy/__init__.py

@@ -15,7 +15,6 @@
 # under the License.
 from oslo_log import log as logging
 from oslo_utils.secretutils import md5
-from oslo_utils import timeutils
 from sqlalchemy import case, select, distinct, func
 from sqlalchemy.sql.expression import or_, literal_column
 
@@ -621,10 +620,9 @@ class SQLAlchemyStorage(base.SQLAlchemy):
 
         return updated_zone
 
-    def increment_serial(self, context, zone_id):
+    def increment_serial(self, context, zone_id, new_serial):
         """Increment the zone's serial number.
         """
-        new_serial = timeutils.utcnow_ts()
         query = tables.zones.update().where(
             tables.zones.c.id == zone_id).values(
             {'serial': new_serial, 'increment_serial': False}

designate/tests/functional/api/v2/test_zones.py

@@ -422,6 +422,32 @@ class ApiV2ZonesTest(v2.ApiV2TestCase):
         self.assertEqual('prefix-%s' % zone['email'],
                          response.json['email'])
 
+    def test_update_zone_serial(self):
+        # Create a zone
+        zone = self.create_zone()
+
+        new_serial = zone['serial'] + 10000
+
+        # Prepare an update body
+        body = {'serial': new_serial}
+
+        response = self.client.patch_json('/zones/%s' % zone['id'], body,
+                                          status=202)
+
+        # Check the headers are what we expect
+        self.assertEqual(202, response.status_int)
+        self.assertEqual('application/json', response.content_type)
+
+        # Check the body structure is what we expect
+        self.assertIn('links', response.json)
+        self.assertIn('self', response.json['links'])
+        self.assertIn('status', response.json)
+
+        # Check the values returned are what we expect
+        self.assertIn('id', response.json)
+        self.assertIsNotNone(response.json['updated_at'])
+        self.assertEqual(new_serial - 1, response.json['serial'])
+
     def test_update_zone_invalid_id(self):
         self._assert_invalid_uuid(self.client.patch_json, '/zones/%s',
                                   headers={'X-Test-Role': 'member'})

designate/tests/functional/central/test_service.py

@@ -1037,6 +1037,23 @@ class CentralServiceTest(designate.tests.functional.TestCase):
         self.assertFalse(zone.increment_serial)
         self.assertEqual('info@example.net', zone.email)
 
+    def test_update_zone_serial(self):
+        # Create a zone
+        zone = self.create_zone(email='info@example.org')
+        new_serial = zone.serial + 10000
+
+        zone.serial = new_serial
+
+        # Perform the update
+        self.central_service.update_zone(
+            self.admin_context, zone, increment_serial=False)
+
+        # Fetch the zone again
+        zone = self.central_service.get_zone(self.admin_context, zone.id)
+
+        # Ensure the zone was updated correctly
+        self.assertEqual(new_serial, zone.serial)
+
     def test_update_zone_name_fail(self):
         # Create a zone
         zone = self.create_zone(name='example.org.')
@@ -4542,7 +4559,7 @@ class CentralServiceTest(designate.tests.functional.TestCase):
         )
 
         # Ensure that serial is now correct.
-        self.assertEqual(zone_serial + 5, updated_zone.serial)
+        self.assertEqual(zone_serial + 1, updated_zone.serial)
         self.assertFalse(updated_zone.increment_serial)
 
         # But the zone is still in pending status as we haven't notified
@@ -4570,7 +4587,7 @@ class CentralServiceTest(designate.tests.functional.TestCase):
 
         # Validate that the status is now ACTIVE.
         self.assertEqual('ACTIVE', updated_zone.status)
-        self.assertEqual(zone_serial + 5, updated_zone.serial)
+        self.assertEqual(zone_serial + 1, updated_zone.serial)
         for recordset in recordsets:
             self.assertEqual('ACTIVE', recordset.status)
             for record in recordset.records:
@@ -4663,3 +4680,71 @@ class CentralServiceTest(designate.tests.functional.TestCase):
             exceptions.Forbidden,
             self.central_service._enforce_catalog_zone_policy,
             self.get_context(), catalog_zone)
+
+    def test_create_zone_serial_yyyymmddss(self):
+        serial = 2024080201
+        # Create a zone
+        zone = self.create_zone(email='info@example.org', serial=serial)
+        self.assertEqual(serial, self.central_service.get_zone(
+            self.admin_context, zone['id']).serial)
+
+    def test_create_zone_serial_unixtime(self):
+        serial = 1369550494
+        # Create a zone
+        zone = self.create_zone(email='info@example.org', serial=serial)
+        self.assertEqual(serial, self.central_service.get_zone(
+            self.admin_context, zone['id']).serial)
+
+    def test_create_zone_serial_number(self):
+        serial = 1234567
+        # Create a zone
+        zone = self.create_zone(email='info@example.org', serial=serial)
+        self.assertEqual(serial, self.central_service.get_zone(
+            self.admin_context, zone['id']).serial)
+
+    @mock.patch.object(notifier.Notifier, "info")
+    def test_update_zone_serial_to_yyyymmddss(self, mock_notifier):
+        serial = 2024080201
+        # Create a zone
+        zone = self.create_zone(email='info@example.org', serial=1)
+        # Update the object
+        zone.serial = serial
+
+        # Reset the mock to avoid the calls from the create_zone() call
+        mock_notifier.reset_mock()
+
+        # Perform the update
+        self.central_service.update_zone(self.admin_context, zone)
+        self.assertEqual(serial, self.central_service.get_zone(
+            self.admin_context, zone['id']).serial)
+
+    @mock.patch.object(notifier.Notifier, "info")
+    def test_update_zone_serial_to_unixtime(self, mock_notifier):
+        serial = 1708636627
+        # Create a zone
+        zone = self.create_zone(email='info@example.org', serial=1)
+        # Update the object
+        zone.serial = serial
+
+        # Reset the mock to avoid the calls from the create_zone() call
+        mock_notifier.reset_mock()
+
+        # Perform the update
+        self.central_service.update_zone(self.admin_context, zone)
+        self.assertEqual(serial, self.central_service.get_zone(
+            self.admin_context, zone['id']).serial)
+
+    @mock.patch.object(notifier.Notifier, "info")
+    def test_update_zone_serial_to_number(self, mock_notifier):
+        serial = 1234567
+        # Create a zone
+        zone = self.create_zone(email='info@example.org', serial=1)
+        # Update the object
+        zone.serial = serial
+        # Reset the mock to avoid the calls from the create_zone() call
+        mock_notifier.reset_mock()
+
+        # Perform the update
+        self.central_service.update_zone(self.admin_context, zone)
+        self.assertEqual(serial, self.central_service.get_zone(
+            self.admin_context, zone['id']).serial)

devstack/plugin.sh

@@ -146,6 +146,7 @@ function configure_designate_tempest() {
         iniset $TEMPEST_CONFIG dns_feature_enabled api_admin $DESIGNATE_ENABLE_API_ADMIN
         iniset $TEMPEST_CONFIG dns_feature_enabled api_v2_root_recordsets True
         iniset $TEMPEST_CONFIG dns_feature_enabled api_v2_quotas True
+        iniset $TEMPEST_CONFIG dns_feature_enabled zone_serial_update_disabled False
         iniset $TEMPEST_CONFIG dns_feature_enabled api_v2_quotas_verify_project True
         iniset $TEMPEST_CONFIG dns_feature_enabled bug_1573141_fixed True
         iniset $TEMPEST_CONFIG dns_feature_enabled bug_1932026_fixed True