launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06349
[Merge] lp:~allenap/maas/pserv-modify-interfaces into lp:maas
Gavin Panella has proposed merging lp:~allenap/maas/pserv-modify-interfaces into lp:maas.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~allenap/maas/pserv-modify-interfaces/+merge/92779
This does the nasty stuff to get MAC address changes into Cobbler.
--
https://code.launchpad.net/~allenap/maas/pserv-modify-interfaces/+merge/92779
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/pserv-modify-interfaces into lp:maas.
=== modified file 'src/provisioningserver/api.py'
--- src/provisioningserver/api.py 2012-02-13 10:02:40 +0000
+++ src/provisioningserver/api.py 2012-02-13 15:36:41 +0000
@@ -14,6 +14,7 @@
]
from functools import partial
+from itertools import count
from provisioningserver.cobblerclient import (
CobblerDistro,
@@ -80,6 +81,55 @@
postprocess_mapping, function=cobbler_to_papi_distro)
+def mac_addresses_to_cobbler_deltas(interfaces, mac_addresses):
+ """Generate `modify_system` dicts for use with `xapi_object_edit`.
+
+ This takes `interfaces` - the current state of a system's interfaces - and
+ generates the operations required to transform it into a list of
+ interfaces containing exactly `mac_addresses`.
+
+ :param interfaces: A dict of interface-names -> interface-configurations.
+ :param mac_addresses: A collection of desired MAC addresses.
+ """
+ # For the sake of this calculation, ignore interfaces without MACs
+ # assigned. We may end up setting the MAC on these interfaces, but whether
+ # or not that happens is undefined (for now).
+ interfaces = {
+ name: configuration
+ for name, configuration in interfaces.iteritems()
+ if configuration["mac_address"] not in (None, "")
+ }
+
+ interface_names_by_mac_address = {
+ interface["mac_address"]: interface_name
+ for interface_name, interface in interfaces.iteritems()
+ }
+ mac_addresses_to_remove = set(
+ interface_names_by_mac_address).difference(mac_addresses)
+ mac_addresses_to_add = set(
+ mac_addresses).difference(interface_names_by_mac_address)
+
+ interface_names = set(interfaces)
+ interface_names_unused = (
+ "eth%d" % num for num in count(0)
+ if "eth%d" % num not in interface_names)
+
+ for mac_address in sorted(mac_addresses_to_remove):
+ interface_name = interface_names_by_mac_address[mac_address]
+ interface_names.remove(interface_name)
+ yield {
+ "interface": interface_name,
+ "delete_interface": True,
+ }
+
+ for mac_address in sorted(mac_addresses_to_add):
+ interface_name = next(interface_names_unused)
+ yield {
+ "interface": interface_name,
+ "mac_address": mac_address,
+ }
+
+
class ProvisioningAPI:
implements(IProvisioningAPI)
@@ -129,7 +179,17 @@
@inlineCallbacks
def modify_nodes(self, deltas):
for name, delta in deltas.iteritems():
- yield CobblerSystem(self.session, name).modify(delta)
+ system = CobblerSystem(self.session, name)
+ if "mac_addresses" in delta:
+ # This needs to be handled carefully.
+ mac_addresses = delta.pop("mac_addresses")
+ system_state = yield system.get_values()
+ interfaces = system_state.get("interfaces", {})
+ interface_modifications = mac_addresses_to_cobbler_deltas(
+ interfaces, mac_addresses)
+ for interface_modification in interface_modifications:
+ yield system.modify(interface_modification)
+ yield system.modify(delta)
@inlineCallbacks
def get_objects_by_name(self, object_type, names):
=== modified file 'src/provisioningserver/cobblerclient.py'
--- src/provisioningserver/cobblerclient.py 2012-02-12 22:57:35 +0000
+++ src/provisioningserver/cobblerclient.py 2012-02-13 15:36:41 +0000
@@ -650,6 +650,7 @@
'gateway',
# FQDN:
'hostname',
+ 'interfaces',
# Space-separated key=value pairs:
'kernel_options'
'kickstart',
@@ -658,8 +659,6 @@
# Space-separated key=value pairs for preseed:
'ks_meta',
'mgmt_classes',
- # A special dict; see below.
- 'modify_interface',
# Unqualified host name:
'name',
'name_servers',
@@ -684,14 +683,12 @@
'profile',
]
modification_attributes = [
+ 'delete_interface',
+ 'interface', # Required for mac_address and delete_interface.
+ 'mac_address',
'profile',
]
- # The modify_interface dict can contain:
- # * "macaddress-eth0" etc.
- # * "ipaddress-eth0" etc.
- # * "dnsname-eth0" etc.
-
@classmethod
def _callPowerMultiple(cls, session, operation, system_names):
"""Call API's "background_power_system" method.
=== modified file 'src/provisioningserver/testing/fakecobbler.py'
--- src/provisioningserver/testing/fakecobbler.py 2012-02-12 20:53:18 +0000
+++ src/provisioningserver/testing/fakecobbler.py 2012-02-13 15:36:41 +0000
@@ -16,6 +16,7 @@
'make_fake_cobbler_session',
]
+from copy import deepcopy
from fnmatch import fnmatch
from itertools import count
from random import randint
@@ -220,7 +221,7 @@
if len(matches) == 0:
return None
else:
- return matches[0]
+ return deepcopy(matches[0])
def _api_get_objects(self, object_type):
"""Return all saved objects of type `object_type`.
@@ -231,7 +232,7 @@
"""
# Assumption: these operations look only at saved objects.
location = self.store[None].get(object_type, {})
- return [obj.copy() for obj in location.values()]
+ return [deepcopy(obj) for obj in location.values()]
def _api_modify_object(self, token, object_type, handle, key, value):
"""Set an attribute on an object.
@@ -248,7 +249,7 @@
saved_obj = self._get_object_if_present(None, object_type, handle)
if saved_obj is None:
self._raise_bad_handle(object_type, handle)
- session_obj = saved_obj.copy()
+ session_obj = deepcopy(saved_obj)
self._add_object_to_session(
token, object_type, handle, session_obj)
@@ -316,6 +317,25 @@
self.tokens[token] = user
return token
+ def _xapi_edit_system_interfaces(self, token, handle, name, attrs):
+ """Edit system interfaces with Cobbler's crazy protocol."""
+ obj_state = self._api_get_object('system', name)
+ interface_name = attrs.pop("interface")
+ interfaces = obj_state.get("interfaces", {})
+ if "mac_address" in attrs:
+ interface = interfaces.setdefault(interface_name, {})
+ interface["mac_address"] = attrs.pop("mac_address")
+ elif "delete_interface" in attrs:
+ if interface_name in interfaces:
+ del interfaces[interface_name]
+ else:
+ raise AssertionError(
+ "Edit operation defined interface but "
+ "not mac_address or delete_interface. "
+ "Got: %r" % (attrs,))
+ self._api_modify_object(
+ token, 'system', handle, "interfaces", interfaces)
+
def xapi_object_edit(self, object_type, name, operation, attrs, token):
"""Swiss-Army-Knife API: create/rename/copy/edit object."""
if operation == 'remove':
@@ -329,6 +349,8 @@
return self._api_save_object(token, object_type, handle)
elif operation == 'edit':
handle = self._api_get_handle(token, object_type, name)
+ if object_type == "system" and "interface" in attrs:
+ self._xapi_edit_system_interfaces(token, handle, name, attrs)
for key, value in attrs.iteritems():
self._api_modify_object(token, object_type, handle, key, value)
return self._api_save_object(token, object_type, handle)
=== modified file 'src/provisioningserver/tests/test_api.py'
--- src/provisioningserver/tests/test_api.py 2012-02-13 11:02:31 +0000
+++ src/provisioningserver/tests/test_api.py 2012-02-13 15:36:41 +0000
@@ -18,6 +18,7 @@
cobbler_to_papi_distro,
cobbler_to_papi_node,
cobbler_to_papi_profile,
+ mac_addresses_to_cobbler_deltas,
postprocess_mapping,
ProvisioningAPI,
)
@@ -105,6 +106,85 @@
self.assertEqual(expected, observed)
+class TestInterfaceDeltas(TestCase):
+
+ def test_mac_addresses_to_cobbler_deltas_set_1(self):
+ current_interfaces = {
+ "eth0": {
+ "mac_address": "",
+ },
+ }
+ mac_addresses_desired = ["12:34:56:78:90:12"]
+ expected = [
+ {"interface": "eth0",
+ "mac_address": "12:34:56:78:90:12"},
+ ]
+ observed = list(
+ mac_addresses_to_cobbler_deltas(
+ current_interfaces, mac_addresses_desired))
+ self.assertEqual(expected, observed)
+
+ def test_mac_addresses_to_cobbler_deltas_set_2(self):
+ current_interfaces = {
+ "eth0": {
+ "mac_address": "",
+ },
+ }
+ mac_addresses_desired = [
+ "11:11:11:11:11:11", "22:22:22:22:22:22"]
+ expected = [
+ {"interface": "eth0",
+ "mac_address": "11:11:11:11:11:11"},
+ {"interface": "eth1",
+ "mac_address": "22:22:22:22:22:22"},
+ ]
+ observed = list(
+ mac_addresses_to_cobbler_deltas(
+ current_interfaces, mac_addresses_desired))
+ self.assertEqual(expected, observed)
+
+ def test_mac_addresses_to_cobbler_deltas_remove_1(self):
+ current_interfaces = {
+ "eth0": {
+ "mac_address": "11:11:11:11:11:11",
+ },
+ "eth1": {
+ "mac_address": "22:22:22:22:22:22",
+ },
+ }
+ mac_addresses_desired = ["22:22:22:22:22:22"]
+ expected = [
+ {"interface": "eth0",
+ "delete_interface": True},
+ ]
+ observed = list(
+ mac_addresses_to_cobbler_deltas(
+ current_interfaces, mac_addresses_desired))
+ self.assertEqual(expected, observed)
+
+ def test_mac_addresses_to_cobbler_deltas_set_1_remove_1(self):
+ current_interfaces = {
+ "eth0": {
+ "mac_address": "11:11:11:11:11:11",
+ },
+ "eth1": {
+ "mac_address": "22:22:22:22:22:22",
+ },
+ }
+ mac_addresses_desired = [
+ "22:22:22:22:22:22", "33:33:33:33:33:33"]
+ expected = [
+ {"interface": "eth0",
+ "delete_interface": True},
+ {"interface": "eth0",
+ "mac_address": "33:33:33:33:33:33"},
+ ]
+ observed = list(
+ mac_addresses_to_cobbler_deltas(
+ current_interfaces, mac_addresses_desired))
+ self.assertEqual(expected, observed)
+
+
class TestProvisioningAPI(TestCase):
"""Tests for `provisioningserver.api.ProvisioningAPI`."""
@@ -181,6 +261,36 @@
self.assertEqual(profile2_name, values[node_name]["profile"])
@inlineCallbacks
+ def test_modify_nodes_set_mac_addresses(self):
+ papi = self.get_provisioning_api()
+ distro_name = yield papi.add_distro(
+ "distro", "an_initrd", "a_kernel")
+ profile_name = yield papi.add_profile("profile1", distro_name)
+ node_name = yield papi.add_node("node", profile_name)
+ yield papi.modify_nodes(
+ {node_name: {"mac_addresses": ["55:55:55:55:55:55"]}})
+ values = yield papi.get_nodes_by_name([node_name])
+ self.assertEqual(
+ ["55:55:55:55:55:55"], values[node_name]["mac_addresses"])
+
+ @inlineCallbacks
+ def test_modify_nodes_remove_mac_addresses(self):
+ papi = self.get_provisioning_api()
+ distro_name = yield papi.add_distro(
+ "distro", "an_initrd", "a_kernel")
+ profile_name = yield papi.add_profile("profile1", distro_name)
+ node_name = yield papi.add_node("node", profile_name)
+ mac_addresses_from = ["55:55:55:55:55:55", "66:66:66:66:66:66"]
+ mac_addresses_to = ["66:66:66:66:66:66"]
+ yield papi.modify_nodes(
+ {node_name: {"mac_addresses": mac_addresses_from}})
+ yield papi.modify_nodes(
+ {node_name: {"mac_addresses": mac_addresses_to}})
+ values = yield papi.get_nodes_by_name([node_name])
+ self.assertEqual(
+ ["66:66:66:66:66:66"], values[node_name]["mac_addresses"])
+
+ @inlineCallbacks
def test_delete_distros_by_name(self):
# Create a distro via the Provisioning API.
papi = self.get_provisioning_api()
=== modified file 'src/provisioningserver/tests/test_fakecobbler.py'
--- src/provisioningserver/tests/test_fakecobbler.py 2012-02-12 16:02:52 +0000
+++ src/provisioningserver/tests/test_fakecobbler.py 2012-02-13 15:36:41 +0000
@@ -440,6 +440,32 @@
returnValue(handles)
@inlineCallbacks
+ def test_interface_set_mac_address(self):
+ session = yield fake_cobbler_session()
+ name = self.make_name()
+ obj = yield fake_cobbler_object(session, self.cobbler_class, name)
+ yield obj.modify(
+ {"interface": "eth0", "mac_address": "12:34:56:78:90:12"})
+ state = yield obj.get_values()
+ interfaces = state.get("interfaces", {})
+ self.assertEqual(["eth0"], sorted(interfaces))
+ state_eth0 = interfaces["eth0"]
+ self.assertEqual("12:34:56:78:90:12", state_eth0["mac_address"])
+
+ @inlineCallbacks
+ def test_interface_delete_interface(self):
+ session = yield fake_cobbler_session()
+ name = self.make_name()
+ obj = yield fake_cobbler_object(session, self.cobbler_class, name)
+ yield obj.modify(
+ {"interface": "eth0", "mac_address": "12:34:56:78:90:12"})
+ yield obj.modify(
+ {"interface": "eth0", "delete_interface": "ignored"})
+ state = yield obj.get_values()
+ interfaces = state.get("interfaces", {})
+ self.assertEqual([], sorted(interfaces))
+
+ @inlineCallbacks
def test_powerOnMultiple(self):
session = yield fake_cobbler_session()
names, systems = yield self.make_systems(session, 3)
