diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample index d0724ec5f7..1559c4fd44 100644 --- a/etc/ironic/ironic.conf.sample +++ b/etc/ironic/ironic.conf.sample @@ -711,6 +711,17 @@ #mysql_engine=InnoDB +[dhcp] + +# +# Options defined in ironic.common.dhcp_factory +# + +# DHCP provider to use. "neutron" uses Neutron, and "none" +# uses a no-op provider. (string value) +#dhcp_provider=neutron + + [disk_partitioner] # @@ -1043,7 +1054,7 @@ [neutron] # -# Options defined in ironic.common.neutron +# Options defined in ironic.dhcp.neutron # # URL for connecting to neutron. (string value) @@ -1113,7 +1124,7 @@ # (string value) #tftp_master_path=/tftpboot/master_images -# Neutron bootfile DHCP parameter. (string value) +# Bootfile DHCP parameter. (string value) #pxe_bootfile_name=pxelinux.0 # Ironic compute node's HTTP server URL. Example: diff --git a/ironic/common/dhcp_factory.py b/ironic/common/dhcp_factory.py new file mode 100644 index 0000000000..bca51989da --- /dev/null +++ b/ironic/common/dhcp_factory.py @@ -0,0 +1,94 @@ +# Copyright 2014 Rackspace, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import stevedore + +from oslo.config import cfg + +from ironic.common import exception +from ironic.openstack.common import lockutils + + +dhcp_provider_opts = [ + cfg.StrOpt('dhcp_provider', + default='neutron', + help='DHCP provider to use. "neutron" uses Neutron, and ' + '"none" uses a no-op provider.' + ), +] + +CONF = cfg.CONF +CONF.register_opts(dhcp_provider_opts, group='dhcp') + +_dhcp_provider = None + +EM_SEMAPHORE = 'dhcp_provider' + + +class DHCPFactory(object): + + # NOTE(lucasagomes): Instantiate a stevedore.driver.DriverManager + # only once, the first time DHCPFactory.__init__ + # is called. + _dhcp_provider = None + + def __init__(self, **kwargs): + if not DHCPFactory._dhcp_provider: + DHCPFactory._set_dhcp_provider(**kwargs) + + # NOTE(lucasagomes): Use lockutils to avoid a potential race in eventlet + # that might try to create two dhcp factories. + @classmethod + @lockutils.synchronized(EM_SEMAPHORE, 'ironic-') + def _set_dhcp_provider(cls, **kwargs): + """Initialize the dhcp provider + + :raises: DHCPNotFound if the dhcp_provider cannot be loaded. + """ + + # NOTE(lucasagomes): In case multiple greenthreads queue up on + # this lock before _dhcp_provider is initialized, + # prevent creation of multiple DriverManager. + if cls._dhcp_provider: + return + + dhcp_provider_name = CONF.dhcp.dhcp_provider + try: + _extension_manager = stevedore.driver.DriverManager( + 'ironic.dhcp', + dhcp_provider_name, + invoke_kwds=kwargs, + invoke_on_load=True) + except RuntimeError: + raise exception.DHCPNotFound(dhcp_provider_name=dhcp_provider_name) + + cls._dhcp_provider = _extension_manager.driver + + def update_dhcp(self, task, dhcp_opts): + """Send or update the DHCP BOOT options for this node. + + :param task: A TaskManager instance. + :param dhcp_opts: this will be a list of dicts, e.g. + [{'opt_name': 'bootfile-name', + 'opt_value': 'pxelinux.0'}, + {'opt_name': 'server-ip-address', + 'opt_value': '123.123.123.456'}, + {'opt_name': 'tftp-server', + 'opt_value': '123.123.123.123'}] + """ + self.provider.update_dhcp_opts(task, dhcp_opts) + + @property + def provider(self): + return self._dhcp_provider diff --git a/ironic/common/exception.py b/ironic/common/exception.py index 76cfd9f32f..735f1ce1cf 100644 --- a/ironic/common/exception.py +++ b/ironic/common/exception.py @@ -190,6 +190,10 @@ class NotFound(IronicException): code = 404 +class DHCPNotFound(NotFound): + message = _("Failed to load DHCP provider %(dhcp_provider_name)s.") + + class DriverNotFound(NotFound): message = _("Failed to load driver %(driver_name)s.") diff --git a/ironic/common/network.py b/ironic/common/network.py new file mode 100644 index 0000000000..de55979059 --- /dev/null +++ b/ironic/common/network.py @@ -0,0 +1,30 @@ +# Copyright 2014 Rackspace, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + + +def get_node_vif_ids(task): + """Get all VIF ids for a node. + + This function does not handle multi node operations. + + :param task: a TaskManager instance. + :returns: A dict of the Node's port UUIDs and their associated VIFs + + """ + port_vifs = {} + for port in task.ports: + vif = port.extra.get('vif_port_id') + if vif: + port_vifs[port.uuid] = vif + return port_vifs diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index b313a0dc1e..eaca1b2be9 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -52,11 +52,11 @@ from oslo.config import cfg from oslo import messaging from oslo.utils import excutils +from ironic.common import dhcp_factory from ironic.common import driver_factory from ironic.common import exception from ironic.common import hash_ring as hash from ironic.common import i18n -from ironic.common import neutron from ironic.common import rpc from ironic.common import states from ironic.common import utils as ironic_utils @@ -1052,8 +1052,9 @@ class ConductorManager(periodic_task.PeriodicTasks): :param context: request context. :param port_obj: a changed (but not saved) port object. + :raises: DHCPNotFound if the dhcp_provider provider endpoint is invalid :raises: FailedToUpdateMacOnPort if MAC address changed and update - Neutron failed. + failed. :raises: MACAlreadyExists if the update is setting a MAC which is registered on another port already. """ @@ -1065,14 +1066,14 @@ class ConductorManager(periodic_task.PeriodicTasks): if 'address' in port_obj.obj_what_changed(): vif = port_obj.extra.get('vif_port_id') if vif: - api = neutron.NeutronAPI(context) - api.update_port_address(vif, port_obj.address) + api = dhcp_factory.DHCPFactory(token=context.auth_token) + api.provider.update_port_address(vif, port_obj.address) # Log warning if there is no vif_port_id and an instance # is associated with the node. elif node.instance_uuid: LOG.warning(_("No VIF found for instance %(instance)s " - "port %(port)s when attempting to update Neutron " - "port MAC address."), + "port %(port)s when attempting to update port MAC " + "address."), {'port': port_uuid, 'instance': node.instance_uuid}) port_obj.save(context) diff --git a/ironic/dhcp/__init__.py b/ironic/dhcp/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/ironic/dhcp/base.py b/ironic/dhcp/base.py new file mode 100644 index 0000000000..3b30680017 --- /dev/null +++ b/ironic/dhcp/base.py @@ -0,0 +1,69 @@ +# Copyright 2014 Rackspace, Inc. +# All Rights Reserved +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +""" +Abstract base class for dhcp providers. +""" + +import abc + +import six + + +@six.add_metaclass(abc.ABCMeta) +class BaseDHCP(object): + """Base class for DHCP provider APIs.""" + + @abc.abstractmethod + def update_port_dhcp_opts(self, port_id, dhcp_options): + """Update one or more DHCP options on the specified port. + + :param port_id: designate which port these attributes + will be applied to. + :param dhcp_options: this will be a list of dicts, e.g. + [{'opt_name': 'bootfile-name', + 'opt_value': 'pxelinux.0'}, + {'opt_name': 'server-ip-address', + 'opt_value': '123.123.123.456'}, + {'opt_name': 'tftp-server', + 'opt_value': '123.123.123.123'}] + + :raises: FailedToUpdateDHCPOptOnPort + """ + + @abc.abstractmethod + def update_port_address(self, port_id, address): + """Update a port's MAC address. + + :param port_id: port id. + :param address: new MAC address. + :raises: FailedToUpdateMacOnPort + """ + + @abc.abstractmethod + def update_dhcp_opts(self, task, options): + """Send or update the DHCP BOOT options for this node. + + :param task: A TaskManager instance. + :param options: this will be a list of dicts, e.g. + [{'opt_name': 'bootfile-name', + 'opt_value': 'pxelinux.0'}, + {'opt_name': 'server-ip-address', + 'opt_value': '123.123.123.456'}, + {'opt_name': 'tftp-server', + 'opt_value': '123.123.123.123'}] + + :raises: FailedToUpdateDHCPOptOnPort + """ diff --git a/ironic/common/neutron.py b/ironic/dhcp/neutron.py similarity index 64% rename from ironic/common/neutron.py rename to ironic/dhcp/neutron.py index 621267b17b..d940281168 100644 --- a/ironic/common/neutron.py +++ b/ironic/dhcp/neutron.py @@ -21,11 +21,16 @@ from neutronclient.v2_0 import client as clientv20 from oslo.config import cfg from ironic.common import exception +from ironic.common import i18n from ironic.common import keystone +from ironic.common import network +from ironic.dhcp import base from ironic.drivers.modules import ssh from ironic.openstack.common import log as logging +_LW = i18n._LW + neutron_opts = [ cfg.StrOpt('url', default='http://$my_ip:9696', @@ -40,7 +45,7 @@ neutron_opts = [ 'Running neutron in noauth mode (related to but not ' 'affected by this setting) is insecure and should only be ' 'used for testing.') - ] + ] CONF = cfg.CONF CONF.import_opt('my_ip', 'ironic.netconf') @@ -48,11 +53,11 @@ CONF.register_opts(neutron_opts, group='neutron') LOG = logging.getLogger(__name__) -class NeutronAPI(object): +class NeutronDHCPApi(base.BaseDHCP): """API for communicating to neutron 2.x API.""" - def __init__(self, context): - self.context = context + def __init__(self, **kwargs): + token = kwargs.get('token', None) self.client = None params = { 'timeout': CONF.neutron.url_timeout, @@ -68,7 +73,7 @@ class NeutronAPI(object): params['endpoint_url'] = CONF.neutron.url params['auth_strategy'] = 'noauth' elif (CONF.neutron.auth_strategy == 'keystone' and - context.auth_token is None): + token is None): params['endpoint_url'] = (CONF.neutron.url or keystone.get_service_url('neutron')) params['username'] = CONF.keystone_authtoken.admin_user @@ -76,7 +81,7 @@ class NeutronAPI(object): params['password'] = CONF.keystone_authtoken.admin_password params['auth_url'] = (CONF.keystone_authtoken.auth_uri or '') else: - params['token'] = context.auth_token + params['token'] = token params['endpoint_url'] = CONF.neutron.url params['auth_strategy'] = None @@ -120,68 +125,51 @@ class NeutronAPI(object): self.client.update_port(port_id, port_req_body) except neutron_client_exc.NeutronClientException: LOG.exception(_("Failed to update MAC address on Neutron port %s." - ), port_id) + ), port_id) raise exception.FailedToUpdateMacOnPort(port_id=port_id) + def update_dhcp_opts(self, task, options): + """Send or update the DHCP BOOT options for this node. -def get_node_vif_ids(task): - """Get all Neutron VIF ids for a node. + :param task: A TaskManager instance. + :param dhcp_opts: this will be a list of dicts, e.g. + [{'opt_name': 'bootfile-name', + 'opt_value': 'pxelinux.0'}, + {'opt_name': 'server-ip-address', + 'opt_value': '123.123.123.456'}, + {'opt_name': 'tftp-server', + 'opt_value': '123.123.123.123'}] + """ + vifs = network.get_node_vif_ids(task) + if not vifs: + LOG.warning(_LW("No VIFs found for node %(node)s when attempting " + "to update DHCP BOOT options."), + {'node': task.node.uuid}) + return - This function does not handle multi node operations. + failures = [] + for port_id, port_vif in vifs.items(): + try: + self.update_port_dhcp_opts(port_vif, options) + except exception.FailedToUpdateDHCPOptOnPort: + failures.append(port_id) - :param task: a TaskManager instance. - :returns: A dict of the Node's port UUIDs and their associated VIFs + if failures: + if len(failures) == len(vifs): + raise exception.FailedToUpdateDHCPOptOnPort(_( + "Failed to set DHCP BOOT options for any port on node %s.") + % task.node.uuid) + else: + LOG.warning(_LW("Some errors were encountered when updating " + "the DHCP BOOT options for node %(node)s on " + "the following ports: %(ports)s."), + {'node': task.node.uuid, 'ports': failures}) - """ - port_vifs = {} - for port in task.ports: - vif = port.extra.get('vif_port_id') - if vif: - port_vifs[port.uuid] = vif - return port_vifs - - -def update_neutron(task, options): - """Send or update the DHCP BOOT options to Neutron for this node.""" - vifs = get_node_vif_ids(task) - if not vifs: - LOG.warning(_("No VIFs found for node %(node)s when attempting to " - "update Neutron DHCP BOOT options."), - {'node': task.node.uuid}) - return - - # TODO(deva): decouple instantiation of NeutronAPI from task.context. - # Try to use the user's task.context.auth_token, but if it - # is not present, fall back to a server-generated context. - # We don't need to recreate this in every method call. - api = NeutronAPI(task.context) - failures = [] - for port_id, port_vif in vifs.iteritems(): - try: - api.update_port_dhcp_opts(port_vif, options) - except exception.FailedToUpdateDHCPOptOnPort: - failures.append(port_id) - - if failures: - if len(failures) == len(vifs): - raise exception.FailedToUpdateDHCPOptOnPort(_( - "Failed to set DHCP BOOT options for any port on node %s.") % - task.node.uuid) - else: - LOG.warning(_("Some errors were encountered when updating the " - "DHCP BOOT options for node %(node)s on the " - "following ports: %(ports)s."), - {'node': task.node.uuid, 'ports': failures}) - - _wait_for_neutron_update(task) - - -def _wait_for_neutron_update(task): - """Wait for Neutron agents to process all requested changes if required.""" - # TODO(adam_g): Hack to workaround bug 1334447 until we have a mechanism - # for synchronizing events with Neutron. We need to sleep only if we are - # booting VMs, which is implied by SSHPower, to ensure they do not boot - # before Neutron agents have setup sufficent DHCP config for netboot. - if isinstance(task.driver.power, ssh.SSHPower): - LOG.debug(_("Waiting 15 seconds for Neutron.")) - time.sleep(15) + # TODO(adam_g): Hack to workaround bug 1334447 until we have a + # mechanism for synchronizing events with Neutron. We need to sleep + # only if we are booting VMs, which is implied by SSHPower, to ensure + # they do not boot before Neutron agents have setup sufficent DHCP + # config for netboot. + if isinstance(task.driver.power, ssh.SSHPower): + LOG.debug("Waiting 15 seconds for Neutron.") + time.sleep(15) diff --git a/ironic/dhcp/none.py b/ironic/dhcp/none.py new file mode 100644 index 0000000000..eeae828adc --- /dev/null +++ b/ironic/dhcp/none.py @@ -0,0 +1,28 @@ +# Copyright 2014 Rackspace, Inc. +# All Rights Reserved +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from ironic.dhcp import base + + +class NoneDHCPApi(base.BaseDHCP): + """No-op DHCP API.""" + def update_port_dhcp_opts(self, port_id, dhcp_options): + pass + + def update_dhcp_opts(self, task, options): + pass + + def update_port_address(self, port_id, address): + pass diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index b8e7cc2a9b..d07ce5145c 100644 --- a/ironic/drivers/modules/agent.py +++ b/ironic/drivers/modules/agent.py @@ -18,11 +18,11 @@ import time from oslo.config import cfg from oslo.utils import excutils +from ironic.common import dhcp_factory from ironic.common import exception from ironic.common import i18n from ironic.common import image_service from ironic.common import keystone -from ironic.common import neutron from ironic.common import paths from ironic.common import pxe_utils from ironic.common import states @@ -217,7 +217,8 @@ class AgentDeploy(base.DeployInterface): :returns: status of the deploy. One of ironic.common.states. """ dhcp_opts = pxe_utils.dhcp_options_for_instance() - neutron.update_neutron(task, dhcp_opts) + provider = dhcp_factory.DHCPFactory(token=task.context.auth_token) + provider.update_dhcp(task, dhcp_opts) manager_utils.node_set_boot_device(task, 'pxe', persistent=True) manager_utils.node_power_action(task, states.REBOOT) @@ -290,7 +291,8 @@ class AgentDeploy(base.DeployInterface): :param task: a TaskManager instance. """ - neutron.update_neutron(task, CONF.agent.agent_pxe_bootfile_name) + provider = dhcp_factory.DHCPFactory(token=task.context.auth_token) + provider.update_dhcp(task, CONF.agent.agent_pxe_bootfile_name) class AgentVendorInterface(base.VendorInterface): diff --git a/ironic/drivers/modules/pxe.py b/ironic/drivers/modules/pxe.py index 4536ae6f2c..82a535bce9 100644 --- a/ironic/drivers/modules/pxe.py +++ b/ironic/drivers/modules/pxe.py @@ -22,10 +22,10 @@ import shutil from oslo.config import cfg +from ironic.common import dhcp_factory from ironic.common import exception from ironic.common import i18n from ironic.common import image_service as service -from ironic.common import neutron from ironic.common import paths from ironic.common import pxe_utils from ironic.common import states @@ -58,7 +58,7 @@ pxe_opts = [ # other architectures require different boot files. cfg.StrOpt('pxe_bootfile_name', default='pxelinux.0', - help='Neutron bootfile DHCP parameter.'), + help='Bootfile DHCP parameter.'), cfg.StrOpt('http_url', help='Ironic compute node\'s HTTP server URL. ' 'Example: http://192.1.2.3:8080'), @@ -283,8 +283,8 @@ class PXEDeploy(base.DeployInterface): """Start deployment of the task's node'. Fetches instance image, creates a temporary keystone token file, - updates the Neutron DHCP port options for next boot, and issues a - reboot request to the power driver. + updates the DHCP port options for next boot, and issues a reboot + request to the power driver. This causes the node to boot into the deployment ramdisk and triggers the next phase of PXE-based deployment via VendorPassthru._continue_deploy(). @@ -299,7 +299,8 @@ class PXEDeploy(base.DeployInterface): # to deploy ramdisk _create_token_file(task) dhcp_opts = pxe_utils.dhcp_options_for_instance() - neutron.update_neutron(task, dhcp_opts) + provider = dhcp_factory.DHCPFactory(token=task.context.auth_token) + provider.update_dhcp(task, dhcp_opts) manager_utils.node_set_boot_device(task, 'pxe', persistent=True) manager_utils.node_power_action(task, states.REBOOT) @@ -363,7 +364,8 @@ class PXEDeploy(base.DeployInterface): def take_over(self, task): dhcp_opts = pxe_utils.dhcp_options_for_instance() - neutron.update_neutron(task, dhcp_opts) + provider = dhcp_factory.DHCPFactory(token=task.context.auth_token) + provider.update_dhcp(task, dhcp_opts) class VendorPassthru(base.VendorInterface): diff --git a/ironic/tests/conductor/test_manager.py b/ironic/tests/conductor/test_manager.py index 977b538663..6e385b6ec9 100644 --- a/ironic/tests/conductor/test_manager.py +++ b/ironic/tests/conductor/test_manager.py @@ -1245,7 +1245,7 @@ class UpdatePortTestCase(_ServiceSetUpMixin, tests_db_base.DbTestCase): # Compare true exception hidden by @messaging.expected_exceptions self.assertEqual(exception.NodeLocked, exc.exc_info[0]) - @mock.patch('ironic.common.neutron.NeutronAPI.update_port_address') + @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_address') def test_update_port_address(self, mac_update_mock): obj_utils.create_test_node(self.context, driver='fake') port = obj_utils.create_test_port(self.context, @@ -1256,7 +1256,7 @@ class UpdatePortTestCase(_ServiceSetUpMixin, tests_db_base.DbTestCase): self.assertEqual(new_address, res.address) mac_update_mock.assert_called_once_with('fake-id', new_address) - @mock.patch('ironic.common.neutron.NeutronAPI.update_port_address') + @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_address') def test_update_port_address_fail(self, mac_update_mock): obj_utils.create_test_node(self.context, driver='fake') port = obj_utils.create_test_port(self.context, @@ -1273,7 +1273,7 @@ class UpdatePortTestCase(_ServiceSetUpMixin, tests_db_base.DbTestCase): port.refresh(self.context) self.assertEqual(old_address, port.address) - @mock.patch('ironic.common.neutron.NeutronAPI.update_port_address') + @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_address') def test_update_port_address_no_vif_id(self, mac_update_mock): obj_utils.create_test_node(self.context, driver='fake') port = obj_utils.create_test_port(self.context) @@ -2140,7 +2140,7 @@ class ManagerCheckDeployTimeoutsTestCase(_CommonMixIn, tests_base.TestCase): self.task.spawn_after.call_args_list) @mock.patch.object(dbapi.IMPL, 'update_port') - @mock.patch('ironic.common.neutron.NeutronAPI.update_port_address') + @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_address') def test_update_port_duplicate_mac(self, get_nodeinfo_mock, mapped_mock, acquire_mock, mac_update_mock, mock_up): ndict = utils.get_test_node(driver='fake') diff --git a/ironic/tests/dhcp/__init__.py b/ironic/tests/dhcp/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/ironic/tests/dhcp/test_factory.py b/ironic/tests/dhcp/test_factory.py new file mode 100644 index 0000000000..869abd9240 --- /dev/null +++ b/ironic/tests/dhcp/test_factory.py @@ -0,0 +1,72 @@ +# Copyright 2014 Rackspace, Inc. +# All Rights Reserved +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import mock + +from ironic.common import dhcp_factory +from ironic.common import exception +from ironic.dhcp import neutron +from ironic.dhcp import none +from ironic.openstack.common import context +from ironic.tests import base + + +class TestDHCPFactory(base.TestCase): + + def setUp(self): + super(TestDHCPFactory, self).setUp() + self.config(enabled_drivers=['fake']) + self.config(url='test-url', + url_timeout=30, + group='neutron') + self.context = context.get_admin_context() + dhcp_factory.DHCPFactory._dhcp_provider = None + + def test_default_dhcp(self): + # dhcp provider should default to neutron + api = dhcp_factory.DHCPFactory() + self.assertIsInstance(api.provider, neutron.NeutronDHCPApi) + + def test_set_none_dhcp(self): + self.config(dhcp_provider='none', + group='dhcp') + + api = dhcp_factory.DHCPFactory() + self.assertIsInstance(api.provider, none.NoneDHCPApi) + + def test_set_neutron_dhcp(self): + self.config(dhcp_provider='neutron', + group='dhcp') + + api = dhcp_factory.DHCPFactory() + self.assertIsInstance(api.provider, neutron.NeutronDHCPApi) + + def test_only_one_dhcp(self): + self.config(dhcp_provider='none', + group='dhcp') + dhcp_factory.DHCPFactory() + + with mock.patch.object(dhcp_factory.DHCPFactory, + '_set_dhcp_provider') as mock_set_dhcp: + # There is already a dhcp_provider, so this shouldn't call + # _set_dhcp_provider again. + dhcp_factory.DHCPFactory() + self.assertEqual(0, mock_set_dhcp.call_count) + + def test_set_bad_dhcp(self): + self.config(dhcp_provider='bad_dhcp', + group='dhcp') + + self.assertRaises(exception.DHCPNotFound, dhcp_factory.DHCPFactory) diff --git a/ironic/tests/test_neutron.py b/ironic/tests/dhcp/test_neutron.py similarity index 57% rename from ironic/tests/test_neutron.py rename to ironic/tests/dhcp/test_neutron.py index 27a59f7af1..1ce4af390f 100644 --- a/ironic/tests/test_neutron.py +++ b/ironic/tests/dhcp/test_neutron.py @@ -17,29 +17,26 @@ import mock from neutronclient.common import exceptions as neutron_client_exc from neutronclient.v2_0 import client -from oslo.config import cfg +from ironic.common import dhcp_factory from ironic.common import exception -from ironic.common import neutron from ironic.common import pxe_utils -from ironic.common import utils from ironic.conductor import task_manager -from ironic.db import api as dbapi +from ironic.dhcp import neutron from ironic.openstack.common import context from ironic.tests import base from ironic.tests.conductor import utils as mgr_utils from ironic.tests.objects import utils as object_utils -CONF = cfg.CONF - - class TestNeutron(base.TestCase): def setUp(self): super(TestNeutron, self).setUp() mgr_utils.mock_the_extension_manager(driver='fake') self.config(enabled_drivers=['fake']) + self.config(dhcp_provider='neutron', + group='dhcp') self.config(url='test-url', url_timeout=30, group='neutron') @@ -50,19 +47,16 @@ class TestNeutron(base.TestCase): admin_password='test-admin-password', auth_uri='test-auth-uri', group='keystone_authtoken') - self.dbapi = dbapi.get_instance() self.context = context.get_admin_context() self.node = object_utils.create_test_node(self.context) + dhcp_factory.DHCPFactory._dhcp_provider = None def test_invalid_auth_strategy(self): self.config(auth_strategy='wrong_config', group='neutron') token = 'test-token-123' - my_context = context.RequestContext(user='test-user', - tenant='test-tenant', - auth_token=token) self.assertRaises(exception.ConfigInvalid, - neutron.NeutronAPI, - my_context) + neutron.NeutronDHCPApi, + token=token) def test_create_with_token(self): token = 'test-token-123' @@ -78,7 +72,7 @@ class TestNeutron(base.TestCase): with mock.patch.object(client.Client, "__init__") as mock_client_init: mock_client_init.return_value = None - neutron.NeutronAPI(my_context) + neutron.NeutronDHCPApi(token=my_context.auth_token) mock_client_init.assert_called_once_with(**expected) def test_create_without_token(self): @@ -95,7 +89,7 @@ class TestNeutron(base.TestCase): with mock.patch.object(client.Client, "__init__") as mock_client_init: mock_client_init.return_value = None - neutron.NeutronAPI(my_context) + neutron.NeutronDHCPApi(token=my_context.auth_token) mock_client_init.assert_called_once_with(**expected) def test_create_noauth(self): @@ -109,16 +103,16 @@ class TestNeutron(base.TestCase): with mock.patch.object(client.Client, "__init__") as mock_client_init: mock_client_init.return_value = None - neutron.NeutronAPI(my_context) + neutron.NeutronDHCPApi(token=my_context.auth_token) mock_client_init.assert_called_once_with(**expected) def test_neutron_port_update(self): opts = [{'opt_name': 'bootfile-name', - 'opt_value': 'pxelinux.0'}, + 'opt_value': 'pxelinux.0'}, {'opt_name': 'tftp-server', - 'opt_value': '1.1.1.1'}, + 'opt_value': '1.1.1.1'}, {'opt_name': 'server-ip-address', - 'opt_value': '1.1.1.1'}] + 'opt_value': '1.1.1.1'}] port_id = 'fake-port-id' expected = {'port': {'extra_dhcp_opts': opts}} my_context = context.RequestContext(user='test-user', @@ -126,11 +120,12 @@ class TestNeutron(base.TestCase): with mock.patch.object(client.Client, "__init__") as mock_client_init: mock_client_init.return_value = None - api = neutron.NeutronAPI(my_context) + api = dhcp_factory.DHCPFactory(token=my_context.auth_token) with mock.patch.object(client.Client, "update_port") as mock_update_port: mock_update_port.return_value = None - api.update_port_dhcp_opts(port_id, opts) + api = dhcp_factory.DHCPFactory(token=my_context.auth_token) + api.provider.update_port_dhcp_opts(port_id, opts) mock_update_port.assert_called_once_with(port_id, expected) def test_neutron_port_update_with_execption(self): @@ -140,14 +135,15 @@ class TestNeutron(base.TestCase): tenant='test-tenant') with mock.patch.object(client.Client, "__init__") as mock_client_init: mock_client_init.return_value = None - api = neutron.NeutronAPI(my_context) + api = dhcp_factory.DHCPFactory(token=my_context.auth_token) with mock.patch.object(client.Client, "update_port") as mock_update_port: mock_update_port.side_effect = ( neutron_client_exc.NeutronClientException()) + api = dhcp_factory.DHCPFactory(token=my_context.auth_token) self.assertRaises( exception.FailedToUpdateDHCPOptOnPort, - api.update_port_dhcp_opts, + api.provider.update_port_dhcp_opts, port_id, opts) @mock.patch.object(client.Client, 'update_port') @@ -159,9 +155,9 @@ class TestNeutron(base.TestCase): my_context = context.RequestContext(user='test-user', tenant='test-tenant') mock_client_init.return_value = None - api = neutron.NeutronAPI(my_context) + api = dhcp_factory.DHCPFactory(token=my_context.auth_token) mock_update_port.return_value = None - api.update_port_address(port_id, address) + api.provider.update_port_address(port_id, address) mock_update_port.assert_called_once_with(port_id, expected) @mock.patch.object(client.Client, 'update_port') @@ -173,121 +169,58 @@ class TestNeutron(base.TestCase): my_context = context.RequestContext(user='test-user', tenant='test-tenant') mock_client_init.return_value = None - api = neutron.NeutronAPI(my_context) + api = dhcp_factory.DHCPFactory(token=my_context.auth_token) mock_update_port.side_effect = ( - neutron_client_exc.NeutronClientException()) + neutron_client_exc.NeutronClientException()) self.assertRaises(exception.FailedToUpdateMacOnPort, - api.update_port_address, port_id, address) + api.provider.update_port_address, + port_id, address) - def test_get_node_vif_ids_no_ports(self): - expected = {} - with task_manager.acquire(self.context, self.node.uuid) as task: - result = neutron.get_node_vif_ids(task) - self.assertEqual(expected, result) - - def test__get_node_vif_ids_one_port(self): - port1 = object_utils.create_test_port(self.context, - node_id=self.node.id, - id=6, address='aa:bb:cc', - uuid=utils.generate_uuid(), - extra={'vif_port_id': 'test-vif-A'}, - driver='fake') - expected = {port1.uuid: 'test-vif-A'} - with task_manager.acquire(self.context, self.node.uuid) as task: - result = neutron.get_node_vif_ids(task) - self.assertEqual(expected, result) - - def test__get_node_vif_ids_two_ports(self): - port1 = object_utils.create_test_port(self.context, - node_id=self.node.id, - id=6, - address='aa:bb:cc', - uuid=utils.generate_uuid(), - extra={'vif_port_id': 'test-vif-A'}, - driver='fake') - port2 = object_utils.create_test_port(self.context, - node_id=self.node.id, - id=7, - address='dd:ee:ff', - uuid=utils.generate_uuid(), - extra={'vif_port_id': 'test-vif-B'}, - driver='fake') - expected = {port1.uuid: 'test-vif-A', port2.uuid: 'test-vif-B'} - with task_manager.acquire(self.context, self.node.uuid) as task: - result = neutron.get_node_vif_ids(task) - self.assertEqual(expected, result) - - @mock.patch('ironic.common.neutron._wait_for_neutron_update') - @mock.patch('ironic.common.neutron.NeutronAPI.update_port_dhcp_opts') - @mock.patch('ironic.common.neutron.get_node_vif_ids') - def test_update_neutron(self, mock_gnvi, mock_updo, mock_wait_neutron): + @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_dhcp_opts') + @mock.patch('ironic.common.network.get_node_vif_ids') + def test_update_dhcp(self, mock_gnvi, mock_updo): opts = pxe_utils.dhcp_options_for_instance() mock_gnvi.return_value = {'port-uuid': 'vif-uuid'} with task_manager.acquire(self.context, self.node.uuid) as task: - neutron.update_neutron(task, self.node) + api = dhcp_factory.DHCPFactory(token=self.context.auth_token) + api.update_dhcp(task, self.node) mock_updo.assertCalleOnceWith('vif-uuid', opts) - mock_wait_neutron.assert_called_once_with(task) - @mock.patch('ironic.common.neutron._wait_for_neutron_update') - @mock.patch('ironic.common.neutron.NeutronAPI.__init__') - @mock.patch('ironic.common.neutron.get_node_vif_ids') - def test_update_neutron_no_vif_data(self, mock_gnvi, mock_init, - mock_wait_neutron): + @mock.patch('ironic.common.network.get_node_vif_ids') + def test_update_dhcp_no_vif_data(self, mock_gnvi): mock_gnvi.return_value = {} with task_manager.acquire(self.context, self.node.uuid) as task: - neutron.update_neutron(task, self.node) - self.assertFalse(mock_init.called) - self.assertFalse(mock_wait_neutron.called) + api = dhcp_factory.DHCPFactory(token=self.context.auth_token) + api.update_dhcp(task, self.node) - @mock.patch('ironic.common.neutron._wait_for_neutron_update') - @mock.patch('ironic.common.neutron.NeutronAPI.update_port_dhcp_opts') - @mock.patch('ironic.common.neutron.get_node_vif_ids') - def test_update_neutron_some_failures(self, mock_gnvi, mock_updo, - mock_wait_neutron): + @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_dhcp_opts') + @mock.patch('ironic.common.network.get_node_vif_ids') + def test_update_dhcp_some_failures(self, mock_gnvi, mock_updo): # confirm update is called twice, one fails, but no exception raised mock_gnvi.return_value = {'p1': 'v1', 'p2': 'v2'} exc = exception.FailedToUpdateDHCPOptOnPort('fake exception') mock_updo.side_effect = [None, exc] with task_manager.acquire(self.context, self.node.uuid) as task: - neutron.update_neutron(task, self.node) + api = dhcp_factory.DHCPFactory(token=self.context.auth_token) + api.update_dhcp(task, self.node) + mock_gnvi.assertCalleOnceWith(task) self.assertEqual(2, mock_updo.call_count) - mock_wait_neutron.assert_called_once_with(task) - @mock.patch('ironic.common.neutron._wait_for_neutron_update') - @mock.patch('ironic.common.neutron.NeutronAPI.update_port_dhcp_opts') - @mock.patch('ironic.common.neutron.get_node_vif_ids') - def test_update_neutron_fails(self, mock_gnvi, mock_updo, - mock_wait_neutron): + @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_dhcp_opts') + @mock.patch('ironic.common.network.get_node_vif_ids') + def test_update_dhcp_fails(self, mock_gnvi, mock_updo): # confirm update is called twice, both fail, and exception is raised mock_gnvi.return_value = {'p1': 'v1', 'p2': 'v2'} exc = exception.FailedToUpdateDHCPOptOnPort('fake exception') mock_updo.side_effect = [exc, exc] with task_manager.acquire(self.context, self.node.uuid) as task: + api = dhcp_factory.DHCPFactory(token=self.context.auth_token) self.assertRaises(exception.FailedToUpdateDHCPOptOnPort, - neutron.update_neutron, + api.update_dhcp, task, self.node) + mock_gnvi.assertCalleOnceWith(task) self.assertEqual(2, mock_updo.call_count) - self.assertFalse(mock_wait_neutron.called) - - def test__wait_for_neutron_update(self): - kw = { - 'id': 190238451205398, - 'uuid': utils.generate_uuid(), - 'driver': 'fake_ssh' - } - node = object_utils.create_test_node(self.context, **kw) - mgr_utils.mock_the_extension_manager(driver="fake_ssh") - with task_manager.acquire(self.context, node.uuid) as task: - with mock.patch('time.sleep') as mock_sleep: - neutron._wait_for_neutron_update(task) - mock_sleep.assert_called_once_with(15) - - def test__wait_for_neutron_update_no_sleep(self): - with task_manager.acquire(self.context, self.node.uuid) as task: - with mock.patch('time.sleep') as mock_sleep: - neutron._wait_for_neutron_update(task) - self.assertFalse(mock_sleep.called) diff --git a/ironic/tests/drivers/test_agent.py b/ironic/tests/drivers/test_agent.py index bd3a188cee..504f3ee528 100644 --- a/ironic/tests/drivers/test_agent.py +++ b/ironic/tests/drivers/test_agent.py @@ -15,8 +15,8 @@ import mock from oslo.config import cfg +from ironic.common import dhcp_factory from ironic.common import exception -from ironic.common import neutron from ironic.common import pxe_utils from ironic.common import states from ironic.conductor import task_manager @@ -53,16 +53,16 @@ class TestAgentDeploy(db_base.DbTestCase): self.context, self.node['uuid'], shared=False) as task: self.driver.validate(task) - @mock.patch.object(neutron, 'update_neutron') + @mock.patch.object(dhcp_factory.DHCPFactory, 'update_dhcp') @mock.patch('ironic.conductor.utils.node_set_boot_device') @mock.patch('ironic.conductor.utils.node_power_action') - def test_deploy(self, power_mock, bootdev_mock, neutron_mock): + def test_deploy(self, power_mock, bootdev_mock, dhcp_mock): dhcp_opts = pxe_utils.dhcp_options_for_instance() with task_manager.acquire( self.context, self.node['uuid'], shared=False) as task: driver_return = self.driver.deploy(task) self.assertEqual(driver_return, states.DEPLOYWAIT) - neutron_mock.assert_called_once_with(task, dhcp_opts) + dhcp_mock.assert_called_once_with(task, dhcp_opts) bootdev_mock.assert_called_once_with(task, 'pxe', persistent=True) power_mock.assert_called_once_with(task, states.REBOOT) @@ -81,12 +81,12 @@ class TestAgentDeploy(db_base.DbTestCase): def test_clean_up(self): pass - @mock.patch.object(neutron, 'update_neutron') - def test_take_over(self, update_neutron_mock): + @mock.patch.object(dhcp_factory.DHCPFactory, 'update_dhcp') + def test_take_over(self, update_dhcp_mock): with task_manager.acquire( self.context, self.node['uuid'], shared=True) as task: task.driver.deploy.take_over(task) - update_neutron_mock.assert_called_once_with( + update_dhcp_mock.assert_called_once_with( task, CONF.agent.agent_pxe_bootfile_name) diff --git a/ironic/tests/drivers/test_pxe.py b/ironic/tests/drivers/test_pxe.py index ebbbef7061..2b383023ff 100644 --- a/ironic/tests/drivers/test_pxe.py +++ b/ironic/tests/drivers/test_pxe.py @@ -24,10 +24,10 @@ import tempfile from oslo.config import cfg +from ironic.common import dhcp_factory from ironic.common import exception from ironic.common.glance_service import base_image_service from ironic.common import keystone -from ironic.common import neutron from ironic.common import pxe_utils from ironic.common import states from ironic.common import utils @@ -471,11 +471,11 @@ class PXEDriverTestCase(db_base.DbTestCase): @mock.patch.object(deploy_utils, 'get_image_mb') @mock.patch.object(iscsi_deploy, '_get_image_file_path') @mock.patch.object(iscsi_deploy, 'cache_instance_image') - @mock.patch.object(neutron, 'update_neutron') + @mock.patch.object(dhcp_factory.DHCPFactory, 'update_dhcp') @mock.patch.object(manager_utils, 'node_power_action') @mock.patch.object(manager_utils, 'node_set_boot_device') def test_deploy(self, mock_node_set_boot, mock_node_power_action, - mock_update_neutron, mock_cache_instance_image, + mock_update_dhcp, mock_cache_instance_image, mock_get_image_file_path, mock_get_image_mb): fake_img_path = '/test/path/test.img' mock_get_image_file_path.return_value = fake_img_path @@ -483,14 +483,14 @@ class PXEDriverTestCase(db_base.DbTestCase): dhcp_opts = pxe_utils.dhcp_options_for_instance() with task_manager.acquire(self.context, - self.node.uuid, shared=False) as task: + self.node.uuid, shared=False) as task: state = task.driver.deploy.deploy(task) self.assertEqual(state, states.DEPLOYWAIT) mock_cache_instance_image.assert_called_once_with( self.context, task.node) mock_get_image_file_path.assert_called_once_with(task.node.uuid) mock_get_image_mb.assert_called_once_with(fake_img_path) - mock_update_neutron.assert_called_once_with( + mock_update_dhcp.assert_called_once_with( task, dhcp_opts) mock_node_set_boot.assert_called_once_with(task, 'pxe', persistent=True) @@ -528,13 +528,13 @@ class PXEDriverTestCase(db_base.DbTestCase): self.assertEqual(states.DELETED, state) node_power_mock.assert_called_once_with(task, states.POWER_OFF) - @mock.patch.object(neutron, 'update_neutron') - def test_take_over(self, update_neutron_mock): + @mock.patch.object(dhcp_factory.DHCPFactory, 'update_dhcp') + def test_take_over(self, update_dhcp_mock): dhcp_opts = pxe_utils.dhcp_options_for_instance() with task_manager.acquire( self.context, self.node.uuid, shared=True) as task: task.driver.deploy.take_over(task) - update_neutron_mock.assert_called_once_with( + update_dhcp_mock.assert_called_once_with( task, dhcp_opts) @mock.patch.object(deploy_utils, 'notify_deploy_complete') diff --git a/ironic/tests/test_network.py b/ironic/tests/test_network.py new file mode 100644 index 0000000000..bfeb4074dc --- /dev/null +++ b/ironic/tests/test_network.py @@ -0,0 +1,74 @@ +# Copyright 2014 Rackspace Inc. +# All Rights Reserved +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from ironic.common import network +from ironic.common import utils +from ironic.conductor import task_manager +from ironic.db import api as dbapi +from ironic.openstack.common import context +from ironic.tests import base +from ironic.tests.conductor import utils as mgr_utils +from ironic.tests.db import utils as db_utils +from ironic.tests.objects import utils as object_utils + + +class TestNetwork(base.TestCase): + + def setUp(self): + super(TestNetwork, self).setUp() + mgr_utils.mock_the_extension_manager(driver='fake') + self.dbapi = dbapi.get_instance() + self.context = context.get_admin_context() + self.node = object_utils.create_test_node(self.context) + + def _create_test_port(self, **kwargs): + p = db_utils.get_test_port(**kwargs) + return self.dbapi.create_port(p) + + def test_get_node_vif_ids_no_ports(self): + expected = {} + with task_manager.acquire(self.context, self.node.uuid) as task: + result = network.get_node_vif_ids(task) + self.assertEqual(expected, result) + + def test_get_node_vif_ids_one_port(self): + port1 = self._create_test_port(node_id=self.node.id, + id=6, + address='aa:bb:cc', + uuid=utils.generate_uuid(), + extra={'vif_port_id': 'test-vif-A'}, + driver='fake') + expected = {port1.uuid: 'test-vif-A'} + with task_manager.acquire(self.context, self.node.uuid) as task: + result = network.get_node_vif_ids(task) + self.assertEqual(expected, result) + + def test_get_node_vif_ids_two_ports(self): + port1 = self._create_test_port(node_id=self.node.id, + id=6, + address='aa:bb:cc', + uuid=utils.generate_uuid(), + extra={'vif_port_id': 'test-vif-A'}, + driver='fake') + port2 = self._create_test_port(node_id=self.node.id, + id=7, + address='dd:ee:ff', + uuid=utils.generate_uuid(), + extra={'vif_port_id': 'test-vif-B'}, + driver='fake') + expected = {port1.uuid: 'test-vif-A', port2.uuid: 'test-vif-B'} + with task_manager.acquire(self.context, self.node.uuid) as task: + result = network.get_node_vif_ids(task) + self.assertEqual(expected, result) diff --git a/setup.cfg b/setup.cfg index fd1df2586a..1eab0febc1 100644 --- a/setup.cfg +++ b/setup.cfg @@ -29,6 +29,10 @@ console_scripts = ironic-conductor = ironic.cmd.conductor:main ironic-rootwrap = oslo.rootwrap.cmd:main +ironic.dhcp = + neutron = ironic.dhcp.neutron:NeutronDHCPApi + none = ironic.dhcp.none:NoneDHCPApi + ironic.drivers = agent_ipmitool = ironic.drivers.agent:AgentAndIPMIToolDriver agent_pyghmi = ironic.drivers.agent:AgentAndIPMINativeDriver