From 6a5be4ae250b9ff1b8e48947ab80a65827460513 Mon Sep 17 00:00:00 2001
From: mb <mb@citynetwork.se>
Date: Fri, 29 Mar 2019 10:20:58 +0100
Subject: [PATCH] Fix bug preventing forbidden traits from working

Modifies _clean_empties function to take forbidden traits into account
in addition to required traits.

Added unit test test_resources_from_request_spec_flavor_forbidden_trait
to test that a single forbidden trait doesn't get lost in the
resources_from_request_spec function.

Also updated the functional test
test_flavor_forbidden_traits_based_scheduling to do the right thing.

Change-Id: I491b10c9c202baae4a37034848147f910a50eebf
Closes-Bug: #1821824
(cherry picked from commit c088856c8c08f0ab0746db7c120da494c9dd42d4)
---
 nova/scheduler/utils.py                 |  4 ++--
 nova/tests/functional/test_servers.py   | 20 +++++---------------
 nova/tests/unit/scheduler/test_utils.py | 22 ++++++++++++++++++++++
 3 files changed, 29 insertions(+), 17 deletions(-)

diff --git a/nova/scheduler/utils.py b/nova/scheduler/utils.py
index adcc993144ab..64c37e01467e 100644
--- a/nova/scheduler/utils.py
+++ b/nova/scheduler/utils.py
@@ -243,8 +243,8 @@ class ResourceRequest(object):
     def _clean_empties(self):
         """Get rid of any empty ResourceGroup instances."""
         for ident, rg in list(self._rg_by_id.items()):
-            # TODO(magnusbe) Fix bug #1821824
-            if not any((rg.resources, rg.required_traits)):
+            if not any((rg.resources, rg.required_traits,
+                        rg.forbidden_traits)):
                 self._rg_by_id.pop(ident)
 
     def strip_zeros(self):
diff --git a/nova/tests/functional/test_servers.py b/nova/tests/functional/test_servers.py
index 086db9dd3a6c..d46624d32998 100644
--- a/nova/tests/functional/test_servers.py
+++ b/nova/tests/functional/test_servers.py
@@ -4579,11 +4579,9 @@ class TraitsBasedSchedulingTest(integrated_helpers.ProviderUsageBaseTestCase):
 
         server = self._wait_for_state_change(self.admin_api, server, 'ACTIVE')
 
-        # Temporarily pass until bug #1821824 is fixed
         # Assert the server ended up on the expected compute host that doesn't
         # have the forbidden trait.
-        # TODO(magnusbe) Uncomment when commiting fix for bug #1821824
-        # self.assertEqual(self.compute2.host, server['OS-EXT-SRV-ATTR:host'])
+        self.assertEqual(self.compute2.host, server['OS-EXT-SRV-ATTR:host'])
 
         # Disable the compute node that doesn't have the forbidden trait
         compute2_service_id = self.admin_api.get_services(
@@ -4596,20 +4594,12 @@ class TraitsBasedSchedulingTest(integrated_helpers.ProviderUsageBaseTestCase):
             self.image_id_without_trait
         )
 
-        # Temporarily pass if server goes to ACTIVE STATE until bug #1821824
-        # is fixed.
-        # TODO(magnusbe) Remove when commiting fix for bug #1821824
-        server = self._wait_for_state_change(self.admin_api, server, 'ACTIVE')
-
         # The server should go to ERROR state because there is no valid host.
-        # This is the expected behaviour. However, at this time bug #1821824
-        # prevents this from working.
-        # TODO(magnusbe) Uncomment when commiting fix for bug #1821824
-        # server = self._wait_for_state_change(self.admin_api, server, 'ERROR')
-        # self.assertIsNone(server['OS-EXT-SRV-ATTR:host'])
+        server = self._wait_for_state_change(self.admin_api, server, 'ERROR')
+        self.assertIsNone(server['OS-EXT-SRV-ATTR:host'])
         # Make sure the failure was due to NoValidHost by checking the fault.
-        # self.assertIn('fault', server)
-        # self.assertIn('No valid host', server['fault']['message'])
+        self.assertIn('fault', server)
+        self.assertIn('No valid host', server['fault']['message'])
 
     def test_image_traits_based_scheduling(self):
         """Tests that a server create request using a required trait on image
diff --git a/nova/tests/unit/scheduler/test_utils.py b/nova/tests/unit/scheduler/test_utils.py
index d4dc2b315124..248be7af2ab0 100644
--- a/nova/tests/unit/scheduler/test_utils.py
+++ b/nova/tests/unit/scheduler/test_utils.py
@@ -103,6 +103,28 @@ class TestUtils(test.NoDBTestCase):
         self._test_resources_from_request_spec(expected_resources, flavor,
                                                image)
 
+    def test_resources_from_request_spec_flavor_forbidden_trait(self):
+        flavor = objects.Flavor(vcpus=1,
+                                memory_mb=1024,
+                                root_gb=10,
+                                ephemeral_gb=5,
+                                swap=0,
+                                extra_specs={
+                                    'trait:CUSTOM_FLAVOR_TRAIT': 'forbidden'})
+        expected_resources = utils.ResourceRequest()
+        expected_resources._rg_by_id[None] = objects.RequestGroup(
+            use_same_provider=False,
+            resources={
+                'VCPU': 1,
+                'MEMORY_MB': 1024,
+                'DISK_GB': 15,
+            },
+            forbidden_traits={
+                'CUSTOM_FLAVOR_TRAIT',
+            }
+        )
+        self._test_resources_from_request_spec(expected_resources, flavor)
+
     def test_resources_from_request_spec_with_no_disk(self):
         flavor = objects.Flavor(vcpus=1,
                                 memory_mb=1024,