Add functional regression test for bug 1853009

Bug 1853009 describes a race condition involving multiple nova-compute
services with ironic. As the compute services start up, the hash ring
rebalances, and the compute services have an inconsistent view of which
is responsible for a compute node.

The sequence of actions here is adapted from a real world log [1], where
multiple nova-compute services were started simultaneously. In some
cases mocks are used to simulate race conditions.

There are three main issues with the behaviour:

* host2 deletes the orphan node compute node after host1 has taken
  ownership of it.

* host1 assumes that another compute service will not delete its nodes.
  Once a node is in rt.compute_nodes, it is not removed again unless the
  node is orphaned. This prevents host1 from recreating the compute
  node.

* host1 assumes that another compute service will not delete its
  resource providers. Once an RP is in the provider tree, it is not
  removed.

This functional test documents the current behaviour, with the idea that
it can be updated as this behaviour is fixed.

[1] http://paste.openstack.org/show/786272/

Co-Authored-By: Matt Riedemann <[email protected]>

Change-Id: Ice4071722de54e8d20bb8c3795be22f1995940cd
Related-Bug: #1853009
Related-Bug: #1853159

Author: markgoddard

nova/tests/functional/regressions/test_bug_1853009.py

@@ -0,0 +1,162 @@
+# 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 nova import context
+from nova import objects
+from nova.tests.functional import integrated_helpers
+
+
+class NodeRebalanceDeletedComputeNodeRaceTestCase(
+        integrated_helpers.ProviderUsageBaseTestCase):
+    """Regression test for bug 1853009 observed in Rocky & later.
+
+    When an ironic node re-balances from one host to another, there can be a
+    race where the old host deletes the orphan compute node after the new host
+    has taken ownership of it which results in the new host failing to create
+    the compute node and resource provider because the ResourceTracker does not
+    detect a change.
+    """
+    # Make sure we're using the fake driver that has predictable uuids
+    # for each node.
+    compute_driver = 'fake.PredictableNodeUUIDDriver'
+
+    def _assert_hypervisor_api(self, nodename, expected_host):
+        # We should have one compute node shown by the API.
+        hypervisors = self.api.api_get(
+            '/os-hypervisors/detail').body['hypervisors']
+        self.assertEqual(1, len(hypervisors), hypervisors)
+        hypervisor = hypervisors[0]
+        self.assertEqual(nodename, hypervisor['hypervisor_hostname'])
+        self.assertEqual(expected_host, hypervisor['service']['host'])
+
+    def _start_compute(self, host):
+        host = self.start_service('compute', host)
+        # Ironic compute driver has rebalances_nodes = True.
+        host.manager.driver.rebalances_nodes = True
+        return host
+
+    def setUp(self):
+        super(NodeRebalanceDeletedComputeNodeRaceTestCase, self).setUp()
+        nodename = 'fake-node'
+        ctxt = context.get_admin_context()
+
+        # Simulate a service running and then stopping.
+        # host2 runs, creates fake-node, then is stopped. The fake-node compute
+        # node is destroyed. This leaves a soft-deleted node in the DB.
+        host2 = self._start_compute('host2')
+        host2.manager.driver._set_nodes([nodename])
+        host2.manager.update_available_resource(ctxt)
+        host2.stop()
+        cn = objects.ComputeNode.get_by_host_and_nodename(
+            ctxt, 'host2', nodename)
+        cn.destroy()
+
+    def test_node_rebalance_deleted_compute_node_race(self):
+        nodename = 'fake-node'
+        ctxt = context.get_admin_context()
+
+        # First we create a compute service to manage our node.
+        # When start_service runs, it will create a host1 ComputeNode. We want
+        # to delete that and inject our fake node into the driver which will
+        # be re-balanced to another host later.
+        host1 = self._start_compute('host1')
+        host1.manager.driver._set_nodes([nodename])
+
+        # Run the update_available_resource periodic to register fake-node and
+        # have it managed by host1. This will also detect the "host1" node as
+        # orphaned and delete it along with its resource provider.
+
+        # host1[1]: Finds no compute record in RT. Tries to create one
+        # (_init_compute_node).
+        # FIXME(mgoddard): This shows a traceback with SQL rollback due to
+        # soft-deleted node. The create seems to succeed but breaks the RT
+        # update for this node. See
+        # https://bugs.launchpad.net/nova/+bug/1853159.
+        host1.manager.update_available_resource(ctxt)
+        self._assert_hypervisor_api(nodename, 'host1')
+        # There should only be one resource provider (fake-node).
+        original_rps = self._get_all_providers()
+        self.assertEqual(1, len(original_rps), original_rps)
+        self.assertEqual(nodename, original_rps[0]['name'])
+
+        # Simulate a re-balance by starting host2 and make it manage fake-node.
+        # At this point both host1 and host2 think they own fake-node.
+        host2 = self._start_compute('host2')
+        host2.manager.driver._set_nodes([nodename])
+
+        # host2[1]: Finds no compute record in RT, 'moves' existing node from
+        # host1
+        host2.manager.update_available_resource(ctxt)
+        # Assert that fake-node was re-balanced from host1 to host2.
+        self.assertIn('ComputeNode fake-node moving from host1 to host2',
+                      self.stdlog.logger.output)
+        self._assert_hypervisor_api(nodename, 'host2')
+
+        # host2[2]: Begins periodic update, queries compute nodes for this
+        # host, finds the fake-node.
+        cn = objects.ComputeNode.get_by_host_and_nodename(
+            ctxt, 'host2', nodename)
+
+        # host1[2]: Finds no compute record in RT, 'moves' existing node from
+        # host2
+        host1.manager.update_available_resource(ctxt)
+        # Assert that fake-node was re-balanced from host2 to host1.
+        self.assertIn('ComputeNode fake-node moving from host2 to host1',
+                      self.stdlog.logger.output)
+        self._assert_hypervisor_api(nodename, 'host1')
+
+        # Complete rebalance, as host2 realises it does not own fake-node.
+        host2.manager.driver._set_nodes([])
+
+        # host2[2]: Deletes orphan compute node.
+        # Mock out the compute node query to simulate a race condition where
+        # the list includes an orphan compute node that is taken ownership of
+        # by host1 by the time host2 deletes it.
+        # FIXME(mgoddard): Ideally host2 would not delete a node that does not
+        # belong to it. See https://bugs.launchpad.net/nova/+bug/1853009.
+        with mock.patch('nova.compute.manager.ComputeManager.'
+                        '_get_compute_nodes_in_db') as mock_get:
+            mock_get.return_value = [cn]
+            host2.manager.update_available_resource(ctxt)
+
+        # Verify that the node was deleted.
+        self.assertIn("Deleting orphan compute node %s hypervisor host "
+                      "is fake-node, nodes are" % cn.id,
+                      self.stdlog.logger.output)
+        hypervisors = self.api.api_get(
+            '/os-hypervisors/detail').body['hypervisors']
+        self.assertEqual(0, len(hypervisors), hypervisors)
+        rps = self._get_all_providers()
+        self.assertEqual(0, len(rps), rps)
+
+        # host1[3]: Should recreate compute node and resource provider.
+        # FIXME(mgoddard): Compute node not recreated here, because it is
+        # already in RT.compute_nodes. See
+        # https://bugs.launchpad.net/nova/+bug/1853009.
+        host1.manager.update_available_resource(ctxt)
+
+        # Verify that the node was not recreated.
+        hypervisors = self.api.api_get(
+            '/os-hypervisors/detail').body['hypervisors']
+        self.assertEqual(0, len(hypervisors), hypervisors)
+
+        rt = host1.manager._get_resource_tracker()
+
+        # But the compute node exists in the RT.
+        self.assertIn(nodename, rt.compute_nodes)
+
+        # The RP exists in Rocky, due to the lack of a provider tree cache.
+        rps = self._get_all_providers()
+        self.assertEqual(1, len(rps), rps)
+        self.assertEqual(nodename, rps[0]['name'])