Add functional regression test for bug 1669054

Change Ic3968721d257a167f3f946e5387cd227a7eeec6c in Newton
started setting the RequestSpec.ignore_hosts field to the
source instance.host during resize/cold migrate if
allow_resize_to_same_host=False in config, which it is by
default.

Change I8abdf58a6537dd5e15a012ea37a7b48abd726579 also in
Newton persists changes to the RequestSpec in conductor
in order to save the RequestSpec.flavor for the new flavor.
This inadvertently persists the ignore_hosts field as well.

Later if you try to evacuate or unshelve the server it will ignore
the original source host because of the persisted ignore_hosts value.
This is obviously a problem in a small deployment with only a few
compute nodes (like an edge deployment). As a result, an evacuation
can fail if the only available host is the one being ignored.

This adds a functional regression recreate test for the bug.

Conflicts:
      nova/tests/functional/api/client.py

NOTE(mriedem): The conflict is due to not having change
I71e9d8dae55653ad3ee70f708a6d92c98ed20c1c in Ocata which
added the 201 value to the check_response_status default.

NOTE(mriedem): This backport differs slightly in that
(a) REQUIRES_LOCKING is removed and NeutronFixture is
used explicitly since change I9b35ed7497db8bd1eb74f4bb89631aabbcfeec0d
is not in Ocata and (b) kwargs are passed through post_server_action
because change If7b02bcd8d77e94c7fb42b721792c1391bc0e3b7 is not in
Ocata.

Change-Id: I6ce2d6b1baf47796f867aede1acf292ec9739d6d
Related-Bug: #1669054
(cherry picked from commit 556cf103b22ab6bebecc9d824d6f918cda38fe3e)
(cherry picked from commit 20c1414945db633ed00c1f19f1f0d163028454d9)
(cherry picked from commit 77164128bf5eef53b49547658be2ef902c020207)
(cherry picked from commit 9fd4082d7c076146ec314b86e0e4772d0a021712)
(cherry picked from commit fcd718dcdd3cb7ba46f16ff97ecee068d55c8801)

2 files changed, 93 insertions, 3 deletions

diff --git a/nova/tests/functional/api/client.py b/nova/tests/functional/api/client.py
index 64a9a0bb1a..eaf747552d 100644
--- a/nova/tests/functional/api/client.py
+++ b/nova/tests/functional/api/client.py
@@ -228,7 +228,7 @@ class TestOpenStackClient(object):
             headers['Content-Type'] = 'application/json'
             kwargs['body'] = jsonutils.dumps(body)
 
-        kwargs.setdefault('check_response_status', [200, 202])
+        kwargs.setdefault('check_response_status', [200, 202, 204])
         return APIResponse(self.api_request(relative_uri, **kwargs))
 
     def api_put(self, relative_uri, body, **kwargs):
@@ -286,8 +286,9 @@ class TestOpenStackClient(object):
     def put_server(self, server_id, server):
         return self.api_put('/servers/%s' % server_id, server).body
 
-    def post_server_action(self, server_id, data):
-        return self.api_post('/servers/%s/action' % server_id, data).body
+    def post_server_action(self, server_id, data, **kwargs):
+        return self.api_post(
+            '/servers/%s/action' % server_id, data, **kwargs).body
 
     def delete_server(self, server_id):
         return self.api_delete('/servers/%s' % server_id)
diff --git a/nova/tests/functional/regressions/test_bug_1669054.py b/nova/tests/functional/regressions/test_bug_1669054.py
new file mode 100644
index 0000000000..149e0c09df
--- /dev/null
+++ b/nova/tests/functional/regressions/test_bug_1669054.py
@@ -0,0 +1,89 @@
+# 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 import context
+from nova import objects
+from nova.tests import fixtures as nova_fixtures
+from nova.tests.functional import integrated_helpers
+from nova.tests.unit import fake_network
+from nova.virt import fake
+
+
+class ResizeEvacuateTestCase(integrated_helpers._IntegratedTestBase,
+                             integrated_helpers.InstanceHelperMixin):
+    """Regression test for bug 1669054 introduced in Newton.
+
+    When resizing a server, if CONF.allow_resize_to_same_host is False,
+    the API will set RequestSpec.ignore_hosts = [instance.host] and then
+    later in conductor the RequestSpec changes are saved to persist the new
+    flavor. This inadvertently saves the ignore_hosts value. Later if you
+    try to migrate, evacuate or unshelve the server, that original source
+    host will be ignored. If the deployment has a small number of computes,
+    like two in an edge node, then evacuate will fail because the only other
+    available host is ignored. This test recreates the scenario.
+    """
+    # Set variables used in the parent class.
+    ADMIN_API = True
+    USE_NEUTRON = True
+    _image_ref_parameter = 'imageRef'
+    _flavor_ref_parameter = 'flavorRef'
+    api_major_version = 'v2.1'
+    microversion = '2.11'  # Need at least 2.11 for the force-down API
+
+    def setUp(self):
+        super(ResizeEvacuateTestCase, self).setUp()
+        fake_network.set_stub_network_methods(self)
+        self.useFixture(nova_fixtures.NeutronFixture(self))
+
+    def test_resize_then_evacuate(self):
+        # Create a server. At this point there is only one compute service.
+        flavors = self.api.get_flavors()
+        flavor1 = flavors[0]['id']
+        server = self._build_server(flavor1)
+        server = self.api.post_server({'server': server})
+        self._wait_for_state_change(self.api, server, 'ACTIVE')
+
+        # Start up another compute service so we can resize.
+        fake.set_nodes(['host2'])
+        self.addCleanup(fake.restore_nodes)
+        host2 = self.start_service('compute', host='host2')
+
+        # Now resize the server to move it to host2.
+        flavor2 = flavors[1]['id']
+        req = {'resize': {'flavorRef': flavor2}}
+        self.api.post_server_action(server['id'], req)
+        server = self._wait_for_state_change(self.api, server, 'VERIFY_RESIZE')
+        self.assertEqual('host2', server['OS-EXT-SRV-ATTR:host'])
+        self.api.post_server_action(server['id'], {'confirmResize': None})
+        server = self._wait_for_state_change(self.api, server, 'ACTIVE')
+
+        # Disable the host on which the server is now running (host2).
+        host2.stop()
+        self.api.force_down_service('host2', 'nova-compute', forced_down=True)
+
+        # Now try to evacuate the server back to the original source compute.
+        # FIXME(mriedem): This is bug 1669054 where the evacuate fails with
+        # NoValidHost because the RequestSpec.ignore_hosts field has the
+        # original source host in it which is the only other available host to
+        # which we can evacuate the server.
+        req = {'evacuate': {'onSharedStorage': False}}
+        self.api.post_server_action(server['id'], req,
+                                    check_response_status=[500])
+        # There should be fault recorded with the server.
+        server = self._wait_for_state_change(self.api, server, 'ERROR')
+        self.assertIn('fault', server)
+        self.assertIn('No valid host was found', server['fault']['message'])
+        # Assert the RequestSpec.ignore_hosts is still populated.
+        reqspec = objects.RequestSpec.get_by_instance_uuid(
+            context.get_admin_context(), server['id'])
+        self.assertIsNotNone(reqspec.ignore_hosts)
+        self.assertIn(self.compute.host, reqspec.ignore_hosts)