Merge lp:~jtv/maas/bug-1373103 into lp:~maas-committers/maas/trunk

[Jeroen T. Vermeulen]

Install/run network configuration script only on Ubuntu. Installing it through curtin and then running it through curtin doesn't seem to work on other operating systems, even CentOS: for some reason the file is not found.

Unfortunately I can't Q/A this with full precision because of bug 1373207. I'm trying it out by hand-editing the code on a test machine.

Jeroen

[Raphaël Badin (rvb)]

Okay, I understand it is quite urgent to fix this so I'm approving this MP.

It's probably worth adding a little note to state that this is a temporary fix because it's really ugly as it stands.

Not trying to configure the interfaces if the file is not present might be a better fix.

In any case, if it wasn't temporary, this code would really belong to the OS code.

review: Approve

[Jeroen T. Vermeulen (jtv)]

The script is about to be removed. The replacement could be integrated with the OS registry, although the RPC calls involved might complicate things.

[Scott Moser (smoser)]

I'd not bother carrying the hack much further than we have it already.
the goal is for maas to declarel networking information to the installer (curtin), and that to handle figuring out what to do with that network information.

i wouldn't bother wan an "OS registry" and scripts. we want maas to be stupid and images it installs to be smart. The image it installs provides a /opt/curtin/hook that has access to the network config that maas provided, and has to render that to in-operating system networking.

that way, maas is stupid, and the information about the OS is in the OS.

the change you have here is fine, but to me it seems like the wrong direction to "integrate with the OS registry", whatever that means.

[Blake Rouse (blake-rouse)]

review: Needs Fixing

[Jeroen T. Vermeulen (jtv)]

Thanks for pointing that out, Blake. I have a fix up for review: https://code.launchpad.net/~jtv/maas/use-get_osystem-not-osystem/+merge/235895

=== modified file 'src/maasserver/preseed.py'
--- src/maasserver/preseed.py	2014-09-24 02:58:05 +0000
+++ src/maasserver/preseed.py	2014-09-24 03:00:34 +0000
@@ -19,6 +19,7 @@
     'get_enlist_preseed',
     'get_preseed',
     'get_preseed_context',
+    'OS_WITH_IPv6_SUPPORT',
     ]
 
 from collections import namedtuple
@@ -71,6 +72,10 @@
 GENERIC_FILENAME = 'generic'
 
 
+# Node operating systems which we can deploy with IPv6 networking.
+OS_WITH_IPv6_SUPPORT = ['ubuntu']
+
+
 def get_enlist_preseed(nodegroup=None):
     """Return the enlistment preseed.
 
@@ -117,6 +122,10 @@
     the `maas_configure_interfaces` script to the node during installation, and
     running it.
     """
+    if node.osystem not in OS_WITH_IPv6_SUPPORT:
+        # Don't configure networking on this node.  It breaks.
+        return []
+
     # Read the script that we will run on the node.  It really isn't a
     # template; it's a simple Python module and it has tests.
     script = locate_config(

=== modified file 'src/maasserver/tests/test_preseed.py'
--- src/maasserver/tests/test_preseed.py	2014-09-24 02:58:05 +0000
+++ src/maasserver/tests/test_preseed.py	2014-09-24 03:00:34 +0000
@@ -730,7 +730,8 @@
 class TestComposeCurtinNetworkPreseed(MAASServerTestCase):
 
     def test__returns_list_of_yaml_strings(self):
-        preseeds = compose_curtin_network_preseed(factory.make_Node())
+        preseeds = compose_curtin_network_preseed(
+            factory.make_Node(osystem='ubuntu'))
         self.assertIsInstance(preseeds, list)
         self.assertThat(preseeds, HasLength(2))
         [write_files_yaml, late_commands_yaml] = preseeds
@@ -741,11 +742,17 @@
         self.assertIsInstance(late_commands, dict)
         self.assertEqual(['late_commands'], list(late_commands.keys()))
 
-    def test__uploads_script(self):
+    def test__returns_empty_if_unsupported_OS(self):
+        self.assertEqual(
+            [],
+            compose_curtin_network_preseed(
+                factory.make_Node(osystem='windows')))
+
+    def test__uploads_script_if_supported_OS(self):
         [write_files_yaml, _] = compose_curtin_network_preseed(
-            factory.make_Node())
+            factory.make_Node(osystem='ubuntu'))
         write_files = yaml.safe_load(write_files_yaml)
