MAAS

Merge lp:~julian-edwards/maas/ipmi-with-mac into lp:~maas-committers/maas/trunk

  1. ipmi-with-mac
  2. Merge into trunk
Proposed by Julian Edwards
Status: Merged
Approved by: Julian Edwards
Approved revision: no longer in the source branch.
Merged at revision: 2029
Proposed branch: lp:~julian-edwards/maas/ipmi-with-mac
Merge into: lp:~maas-committers/maas/trunk
Diff against target: 102 lines (+21/-8)
6 files modified
etc/maas/templates/power/ipmi.template (+6/-0)
src/maasserver/models/node.py (+2/-2)
src/maasserver/models/tests/test_node.py (+6/-4)
src/maasserver/power_parameters.py (+5/-1)
src/provisioningserver/power/tests/test_poweraction.py (+1/-1)
src/provisioningserver/tasks.py (+1/-0)
To merge this branch: bzr merge lp:~julian-edwards/maas/ipmi-with-mac
Reviewer Review Type Date Requested Status
Jeroen T. Vermeulen (community) Approve
Review via email: mp+208286@code.launchpad.net

Commit message

Allow IPMI to define the MAC address of the BMC in its parameters, and use the ARPed IP address in preference to the IP address in power_address. This is the first exploratory step in removing power_address entirely because it is too unreliable in the face of changing DHCP addresses.

Description of the change

This branch has the side effect of fixing an AMT problem I just noticed - if someone was not setting any power parameters and relying on the node's mac address, the power_pass would never get sent to the template!

To post a comment you must log in.
Revision history for this message
Tycho Andersen (tycho-s) wrote :

Looks good, thanks!

Revision history for this message
MAAS Lander (maas-lander) wrote :

Attempt to merge into lp:maas failed due to conflicts:

text conflict in src/maasserver/power_parameters.py

Revision history for this message
Julian Edwards (julian-edwards) wrote :

Let me also add that I have tested this on my Microservers. If the MAC is invalid or can't be found it calls back to the configured IP address, otherwise seems to work fine.

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Well done for actually documenting your intentions. I guess the lookup of the IP address was already in there somewhere.

One thing that still makes me nervous is the "primary MAC" idea — what if an existing installation upgrades, has its power addresses changed, and now finds that a node's primary MAC wasn't on a network that talks to the cluster controller? Once we start auto-discovering networks, we can start making more informed choices.

review: Approve
Revision history for this message
Julian Edwards (julian-edwards) wrote :

Yes, ip_address is populated if mac_address is set. Hence my change to the code you wrote and my questions last night :)

Revision history for this message
MAAS Lander (maas-lander) wrote :

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

Revision history for this message
MAAS Lander (maas-lander) wrote :
Download full text (15.5 KiB)

The attempt to merge lp:~julian-edwards/maas/ipmi-with-mac into lp:maas failed. Below is the output from the failed tests.

