launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #12513
[Merge] lp:~jtv/maas/cluster-params into lp:maas
Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/cluster-params into lp:maas with lp:~jtv/maas/list-networks as a prerequisite.
Commit message:
In start_cluster_controller: require a uuid argument, discover network interfaces, and pass both to the "register" API call which needs to know both items.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jtv/maas/cluster-params/+merge/126520
See commit message. As discussed with Julian. There is no need to pass a nodegroup name to the API, nor DHCP ranges and broadcast addresses; both can be set in the UI when appropriate.
Jeroen
--
https://code.launchpad.net/~jtv/maas/cluster-params/+merge/126520
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/cluster-params into lp:maas.
=== modified file 'src/provisioningserver/network.py'
--- src/provisioningserver/network.py 2012-09-26 17:47:23 +0000
+++ src/provisioningserver/network.py 2012-09-26 17:47:23 +0000
@@ -18,9 +18,8 @@
'discover_networks',
]
-from io import BytesIO
import os
-from subprocess import check_call
+from subprocess import check_output
class InterfaceInfo:
@@ -29,34 +28,37 @@
def __init__(self, interface):
self.interface = interface
self.ip = None
- self.mask = None
+ self.subnet_mask = None
def may_be_subnet(self):
"""Could this be a subnet that MAAS is interested in?"""
return all([
self.interface != 'lo',
self.ip is not None,
- self.mask is not None,
+ self.subnet_mask is not None,
])
def as_dict(self):
+ """Return information as a dictionary.
+
+ The return value's format is suitable as an interface description
+ for use with the `register` API call.
+ """
return {
'interface': self.interface,
'ip': self.ip,
- 'mask': self.mask,
+ 'subnet_mask': self.subnet_mask,
}
def run_ifconfig():
"""Run `ifconfig` to list active interfaces. Return output."""
env = dict(os.environ, LC_ALL='C')
- stdout = BytesIO()
- check_call(['/sbin/ifconfig'], env=env, stdout=stdout)
- stdout.seek(0)
- return stdout.read().decode('ascii')
-
-
-def extract_ip_and_mask(line):
+ output = check_output(['/sbin/ifconfig'], env=env)
+ return output.decode('ascii')
+
+
+def extract_ip_and_subnet_mask(line):
"""Get IP address and subnet mask from an inet address line."""
# This line consists of key:value pairs separated by double spaces.
# The "inet addr" key contains a space. There is typically a
@@ -80,7 +82,7 @@
info = InterfaceInfo(header.split()[0])
for line in lines[1:]:
if line.split()[0] == 'inet':
- info.ip, info.mask = extract_ip_and_mask(line)
+ info.ip, info.subnet_mask = extract_ip_and_subnet_mask(line)
return info
@@ -100,6 +102,6 @@
"""Find the networks attached to this system."""
output = run_ifconfig()
return [
- interface
+ interface.as_dict()
for interface in parse_ifconfig(output)
if interface.may_be_subnet()]
=== modified file 'src/provisioningserver/start_cluster_controller.py'
--- src/provisioningserver/start_cluster_controller.py 2012-09-25 17:26:46 +0000
+++ src/provisioningserver/start_cluster_controller.py 2012-09-26 17:47:23 +0000
@@ -31,6 +31,7 @@
NoAuth,
)
from provisioningserver.logging import task_logger
+from provisioningserver.network import discover_networks
class ClusterControllerRejected(Exception):
@@ -41,6 +42,8 @@
"""For use by :class:`MainScript`."""
parser.add_argument(
'server_url', metavar='URL', help="URL to the MAAS region controller.")
+ parser.add_argument(
+ '--uuid', metavar='UUID', help="UUID for this cluster controller.")
def log_error(exception):
@@ -54,12 +57,14 @@
return MAASClient(NoAuth(), MAASDispatcher(), server_url)
-def register(server_url):
+def register(server_url, uuid):
"""Request Rabbit connection details from the domain controller.
Offers this machine to the region controller as a potential cluster
controller.
+ :param server_url: URL to the region controller's MAAS API.
+ :param uuid: UUID for this cluster controller.
:return: A dict of connection details if this cluster controller has been
accepted, or `None` if there is no definite response yet. If there
is no definite response, retry this call later.
@@ -67,9 +72,13 @@
cluster controller.
"""
known_responses = {httplib.OK, httplib.FORBIDDEN, httplib.ACCEPTED}
+
+ interfaces = json.dumps(discover_networks())
client = make_anonymous_api_client(server_url)
try:
- response = client.post('api/1.0/nodegroups/', 'register')
+ response = client.post(
+ 'api/1.0/nodegroups/', 'register',
+ uuid=uuid, interfaces=interfaces)
except HTTPError as e:
status_code = e.code
if e.code not in known_responses:
@@ -150,7 +159,7 @@
If this system is still awaiting approval as a cluster controller, this
command will keep looping until it gets a definite answer.
"""
- connection_details = register(args.server_url)
+ connection_details = register(args.server_url, args.uuid)
while connection_details is None:
sleep(60)
connection_details = register(args.server_url)
=== modified file 'src/provisioningserver/tests/test_network.py'
--- src/provisioningserver/tests/test_network.py 2012-09-26 17:47:23 +0000
+++ src/provisioningserver/tests/test_network.py 2012-09-26 17:47:23 +0000
@@ -22,31 +22,18 @@
from provisioningserver import network
-class FakeCheckCall:
- """Test double for `check_call`."""
-
- def __init__(self, output_text):
- self.output_text = output_text
- self.calls = []
-
- def __call__(self, command, stdout=None, env=None):
- stdout.write(self.output_text.encode('ascii'))
- self.calls.append(dict(command=command, env=env))
- return 0
-
-
def make_address_line(**kwargs):
"""Create an inet address line."""
# First word on this line is inet or inet6.
kwargs.setdefault('inet', 'inet')
kwargs.setdefault('broadcast', '10.255.255.255')
- kwargs.setdefault('mask', '255.0.0.0')
+ kwargs.setdefault('subnet_mask', '255.0.0.0')
items = [
"%(inet)s addr:%(ip)s"
]
if len(kwargs['broadcast']) > 0:
items.append("Bcast:%(broadcast)s")
- items.append("Mask:%(mask)s")
+ items.append("Mask:%(subnet_mask)s")
return ' '.join(items) % kwargs
@@ -189,7 +176,7 @@
def test_run_ifconfig_returns_ifconfig_output(self):
text = join_stanzas([make_stanza()])
- self.patch(network, 'check_call', FakeCheckCall(text))
+ self.patch(network, 'check_output').return_value = text.encode('ascii')
self.assertEqual(text, network.run_ifconfig())
def test_parse_ifconfig_produces_interface_info(self):
@@ -205,7 +192,7 @@
parms = {
'interface': factory.make_name('eth'),
'ip': factory.getRandomIPAddress(),
- 'mask': '255.255.255.128',
+ 'subnet_mask': '255.255.255.128',
}
info = network.parse_stanza(make_stanza(**parms))
self.assertEqual(parms, info.as_dict())
@@ -214,7 +201,7 @@
parms = {
'interface': factory.make_name('eth'),
'ip': factory.getRandomIPAddress(),
- 'mask': '255.255.255.128',
+ 'subnet_mask': '255.255.255.128',
'interrupt': '',
}
info = network.parse_stanza(make_stanza(**parms))
@@ -230,14 +217,14 @@
info = network.parse_stanza(make_stanza(**parms))
expected = parms.copy()
expected['ip'] = None
- expected['mask'] = None
+ expected['subnet_mask'] = None
self.assertEqual(expected, info.as_dict())
def test_parse_stanza_returns_nothing_for_loopback(self):
parms = {
'interface': 'lo',
'ip': '127.1.2.3',
- 'mask': '255.0.0.0',
+ 'subnet_mask': '255.0.0.0',
'encapsulation': 'Local Loopback',
'broadcast': '',
'interrupt': '',
@@ -258,17 +245,21 @@
[stanza.strip() for stanza in stanzas],
network.split_stanzas(full_output))
+ def test_discover_networks_runs_in_real_life(self):
+ interfaces = network.discover_networks()
+ self.assertIsInstance(interfaces, list)
+
def test_discover_networks_returns_suitable_interfaces(self):
params = {
'interface': factory.make_name('eth'),
'ip': factory.getRandomIPAddress(),
- 'mask': '255.255.255.0',
+ 'subnet_mask': '255.255.255.0',
}
regular_interface = make_stanza(**params)
loopback = make_stanza(
interface='lo', encapsulation='Local loopback', broadcast='',
interrupt='')
- disabled_interface = make_stanza(ip='', broadcast='', mask='')
+ disabled_interface = make_stanza(ip='', broadcast='', subnet_mask='')
text = join_stanzas([regular_interface, loopback, disabled_interface])
self.patch(network, 'run_ifconfig').return_value = text
@@ -277,11 +268,11 @@
self.assertEqual(
[params],
- [interface.as_dict() for interface in interfaces])
+ [interface for interface in interfaces])
def test_discover_networks_processes_real_ifconfig_output(self):
self.patch(network, 'run_ifconfig').return_value = sample_output
info = network.discover_networks()
self.assertEqual(
['eth1', 'maasbr0', 'virbr0'],
- [interface.interface for interface in info])
+ [interface['interface'] for interface in info])
=== modified file 'src/provisioningserver/tests/test_start_cluster_controller.py'
--- src/provisioningserver/tests/test_start_cluster_controller.py 2012-09-25 17:26:46 +0000
+++ src/provisioningserver/tests/test_start_cluster_controller.py 2012-09-26 17:47:23 +0000
@@ -42,7 +42,22 @@
"""
-FakeArgs = namedtuple('FakeArgs', ['server_url'])
+def make_url(name_hint='host'):
+ return "http://%s.example.com/%s/"; % (
+ factory.make_name(name_hint),
+ factory.make_name('path'),
+ )
+
+
+FakeArgs = namedtuple('FakeArgs', ['server_url', 'uuid'])
+
+
+def make_args(server_url=None, uuid=None):
+ if server_url is None:
+ server_url = make_url('region')
+ if uuid is None:
+ uuid = factory.getRandomUUID()
+ return FakeArgs(server_url, uuid)
class FakeURLOpenResponse:
@@ -59,13 +74,6 @@
return self._status_code
-def make_url(name_hint='host'):
- return "http://%s.example.com/%s/"; % (
- factory.make_name(name_hint),
- factory.make_name('path'),
- )
-
-
class TestStartClusterController(PservTestCase):
def setUp(self):
@@ -79,6 +87,20 @@
'BROKER_URL': make_url('broker'),
}
+ def parse_headers_and_body(self, headers, body):
+ """Parse ingredients of a web request.
+
+ The headers and body are as passed to :class:`MAASDispatcher`.
+ """
+ # Make Django STFU; just using Django's multipart code causes it to
+ # pull in a settings module, and it will throw up if it can't.
+ self.useFixture(
+ EnvironmentVariableFixture(
+ "DJANGO_SETTINGS_MODULE", __name__))
+
+ post, files = parse_headers_and_body_with_django(headers, body)
+ return post, files
+
def prepare_response(self, http_code, content=""):
"""Prepare to return the given http response from API request."""
fake = self.patch(MAASDispatcher, 'dispatch_query')
@@ -108,14 +130,17 @@
self.prepare_success_response()
parser = ArgumentParser()
start_cluster_controller.add_arguments(parser)
- start_cluster_controller.run(parser.parse_args((make_url(), )))
+ start_cluster_controller.run(parser.parse_args((
+ make_url(),
+ '--uuid', factory.getRandomUUID(),
+ )))
self.assertNotEqual(0, start_cluster_controller.Popen.call_count)
def test_uses_given_url(self):
url = make_url('region')
self.patch(start_cluster_controller, 'start_up')
self.prepare_success_response()
- start_cluster_controller.run(FakeArgs(url))
+ start_cluster_controller.run(make_args(server_url=url))
(args, kwargs) = MAASDispatcher.dispatch_query.call_args
self.assertEqual(url + 'api/1.0/nodegroups/', args[0])
@@ -124,7 +149,7 @@
self.prepare_rejection_response()
self.assertRaises(
start_cluster_controller.ClusterControllerRejected,
- start_cluster_controller.run, FakeArgs(make_url()))
+ start_cluster_controller.run, make_args())
self.assertItemsEqual([], start_cluster_controller.start_up.calls_list)
def test_polls_while_pending(self):
@@ -132,7 +157,7 @@
self.prepare_pending_response()
self.assertRaises(
Sleeping,
- start_cluster_controller.run, FakeArgs(make_url()))
+ start_cluster_controller.run, make_args())
self.assertItemsEqual([], start_cluster_controller.start_up.calls_list)
def test_polls_on_unexpected_errors(self):
@@ -141,14 +166,33 @@
make_url(), httplib.REQUEST_TIMEOUT, "Timeout.", '', BytesIO())
self.assertRaises(
Sleeping,
- start_cluster_controller.run, FakeArgs(make_url()))
+ start_cluster_controller.run, make_args())
self.assertItemsEqual([], start_cluster_controller.start_up.calls_list)
+ def test_register_passes_cluster_information(self):
+ self.prepare_success_response()
+ uuid = factory.getRandomUUID()
+ interface = {
+ 'interface': factory.make_name('eth'),
+ 'ip': factory.getRandomIPAddress(),
+ 'subnet_mask': '255.255.255.0',
+ }
+ discover = self.patch(start_cluster_controller, 'discover_networks')
+ discover.return_value = [interface]
+
+ start_cluster_controller.register(make_url(), uuid)
+
+ (args, kwargs) = MAASDispatcher.dispatch_query.call_args
+ headers, body = kwargs["headers"], kwargs["data"]
+ post, files = self.parse_headers_and_body(headers, body)
+ self.assertEqual([interface], json.loads(post['interfaces']))
+ self.assertEqual(uuid, post['uuid'])
+
def test_starts_up_once_accepted(self):
self.patch(start_cluster_controller, 'start_up')
connection_details = self.prepare_success_response()
server_url = make_url()
- start_cluster_controller.run(FakeArgs(server_url))
+ start_cluster_controller.run(make_args(server_url=server_url))
start_cluster_controller.start_up.assert_called_once_with(
server_url, connection_details)
@@ -157,7 +201,7 @@
connection_details = self.make_connection_details()
self.patch(start_cluster_controller, 'Popen')
self.patch(start_cluster_controller, 'sleep')
- self.prepare_response('OK', httplib.OK)
+ self.prepare_success_response()
start_cluster_controller.start_up(url, connection_details)
@@ -165,14 +209,8 @@
self.assertEqual(url + 'api/1.0/nodegroups/', args[0])
self.assertEqual('POST', kwargs['method'])
- # Make Django STFU; just using Django's multipart code causes it to
- # pull in a settings module, and it will throw up if it can't.
- self.useFixture(
- EnvironmentVariableFixture(
- "DJANGO_SETTINGS_MODULE", __name__))
-
headers, body = kwargs["headers"], kwargs["data"]
- post, files = parse_headers_and_body_with_django(headers, body)
+ post, files = self.parse_headers_and_body(headers, body)
self.assertEqual("refresh_workers", post["op"])
def test_start_up_ignores_failure_on_refresh_secrets(self):
