From feb4bae60fd86d107b1f6479bdf38c16d623398d Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Thu, 4 Oct 2018 12:14:13 +0300 Subject: [PATCH] Switch to new instance failover listener We had a bunch of issues with the WMI based event listener, which appear to be bugs in the WMI provider. We've added a new listener that uses the underlying C library. The only issue is that this new listener cannot tell us the source of the migrated VMs, so we'll have to rely on the information from Nova. Closes-Bug: #1798069 Depends-On: I63d1aa3a6f9fb12d08478eb41fe973b1582b540c Change-Id: I9e777351eca84274cafc3f04b6bc27c3f6b572ca --- compute_hyperv/nova/cluster/clusterops.py | 22 +++++++++---------- .../tests/unit/cluster/test_clusterops.py | 15 ++++++------- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/compute_hyperv/nova/cluster/clusterops.py b/compute_hyperv/nova/cluster/clusterops.py index 7b46e1d1..5cb83e9b 100644 --- a/compute_hyperv/nova/cluster/clusterops.py +++ b/compute_hyperv/nova/cluster/clusterops.py @@ -82,7 +82,7 @@ class ClusterOps(object): def start_failover_listener_daemon(self): """Start the daemon failover listener.""" - listener = self._clustutils.get_vm_owner_change_listener() + listener = self._clustutils.get_vm_owner_change_listener_v2() cbk = functools.partial(utils.spawn_n, self._failover_migrate) utils.spawn_n(listener, cbk) @@ -105,18 +105,12 @@ class ClusterOps(object): instance.name, instance.host, self._this_node) - def _failover_migrate(self, instance_name, old_host, new_host): + def _failover_migrate(self, instance_name, new_host): """This method will check if the generated event is a legitimate failover to this node. If it is, it will proceed to prepare the failovered VM if necessary and update the owner of the compute vm in nova and ports in neutron. """ - LOG.info('Checking instance failover %(instance)s to %(new_host)s ' - 'from host %(old_host)s.', - {'instance': instance_name, - 'new_host': new_host, - 'old_host': old_host}) - instance = self._get_instance_by_name(instance_name) if not instance: # Some instances on the hypervisor may not be tracked by nova @@ -124,6 +118,13 @@ class ClusterOps(object): instance_name) return + old_host = instance.host + LOG.info('Checking instance failover %(instance)s to %(new_host)s ' + 'from host %(old_host)s.', + {'instance': instance_name, + 'new_host': new_host, + 'old_host': old_host}) + if instance.task_state == task_states.MIGRATING: # In case of live migration triggered by the user, we get the # event that the instance changed host but we do not want @@ -142,9 +143,8 @@ class ClusterOps(object): instance_name) return - LOG.info('Instance %(instance)s failover to %(host)s.', - {'instance': instance_name, - 'host': new_host}) + LOG.info('Handling instance %(instance)s failover to this host.', + {'instance': instance_name}) self._nova_failover_server(instance, new_host) self._failover_migrate_networks(instance, old_host) diff --git a/compute_hyperv/tests/unit/cluster/test_clusterops.py b/compute_hyperv/tests/unit/cluster/test_clusterops.py index d6971ee0..c3d26b1a 100644 --- a/compute_hyperv/tests/unit/cluster/test_clusterops.py +++ b/compute_hyperv/tests/unit/cluster/test_clusterops.py @@ -112,7 +112,7 @@ class ClusterOpsTestCase(test_base.HyperVBaseTestCase): spawn_args = mock_spawn.call_args_list[0][0] self.assertEqual( - self._clustutils.get_vm_owner_change_listener.return_value, + self._clustutils.get_vm_owner_change_listener_v2.return_value, spawn_args[0]) cbk = spawn_args[1] @@ -149,7 +149,6 @@ class ClusterOpsTestCase(test_base.HyperVBaseTestCase): mock_get_instance_by_name.return_value = None self.clusterops._failover_migrate(mock.sentinel.instance_name, - mock.sentinel.old_host, mock.sentinel.new_host) mock_LOG.debug.assert_called_once_with( @@ -166,7 +165,6 @@ class ClusterOpsTestCase(test_base.HyperVBaseTestCase): instance.task_state = task_states.MIGRATING self.clusterops._failover_migrate(mock.sentinel.instance_name, - mock.sentinel.old_host, mock.sentinel.new_host) mock_LOG.debug.assert_called_once_with( @@ -175,11 +173,11 @@ class ClusterOpsTestCase(test_base.HyperVBaseTestCase): @mock.patch.object(clusterops.ClusterOps, '_get_instance_by_name') def test_failover_migrate_at_source_node(self, mock_get_instance_by_name): instance = mock_get_instance_by_name.return_value - old_host = 'old_host' - self.clusterops._this_node = old_host + instance.host = 'old_host' + self.clusterops._this_node = instance.host self.clusterops._failover_migrate(mock.sentinel.instance_name, - old_host, mock.sentinel.new_host) + mock.sentinel.new_host) self.clusterops._vmops.unplug_vifs.assert_called_once_with(instance, self.clusterops._network_api.get_instance_nw_info.return_value) @@ -191,7 +189,7 @@ class ClusterOpsTestCase(test_base.HyperVBaseTestCase): self.clusterops._this_node = 'new_host' self.clusterops._failover_migrate(mock.sentinel.instance_name, - None, 'host') + 'host') mock_LOG.debug.assert_called_once_with( 'Instance %s did not failover to this node.', @@ -206,10 +204,11 @@ class ClusterOpsTestCase(test_base.HyperVBaseTestCase): instance = mock_get_instance_by_name.return_value old_host = 'old_host' new_host = 'new_host' + instance.host = old_host self.clusterops._this_node = new_host self.clusterops._failover_migrate(mock.sentinel.instance_name, - old_host, new_host) + new_host) mock_get_instance_by_name.assert_called_once_with( mock.sentinel.instance_name)