Fallback to same-cell resize with qos ports

Balazs Gibizer · Balazs Gibizer · commit 6b57575092aa · 2021-01-11T16:20:03.000+01:00
The cross-cell resize code does not consider neutron ports with resource request. To avoid migration failures this patch makes nova to fall back to same cell resize if the instance has neutron ports with resource request. The stable only difference is due to change in the reproduction patch Id91d2e817ef6bd21124bb840bdb098054e9753b8 on stable. Change-Id: Icaad4b2375b491c8a7e87fb6f4977ae2e13e8190 Closes-Bug: #1907522 Closes-Bug: #1907511 (cherry picked from commit 1c056c3)
diff --git a/api-guide/source/port_with_resource_request.rst b/api-guide/source/port_with_resource_request.rst
@@ -30,7 +30,8 @@ neutron ports having resource requests if both the source and destination
 compute services are upgraded to 20.0.0 (Train) and the
 ``[upgrade_levels]/compute`` configuration does not prevent the computes from
 using the latest RPC version. However cross cell resize and cross cell migrate
-operations are still not supported with such ports.
+operations are still not supported with such ports and Nova will fall back to
+same-cell resize if the server has such ports.
 
 As of 21.0.0 (Ussuri), nova supports evacuating, live migrating and unshelving
 servers with neutron ports having resource requests.
diff --git a/doc/source/admin/configuration/cross-cell-resize.rst b/doc/source/admin/configuration/cross-cell-resize.rst
@@ -237,7 +237,8 @@ These are known to not yet be supported in the code:
 
 * Instances with ports attached that have
   :doc:`bandwidth-aware </admin/ports-with-resource-requests>` resource
-  provider allocations.
+  provider allocations. Nova falls back to same-cell resize if the server has
+  such ports.
 * Rescheduling to alternative hosts within the same target cell in case the
   primary selected host fails the ``prep_snapshot_based_resize_at_dest`` call.
 
diff --git a/nova/compute/api.py b/nova/compute/api.py
@@ -3870,17 +3870,14 @@ def confirm_resize(self, context, instance, migration=None):
                                                migration,
                                                migration.source_compute)
 
-    @staticmethod
-    def _allow_cross_cell_resize(context, instance):
+    def _allow_cross_cell_resize(self, context, instance):
         """Determine if the request can perform a cross-cell resize on this
         instance.
 
         :param context: nova auth request context for the resize operation
         :param instance: Instance object being resized
         :returns: True if cross-cell resize is allowed, False otherwise
         """
-        # TODO(gibi): do not allow cross cell migration if the instance has
-        # neutron ports with resource request. See bug 1907522.
         # First check to see if the requesting project/user is allowed by
         # policy to perform cross-cell resize.
         allowed = context.can(
@@ -3903,7 +3900,17 @@ def _allow_cross_cell_resize(context, instance):
                           'version in the deployment %s is less than %s so '
                           'cross-cell resize is not allowed at this time.',
                           min_compute_version, MIN_COMPUTE_CROSS_CELL_RESIZE)
-                allowed = False
+                return False
+
+            if self.network_api.get_requested_resource_for_instance(
+                    context, instance.uuid):
+                LOG.info(
+                    'Request is allowed by policy to perform cross-cell '
+                    'resize but the instance has ports with resource request '
+                    'and cross-cell resize is not supported with such ports.',
+                    instance=instance)
+                return False
+
         return allowed
 
     @staticmethod
diff --git a/nova/tests/functional/test_servers.py b/nova/tests/functional/test_servers.py
@@ -8333,35 +8333,40 @@ def spy_on_create_binding(context, client, port_id, data):
             # host is in a different cell and while cross cell migration is
             # enabled it is not supported for neutron ports with resource
             # request.
-            # FIXME(gibi): We expect this to fail with NoValidHost.
-            # Unfortunately it fails by not finding the target compute service
-            # in the same cell the source service. This is bug 1907511. If
-            # there would be a standalone fix for 1907511 then the next failure
-            # would be 1907522. Our coming fix will fix both bug with a same
-            # fix.
             self.api.post_server_action(server['id'], {'migrate': None})
             self._wait_for_migration_status(server, ['error'])
-            self._wait_for_action_fail_completion(
+            self._wait_for_server_parameter(
+                server,
+                {'status': 'ACTIVE', 'OS-EXT-SRV-ATTR:host': 'host1'})
+            event = self._wait_for_action_fail_completion(
                 server, 'migrate', 'conductor_migrate_server')
-            # This is the root case
             self.assertIn(
+                'exception.NoValidHost', event['traceback'])
+            log = self.stdlog.logger.output
+            self.assertIn(
+                'Request is allowed by policy to perform cross-cell resize '
+                'but the instance has ports with resource request and '
+                'cross-cell resize is not supported with such ports.',
+                log)
+            self.assertNotIn(
+                'nova.exception.PortBindingFailed: Binding failed for port',
+                log)
+            self.assertNotIn(
                 "AttributeError: 'NoneType' object has no attribute 'version'",
-                self.stdlog.logger.output)
+                log)
 
         # Now start a new compute in the same cell as the instance and retry
         # the migration.
