Repro gen conflict in COMPUTE_STATUS_DISABLED handling

The COMPUTE_STATUS_DISABLED trait is supposed to be added to the compute
RP when the compute service is disabled, and the trait supposed to be
removed when the service is enabled again. However adding and removing
traits is subject to generation conflict in placement. The original
implementation of blueprint pre-filter-disabled-computes noticed this
and prints a detailed warning message while the API operation succeeds.
We can ignore the conflict this way because the periodic
update_available_resource() call will re-sync the traits later.

Still this gives human noticeable time window where the trait and the
service state are not in sync.

Setting the compute service disable is the smaller problem as the
scheduler still uses the ComputeFilter that filters the computes based
on the service api. So during the enable ->disable race window we only
lose scheduling performance as the placement filter is inefficient.

In case of setting the compute service to enabled the race is more
visible as the placement pre_filter will filter out the compute that
is enable by the admin until the re-sync happens. If the de-sync would
only happen due to high load on the given compute the such delay could
be explained by the load itself. However de-sync can happen simply due
to a new instance boot on the compute.

This patch adds a functional test that reproduce the original problem.

Related-Bug: #1886418

Change-Id: Ib980b1ba68ffcfe51a15dce10eb9f42ef12d7260

Author: Balazs Gibizer

nova/tests/functional/regressions/test_bug_1886418.py

@@ -0,0 +1,84 @@
+# 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.
+
+from nova.tests.functional import integrated_helpers
+
+
+class TestServices(integrated_helpers._IntegratedTestBase):
+    api_major_version = 'v2.1'
+    microversion = 'latest'
+
+    def setUp(self):
+        super(TestServices, self).setUp()
+        self.compute_rp_uuid = self.admin_api.api_get(
+            'os-hypervisors?hypervisor_hostname_pattern=fake-mini'
+        ).body['hypervisors'][0]['id']
+        self.compute_service_id = self.admin_api.get_services(
+            host='compute', binary='nova-compute')[0]['id']
+
+    def _get_traits_on_compute(self):
+        return self.placement_api.get(
+            '/resource_providers/%s/traits' % self.compute_rp_uuid,
+            version='1.6'
+        ).body['traits']
+
+    def _disable_compute(self):
+        self.api.put_service(
+            self.compute_service_id, {'status': 'disabled'})
+
+    def _enable_compute(self):
+        self.api.put_service(
+            self.compute_service_id, {'status': 'enabled'})
+
+    def _has_disabled_trait(self):
+        return "COMPUTE_STATUS_DISABLED" in self._get_traits_on_compute()
+
+    def test_compute_disable_after_server_create(self):
+        # Check that COMPUTE_STATUS_DISABLED is not on the compute
+        self.assertFalse(self._has_disabled_trait())
+
+        self._disable_compute()
+        # Check that COMPUTE_STATUS_DISABLED is now on the compute
+        self.assertTrue(self._has_disabled_trait())
+
+        self._enable_compute()
+        # Check that COMPUTE_STATUS_DISABLED is not on the compute
+        self.assertFalse(self._has_disabled_trait())
+
+        # Create a server.
+        self._create_server(networks=[])
+
+        self._disable_compute()
+        # FIXME(gibi): Check that COMPUTE_STATUS_DISABLED is now on the
+        # compute. Unfortunately it is not true as the compute manager failed
+        # to update the traits in placement due to a stale provide tree cache.
+        # It is stale because a server is booted on the compute since the last
+        # update_available_resource periodic was run.
+        self.assertIn(
+            'An error occurred while updating COMPUTE_STATUS_DISABLED trait '
+            'on compute node resource provider',
+            self.stdlog.logger.output)
+        self.assertFalse(self._has_disabled_trait())
+
+        # This would be the expected behavior
+        #
+        # self.assertTrue(self._has_disabled_trait())
+        #
+        # Alternatively the test could wait for the periodic to run or trigger
+        # it manually.
+
+        # This passes now but not because enabling works but because the
+        # above fault caused that COMPUTE_STATUS_DISABLED is not on the compute
+        # RP in the first place.
+        self._enable_compute()
+        # Check that COMPUTE_STATUS_DISABLED is removed from the compute
+        self.assertFalse(self._has_disabled_trait())