Merge ~cgrabowski/maas:fix_dns_tx_serialization into maas:master

Proposed by Christian Grabowski
Status: Merged
Approved by: Christian Grabowski
Approved revision: d161385a8d3f045786adc9de3089895b853d07af
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~cgrabowski/maas:fix_dns_tx_serialization
Merge into: maas:master
Diff against target: 244 lines (+130/-15)
5 files modified
src/maasserver/region_controller.py (+17/-0)
src/maasserver/tests/test_region_controller.py (+20/-0)
src/maasserver/triggers/system.py (+16/-11)
src/maasserver/triggers/testing.py (+4/-4)
src/maasserver/triggers/tests/test_system.py (+73/-0)
Reviewer Review Type Date Requested Status
MAAS Lander Approve
Alexsander de Souza Approve
Review via email: mp+441964@code.launchpad.net

Commit message

skip checking serial if a newer one exists

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

To post a comment you must log in.
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b fix_dns_tx_serialization lp:~cgrabowski/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas-tester/2403/console
COMMIT: 139936def8b8cb1e50fd2c1fa45e959995f80054

review: Needs Fixing
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b fix_dns_tx_serialization lp:~cgrabowski/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas-tester/2404/console
COMMIT: 7bc0cde0ab971840e75efed644f7256e9c0df177

review: Needs Fixing
Revision history for this message
Alexsander de Souza (alexsander-souza) wrote :

+1

review: Approve
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b fix_dns_tx_serialization lp:~cgrabowski/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas-tester/2405/console
COMMIT: d161385a8d3f045786adc9de3089895b853d07af

review: Needs Fixing
Revision history for this message
Christian Grabowski (cgrabowski) wrote :

jenkins: !test

Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b fix_dns_tx_serialization lp:~cgrabowski/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas-tester/2406/console
COMMIT: d161385a8d3f045786adc9de3089895b853d07af

review: Needs Fixing
Revision history for this message
Christian Grabowski (cgrabowski) wrote :