-        #
-        # This should work after the fallback to same cell resize is
-        # implemented
-        #
-        # self._start_compute('host3', cell_name='cell1')
-        #
-        # with mock.patch(
-        #         'nova.network.neutron.API._create_port_binding',
-        #         side_effect=spy_on_create_binding, autospec=True
-        # ):
-        #     server = self._migrate_server(server)
-        #     self.assertEqual('host3', server['OS-EXT-SRV-ATTR:host'])
+        self._start_compute('host3', cell_name='cell1')
+        self.compute3_rp_uuid = self._get_provider_uuid_by_host('host3')
+        self._create_networking_rp_tree('host3', self.compute3_rp_uuid)
+
+        with mock.patch(
+                'nova.network.neutron.API._create_port_binding',
+                side_effect=spy_on_create_binding, autospec=True
+        ):
+            server = self._migrate_server(server)
+            self.assertEqual('host3', server['OS-EXT-SRV-ATTR:host'])
 
         self._delete_server_and_check_allocations(
             server, qos_normal_port, qos_sriov_port)
diff --git a/nova/tests/unit/compute/test_api.py b/nova/tests/unit/compute/test_api.py
@@ -7522,28 +7522,54 @@ def test_allow_cross_cell_resize_false_old_version(self, mock_get_min_ver):
         version is not new enough.
         """
         instance = objects.Instance(
-            project_id='fake-project', user_id='fake-user')
+            project_id='fake-project', user_id='fake-user',
+            uuid=uuids.instance)
+        with mock.patch.object(self.context, 'can', return_value=True) as can:
+            self.assertFalse(self.compute_api._allow_cross_cell_resize(
+                self.context, instance))
+        can.assert_called_once()
+        mock_get_min_ver.assert_called_once_with(
+            self.context, ['nova-compute'])
+
+    @mock.patch('nova.network.neutron.API.get_requested_resource_for_instance',
+                return_value=[objects.RequestGroup()])
+    @mock.patch('nova.objects.service.get_minimum_version_all_cells',
+                return_value=compute_api.MIN_COMPUTE_CROSS_CELL_RESIZE)
+    def test_allow_cross_cell_resize_false_port_with_resource_req(
+            self, mock_get_min_ver, mock_get_res_req):
+        """Policy allows cross-cell resize but minimum nova-compute service
+        version is not new enough.
+        """
+        instance = objects.Instance(
+            project_id='fake-project', user_id='fake-user',
+            uuid=uuids.instance)
         with mock.patch.object(self.context, 'can', return_value=True) as can:
             self.assertFalse(self.compute_api._allow_cross_cell_resize(
                 self.context, instance))
         can.assert_called_once()
         mock_get_min_ver.assert_called_once_with(
             self.context, ['nova-compute'])
+        mock_get_res_req.assert_called_once_with(self.context, uuids.instance)
 
+    @mock.patch('nova.network.neutron.API.get_requested_resource_for_instance',
+                return_value=[])
     @mock.patch('nova.objects.service.get_minimum_version_all_cells',
                 return_value=compute_api.MIN_COMPUTE_CROSS_CELL_RESIZE)
-    def test_allow_cross_cell_resize_true(self, mock_get_min_ver):
+    def test_allow_cross_cell_resize_true(
+            self, mock_get_min_ver, mock_get_res_req):
         """Policy allows cross-cell resize and minimum nova-compute service
         version is new enough.
         """
         instance = objects.Instance(
-            project_id='fake-project', user_id='fake-user')
+            project_id='fake-project', user_id='fake-user',
+            uuid=uuids.instance)
         with mock.patch.object(self.context, 'can', return_value=True) as can:
             self.assertTrue(self.compute_api._allow_cross_cell_resize(
                 self.context, instance))
         can.assert_called_once()
         mock_get_min_ver.assert_called_once_with(
             self.context, ['nova-compute'])
+        mock_get_res_req.assert_called_once_with(self.context, uuids.instance)
 
     def _test_block_accelerators(self, instance, args_info,
                                  until_service=None):
diff --git a/releasenotes/notes/cros-scell-resize-not-supported-with-ports-having-resource-request-a8e1029ef5983793.yaml b/releasenotes/notes/cros-scell-resize-not-supported-with-ports-having-resource-request-a8e1029ef5983793.yaml
@@ -4,4 +4,6 @@ issues:
     When the tempest test coverage was added for resize and cold migrate
     with neutron ports having QoS minimum bandwidth policy rules we
     discovered that the cross cell resize code path cannot handle such ports.
-    See bug https://bugs.launchpad.net/nova/+bug/1907522 for details.
+    See bug https://bugs.launchpad.net/nova/+bug/1907522 for details. A fix
+    was implemented that makes sure that Nova falls back to same-cell resize if
+    the server has such ports.
