sts-sponsors team mailing list archive

[Christian Grabowski <mp+441964@xxxxxxxxxxxxxxxxxx>]

Christian Grabowski has proposed merging ~cgrabowski/maas:fix_dns_tx_serialization into maas:master.

Commit message:
skip checking serial if a newer one exists

update interface+ip trigger to ignore controllers handled in other trigger



Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~cgrabowski/maas/+git/maas/+merge/441964
-- 
Your team MAAS Committers is subscribed to branch maas:master.

diff --git a/src/maasserver/region_controller.py b/src/maasserver/region_controller.py
index de5e1e8..eb193be 100644
--- a/src/maasserver/region_controller.py
+++ b/src/maasserver/region_controller.py
@@ -101,6 +101,7 @@ class RegionControllerService(Service):
         self._queued_updates = []
         self._dns_update_in_progress = False
         self._dns_requires_full_reload = True
+        self._dns_latest_serial = None
         self.postgresListener = postgresListener
         self.dnsResolver = Resolver(
             resolv=None,
@@ -232,6 +233,13 @@ class RegionControllerService(Service):
             self._dns_update_in_progress = False
             return d
 
+        def _set_latest_serial(result):
+            if result:
+                (serial, _, _ ) = result
+                if not self._dns_latest_serial or self._dns_latest_serial < serial:
+                    self._dns_latest_serial = serial
+            return result
+
         defers = []
         if self.needsDNSUpdate:
             self.needsDNSUpdate = False
@@ -244,6 +252,7 @@ class RegionControllerService(Service):
                 requires_reload=self._dns_requires_full_reload,
             )
             d.addCallback(_clear_dynamic_dns_updates)
+            d.addCallback(_set_latest_serial)
             d.addCallback(self._checkSerial)
             d.addCallback(self._logDNSReload)
             # Order here matters, first needsDNSUpdate is set then pass the
@@ -284,6 +293,11 @@ class RegionControllerService(Service):
         if result is None:
             return None
         serial, reloaded, domain_names = result
+
+        # check that there is not a newer serial we should query instead
+        if self._dns_latest_serial and self._dns_latest_serial > serial:
+            return result
+        
         if not reloaded:
             raise DNSReloadError(
                 "Failed to reload DNS; timeout or rdnc command failed."
diff --git a/src/maasserver/tests/test_region_controller.py b/src/maasserver/tests/test_region_controller.py
index e5a1f09..ef75417 100644
--- a/src/maasserver/tests/test_region_controller.py
+++ b/src/maasserver/tests/test_region_controller.py
@@ -163,6 +163,11 @@ class TestRegionControllerService(MAASServerTestCase):
         mock_dns_update_all_zones = self.patch(
             region_controller, "dns_update_all_zones"
         )
+        mock_dns_update_all_zones.returnValue = (
+            random.randint(1, 1000),
+            True,
+            [factory.make_name("domain") for _ in range(3)],
+        )
         service.startProcessing()
         yield service.processingDefer
         mock_dns_update_all_zones.assert_called_once()
@@ -947,3 +952,19 @@ class TestRegionControllerServiceTransactional(MAASTransactionServerTestCase):
                 call(dynamic_updates=expected_updates, requires_reload=False),
             ]
         )
+
+    @wait_for_reactor
+    @inlineCallbacks
+    def test_check_serial_is_skipped_if_a_newer_serial_exists(self):
+        domain = yield deferToDatabase(factory.make_Domain)
+        update_result = (random.randint(0, 10), True, [domain.name])
+        record = yield deferToDatabase(factory.make_DNSResource, domain=domain)
+        service = RegionControllerService(sentinel.listener)
+
+        query = self.patch(service.dnsResolver, "lookupAuthority")
+
+        service._dns_latest_serial = update_result[0]+1
+         
+        yield service._checkSerial(update_result)
+
+        query.assert_not_called()
diff --git a/src/maasserver/triggers/system.py b/src/maasserver/triggers/system.py
index c6313f9..3c38339 100644
--- a/src/maasserver/triggers/system.py
+++ b/src/maasserver/triggers/system.py
@@ -2061,6 +2061,7 @@ def render_dns_dynamic_update_interface_static_ip_address(op):
         CREATE OR REPLACE FUNCTION sys_dns_updates_interface_ip_{op}()
         RETURNS trigger as $$
         DECLARE
+          node_type int;
           current_hostname text;
           domain text;
           iface_name text;