Ign http://security.ubuntu.com trusty-security InRelease
Hit http://security.ubuntu.com trusty-security Release.gpg
Hit http://security.ubuntu.com trusty-security Release
Ign http://nova.clouds.archive.ubuntu.com trusty InRelease
Ign http://nova.clouds.archive.ubuntu.com trusty-updates InRelease
Get:1 http://nova.clouds.archive.ubuntu.com trusty Release.gpg [933 B]
Hit http://nova.clouds.archive.ubuntu.com trusty-updates Release.gpg
Get:2 http://nova.clouds.archive.ubuntu.com trusty Release [58.5 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty-updates Release
Hit http://security.ubuntu.com trusty-security/main Sources
Hit http://security.ubuntu.com trusty-security/universe Sources
Hit http://security.ubuntu.com trusty-security/main amd64 Packages
Hit http://security.ubuntu.com trusty-security/universe amd64 Packages
Hit http://security.ubuntu.com trusty-security/main Translation-en
Hit http://security.ubuntu.com trusty-security/universe Translation-en
Get:3 http://nova.clouds.archive.ubuntu.com trusty/main Sources [1,062 kB]
Ign http://security.ubuntu.com trusty-security/main Translation-en_US
Ign http://security.ubuntu.com trusty-security/universe Translation-en_US
Get:4 http://nova.clouds.archive.ubuntu.com trusty/universe Sources [6,404 kB]
Get:5 http://nova.clouds.archive.ubuntu.com trusty/main amd64 Packages [1,344 kB]
Get:6 http://nova.clouds.archive.ubuntu.com trusty/universe amd64 Packages [5,876 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main Sources
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe Sources
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en
Ign http://nova.clouds.archive.ubuntu.com trusty/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en_US
Fetched 14.7 MB in 5s (2,933 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
     --no-install-recommends install apache2 bind9 bind9utils build-essential curl daemontools distro-info dnsutils firefox freeipmi-tools ipython isc-dhcp-common libjs-raphael libjs-yui3-full libjs-yui3-min libpq-dev make postgresql python-amqplib python-bzrlib python-celery python-convoy python-cssselect python-curtin python-dev python-distro-info python-django python-django-piston python-django-south python-djorm-ext-pgarray python-docutils python-formencode python-httplib2 python-jinja2 python-jsonschema python-lockfile python-lxml python-netaddr python-netifaces python-oauth python-oops python-oops-amqp ...

Revision history for this message
MAAS Lander (maas-lander) wrote :
Download full text (15.7 KiB)

The attempt to merge lp:~julian-edwards/maas/ipmi-with-mac into lp:maas failed. Below is the output from the failed tests.

Ign http://security.ubuntu.com trusty-security InRelease
Ign http://nova.clouds.archive.ubuntu.com trusty InRelease
Hit http://security.ubuntu.com trusty-security Release.gpg
Ign http://nova.clouds.archive.ubuntu.com trusty-updates InRelease
Hit http://security.ubuntu.com trusty-security Release
Get:1 http://nova.clouds.archive.ubuntu.com trusty Release.gpg [933 B]
Hit http://nova.clouds.archive.ubuntu.com trusty-updates Release.gpg
Get:2 http://nova.clouds.archive.ubuntu.com trusty Release [58.5 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty-updates Release
Hit http://security.ubuntu.com trusty-security/main Sources
Hit http://security.ubuntu.com trusty-security/universe Sources
Hit http://security.ubuntu.com trusty-security/main amd64 Packages
Hit http://security.ubuntu.com trusty-security/universe amd64 Packages
Hit http://security.ubuntu.com trusty-security/main Translation-en
Hit http://security.ubuntu.com trusty-security/universe Translation-en
Get:3 http://nova.clouds.archive.ubuntu.com trusty/main Sources [1,062 kB]
Ign http://security.ubuntu.com trusty-security/main Translation-en_US
Ign http://security.ubuntu.com trusty-security/universe Translation-en_US
Get:4 http://nova.clouds.archive.ubuntu.com trusty/universe Sources [6,404 kB]
Get:5 http://nova.clouds.archive.ubuntu.com trusty/main amd64 Packages [1,344 kB]
Get:6 http://nova.clouds.archive.ubuntu.com trusty/universe amd64 Packages [5,876 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main Sources
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe Sources
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en
Ign http://nova.clouds.archive.ubuntu.com trusty/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en_US
Fetched 14.7 MB in 4s (2,980 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
     --no-install-recommends install apache2 bind9 bind9utils build-essential curl daemontools distro-info dnsutils firefox freeipmi-tools ipython isc-dhcp-common libjs-raphael libjs-yui3-full libjs-yui3-min libpq-dev make postgresql python-amqplib python-bzrlib python-celery python-convoy python-cssselect python-curtin python-dev python-distro-info python-django python-django-piston python-django-south python-djorm-ext-pgarray python-docutils python-formencode python-httplib2 python-jinja2 python-jsonschema python-lockfile python-lxml python-netaddr python-netifaces python-oauth python-oops python-oops-amqp ...

Revision history for this message
Julian Edwards (julian-edwards) wrote :

On 27/02/14 12:49, MaaS Lander wrote:
> FAIL: metadataserver.models.tests.test_commissioningscript.TestLLDPScripts.test_wait_script_waits_for_lldpd
> ----------------------------------------------------------------------
> _StringException: Traceback (most recent call last):
> File "/tmp/tarmac/branch.VURdtZ/src/metadataserver/models/tests/test_commissioningscript.py", line 268, in test_wait_script_waits_for_lldpd
> time.time.return_value))
> File "/home/ubuntu/.buildout/eggs/testtools-0.9.34-py2.7.egg/testtools/testcase.py", line 414, in assertThat
> raise MismatchError(matchee, matcher, mismatch, verbose)
> MismatchError: Expected to be called once. Called 206 times.

ARGHHHHHHHH.

