Fixes timeout waiting for vif plugging callback for instance

Currently, plug_vifs is called when powering on the instance.
The call to wait_for_plug_events will give a timeout waiting
for the plug event when using neutron-ovs-agent.

This patch addresses this issue by moving the call to plug_vifs
from power_on to after the create_instance call, which is prior
to waiting for plug events. This change modifies the behaviour
of power_on only during the instance spawn, the other calls
remaining the same.

Change-Id: I13b7caf84434a4b9b6f2795cddc7401874c46647
Closes-Bug: #1684250

hyperv/nova/vmops.py

@@ -305,6 +305,8 @@ class VMOps(object):
                 # waiting will occur after the instance is created.
                 self.create_instance(context, instance, network_info,
                                      block_device_info, vm_gen, image_meta)
+                # This is supported starting from OVS version 2.5
+                self.plug_vifs(instance, network_info)
 
             self._save_device_metadata(context, instance, block_device_info)
 
@@ -317,7 +319,12 @@ class VMOps(object):
 
                 self.attach_config_drive(instance, configdrive_path, vm_gen)
             self.set_boot_order(instance.name, vm_gen, block_device_info)
-            self.power_on(instance, network_info=network_info)
+            # vifs are already plugged in at this point. We waited on the vif
+            # plug event previously when we created the instance. Skip the
+            # plug vifs during power on in this case
+            self.power_on(instance,
+                          network_info=network_info,
+                          should_plug_vifs=False)
         except Exception:
             with excutils.save_and_reraise_exception():
                 self.destroy(instance)
@@ -922,7 +929,8 @@ class VMOps(object):
             LOG.debug("Instance not found. Skipping power off",
                       instance=instance)
 
-    def power_on(self, instance, block_device_info=None, network_info=None):
+    def power_on(self, instance, block_device_info=None, network_info=None,
+                 should_plug_vifs=True):
         """Power on the specified instance."""
         LOG.debug("Power on instance", instance=instance)
 
@@ -930,8 +938,9 @@ class VMOps(object):
             self._volumeops.fix_instance_volume_disk_paths(instance.name,
                                                            block_device_info)
 
+        if should_plug_vifs:
+            self.plug_vifs(instance, network_info)
         self._set_vm_state(instance, os_win_const.HYPERV_VM_STATE_ENABLED)
-        self.plug_vifs(instance, network_info)
 
     def _set_vm_state(self, instance, req_state):
         instance_name = instance.name

hyperv/tests/unit/test_vmops.py

@@ -417,6 +417,7 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
         self._vmops._vmutils.set_boot_order.assert_called_once_with(
             mock.sentinel.instance_name, mock_get_boot_order.return_value)
 
+    @mock.patch.object(vmops.VMOps, 'plug_vifs')
     @mock.patch('hyperv.nova.vmops.VMOps.destroy')
     @mock.patch('hyperv.nova.vmops.VMOps.power_on')
     @mock.patch.object(vmops.VMOps, 'set_boot_order')
@@ -442,8 +443,8 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
                     mock_configdrive_required,
                     mock_create_config_drive, mock_attach_config_drive,
                     mock_set_boot_order,
-                    mock_power_on, mock_destroy, exists,
-                    configdrive_required, fail,
+                    mock_power_on, mock_destroy, mock_plug_vifs,
+                    exists, configdrive_required, fail,
                     fake_vm_gen=constants.VM_GEN_2):
         mock_instance = fake_instance.fake_instance_obj(self.context)
         mock_image_meta = mock.MagicMock()
@@ -490,6 +491,8 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
             mock_create_instance.assert_called_once_with(
                 self.context, mock_instance, mock.sentinel.INFO,
                 block_device_info, fake_vm_gen, mock_image_meta)
+            mock_plug_vifs.assert_called_once_with(mock_instance,
+                                                   mock.sentinel.INFO)
             mock_save_device_metadata.assert_called_once_with(
                 self.context, mock_instance, block_device_info)
             mock_configdrive_required.assert_called_once_with(mock_instance)
@@ -503,7 +506,9 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
             mock_set_boot_order.assert_called_once_with(
                 mock_instance.name, fake_vm_gen, block_device_info)
             mock_power_on.assert_called_once_with(
-                mock_instance, network_info=mock.sentinel.INFO)
+                mock_instance,
+                network_info=mock.sentinel.INFO,
+                should_plug_vifs=False)
 
     def test_spawn(self):
         self._test_spawn(exists=False, configdrive_required=True, fail=None)
@@ -1453,6 +1458,14 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
         mock_plug_vifs.assert_called_once_with(
             mock_instance, mock.sentinel.fake_network_info)
 
+    @mock.patch.object(vmops.VMOps, 'plug_vifs')
+    def test_power_on_vifs_already_plugged(self, mock_plug_vifs):
+        mock_instance = fake_instance.fake_instance_obj(self.context)
+
+        self._vmops.power_on(mock_instance,
+                             should_plug_vifs=False)
+        self.assertFalse(mock_plug_vifs.called)
+
     def _test_set_vm_state(self, state):
         mock_instance = fake_instance.fake_instance_obj(self.context)