Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/pre-virsh-power into
lp:maas.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jtv/maas/pre-virsh-power/+merge/107963
This rearranges the way we compose power parameters a bit, so that it can fit
the virsh use-case without special-purpose code. The virsh power type needs to
know the system_id, because it does not identify a node by its MAC. (I know
because I have that branch already underway; this work has been discussed with
Julian).
The change means that sometimes we'll look up a node's primary MAC when it's
not needed. I don't think it's a very expensive operation; there's things we
could do to avoid the database access, at the cost of some special-casing, but
that's micro-optimization.
I'm not entirely convinced that the defaulting behaviour I chose is what we
want, but I just kept the existing behaviour and created a place to encapsulate
it. We can change it later as insight progresses.
You'll also see a new Node method to obtain the node's primary MAC address.
Something in the back of my head says we probably had code elsewhere that could
benefit from reusing the new method, but I was unable to find it. Maybe it was
in the provisioning server, in which case there's not much point trying to
share the same code — one has access to the ORM and the other does not.
Jeroen
--
https://code.launchpad.net/~jtv/maas/pre-virsh-power/+merge/107963
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~jtv/maas/pre-virsh-power into lp:maas.
=== modified file 'src/maasserver/models/__init__.py'
--- src/maasserver/models/__init__.py 2012-05-28 14:28:46 +0000
+++ src/maasserver/models/__init__.py 2012-05-30 12:37:23 +0000
@@ -340,35 +340,24 @@
:return: Those Nodes for which power-on was actually requested.
:rtype: list
"""
- # TODO: File structure needs sorting out to avoid this circular
- # import dance.
+ # Avoid circular imports.
from metadataserver.models import NodeUserData
+
nodes = self.get_nodes(by_user, NODE_PERMISSION.EDIT, ids=ids)
+ for node in nodes:
+ NodeUserData.objects.set_user_data(node, user_data)
processed_nodes = []
for node in nodes:
- NodeUserData.objects.set_user_data(node, user_data)
- # Wake on LAN is a special case, deal with it first.
+ power_params = node.get_effective_power_parameters()
node_power_type = node.get_effective_power_type()
if node_power_type == POWER_TYPE.WAKE_ON_LAN:
- # If power_parameters is set, use it. Otherwise, use the
- # first registered MAC address.
- mac = None
- if node.power_parameters:
- mac = node.power_parameters.get("mac", None)
- else:
- try:
- macaddress = node.macaddress_set.order_by('created')[0]
- except IndexError:
- pass # No MAC recorded for this node.
- else:
- mac = macaddress.mac_address
- if mac is not None and mac != "":
- power_on.delay(node_power_type, mac=mac)
- processed_nodes.append(node)
+ mac = power_params.get('mac')
+ do_start = (mac != '' and mac is not None)
else:
- if node.power_parameters:
- power_on.delay(node_power_type, **node.power_parameters)
- processed_nodes.append(node)
+ do_start = True
+ if do_start:
+ power_on.delay(node_power_type, **power_params)
+ processed_nodes.append(node)
return processed_nodes
@@ -596,6 +585,31 @@
power_type = self.power_type
return power_type
+ def get_primary_mac(self):
+ """Return the primary :class:`MACAddress` for this node."""
+ macs = self.macaddress_set.order_by('created')[:1]
+ if len(macs) > 0:
+ return macs[0]
+ else:
+ return None
+
+ def get_effective_power_parameters(self):
+ """Return effective power parameters, including any defaults."""
+ if self.power_parameters:
+ power_params = self.power_parameters.copy()
+ else:
+ # An empty power_parameters comes out as an empty unicode string!
+ power_params = {}
+
+ power_params.setdefault('system_id', self.system_id)
+ # The "mac" parameter defaults to the node's primary MAC
+ # address, but only if no power parameters were set at all.
+ if not self.power_parameters:
+ primary_mac = self.get_primary_mac()
+ if primary_mac is not None:
+ power_params['mac'] = primary_mac.mac_address
+ return power_params
+
def acquire(self, user, token=None):
"""Mark commissioned node as acquired by the given user and token."""
assert self.owner is None
=== modified file 'src/maasserver/tests/test_models.py'
--- src/maasserver/tests/test_models.py 2012-05-30 01:41:24 +0000
+++ src/maasserver/tests/test_models.py 2012-05-30 12:37:23 +0000
@@ -12,7 +12,10 @@
__metaclass__ = type
__all__ = []
-from datetime import datetime
+from datetime import (
+ datetime,
+ timedelta,
+ )
from django.conf import settings
from django.contrib.auth.models import User
@@ -187,6 +190,27 @@
node=node, mac_address='AA:BB:CC:DD:EE:FF').count()
self.assertEqual(0, macs)
+ def test_get_primary_mac_returns_mac_address(self):
+ node = factory.make_node()
+ mac = factory.getRandomMACAddress()
+ node.add_mac_address(mac)
+ self.assertEqual(mac, node.get_primary_mac().mac_address)
+
+ def test_get_primary_mac_returns_None_if_node_has_no_mac(self):
+ node = factory.make_node()
+ self.assertIsNone(node.get_primary_mac())
+
+ def test_get_primary_mac_returns_oldest_mac(self):
+ node = factory.make_node()
+ macs = [factory.getRandomMACAddress() for counter in range(3)]
+ offset = timedelta(0)
+ for mac in macs:
+ mac_address = node.add_mac_address(mac)
+ mac_address.created += offset
+ mac_address.save()
+ offset += timedelta(1)
+ self.assertEqual(macs[0], node.get_primary_mac().mac_address)
+
def test_delete_node_deletes_related_mac(self):
node = factory.make_node()
mac = node.add_mac_address('AA:BB:CC:DD:EE:FF')
@@ -267,6 +291,31 @@
node = factory.make_node(power_type=POWER_TYPE.DEFAULT)
self.assertEqual("", node.power_parameters)
+ def test_get_effective_power_parameters_returns_power_parameters(self):
+ params = {'test_parameter': factory.getRandomString()}
+ node = factory.make_node(power_parameters=params)
+ self.assertEqual(
+ params['test_parameter'],
+ node.get_effective_power_parameters()['test_parameter'])
+
+ def test_get_effective_power_parameters_adds_system_id(self):
+ node = factory.make_node()
+ self.assertEqual(
+ node.system_id,
+ node.get_effective_power_parameters()['system_id'])
+
+ def test_get_effective_power_parameters_adds_mac_if_no_params_set(self):
+ node = factory.make_node()
+ mac = factory.getRandomMACAddress()
+ node.add_mac_address(mac)
+ self.assertEqual(mac, node.get_effective_power_parameters()['mac'])
+
+ def test_get_effective_power_parameters_adds_no_mac_if_params_set(self):
+ node = factory.make_node(power_parameters={'foo': 'bar'})
+ mac = factory.getRandomMACAddress()
+ node.add_mac_address(mac)
+ self.assertNotIn('mac', node.get_effective_power_parameters())
+
def test_acquire(self):
node = factory.make_node(status=NODE_STATUS.READY)
user = factory.make_user()
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
