Merge lp:~jtv/maas/lint-2014-07-07 into lp:~maas-committers/maas/trunk

[Jeroen T. Vermeulen]

Nontrivial lint. Extract awkwardly formatted, repeated check "is cluster interface managed with a static IP range?" into a helper.

[Raphaël Badin (rvb)]

Looks good. Weird that the static range can be partially defined; it doesn't look like it's checked by the form.

review: Approve

[Jeroen T. Vermeulen (jtv)]

Yeah. I didn't want to drag yet more discussion into a lint branch, but it does seem strange.

[Julian Edwards (julian-edwards)]

On Monday 07 Jul 2014 09:50:22 you wrote:
> Yeah. I didn't want to drag yet more discussion into a lint branch, but it
> does seem strange.

It's an oversight.

=== modified file 'src/maasserver/models/nodegroupinterface.py'
--- src/maasserver/models/nodegroupinterface.py	2014-07-07 07:00:06 +0000
+++ src/maasserver/models/nodegroupinterface.py	2014-07-07 09:33:23 +0000
@@ -223,10 +223,19 @@
             # form; validation breaks if we pass an IPAddress.
             self.broadcast_ip = unicode(network.broadcast)
 
+    def manages_static_range(self):
+        """Is this a managed interface with a static IP range configured?"""
+        # Deliberately vague implicit conversion to bool: a blank IP address
+        # can show up internally as either None or an empty string.
+        return (
+            self.is_managed and
+            self.static_ip_range_low and
+            self.static_ip_range_high
+            )
+
     def clean_ip_range_bounds(self):
         """Ensure that the static and dynamic ranges have sane bounds."""
-        if not (self.is_managed and (self.static_ip_range_low and
-            self.static_ip_range_high)):
+        if not self.manages_static_range():
             # Exit early with nothing to do.
             return
 
@@ -259,8 +268,7 @@
 
     def clean_ip_ranges(self):
         """Ensure that the static and dynamic ranges don't overlap."""
-        if not (self.is_managed and (self.static_ip_range_low and
-            self.static_ip_range_high)):
+        if not self.manages_static_range():
             # Nothing to do; bail out.
             return
 

=== modified file 'src/maasserver/models/tests/test_nodegroupinterface.py'
--- src/maasserver/models/tests/test_nodegroupinterface.py	2014-07-03 16:06:59 +0000
+++ src/maasserver/models/tests/test_nodegroupinterface.py	2014-07-07 09:33:23 +0000
@@ -305,3 +305,39 @@
                     interface.static_ip_range_high)],
             }
         self.assertEqual(errors, exception.message_dict)
+
+    def test_manages_static_range_returns_False_if_not_managed(self):
+        cluster = factory.make_node_group()
+        network = IPNetwork("10.9.9.0/24")
+        interface = factory.make_node_group_interface(
+            cluster, network=network,
+            ip_range_low='10.9.9.10', ip_range_high='10.9.9.50',
+            static_ip_range_low='10.9.9.100',
+            static_ip_range_high='10.9.9.200',
+            management=NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED)
+        self.assertFalse(interface.manages_static_range())
+
+    def test_manages_static_range_returns_False_if_no_static_range(self):
+        network = IPNetwork("10.9.9.0/24")
+        interface = make_interface(network)
+        interface.static_ip_range_low = None
+        interface.static_ip_range_high = None
+        self.assertFalse(interface.manages_static_range())
+
+    def test_manages_static_range_returns_False_if_partial_static_range(self):
+        network = IPNetwork("10.9.9.0/24")
+        interface = make_interface(network)
+        interface.static_ip_range_low = '10.99.99.100'
+        interface.static_ip_range_high = None
+        self.assertFalse(interface.manages_static_range())
+
+    def test_manages_static_range_returns_True_if_manages_static_range(self):
+        cluster = factory.make_node_group()
+        network = IPNetwork("10.9.9.0/24")
+        interface = factory.make_node_group_interface(
+            cluster, network=network,
+            ip_range_low='10.9.9.10', ip_range_high='10.9.9.50',
+            static_ip_range_low='10.9.9.100',
+            static_ip_range_high='10.9.9.200',
+            management=NODEGROUPINTERFACE_MANAGEMENT.DHCP)
+        self.assertTrue(interface.manages_static_range())