@@ -2070,26 +2071,30 @@ def render_dns_dynamic_update_interface_static_ip_address(op):
           ASSERT TG_WHEN = 'AFTER', 'May only run as an AFTER trigger';
           ASSERT TG_LEVEL <> 'STATEMENT', 'Should not be used as a STATEMENT level trigger', TG_NAME;
           IF (TG_OP = 'INSERT' AND TG_LEVEL = 'ROW') THEN
-            SELECT iface.name, node.hostname, domain_tbl.name, COALESCE(domain_tbl.ttl, 0) INTO iface_name, current_hostname, domain, address_ttl
+            SELECT iface.name, node.hostname, node.node_type, domain_tbl.name, COALESCE(domain_tbl.ttl, 0) INTO iface_name, current_hostname, node_type, domain, address_ttl
               FROM maasserver_interface AS iface
               JOIN maasserver_node AS node ON iface.node_config_id = node.current_config_id
               JOIN maasserver_domain AS domain_tbl ON domain_tbl.id=node.domain_id WHERE iface.id=NEW.interface_id;
             SELECT host(ip) INTO ip_addr FROM maasserver_staticipaddress WHERE id=NEW.staticipaddress_id;
-            PERFORM pg_notify('sys_dns_updates', 'INSERT ' || domain || ' ' || current_hostname || ' A ' || address_ttl || ' ' || ip_addr);
-            PERFORM pg_notify('sys_dns_updates', 'INSERT ' || domain || ' ' || iface_name || '.' || current_hostname || ' A ' || address_ttl || ' ' || ip_addr);
+            IF (node_type={NODE_TYPE.MACHINE} OR node_type={NODE_TYPE.DEVICE}) THEN
+                PERFORM pg_notify('sys_dns_updates', 'INSERT ' || domain || ' ' || current_hostname || ' A ' || address_ttl || ' ' || ip_addr);
+                PERFORM pg_notify('sys_dns_updates', 'INSERT ' || domain || ' ' || iface_name || '.' || current_hostname || ' A ' || address_ttl || ' ' || ip_addr);
+            END IF;
           ELSIF (TG_OP = 'DELETE' AND TG_LEVEL = 'ROW') THEN
             IF EXISTS(SELECT id FROM maasserver_interface WHERE id=OLD.interface_id) THEN
-                SELECT iface.name, node.hostname, domain_tbl.name, COALESCE(domain_tbl.ttl, 0) INTO iface_name, current_hostname, domain, address_ttl
+                SELECT iface.name, node.hostname, node.node_type, domain_tbl.name, COALESCE(domain_tbl.ttl, 0) INTO iface_name, current_hostname, node_type, domain, address_ttl
                   FROM maasserver_interface AS iface
                   JOIN maasserver_node AS node ON iface.node_config_id = node.current_config_id
                   JOIN maasserver_domain AS domain_tbl ON domain_tbl.id=node.domain_id WHERE iface.id=OLD.interface_id;
-                IF EXISTS(SELECT id FROM maasserver_staticipaddress WHERE id=OLD.staticipaddress_id) THEN
-                  SELECT host(ip) INTO ip_addr FROM maasserver_staticipaddress WHERE id=OLD.staticipaddress_id;
-                  PERFORM pg_notify('sys_dns_updates', 'DELETE ' || domain || ' ' || current_hostname || ' A ' || ip_addr);
-                  PERFORM pg_notify('sys_dns_updates', 'DELETE ' || domain || ' ' || iface_name || '.' || current_hostname || ' A ' || ip_addr);
-                ELSE
-                  PERFORM pg_notify('sys_dns_updates', 'DELETE-IFACE-IP ' || domain || ' ' || current_hostname || ' A ' || OLD.interface_id);
-                  PERFORM pg_notify('sys_dns_updates', 'DELETE-IFACE-IP ' || domain || ' ' || current_hostname || ' AAAA ' || OLD.interface_id);
+                IF (node_type={NODE_TYPE.MACHINE} OR node_type={NODE_TYPE.DEVICE}) THEN
+                    IF EXISTS(SELECT id FROM maasserver_staticipaddress WHERE id=OLD.staticipaddress_id) THEN
+                      SELECT host(ip) INTO ip_addr FROM maasserver_staticipaddress WHERE id=OLD.staticipaddress_id;
+                      PERFORM pg_notify('sys_dns_updates', 'DELETE ' || domain || ' ' || current_hostname || ' A ' || ip_addr);
+                      PERFORM pg_notify('sys_dns_updates', 'DELETE ' || domain || ' ' || iface_name || '.' || current_hostname || ' A ' || ip_addr);
+                    ELSE
+                      PERFORM pg_notify('sys_dns_updates', 'DELETE-IFACE-IP ' || domain || ' ' || current_hostname || ' A ' || OLD.interface_id);
+                      PERFORM pg_notify('sys_dns_updates', 'DELETE-IFACE-IP ' || domain || ' ' || current_hostname || ' AAAA ' || OLD.interface_id);
+                    END IF;
                 END IF;
             END IF;
           END IF;
