From 7e39428691eab57bfb23fda2185695da4a666fc4 Mon Sep 17 00:00:00 2001 From: Sahid Orentino Ferdjaoui Date: Wed, 19 Dec 2018 10:00:30 -0500 Subject: [PATCH] add mutex to protect container instances (SO) Without protecting it, several methods can access in same time to the container instance and updating the state. (AJK) Also fix py27 change where nova.network.linux_utils has moved to/as nova.privsep.linux_net Closes-Bug: #1809114 Change-Id: I28e68e150f5d6e3efdb243aae9e3cf15fda01a65 Co-authored-by: Sahid Orentino Ferdjaoui Co-authored-by: Alex Kavanagh --- nova/tests/unit/virt/lxd/test_driver.py | 9 +++-- nova/tests/unit/virt/lxd/test_vif.py | 50 ++++++++++++------------ nova/virt/lxd/driver.py | 51 ++++++++++++++----------- nova/virt/lxd/vif.py | 22 +++++------ 4 files changed, 71 insertions(+), 61 deletions(-) diff --git a/nova/tests/unit/virt/lxd/test_driver.py b/nova/tests/unit/virt/lxd/test_driver.py index 223c4179..b417186a 100644 --- a/nova/tests/unit/virt/lxd/test_driver.py +++ b/nova/tests/unit/virt/lxd/test_driver.py @@ -524,7 +524,8 @@ class LXDDriverTest(test.NoDBTestCase): self._test_spawn_instance_with_network_events( neutron_failure='timeout') - def test_destroy(self): + @mock.patch('nova.virt.lxd.driver.lockutils.lock') + def test_destroy(self, lock): mock_container = mock.Mock() mock_container.status = 'Running' self.client.containers.get.return_value = mock_container @@ -545,7 +546,8 @@ class LXDDriverTest(test.NoDBTestCase): mock_container.stop.assert_called_once_with(wait=True) mock_container.delete.assert_called_once_with(wait=True) - def test_destroy_when_in_rescue(self): + @mock.patch('nova.virt.lxd.driver.lockutils.lock') + def test_destroy_when_in_rescue(self, lock): mock_stopped_container = mock.Mock() mock_stopped_container.status = 'Stopped' mock_rescued_container = mock.Mock() @@ -579,7 +581,8 @@ class LXDDriverTest(test.NoDBTestCase): mock_rescued_container.stop.assert_called_once_with(wait=True) mock_rescued_container.delete.assert_called_once_with(wait=True) - def test_destroy_without_instance(self): + @mock.patch('nova.virt.lxd.driver.lockutils.lock') + def test_destroy_without_instance(self, lock): def side_effect(*args, **kwargs): raise lxdcore_exceptions.LXDAPIException(MockResponse(404)) self.client.containers.get.side_effect = side_effect diff --git a/nova/tests/unit/virt/lxd/test_vif.py b/nova/tests/unit/virt/lxd/test_vif.py index 7e2f37c6..8554c0d7 100644 --- a/nova/tests/unit/virt/lxd/test_vif.py +++ b/nova/tests/unit/virt/lxd/test_vif.py @@ -201,12 +201,12 @@ class LXDGenericVifDriverTest(test.NoDBTestCase): _post_plug_wiring.assert_called_with(INSTANCE, TAP_VIF) @mock.patch.object(vif, '_post_unplug_wiring') - @mock.patch('nova.virt.lxd.vif.network_utils') + @mock.patch('nova.virt.lxd.vif.linux_net') @mock.patch('nova.virt.lxd.vif.os_vif') - def test_unplug_tap(self, os_vif, network_utils, _post_unplug_wiring): + def test_unplug_tap(self, os_vif, linux_net, _post_unplug_wiring): self.vif_driver.unplug(INSTANCE, TAP_VIF) os_vif.plug.assert_not_called() - network_utils.delete_net_dev.assert_called_with('tapda5cc4bf-f1') + linux_net.delete_net_dev.assert_called_with('tapda5cc4bf-f1') _post_unplug_wiring.assert_called_with(INSTANCE, TAP_VIF) @@ -218,16 +218,16 @@ class PostPlugTest(test.NoDBTestCase): @mock.patch('nova.virt.lxd.vif._create_veth_pair') @mock.patch('nova.virt.lxd.vif._add_bridge_port') - @mock.patch('nova.virt.lxd.vif.network_utils') + @mock.patch('nova.virt.lxd.vif.linux_net') def test_post_plug_ovs_hybrid(self, - network_utils, + linux_net, add_bridge_port, create_veth_pair): - network_utils.device_exists.return_value = False + linux_net.device_exists.return_value = False vif._post_plug_wiring(INSTANCE, OVS_HYBRID_VIF) - network_utils.device_exists.assert_called_with('tapda5cc4bf-f1') + linux_net.device_exists.assert_called_with('tapda5cc4bf-f1') create_veth_pair.assert_called_with('tapda5cc4bf-f1', 'tinda5cc4bf-f1', 1000) @@ -237,18 +237,18 @@ class PostPlugTest(test.NoDBTestCase): @mock.patch('nova.virt.lxd.vif._create_veth_pair') @mock.patch('nova.virt.lxd.vif._add_bridge_port') @mock.patch.object(vif, '_create_ovs_vif_port') - @mock.patch('nova.virt.lxd.vif.network_utils') + @mock.patch('nova.virt.lxd.vif.linux_net') def test_post_plug_ovs(self, - network_utils, + linux_net, create_ovs_vif_port, add_bridge_port, create_veth_pair): - network_utils.device_exists.return_value = False + linux_net.device_exists.return_value = False vif._post_plug_wiring(INSTANCE, OVS_VIF) - network_utils.device_exists.assert_called_with('tapda5cc4bf-f1') + linux_net.device_exists.assert_called_with('tapda5cc4bf-f1') create_veth_pair.assert_called_with('tapda5cc4bf-f1', 'tinda5cc4bf-f1', 1000) @@ -264,16 +264,16 @@ class PostPlugTest(test.NoDBTestCase): @mock.patch('nova.virt.lxd.vif._create_veth_pair') @mock.patch('nova.virt.lxd.vif._add_bridge_port') - @mock.patch('nova.virt.lxd.vif.network_utils') + @mock.patch('nova.virt.lxd.vif.linux_net') def test_post_plug_bridge(self, - network_utils, + linux_net, add_bridge_port, create_veth_pair): - network_utils.device_exists.return_value = False + linux_net.device_exists.return_value = False vif._post_plug_wiring(INSTANCE, LB_VIF) - network_utils.device_exists.assert_called_with('tapda5cc4bf-f1') + linux_net.device_exists.assert_called_with('tapda5cc4bf-f1') create_veth_pair.assert_called_with('tapda5cc4bf-f1', 'tinda5cc4bf-f1', 1000) @@ -282,25 +282,25 @@ class PostPlugTest(test.NoDBTestCase): @mock.patch('nova.virt.lxd.vif._create_veth_pair') @mock.patch('nova.virt.lxd.vif._add_bridge_port') - @mock.patch('nova.virt.lxd.vif.network_utils') + @mock.patch('nova.virt.lxd.vif.linux_net') def test_post_plug_tap(self, - network_utils, + linux_net, add_bridge_port, create_veth_pair): - network_utils.device_exists.return_value = False + linux_net.device_exists.return_value = False vif._post_plug_wiring(INSTANCE, TAP_VIF) - network_utils.device_exists.assert_not_called() + linux_net.device_exists.assert_not_called() class PostUnplugTest(test.NoDBTestCase): """Tests for post unplug operations""" - @mock.patch('nova.virt.lxd.vif.network_utils') - def test_post_unplug_ovs_hybrid(self, network_utils): + @mock.patch('nova.virt.lxd.vif.linux_net') + def test_post_unplug_ovs_hybrid(self, linux_net): vif._post_unplug_wiring(INSTANCE, OVS_HYBRID_VIF) - network_utils.delete_net_dev.assert_called_with('tapda5cc4bf-f1') + linux_net.delete_net_dev.assert_called_with('tapda5cc4bf-f1') @mock.patch.object(vif, '_delete_ovs_vif_port') def test_post_unplug_ovs(self, delete_ovs_vif_port): @@ -309,10 +309,10 @@ class PostUnplugTest(test.NoDBTestCase): 'tapda5cc4bf-f1', True) - @mock.patch('nova.virt.lxd.vif.network_utils') - def test_post_unplug_bridge(self, network_utils): + @mock.patch('nova.virt.lxd.vif.linux_net') + def test_post_unplug_bridge(self, linux_net): vif._post_unplug_wiring(INSTANCE, LB_VIF) - network_utils.delete_net_dev.assert_called_with('tapda5cc4bf-f1') + linux_net.delete_net_dev.assert_called_with('tapda5cc4bf-f1') class MiscHelpersTest(test.NoDBTestCase): diff --git a/nova/virt/lxd/driver.py b/nova/virt/lxd/driver.py index 8bd7939d..b196a264 100644 --- a/nova/virt/lxd/driver.py +++ b/nova/virt/lxd/driver.py @@ -627,27 +627,34 @@ class LXDDriver(driver.ComputeDriver): See `nova.virt.driver.ComputeDriver.destroy` for more information. """ - try: - container = self.client.containers.get(instance.name) - if container.status != 'Stopped': - container.stop(wait=True) - container.delete(wait=True) - if (instance.vm_state == vm_states.RESCUED): - rescued_container = self.client.containers.get( - '%s-rescue' % instance.name) - if rescued_container.status != 'Stopped': - rescued_container.stop(wait=True) - rescued_container.delete(wait=True) - except lxd_exceptions.LXDAPIException as e: - if e.response.status_code == 404: - LOG.warning("Failed to delete instance. " - "Container does not exist for {instance}." - .format(instance=instance.name)) - else: - raise - finally: - self.cleanup( - context, instance, network_info, block_device_info) + lock_path = os.path.join(CONF.instances_path, 'locks') + + with lockutils.lock( + lock_path, external=True, + lock_file_prefix='lxd-container-{}'.format(instance.name)): + # TODO(sahid): Each time we get a container we should + # protect it by using a mutex. + try: + container = self.client.containers.get(instance.name) + if container.status != 'Stopped': + container.stop(wait=True) + container.delete(wait=True) + if (instance.vm_state == vm_states.RESCUED): + rescued_container = self.client.containers.get( + '{}-rescue'.format(instance.name)) + if rescued_container.status != 'Stopped': + rescued_container.stop(wait=True) + rescued_container.delete(wait=True) + except lxd_exceptions.LXDAPIException as e: + if e.response.status_code == 404: + LOG.warning("Failed to delete instance. " + "Container does not exist for {instance}." + .format(instance=instance.name)) + else: + raise + finally: + self.cleanup( + context, instance, network_info, block_device_info) def cleanup(self, context, instance, network_info, block_device_info=None, destroy_disks=True, migrate_data=None, destroy_vifs=True): @@ -850,7 +857,7 @@ class LXDDriver(driver.ComputeDriver): with lockutils.lock( lock_path, external=True, - lock_file_prefix=('lxd-snapshot-%s' % instance.name)): + lock_file_prefix='lxd-container-{}'.format(instance.name)): update_task_state(task_state=task_states.IMAGE_PENDING_UPLOAD) diff --git a/nova/virt/lxd/vif.py b/nova/virt/lxd/vif.py index 6df6cf32..1cd7a715 100644 --- a/nova/virt/lxd/vif.py +++ b/nova/virt/lxd/vif.py @@ -20,7 +20,7 @@ from nova import exception from nova import utils from nova.network import model as network_model from nova.network import os_vif_util -from nova.network import linux_utils as network_utils +from nova.privsep import linux_net import os_vif @@ -47,14 +47,14 @@ def _create_veth_pair(dev1_name, dev2_name, mtu=None): deleting any previous devices with those names. """ for dev in [dev1_name, dev2_name]: - network_utils.delete_net_dev(dev) + linux_net.delete_net_dev(dev) utils.execute('ip', 'link', 'add', dev1_name, 'type', 'veth', 'peer', 'name', dev2_name, run_as_root=True) for dev in [dev1_name, dev2_name]: utils.execute('ip', 'link', 'set', dev, 'up', run_as_root=True) - network_utils.set_device_mtu(dev, mtu) + linux_net.set_device_mtu(dev, mtu) def _add_bridge_port(bridge, dev): @@ -119,13 +119,13 @@ def _create_ovs_vif_port(bridge, dev, iface_id, mac, instance_id, _ovs_vsctl(_create_ovs_vif_cmd(bridge, dev, iface_id, mac, instance_id, interface_type)) - network_utils.set_device_mtu(dev, mtu) + linux_net.set_device_mtu(dev, mtu) def _delete_ovs_vif_port(bridge, dev, delete_dev=True): _ovs_vsctl(['--', '--if-exists', 'del-port', bridge, dev]) if delete_dev: - network_utils.delete_net_dev(dev) + linux_net.delete_net_dev(dev) CONFIG_GENERATORS = { @@ -162,7 +162,7 @@ def _post_plug_wiring_veth_and_bridge(instance, vif): mtu = network.get_meta('mtu') if network else None v1_name = get_vif_devname(vif) v2_name = get_vif_internal_devname(vif) - if not network_utils.device_exists(v1_name): + if not linux_net.device_exists(v1_name): _create_veth_pair(v1_name, v2_name, mtu) if _is_ovs_vif_port(vif): # NOTE(jamespage): wire tap device directly to ovs bridge @@ -176,7 +176,7 @@ def _post_plug_wiring_veth_and_bridge(instance, vif): # NOTE(jamespage): wire tap device linux bridge _add_bridge_port(config['bridge'], v1_name) else: - network_utils.set_device_mtu(v1_name, mtu) + linux_net.set_device_mtu(v1_name, mtu) POST_PLUG_WIRING = { @@ -229,7 +229,7 @@ def _post_unplug_wiring_delete_veth(instance, vif): _delete_ovs_vif_port(vif['network']['bridge'], v1_name, True) else: - network_utils.delete_net_dev(v1_name) + linux_net.delete_net_dev(v1_name) except processutils.ProcessExecutionError: LOG.exception("Failed to delete veth for vif {}".foramt(vif), instance=instance) @@ -320,16 +320,16 @@ class LXDGenericVifDriver(object): # NOTE(jamespage): For nova-lxd this is really a veth pair # so that a) security rules get applied on the host # and b) that the container can still be wired. - if not network_utils.device_exists(v1_name): + if not linux_net.device_exists(v1_name): _create_veth_pair(v1_name, v2_name, mtu) else: - network_utils.set_device_mtu(v1_name, mtu) + linux_net.set_device_mtu(v1_name, mtu) def unplug_tap(self, instance, vif): """Unplug a VIF_TYPE_TAP virtual interface.""" dev = get_vif_devname(vif) try: - network_utils.delete_net_dev(dev) + linux_net.delete_net_dev(dev) except processutils.ProcessExecutionError: LOG.exception("Failed while unplugging vif for instance", instance=instance)