Merge "Fix failed count for anti-affinity check" into stable/2023.1

Author: Zuul

nova/compute/build_results.py

@@ -24,3 +24,11 @@
 ACTIVE = 'active'  # Instance is running
 FAILED = 'failed'  # Instance failed to build and was not rescheduled
 RESCHEDULED = 'rescheduled'  # Instance failed to build, but was rescheduled
+# Instance failed by policy violation (such as affinity or anti-affinity)
+# and was not rescheduled. In this case, the node's failed count won't be
+# increased.
+FAILED_BY_POLICY = 'failed_by_policy'
+# Instance failed by policy violation (such as affinity or anti-affinity)
+# but was rescheduled. In this case, the node's failed count won't be
+# increased.
+RESCHEDULED_BY_POLICY = 'rescheduled_by_policy'

nova/compute/manager.py

@@ -1891,11 +1891,8 @@ def _do_validation(context, instance, group):
                 else:
                     max_server = 1
                 if len(members_on_host) >= max_server:
-                    msg = _("Anti-affinity instance group policy "
-                            "was violated.")
-                    raise exception.RescheduledException(
-                            instance_uuid=instance.uuid,
-                            reason=msg)
+                    raise exception.GroupAffinityViolation(
+                        instance_uuid=instance.uuid, policy='Anti-affinity')
 
             # NOTE(ganso): The check for affinity below does not work and it
             # can easily be violated because the lock happens in different
@@ -1905,10 +1902,8 @@ def _do_validation(context, instance, group):
             elif group.policy and 'affinity' == group.policy:
                 group_hosts = group.get_hosts(exclude=[instance.uuid])
                 if group_hosts and self.host not in group_hosts:
-                    msg = _("Affinity instance group policy was violated.")
-                    raise exception.RescheduledException(
-                            instance_uuid=instance.uuid,
-                            reason=msg)
+                    raise exception.GroupAffinityViolation(
+                        instance_uuid=instance.uuid, policy='Affinity')
 
         _do_validation(context, instance, group)
 
@@ -2348,6 +2343,9 @@ def _locked_do_build_and_run_instance(*args, **kwargs):
                         self.reportclient.delete_allocation_for_instance(
                             context, instance.uuid, force=True)
 
+                    if result in (build_results.FAILED_BY_POLICY,
+                                  build_results.RESCHEDULED_BY_POLICY):
+                        return
                     if result in (build_results.FAILED,
                                   build_results.RESCHEDULED):
                         self._build_failed(node)
@@ -2446,6 +2444,8 @@ def _do_build_and_run_instance(self, context, instance, image,
                 self._nil_out_instance_obj_host_and_node(instance)
                 self._set_instance_obj_error_state(instance,
                                                    clean_task_state=True)
+                if isinstance(e, exception.RescheduledByPolicyException):
+                    return build_results.FAILED_BY_POLICY
                 return build_results.FAILED
             LOG.debug(e.format_message(), instance=instance)
             # This will be used for logging the exception
@@ -2472,6 +2472,10 @@ def _do_build_and_run_instance(self, context, instance, image,
                     injected_files, requested_networks, security_groups,
                     block_device_mapping, request_spec=request_spec,
                     host_lists=[host_list])
+
+            if isinstance(e, exception.RescheduledByPolicyException):
+                return build_results.RESCHEDULED_BY_POLICY
+
             return build_results.RESCHEDULED
         except (exception.InstanceNotFound,
                 exception.UnexpectedDeletingTaskStateError):
@@ -2691,6 +2695,17 @@ def _build_and_run_instance(self, context, instance, image, injected_files,
                     bdms=block_device_mapping)
             raise exception.BuildAbortException(instance_uuid=instance.uuid,
                     reason=e.format_message())
+        except exception.GroupAffinityViolation as e:
+            LOG.exception('Failed to build and run instance',
+                          instance=instance)
+            self._notify_about_instance_usage(context, instance,
+                    'create.error', fault=e)
+            compute_utils.notify_about_instance_create(
+                    context, instance, self.host,
+                    phase=fields.NotificationPhase.ERROR, exception=e,
+                    bdms=block_device_mapping)
+            raise exception.RescheduledByPolicyException(
+                    instance_uuid=instance.uuid, reason=str(e))
         except Exception as e:
             LOG.exception('Failed to build and run instance',
                           instance=instance)

nova/exception.py

@@ -1502,6 +1502,15 @@ class RescheduledException(NovaException):
                 "%(reason)s")
 
 