diff --git a/src/maasserver/triggers/testing.py b/src/maasserver/triggers/testing.py
index 3e6fa68..30d0979 100644
--- a/src/maasserver/triggers/testing.py
+++ b/src/maasserver/triggers/testing.py
@@ -965,13 +965,13 @@ class NotifyHelperMixin:
 
     @inlineCallbacks
     def listen(self, channel, msg):
-        if msg:
+        if msg and channel in self.channel_queues:
             yield self.channel_queues[channel].put(msg)
 
-    @inlineCallbacks
     def get_notify(self, channel):
-        msg = yield self.channel_queues[channel].get()
-        return msg
+        if channel in self.channel_queues:
+            return self.channel_queues[channel].get()
+        return None
 
     def start_reading(self):
         for channel in self.channels:
diff --git a/src/maasserver/triggers/tests/test_system.py b/src/maasserver/triggers/tests/test_system.py
index ce7398e..48c14b6 100644
--- a/src/maasserver/triggers/tests/test_system.py
+++ b/src/maasserver/triggers/tests/test_system.py
@@ -540,6 +540,75 @@ class TestSysDNSUpdates(
 
     @wait_for_reactor
     @inlineCallbacks
+    def test_dns_dynamic_update_interface_static_ip_address_ignores_controllers(self):
+        listener = self.make_listener_without_delay()
+        yield self.set_service(listener)
+        yield deferToDatabase(
+            self.register_trigger,
+            "maasserver_interface_ip_addresses",
+            "sys_dns_updates",
+            ops=("delete",),
+            trigger="sys_dns_updates_interface_ip_delete",
+        )
+        vlan = yield deferToDatabase(self.create_vlan)
+        subnet1 = yield deferToDatabase(
+            self.create_subnet, params={"vlan": vlan}
+        )
+        subnet2 = yield deferToDatabase(
+            self.create_subnet, params={"vlan": vlan}
+        )
+        node1 = yield deferToDatabase(
+            self.create_node_with_interface,
+            params={"node_type": NODE_TYPE.RACK_CONTROLLER, "subnet": subnet1, "status": NODE_STATUS.DEPLOYED},
+        )
+        node2 = yield deferToDatabase(
+            self.create_node_with_interface,
+            params={"subnet": subnet2, "status": NODE_STATUS.DEPLOYED},
+        )
+        domain = yield deferToDatabase(Domain.objects.get_default_domain)
+        iface1 = yield deferToDatabase(
+            lambda: node1.current_config.interface_set.first()
+        )
+        iface2 = yield deferToDatabase(
+            lambda: node2.current_config.interface_set.first()
+        )
+        ip1 = yield deferToDatabase(
+            lambda: self.create_staticipaddress(
+                params={
+                    "ip": subnet1.get_next_ip_for_allocation()[0],
+                    "interface": iface1,
+                    "subnet": subnet1,
+                }
+            )
+        )
+        ip2 = yield deferToDatabase(
+            lambda: self.create_staticipaddress(
+                params={
+                    "ip": subnet2.get_next_ip_for_allocation()[0],
+                    "interface": iface2,
+                    "subnet": subnet2,
+                }
+            )
+        )
+        self.start_reading()
+        try:
+            yield deferToDatabase(iface1.unlink_ip_address, ip1)
+            yield deferToDatabase(iface2.unlink_ip_address, ip2)
+            expected_msgs = (
+                f"DELETE {domain.name} {node2.hostname} A {ip2.ip}",
+                f"DELETE {domain.name} {iface2.name}.{node2.hostname} A {ip2.ip}",
+            )
+            for exp in expected_msgs:
+                msg = yield self.get_notify("sys_dns_updates")
+                self.assertEqual(
+                    msg, exp 
+                )
+        finally:
+            self.stop_reading()
+            yield self.postgres_listener_service.stopService()
+
+    @wait_for_reactor
+    @inlineCallbacks
     def test_dns_dynamc_update_ip_update(self):
         listener = self.make_listener_without_delay()
         yield self.set_service(listener)