That's it I am disabling that test.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'etc/maas/templates/power/ipmi.template'
2--- etc/maas/templates/power/ipmi.template 2013-04-22 16:24:50 +0000
3+++ etc/maas/templates/power/ipmi.template 2014-02-27 02:34:14 +0000
4@@ -13,6 +13,12 @@
5 ipmi_chassis_config={{ipmi_chassis_config}}
6 config={{config_dir}}/{{ipmi_config}}
7
8+# If ip_address was supplied, use it in preference to the power_address,
9+# because it gets discovered on-the-fly based on mac_address.
10+{{if ip_address}}
11+power_address={{ip_address}}
12+{{endif}}
13+
14 # Determines the power command needed to execute the desired
15 # action. This function receives ${power_change} as argument.
16 formulate_power_command() {
17
18=== modified file 'src/maasserver/models/node.py'
19--- src/maasserver/models/node.py 2014-02-25 15:53:10 +0000
20+++ src/maasserver/models/node.py 2014-02-27 02:34:14 +0000
21@@ -774,8 +774,8 @@
22 power_params.setdefault('power_driver', '')
23
24 # The "mac" parameter defaults to the node's primary MAC
25- # address, but only if no power parameters were set at all.
26- if not self.power_parameters:
27+ # address, but only if not already set.
28+ if 'mac_address' not in power_params:
29 primary_mac = self.get_primary_mac()
30 if primary_mac is not None:
31 power_params['mac_address'] = primary_mac.mac_address
32
33=== modified file 'src/maasserver/models/tests/test_node.py'
34--- src/maasserver/models/tests/test_node.py 2014-02-21 09:25:46 +0000
35+++ src/maasserver/models/tests/test_node.py 2014-02-27 02:34:14 +0000
36@@ -1083,16 +1083,18 @@
37 self.celery.tasks[0]['kwargs']['mac_address'],
38 ))
39
40- def test_start_nodes_wakeonlan_ignores_invalid_parameters(self):
41+ def test_start_nodes_wakeonlan_falls_back_to_primary_mac(self):
42 # If node.power_params is set but doesn't have "mac_address" in it,
43- # then the node shouldn't be started.
44+ # then use the node's primary MAC.
45 user = factory.make_user()
46 node, mac = self.make_node_with_mac(
47 user, power_type='ether_wake',
48 power_parameters=dict(jarjar="binks"))
49 output = Node.objects.start_nodes([node.system_id], user)
50- self.assertItemsEqual([], output)
51- self.assertEqual([], self.celery.tasks)
52+ self.assertItemsEqual([node], output)
53+ self.assertEqual(
54+ unicode(node.get_primary_mac()),
55+ self.celery.tasks[0]['kwargs']['mac_address'])
56
57 def test_start_nodes_wakeonlan_ignores_empty_mac_address_parameter(self):
58 user = factory.make_user()
59
60=== modified file 'src/maasserver/power_parameters.py'
61--- src/maasserver/power_parameters.py 2014-02-26 16:20:56 +0000
62+++ src/maasserver/power_parameters.py 2014-02-27 02:34:14 +0000
63@@ -173,9 +173,13 @@
64 make_json_field(
65 'power_driver', "Power driver", field_type='choice',
66 choices=IPMI_DRIVER_CHOICES, default=IPMI_DRIVER.LAN_2_0),
67- make_json_field('power_address', "Power address"),
68+ make_json_field('power_address', "IP address"),
69 make_json_field('power_user', "Power user"),
70 make_json_field('power_pass', "Power password"),
71+ make_json_field(
72+ 'mac_address',
73+ "MAC address - the IP is looked up with ARP and overrides "
74+ "IP address"),
75 ],
76 },
77 {
78
79=== modified file 'src/provisioningserver/power/tests/test_poweraction.py'
80--- src/provisioningserver/power/tests/test_poweraction.py 2014-01-14 01:03:56 +0000
81+++ src/provisioningserver/power/tests/test_poweraction.py 2014-02-27 02:34:14 +0000
82@@ -207,7 +207,7 @@
83 action.get_template(), power_change='on',
84 power_address='mystystem', power_user='me', power_pass='me',
85 ipmipower='echo', ipmi_chassis_config='echo', config_dir='dir',
86- ipmi_config='file.conf', power_driver='LAN')
87+ ipmi_config='file.conf', power_driver='LAN', ip_address='')
88 self.assertIn(conf_dir, script)
89
90 def test_moonshot_checks_state(self):
91
92=== modified file 'src/provisioningserver/tasks.py'
93--- src/provisioningserver/tasks.py 2014-02-24 01:42:18 +0000
94+++ src/provisioningserver/tasks.py 2014-02-27 02:34:14 +0000
95@@ -137,6 +137,7 @@
96 kwargs['power_change'] = power_change
97 if 'mac_address' in kwargs:
98 kwargs['ip_address'] = find_ip_via_arp(kwargs['mac_address'])
99+ kwargs.setdefault('ip_address', None)
100 try:
101 pa = PowerAction(power_type)
102 pa.execute(**kwargs)