Merge "Reject unshelve with port having resource request"

nova/api/openstack/compute/shelve.py

@@ -19,7 +19,10 @@ from webob import exc
 from nova.api.openstack import common
 from nova.api.openstack import wsgi
 from nova import compute
+from nova.compute import vm_states
 from nova import exception
+from nova.i18n import _
+from nova import network
 from nova.policies import shelve as shelve_policies
 
 
@@ -27,6 +30,7 @@ class ShelveController(wsgi.Controller):
     def __init__(self, *args, **kwargs):
         super(ShelveController, self).__init__(*args, **kwargs)
         self.compute_api = compute.API()
+        self.network_api = network.API()
 
     @wsgi.response(202)
     @wsgi.expected_errors((404, 409))
@@ -70,13 +74,23 @@ class ShelveController(wsgi.Controller):
                                                               id)
 
     @wsgi.response(202)
-    @wsgi.expected_errors((404, 409))
+    @wsgi.expected_errors((400, 404, 409))
     @wsgi.action('unshelve')
     def _unshelve(self, req, id, body):
         """Restore an instance from shelved mode."""
         context = req.environ["nova.context"]
         context.can(shelve_policies.POLICY_ROOT % 'unshelve')
         instance = common.get_instance(self.compute_api, context, id)
+
+        if (instance.vm_state == vm_states.SHELVED_OFFLOADED
+                and common.instance_has_port_with_resource_request(
+                    context, instance.uuid, self.network_api)
+                and not common.supports_port_resource_request_during_move(
+                    req)):
+            msg = _("The unshelve server operation on a shelve offloaded "
+                    "server with port having QoS policy is not supported.")
+            raise exc.HTTPBadRequest(explanation=msg)
+
         try:
             self.compute_api.unshelve(context, instance)
         except exception.InstanceUnknownCell as e:

nova/tests/functional/test_servers.py

@@ -5584,3 +5584,67 @@ class PortResourceRequestBasedSchedulingTest(
         self.assertIn(
             'The evacuate server operation with port having QoS policy is '
             'not supported.', six.text_type(ex))
+
+    def test_unshelve_offloaded_server_with_port_resource_request_old_version(
+            self):
+        server = self._create_server(
+            flavor=self.flavor,
+            networks=[{'port': self.neutron.port_1['id']}])
+        self._wait_for_state_change(self.admin_api, server, 'ACTIVE')
+
+        # with default config shelve means immediate offload as well
+        req = {
+            'shelve': {}
+        }
+        self.api.post_server_action(server['id'], req)
+        self._wait_for_server_parameter(
+            self.api, server, {'status': 'SHELVED_OFFLOADED'})
+
+        # We need to simulate that the above server has a port that has
+        # resource request, we cannot boot with such a port but legacy servers
+        # can exists with such a port.
+        bound_port = self.neutron._ports[self.neutron.port_1['id']]
+        fake_resource_request = self.neutron.port_with_resource_request[
+            'resource_request']
+        bound_port['resource_request'] = fake_resource_request
+
+        ex = self.assertRaises(
+            client.OpenStackApiException,
+            self.api.post_server_action, server['id'], {'unshelve': {}})
+
+        self.assertEqual(400, ex.response.status_code)
+        self.assertIn(
+            'The unshelve server operation on a shelve offloaded server with '
+            'port having QoS policy is not supported.', six.text_type(ex))
+
+    def test_unshelve_not_offloaded_server_with_port_resource_request(
+            self):
+        """If the server is not offloaded then unshelving does not cause a new
+        resource allocation therefore having port resource request is
+        irrelevant. This test asserts that such unshelve request is not
+        rejected.
+        """
+        server = self._create_server(
+            flavor=self.flavor,
+            networks=[{'port': self.neutron.port_1['id']}])
+        self._wait_for_state_change(self.admin_api, server, 'ACTIVE')
+
+        # avoid automatic shelve offloading
+        self.flags(shelved_offload_time=-1)
+        req = {
+            'shelve': {}
+        }
+        self.api.post_server_action(server['id'], req)
+        self._wait_for_server_parameter(
+            self.api, server, {'status': 'SHELVED'})
+
+        # We need to simulate that the above server has a port that has
+        # resource request, we cannot boot with such a port but legacy servers
+        # can exists with such a port.
+        bound_port = self.neutron._ports[self.neutron.port_1['id']]
+        fake_resource_request = self.neutron.port_with_resource_request[
+            'resource_request']
+        bound_port['resource_request'] = fake_resource_request
+
+        self.api.post_server_action(server['id'], {'unshelve': {}})
+        self._wait_for_state_change(self.admin_api, server, 'ACTIVE')

nova/tests/unit/api/openstack/compute/test_shelve.py

@@ -15,9 +15,11 @@
 import mock
 from oslo_policy import policy as oslo_policy
 from oslo_utils.fixture import uuidsentinel
+import six
 import webob
 
 from nova.api.openstack.compute import shelve as shelve_v21
+from nova.compute import vm_states
 from nova import exception
 from nova import policy
 from nova import test
@@ -61,6 +63,27 @@ class ShelvePolicyTestV21(test.NoDBTestCase):
                           self.controller._shelve_offload,
                           self.req, uuidsentinel.fake, {})
 
+    @mock.patch('nova.network.neutronv2.api.API.list_ports')
+    @mock.patch('nova.api.openstack.common.get_instance')
+    def test_unshelve_offloaded_with_port_resource_request_old_microversion(
+            self, get_instance_mock, mock_list_ports):
+        mock_list_ports.return_value = {'ports': [
+            {'resource_request': {
+                "resources": {'CUSTOM_FOO': 1}}}]
+        }
+        instance = fake_instance.fake_instance_obj(
+            self.req.environ['nova.context'])
+        instance.vm_state = vm_states.SHELVED_OFFLOADED
+        get_instance_mock.return_value = instance
+
+        ex = self.assertRaises(
+            webob.exc.HTTPBadRequest, self.controller._unshelve, self.req,
+            uuidsentinel.fake, {})
+
+        self.assertIn(
+            'The unshelve server operation on a shelve offloaded server with '
+            'port having QoS policy is not supported.', six.text_type(ex))
+
 
 class ShelvePolicyEnforcementV21(test.NoDBTestCase):