jenkins: !test

Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b fix_dns_tx_serialization lp:~cgrabowski/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: d161385a8d3f045786adc9de3089895b853d07af

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/maasserver/region_controller.py b/src/maasserver/region_controller.py
2index de5e1e8..1c8e014 100644
3--- a/src/maasserver/region_controller.py
4+++ b/src/maasserver/region_controller.py
5@@ -101,6 +101,7 @@ class RegionControllerService(Service):
6 self._queued_updates = []
7 self._dns_update_in_progress = False
8 self._dns_requires_full_reload = True
9+ self._dns_latest_serial = None
10 self.postgresListener = postgresListener
11 self.dnsResolver = Resolver(
12 resolv=None,
13@@ -232,6 +233,16 @@ class RegionControllerService(Service):
14 self._dns_update_in_progress = False
15 return d
16
17+ def _set_latest_serial(result):
18+ if result:
19+ (serial, _, _) = result
20+ if (
21+ not self._dns_latest_serial
22+ or self._dns_latest_serial < serial
23+ ):
24+ self._dns_latest_serial = serial
25+ return result
26+
27 defers = []
28 if self.needsDNSUpdate:
29 self.needsDNSUpdate = False
30@@ -244,6 +255,7 @@ class RegionControllerService(Service):
31 requires_reload=self._dns_requires_full_reload,
32 )
33 d.addCallback(_clear_dynamic_dns_updates)
34+ d.addCallback(_set_latest_serial)
35 d.addCallback(self._checkSerial)
36 d.addCallback(self._logDNSReload)
37 # Order here matters, first needsDNSUpdate is set then pass the
38@@ -284,6 +296,11 @@ class RegionControllerService(Service):
39 if result is None:
40 return None
41 serial, reloaded, domain_names = result
42+
43+ # check that there is not a newer serial we should query instead
44+ if self._dns_latest_serial and self._dns_latest_serial > serial:
45+ return result
46+
47 if not reloaded:
48 raise DNSReloadError(
49 "Failed to reload DNS; timeout or rdnc command failed."
50diff --git a/src/maasserver/tests/test_region_controller.py b/src/maasserver/tests/test_region_controller.py
51index e5a1f09..1a9ee32 100644
52--- a/src/maasserver/tests/test_region_controller.py
53+++ b/src/maasserver/tests/test_region_controller.py
54@@ -163,6 +163,11 @@ class TestRegionControllerService(MAASServerTestCase):
55 mock_dns_update_all_zones = self.patch(
56 region_controller, "dns_update_all_zones"
57 )
58+ mock_dns_update_all_zones.returnValue = (
59+ random.randint(1, 1000),
60+ True,
61+ [factory.make_name("domain") for _ in range(3)],
62+ )
63 service.startProcessing()
64 yield service.processingDefer
65 mock_dns_update_all_zones.assert_called_once()
66@@ -947,3 +952,18 @@ class TestRegionControllerServiceTransactional(MAASTransactionServerTestCase):
67 call(dynamic_updates=expected_updates, requires_reload=False),
68 ]
69 )
70+
71+ @wait_for_reactor
72+ @inlineCallbacks
73+ def test_check_serial_is_skipped_if_a_newer_serial_exists(self):
74+ domain = yield deferToDatabase(factory.make_Domain)
75+ update_result = (random.randint(0, 10), True, [domain.name])
76+ service = RegionControllerService(sentinel.listener)
77+
78+ query = self.patch(service.dnsResolver, "lookupAuthority")
79+
80+ service._dns_latest_serial = update_result[0] + 1
81+
82+ yield service._checkSerial(update_result)
83+
84+ query.assert_not_called()
85diff --git a/src/maasserver/triggers/system.py b/src/maasserver/triggers/system.py
86index c6313f9..3c38339 100644
87--- a/src/maasserver/triggers/system.py
88+++ b/src/maasserver/triggers/system.py
89@@ -2061,6 +2061,7 @@ def render_dns_dynamic_update_interface_static_ip_address(op):
90 CREATE OR REPLACE FUNCTION sys_dns_updates_interface_ip_{op}()
91 RETURNS trigger as $$
92 DECLARE
93+ node_type int;
94 current_hostname text;
95 domain text;
96 iface_name text;
97@@ -2070,26 +2071,30 @@ def render_dns_dynamic_update_interface_static_ip_address(op):
98 ASSERT TG_WHEN = 'AFTER', 'May only run as an AFTER trigger';
99 ASSERT TG_LEVEL <> 'STATEMENT', 'Should not be used as a STATEMENT level trigger', TG_NAME;
100 IF (TG_OP = 'INSERT' AND TG_LEVEL = 'ROW') THEN
101- SELECT iface.name, node.hostname, domain_tbl.name, COALESCE(domain_tbl.ttl, 0) INTO iface_name, current_hostname, domain, address_ttl
102+ 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
103 FROM maasserver_interface AS iface
104 JOIN maasserver_node AS node ON iface.node_config_id = node.current_config_id
105 JOIN maasserver_domain AS domain_tbl ON domain_tbl.id=node.domain_id WHERE iface.id=NEW.interface_id;
106 SELECT host(ip) INTO ip_addr FROM maasserver_staticipaddress WHERE id=NEW.staticipaddress_id;
107- PERFORM pg_notify('sys_dns_updates', 'INSERT ' || domain || ' ' || current_hostname || ' A ' || address_ttl || ' ' || ip_addr);
108- PERFORM pg_notify('sys_dns_updates', 'INSERT ' || domain || ' ' || iface_name || '.' || current_hostname || ' A ' || address_ttl || ' ' || ip_addr);
109+ IF (node_type={NODE_TYPE.MACHINE} OR node_type={NODE_TYPE.DEVICE}) THEN
110+ PERFORM pg_notify('sys_dns_updates', 'INSERT ' || domain || ' ' || current_hostname || ' A ' || address_ttl || ' ' || ip_addr);
111+ PERFORM pg_notify('sys_dns_updates', 'INSERT ' || domain || ' ' || iface_name || '.' || current_hostname || ' A ' || address_ttl || ' ' || ip_addr);
112+ END IF;
113 ELSIF (TG_OP = 'DELETE' AND TG_LEVEL = 'ROW') THEN
114 IF EXISTS(SELECT id FROM maasserver_interface WHERE id=OLD.interface_id) THEN
115- SELECT iface.name, node.hostname, domain_tbl.name, COALESCE(domain_tbl.ttl, 0) INTO iface_name, current_hostname, domain, address_ttl
116+ 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
117 FROM maasserver_interface AS iface
118 JOIN maasserver_node AS node ON iface.node_config_id = node.current_config_id
119 JOIN maasserver_domain AS domain_tbl ON domain_tbl.id=node.domain_id WHERE iface.id=OLD.interface_id;
120- IF EXISTS(SELECT id FROM maasserver_staticipaddress WHERE id=OLD.staticipaddress_id) THEN
121- SELECT host(ip) INTO ip_addr FROM maasserver_staticipaddress WHERE id=OLD.staticipaddress_id;
122- PERFORM pg_notify('sys_dns_updates', 'DELETE ' || domain || ' ' || current_hostname || ' A ' || ip_addr);
123- PERFORM pg_notify('sys_dns_updates', 'DELETE ' || domain || ' ' || iface_name || '.' || current_hostname || ' A ' || ip_addr);
124- ELSE
125- PERFORM pg_notify('sys_dns_updates', 'DELETE-IFACE-IP ' || domain || ' ' || current_hostname || ' A ' || OLD.interface_id);
126- PERFORM pg_notify('sys_dns_updates', 'DELETE-IFACE-IP ' || domain || ' ' || current_hostname || ' AAAA ' || OLD.interface_id);
127+ IF (node_type={NODE_TYPE.MACHINE} OR node_type={NODE_TYPE.DEVICE}) THEN
128+ IF EXISTS(SELECT id FROM maasserver_staticipaddress WHERE id=OLD.staticipaddress_id) THEN
129+ SELECT host(ip) INTO ip_addr FROM maasserver_staticipaddress WHERE id=OLD.staticipaddress_id;
130+ PERFORM pg_notify('sys_dns_updates', 'DELETE ' || domain || ' ' || current_hostname || ' A ' || ip_addr);
131+ PERFORM pg_notify('sys_dns_updates', 'DELETE ' || domain || ' ' || iface_name || '.' || current_hostname || ' A ' || ip_addr);
132+ ELSE
133+ PERFORM pg_notify('sys_dns_updates', 'DELETE-IFACE-IP ' || domain || ' ' || current_hostname || ' A ' || OLD.interface_id);
134+ PERFORM pg_notify('sys_dns_updates', 'DELETE-IFACE-IP ' || domain || ' ' || current_hostname || ' AAAA ' || OLD.interface_id);
135+ END IF;
136 END IF;
137 END IF;
138 END IF;
139diff --git a/src/maasserver/triggers/testing.py b/src/maasserver/triggers/testing.py
140index 3e6fa68..30d0979 100644
141--- a/src/maasserver/triggers/testing.py
142+++ b/src/maasserver/triggers/testing.py
143@@ -965,13 +965,13 @@ class NotifyHelperMixin:
144
145 @inlineCallbacks
146 def listen(self, channel, msg):
147- if msg:
148+ if msg and channel in self.channel_queues:
149 yield self.channel_queues[channel].put(msg)
150
151- @inlineCallbacks
152 def get_notify(self, channel):
153- msg = yield self.channel_queues[channel].get()
154- return msg
155+ if channel in self.channel_queues:
156+ return self.channel_queues[channel].get()
157+ return None
158
159 def start_reading(self):
160 for channel in self.channels:
161diff --git a/src/maasserver/triggers/tests/test_system.py b/src/maasserver/triggers/tests/test_system.py
162index ce7398e..675fe89 100644
163--- a/src/maasserver/triggers/tests/test_system.py
164+++ b/src/maasserver/triggers/tests/test_system.py
165@@ -540,6 +540,79 @@ class TestSysDNSUpdates(
166
167 @wait_for_reactor
168 @inlineCallbacks
169+ def test_dns_dynamic_update_interface_static_ip_address_ignores_controllers(
170+ self,
171+ ):
172+ listener = self.make_listener_without_delay()
173+ yield self.set_service(listener)
174+ yield deferToDatabase(
175+ self.register_trigger,
176+ "maasserver_interface_ip_addresses",
177+ "sys_dns_updates",
178+ ops=("delete",),
179+ trigger="sys_dns_updates_interface_ip_delete",
180+ )
181+ vlan = yield deferToDatabase(self.create_vlan)
182+ subnet1 = yield deferToDatabase(
183+ self.create_subnet, params={"vlan": vlan}
184+ )
185+ subnet2 = yield deferToDatabase(
186+ self.create_subnet, params={"vlan": vlan}
187+ )
188+ node1 = yield deferToDatabase(
189+ self.create_node_with_interface,
190+ params={
191+ "node_type": NODE_TYPE.RACK_CONTROLLER,
192+ "subnet": subnet1,
193+ "status": NODE_STATUS.DEPLOYED,
194+ },
195+ )
196+ node2 = yield deferToDatabase(
197+ self.create_node_with_interface,
198+ params={"subnet": subnet2, "status": NODE_STATUS.DEPLOYED},
199+ )
200+ domain = yield deferToDatabase(Domain.objects.get_default_domain)
201+ iface1 = yield deferToDatabase(
202+ lambda: node1.current_config.interface_set.first()
203+ )
204+ iface2 = yield deferToDatabase(
205+ lambda: node2.current_config.interface_set.first()
206+ )
207+ ip1 = yield deferToDatabase(
208+ lambda: self.create_staticipaddress(
209+ params={
210+ "ip": subnet1.get_next_ip_for_allocation()[0],
211+ "interface": iface1,
212+ "subnet": subnet1,
213+ }
214+ )
215+ )
216+ ip2 = yield deferToDatabase(
217+ lambda: self.create_staticipaddress(
218+ params={
219+ "ip": subnet2.get_next_ip_for_allocation()[0],
220+ "interface": iface2,
221+ "subnet": subnet2,
222+ }
223+ )
224+ )
225+ self.start_reading()
226+ try:
227+ yield deferToDatabase(iface1.unlink_ip_address, ip1)
228+ yield deferToDatabase(iface2.unlink_ip_address, ip2)
229+ expected_msgs = (
230+ f"DELETE {domain.name} {node2.hostname} A {ip2.ip}",
231+ f"DELETE {domain.name} {iface2.name}.{node2.hostname} A {ip2.ip}",
232+ )
233+ for exp in expected_msgs:
234+ msg = yield self.get_notify("sys_dns_updates")
235+ self.assertEqual(msg, exp)
236+ finally:
237+ self.stop_reading()
238+ yield self.postgres_listener_service.stopService()
239+
240+ @wait_for_reactor
241+ @inlineCallbacks
242 def test_dns_dynamc_update_ip_update(self):
243 listener = self.make_listener_without_delay()
244 yield self.set_service(listener)

Subscribers

People subscribed via source and target branches