sts-sponsors team mailing list archive

[Alberto Donato <mp+441761@xxxxxxxxxxxxxxxxxx>]

Alberto Donato has proposed merging ~ack/maas:2017504-allow-empty-address-list into maas:master.

Commit message:
LP:#2017504 don't error if address list in reduce_routable_address_map is empty

also, simplify/cleanup logic in Node.get_default_dns_servers


Requested reviews:
  MAAS Maintainers (maas-maintainers)
Related bugs:
  Bug #2017504 in MAAS: "Cannot deploy from the cli when "Allow DNS resolution" is set on minimal subnet"
  https://bugs.launchpad.net/maas/+bug/2017504

For more details, see:
https://code.launchpad.net/~ack/maas/+git/maas/+merge/441761
-- 
Your team MAAS Maintainers is requested to review the proposed merge of ~ack/maas:2017504-allow-empty-address-list into maas:master.

diff --git a/src/maasserver/models/node.py b/src/maasserver/models/node.py
index 00202dd..a986d5e 100644
--- a/src/maasserver/models/node.py
+++ b/src/maasserver/models/node.py
@@ -5240,7 +5240,6 @@ class Node(CleanSave, TimestampedModel):
         if Config.objects.get_config("use_rack_proxy"):
             # LP:1847537 - Filter out MAAS DNS servers running on subnets
             # which do not allow DNS to be provided from MAAS.
-            routable_addrs_map = {}
             for node, addresses in get_routable_address_map(
                 RackController.objects.all(), self
             ).items():
@@ -5305,12 +5304,13 @@ class Node(CleanSave, TimestampedModel):
 
         # Routable rack controllers come before the region controllers when
         # using the rack DNS proxy.
-        maas_dns_servers = list(
-            OrderedDict.fromkeys([str(ip) for ip in maas_dns_servers])
-        )
-        if routable_addrs_map:
-            routable_addrs = reduce_routable_address_map(routable_addrs_map)
-            routable_addrs = list(map(str, routable_addrs))
+        maas_dns_servers = [str(ip) for ip in maas_dns_servers]
+
+        routable_addrs = [
+            str(addr)
+            for addr in reduce_routable_address_map(routable_addrs_map)
+        ]
+        if routable_addrs:
             return routable_addrs + [
                 ip for ip in maas_dns_servers if ip not in routable_addrs
             ]
diff --git a/src/maasserver/routablepairs.py b/src/maasserver/routablepairs.py
index f3687d1..bb7bd2c 100644
--- a/src/maasserver/routablepairs.py
+++ b/src/maasserver/routablepairs.py
@@ -113,5 +113,5 @@ def reduce_routable_address_map(
     for information on how that's derived) so this simply chooses the first.
     """
     for addresses in routable_addrs_map.values():
-        assert len(addresses) >= 1
-        yield addresses[0]
+        if addresses:
+            yield addresses[0]
diff --git a/src/maasserver/tests/test_routablepairs.py b/src/maasserver/tests/test_routablepairs.py
index d9e4f3e..63142bd 100644
--- a/src/maasserver/tests/test_routablepairs.py
+++ b/src/maasserver/tests/test_routablepairs.py
@@ -11,7 +11,7 @@ from testtools import ExpectedException
 from testtools.matchers import AfterPreprocessing, Equals
 
 from maasserver.models.node import Node
-from maasserver.routablepairs import find_addresses_between_nodes
+from maasserver.routablepairs import find_addresses_between_nodes, reduce_routable_address_map
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import MAASServerTestCase
 
@@ -288,3 +288,26 @@ class TestFindAddressesBetweenNodes(MAASServerTestCase):
             find_addresses_between_nodes({origin}, {node_no_match})
         )
         self.assertEqual([], no_matches)
+
+
+class TestReduceRoutableAddressMap(MAASServerTestCase):
+    def test_first_address(self):
+        address_map = {
+            factory.make_Node(): ["1.1.1.1", "2.2.2.2"],
+            factory.make_Node(): ["3.3.3.3"],
+        }
+        self.assertEqual(
+            list(reduce_routable_address_map(address_map)),
+            ["1.1.1.1", "3.3.3.3"],
+        )
+
+    def test_ignore_empty_address_list(self):
+        address_map = {
+            factory.make_Node(): ["1.1.1.1", "2.2.2.2"],
+            factory.make_Node(): [],
+            factory.make_Node(): ["3.3.3.3"],
+        }
+        self.assertEqual(
+            list(reduce_routable_address_map(address_map)),
+            ["1.1.1.1", "3.3.3.3"],
+        )