-        [file_spec] = list(write_files['write_files'].values())
+        file_spec = write_files['write_files']['maas_configure_interfaces']
         self.expectThat(
             file_spec['path'],
             Equals('/usr/local/bin/maas_configure_interfaces.py'))
@@ -755,11 +762,12 @@
             'maas_configure_interfaces.py')
         self.expectThat(file_spec['content'], Equals(read_text_file(script)))
 
-    def test__runs_script(self):
+    def test__runs_script_if_supported_OS(self):
         [_, late_commands_yaml] = compose_curtin_network_preseed(
-            factory.make_Node())
+            factory.make_Node(osystem='ubuntu'))
         late_commands = yaml.safe_load(late_commands_yaml)
-        [command] = list(late_commands['late_commands'].values())
+        command = (
+            late_commands['late_commands']['90_maas_configure_interfaces'])
         self.assertIsInstance(command, list)
         self.assertEqual(
             ['curtin', 'in-target', '--'],
@@ -771,7 +779,7 @@
     def test__includes_static_IPv6_addresses(self):
         network = factory.make_ipv6_network()
         node = factory.make_node_with_mac_attached_to_nodegroupinterface(
-            network=network)
+            network=network, osystem='ubuntu')
         mac = node.get_primary_mac()
         ip = factory.pick_ip_in_network(network)
         factory.make_StaticIPAddress(mac=mac, ip=ip)
@@ -783,7 +791,7 @@
     def test__ignores_static_IPv4_addresses(self):
         network = factory.make_ipv4_network()
         node = factory.make_node_with_mac_attached_to_nodegroupinterface(
-            network=network)
+            network=network, osystem='ubuntu')
         mac = node.get_primary_mac()
         ip = factory.pick_ip_in_network(network)
         factory.make_StaticIPAddress(mac=mac, ip=ip)
@@ -796,7 +804,7 @@
         network = factory.make_ipv6_network()
         gateway = factory.pick_ip_in_network(network)
         node = factory.make_node_with_mac_attached_to_nodegroupinterface(
-            network=network)
+            network=network, osystem='ubuntu')
         mac = node.get_primary_mac()
         mac.cluster_interface.router_ip = gateway
         mac.cluster_interface.save()
@@ -808,7 +816,7 @@
     def test__ignores_IPv4_gateway_addresses(self):
         network = factory.make_ipv4_network()
         node = factory.make_node_with_mac_attached_to_nodegroupinterface(
-            network=network)
+            network=network, osystem='ubuntu')
         mac = node.get_primary_mac()
         gateway = factory.pick_ip_in_network(network)
         mac.cluster_interface.router_ip = gateway

=== modified file 'src/maasserver/views/nodes.py'
--- src/maasserver/views/nodes.py	2014-09-20 02:22:59 +0000
+++ src/maasserver/views/nodes.py	2014-09-24 03:00:34 +0000
@@ -87,6 +87,7 @@
 from maasserver.preseed import (
     get_enlist_preseed,
     get_preseed,
+    OS_WITH_IPv6_SUPPORT,
     )
 from maasserver.third_party_drivers import get_third_party_driver
 from maasserver.utils.converters import XMLToYAML
@@ -550,7 +551,8 @@
 
         :return: Bool: should the UI show this warning?
         """
-        if node.osystem == 'ubuntu' and node.boot_type == NODE_BOOT.FASTPATH:
+        supported_os = (node.osystem in OS_WITH_IPv6_SUPPORT)
+        if supported_os and node.boot_type == NODE_BOOT.FASTPATH:
             # MAAS knows how to configure IPv6 addresses on an Ubuntu node
             # installed with the fast installer.  No warning needed.
             return False