+class RescheduledByPolicyException(RescheduledException):
+    msg_fmt = _("Build of instance %(instance_uuid)s was re-scheduled: "
+                "%(reason)s")
+
+
+class GroupAffinityViolation(NovaException):
+    msg_fmt = _("%(policy)s instance group policy was violated")
+
+
 class InstanceFaultRollback(NovaException):
     def __init__(self, inner_exception=None):
         message = _("Instance rollback performed due to: %s")

nova/tests/functional/test_server_group.py

@@ -20,6 +20,7 @@
 from nova.compute import instance_actions
 from nova import context
 from nova.db.main import api as db
+from nova import objects
 from nova import test
 from nova.tests import fixtures as nova_fixtures
 from nova.tests.functional.api import client
@@ -499,6 +500,85 @@ def test_soft_affinity_not_supported(self):
         self.assertIn('Invalid input', ex.response.text)
         self.assertIn('soft-affinity', ex.response.text)
 
+    @mock.patch('nova.scheduler.filters.affinity_filter.'
+        'ServerGroupAffinityFilter.host_passes', return_value=True)
+    def test_failed_count_with_affinity_violation(self, mock_host_passes):
+        """Check failed count not incremented after violation of the late
+        affinity check. https://bugs.launchpad.net/nova/+bug/1996732
+        """
+
+        created_group = self.api.post_server_groups(self.affinity)
+        flavor = self.api.get_flavors()[2]
+
+        # Ensure the first instance is on compute1
+        with utils.temporary_mutation(self.admin_api, microversion='2.53'):
+            compute2_service_id = self.admin_api.get_services(
+            host=self.compute2.host, binary='nova-compute')[0]['id']
+            self.admin_api.put_service(compute2_service_id,
+                                        {'status': 'disabled'})
+
+        self._boot_a_server_to_group(created_group, flavor=flavor)
+
+        # Ensure the second instance is on compute2
+        with utils.temporary_mutation(self.admin_api, microversion='2.53'):
+            self.admin_api.put_service(compute2_service_id,
+                                        {'status': 'enabled'})
+            compute1_service_id = self.admin_api.get_services(
+            host=self.compute.host, binary='nova-compute')[0]['id']
+            self.admin_api.put_service(compute1_service_id,
+                                        {'status': 'disabled'})
+
+        # Expects GroupAffinityViolation exception
+        failed_server = self._boot_a_server_to_group(created_group,
+                                                     flavor=flavor,
+                                                     expected_status='ERROR')
+
+        self.assertEqual('Exceeded maximum number of retries. Exhausted all '
+                         'hosts available for retrying build failures for '
+                         'instance %s.' % failed_server['id'],
+                         failed_server['fault']['message'])
+
+        ctxt = context.get_admin_context()
+        computes = objects.ComputeNodeList.get_all(ctxt)
+
+        for node in computes:
+            self.assertEqual(node.stats.get('failed_builds'), '0')
+
+    @mock.patch('nova.scheduler.filters.affinity_filter.'
+        'ServerGroupAntiAffinityFilter.host_passes', return_value=True)
+    def test_failed_count_with_anti_affinity_violation(self, mock_host_passes):
+        """Check failed count after violation of the late affinity check.
+        https://bugs.launchpad.net/nova/+bug/1996732
+        """
+
+        created_group = self.api.post_server_groups(self.anti_affinity)
+        flavor = self.api.get_flavors()[2]
+
+        # Ensure two instances are scheduled on the same host
+        with utils.temporary_mutation(self.admin_api, microversion='2.53'):
+            compute2_service_id = self.admin_api.get_services(
+            host=self.compute2.host, binary='nova-compute')[0]['id']
+            self.admin_api.put_service(compute2_service_id,
+                                        {'status': 'disabled'})
+
+        self._boot_a_server_to_group(created_group, flavor=flavor)
+
+        # Expects GroupAffinityViolation exception
+        failed_server = self._boot_a_server_to_group(created_group,
+                                                     flavor=flavor,
+                                                     expected_status='ERROR')
+
+        self.assertEqual('Exceeded maximum number of retries. Exhausted all '
+                         'hosts available for retrying build failures for '
+                         'instance %s.' % failed_server['id'],
+                         failed_server['fault']['message'])
+
+        ctxt = context.get_admin_context()
+        computes = objects.ComputeNodeList.get_all(ctxt)
+
+        for node in computes:
+            self.assertEqual(node.stats.get('failed_builds'), '0')
+
 
 class ServerGroupAffinityConfTest(ServerGroupTestBase):
     api_major_version = 'v2.1'