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

Proposed by Jeroen T. Vermeulen
Status: Merged
Approved by: Jeroen T. Vermeulen
Approved revision: 3069
Merged at revision: 3072
Proposed branch: lp:~jtv/maas/bug-1373103
Merge into: lp:maas/trunk
Diff against target: 142 lines (+31/-12)
3 files modified
src/maasserver/preseed.py (+9/-0)
src/maasserver/tests/test_preseed.py (+19/-11)
src/maasserver/views/nodes.py (+3/-1)
To merge this branch: bzr merge lp:~jtv/maas/bug-1373103
Reviewer Review Type Date Requested Status
Blake Rouse (community) Needs Fixing
Raphaël Badin (community) Approve
Review via email: mp+235739@code.launchpad.net

Commit message

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.

Description of the change

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

To post a comment you must log in.
Revision history for this message
Raphaël Badin (rvb) wrote :

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
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

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

Revision history for this message
Scott Moser (smoser) wrote :

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.

Revision history for this message
Blake Rouse (blake-rouse) :
review: Needs Fixing
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/preseed.py'
2--- src/maasserver/preseed.py 2014-09-24 02:58:05 +0000
3+++ src/maasserver/preseed.py 2014-09-24 03:00:34 +0000
4@@ -19,6 +19,7 @@
5 'get_enlist_preseed',
6 'get_preseed',
7 'get_preseed_context',
8+ 'OS_WITH_IPv6_SUPPORT',
9 ]
10
11 from collections import namedtuple
12@@ -71,6 +72,10 @@
13 GENERIC_FILENAME = 'generic'
14
15
16+# Node operating systems which we can deploy with IPv6 networking.
17+OS_WITH_IPv6_SUPPORT = ['ubuntu']
18+
19+
20 def get_enlist_preseed(nodegroup=None):
21 """Return the enlistment preseed.
22
23@@ -117,6 +122,10 @@
24 the `maas_configure_interfaces` script to the node during installation, and
25 running it.
26 """
27+ if node.osystem not in OS_WITH_IPv6_SUPPORT:
28+ # Don't configure networking on this node. It breaks.
29+ return []
30+
31 # Read the script that we will run on the node. It really isn't a
32 # template; it's a simple Python module and it has tests.
33 script = locate_config(
34
35=== modified file 'src/maasserver/tests/test_preseed.py'
36--- src/maasserver/tests/test_preseed.py 2014-09-24 02:58:05 +0000
37+++ src/maasserver/tests/test_preseed.py 2014-09-24 03:00:34 +0000
38@@ -730,7 +730,8 @@
39 class TestComposeCurtinNetworkPreseed(MAASServerTestCase):
40
41 def test__returns_list_of_yaml_strings(self):
42- preseeds = compose_curtin_network_preseed(factory.make_Node())
43+ preseeds = compose_curtin_network_preseed(
44+ factory.make_Node(osystem='ubuntu'))
45 self.assertIsInstance(preseeds, list)
46 self.assertThat(preseeds, HasLength(2))
47 [write_files_yaml, late_commands_yaml] = preseeds
48@@ -741,11 +742,17 @@
49 self.assertIsInstance(late_commands, dict)
50 self.assertEqual(['late_commands'], list(late_commands.keys()))
51
52- def test__uploads_script(self):
53+ def test__returns_empty_if_unsupported_OS(self):
54+ self.assertEqual(
55+ [],
56+ compose_curtin_network_preseed(
57+ factory.make_Node(osystem='windows')))
58+
59+ def test__uploads_script_if_supported_OS(self):
60 [write_files_yaml, _] = compose_curtin_network_preseed(
61- factory.make_Node())
62+ factory.make_Node(osystem='ubuntu'))
63 write_files = yaml.safe_load(write_files_yaml)
64- [file_spec] = list(write_files['write_files'].values())
65+ file_spec = write_files['write_files']['maas_configure_interfaces']
66 self.expectThat(
67 file_spec['path'],
68 Equals('/usr/local/bin/maas_configure_interfaces.py'))
69@@ -755,11 +762,12 @@
70 'maas_configure_interfaces.py')
71 self.expectThat(file_spec['content'], Equals(read_text_file(script)))
72
73- def test__runs_script(self):
74+ def test__runs_script_if_supported_OS(self):
75 [_, late_commands_yaml] = compose_curtin_network_preseed(
76- factory.make_Node())
77+ factory.make_Node(osystem='ubuntu'))
78 late_commands = yaml.safe_load(late_commands_yaml)
79- [command] = list(late_commands['late_commands'].values())
80+ command = (
81+ late_commands['late_commands']['90_maas_configure_interfaces'])
82 self.assertIsInstance(command, list)
83 self.assertEqual(
84 ['curtin', 'in-target', '--'],
85@@ -771,7 +779,7 @@
86 def test__includes_static_IPv6_addresses(self):
87 network = factory.make_ipv6_network()
88 node = factory.make_node_with_mac_attached_to_nodegroupinterface(
89- network=network)
90+ network=network, osystem='ubuntu')
91 mac = node.get_primary_mac()
92 ip = factory.pick_ip_in_network(network)
93 factory.make_StaticIPAddress(mac=mac, ip=ip)
94@@ -783,7 +791,7 @@
95 def test__ignores_static_IPv4_addresses(self):
96 network = factory.make_ipv4_network()
97 node = factory.make_node_with_mac_attached_to_nodegroupinterface(
98- network=network)
99+ network=network, osystem='ubuntu')
100 mac = node.get_primary_mac()
101 ip = factory.pick_ip_in_network(network)
102 factory.make_StaticIPAddress(mac=mac, ip=ip)
103@@ -796,7 +804,7 @@
104 network = factory.make_ipv6_network()
105 gateway = factory.pick_ip_in_network(network)
106 node = factory.make_node_with_mac_attached_to_nodegroupinterface(
107- network=network)
108+ network=network, osystem='ubuntu')
109 mac = node.get_primary_mac()
110 mac.cluster_interface.router_ip = gateway
111 mac.cluster_interface.save()
112@@ -808,7 +816,7 @@
113 def test__ignores_IPv4_gateway_addresses(self):
114 network = factory.make_ipv4_network()
115 node = factory.make_node_with_mac_attached_to_nodegroupinterface(
116- network=network)
117+ network=network, osystem='ubuntu')
118 mac = node.get_primary_mac()
119 gateway = factory.pick_ip_in_network(network)
120 mac.cluster_interface.router_ip = gateway
121
122=== modified file 'src/maasserver/views/nodes.py'
123--- src/maasserver/views/nodes.py 2014-09-20 02:22:59 +0000
124+++ src/maasserver/views/nodes.py 2014-09-24 03:00:34 +0000
125@@ -87,6 +87,7 @@
126 from maasserver.preseed import (
127 get_enlist_preseed,
128 get_preseed,
129+ OS_WITH_IPv6_SUPPORT,
130 )
131 from maasserver.third_party_drivers import get_third_party_driver
132 from maasserver.utils.converters import XMLToYAML
133@@ -550,7 +551,8 @@
134
135 :return: Bool: should the UI show this warning?
136 """
137- if node.osystem == 'ubuntu' and node.boot_type == NODE_BOOT.FASTPATH:
138+ supported_os = (node.osystem in OS_WITH_IPv6_SUPPORT)
139+ if supported_os and node.boot_type == NODE_BOOT.FASTPATH:
140 # MAAS knows how to configure IPv6 addresses on an Ubuntu node
141 # installed with the fast installer. No warning needed.
142 return False