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 <sahid.ferdjaoui@canonical.com>
Co-authored-by: Alex Kavanagh <alex.kavanagh@canonical.com>

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

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')
+    @mock.patch('nova.virt.lxd.vif.linux_net')
-    def test_post_unplug_ovs_hybrid(self, network_utils):
+    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')
+    @mock.patch('nova.virt.lxd.vif.linux_net')
-    def test_post_unplug_bridge(self, network_utils):
+    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):

nova/virt/lxd/driver.py

@@ -627,27 +627,34 @@ class LXDDriver(driver.ComputeDriver):
         See `nova.virt.driver.ComputeDriver.destroy` for more
         information.
         """
-        try:
+        lock_path = os.path.join(CONF.instances_path, 'locks')
-            container = self.client.containers.get(instance.name)
+
-            if container.status != 'Stopped':
+        with lockutils.lock(
-                container.stop(wait=True)
+                lock_path, external=True,
-            container.delete(wait=True)
+                lock_file_prefix='lxd-container-{}'.format(instance.name)):
-            if (instance.vm_state == vm_states.RESCUED):
+            # TODO(sahid): Each time we get a container we should
-                rescued_container = self.client.containers.get(
+            # protect it by using a mutex.
-                    '%s-rescue' % instance.name)
+            try:
-                if rescued_container.status != 'Stopped':
+                container = self.client.containers.get(instance.name)
-                    rescued_container.stop(wait=True)
+                if container.status != 'Stopped':
-                rescued_container.delete(wait=True)
+                    container.stop(wait=True)
-        except lxd_exceptions.LXDAPIException as e:
+                container.delete(wait=True)
-            if e.response.status_code == 404:
+                if (instance.vm_state == vm_states.RESCUED):
-                LOG.warning("Failed to delete instance. "
+                    rescued_container = self.client.containers.get(
-                            "Container does not exist for {instance}."
+                        '{}-rescue'.format(instance.name))
-                            .format(instance=instance.name))
+                    if rescued_container.status != 'Stopped':
-            else:
+                        rescued_container.stop(wait=True)
-                raise
+                    rescued_container.delete(wait=True)
-        finally:
+            except lxd_exceptions.LXDAPIException as e:
-            self.cleanup(
+                if e.response.status_code == 404:
-                context, instance, network_info, block_device_info)
+                    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)
 

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)