From ede7866de4865cfd23a996a83084cc75567094c9 Mon Sep 17 00:00:00 2001 From: Lee Yarwood Date: Wed, 28 Jul 2021 17:07:37 +0100 Subject: [PATCH] Add regression test for bug 1938326 Related-Bug: #1938326 Change-Id: Ie7fa120cb7d1739c5de7cd8dbcfaeb3a37fa729b --- nova/tests/functional/integrated_helpers.py | 21 +++ .../regressions/test_bug_1938326.py | 143 ++++++++++++++++++ 2 files changed, 164 insertions(+) create mode 100644 nova/tests/functional/regressions/test_bug_1938326.py diff --git a/nova/tests/functional/integrated_helpers.py b/nova/tests/functional/integrated_helpers.py index 7cb1d7f588..73d54f9a22 100644 --- a/nova/tests/functional/integrated_helpers.py +++ b/nova/tests/functional/integrated_helpers.py @@ -99,6 +99,27 @@ NOT_SPECIFIED = object() class InstanceHelperMixin: + def _wait_for_service_parameter( + self, service_hostname, service_binary, expected_params, + max_retries=10, api=None + ): + api = api or getattr(self, 'admin_api', self.api) + + retry_count = 0 + while True: + service = api.get_services( + host=service_hostname, + binary=service_binary)[0] + if all([service[attr] == expected_params[attr] + for attr in expected_params]): + break + retry_count += 1 + if retry_count == max_retries: + self.fail( + f'Wait for service parameter change failed, ' + f'expected_params={expected_params}, service={service}') + time.sleep(0.5) + def _wait_for_server_parameter( self, server, expected_params, max_retries=10, api=None): api = api or getattr(self, 'admin_api', self.api) diff --git a/nova/tests/functional/regressions/test_bug_1938326.py b/nova/tests/functional/regressions/test_bug_1938326.py new file mode 100644 index 0000000000..f1e3cc8ef3 --- /dev/null +++ b/nova/tests/functional/regressions/test_bug_1938326.py @@ -0,0 +1,143 @@ +# Copyright 2021, Red Hat, Inc. All Rights Reserved. +# +# 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. + +import mock + +from nova.tests.functional.api import client +from nova.tests.functional import integrated_helpers + + +class TestMigrateFromDownHost(integrated_helpers._IntegratedTestBase): + """Regression test for bug #1938326 + + Assert the behaviour of n-api when requests are made to migrate an instance + from a disabled, forced down, down and disabled and down compute + host. + + Bug #1938326 specifically covering the case where a request is made and + accepted to migrate an instance from a disabled and down compute host. + """ + microversion = 'latest' + ADMIN_API = True + + def _setup_compute_service(self): + # We want the service to be marked down in a reasonable time while + # ensuring we don't accidentally mark services as down prematurely + self.flags(report_interval=1) + self.flags(service_down_time=5) + + # Use two compute services to make it easier to assert the call from + # the dest to the src, we could also test this for same host resize. + self._start_compute('src') + self._start_compute('dest') + + def test_migrate_from_disabled_host(self): + """Assert that migration requests for disabled hosts are allowed + """ + # Launch an instance on src + server = self._create_server(host='src', networks='none') + + # Mark the compute service as disabled + source_compute_id = self.api.get_services( + host='src', binary='nova-compute')[0]['id'] + self.api.put_service(source_compute_id, {"status": "disabled"}) + self._wait_for_service_parameter( + 'src', 'nova-compute', + { + 'status': 'disabled', + 'state': 'up' + } + ) + + # Assert that we can migrate and confirm from a disabled but up compute + self._migrate_server(server) + self._confirm_resize(server) + + def test_migrate_from_forced_down_host(self): + """Assert that migration requests for forced down hosts are rejected + """ + # Launch an instance on src + server = self._create_server(host='src', networks='none') + + # Force down the compute + source_compute_id = self.api.get_services( + host='src', binary='nova-compute')[0]['id'] + self.api.put_service(source_compute_id, {'forced_down': 'true'}) + self._wait_for_service_parameter( + 'src', 'nova-compute', + { + 'forced_down': True, + 'state': 'down', + 'status': 'enabled' + } + ) + + # Assert that we cannot migrate from a forced down compute + ex = self.assertRaises( + client.OpenStackApiException, self._migrate_server, server) + self.assertEqual(409, ex.response.status_code) + + def test_migrate_from_down_host(self): + """Assert that migration requests from down hosts are rejected + """ + # Launch an instance on src + server = self._create_server(host='src', networks='none') + + # Stop the compute service and wait until it's down + self.computes['src'].stop() + self._wait_for_service_parameter( + 'src', 'nova-compute', + { + 'state': 'down', + 'status': 'enabled' + } + ) + + # Assert that requests to migrate from down computes are rejected + ex = self.assertRaises( + client.OpenStackApiException, self.api.post_server_action, + server['id'], {'migrate': None}) + self.assertEqual(409, ex.response.status_code) + + def test_migrate_from_disabled_down_host(self): + """Assert that migration requests for disabled down hosts are rejected + """ + # Launch an instance on src + server = self._create_server(host='src', networks='none') + + # Mark the compute service as disabled + source_compute_id = self.api.get_services( + host='src', binary='nova-compute')[0]['id'] + self.api.put_service(source_compute_id, {"status": "disabled"}) + self._wait_for_service_parameter( + 'src', 'nova-compute', {'status': 'disabled'}) + + # Stop the compute service and wait until it's down + self.computes['src'].stop() + self._wait_for_service_parameter( + 'src', 'nova-compute', {'state': 'down'}) + + # FIXME(lyarwood): This is bug #1938326, any request to cold migrate + # from this compute should be rejected earlier in the flow as it is + # currently not running and isn't able to service the eventual request + # from the destination host. + with mock.patch.object( + self.computes['dest'].compute_rpcapi, 'resize_instance' + ) as ( + mock_dest_rpcapi_resize + ): + # We don't want to wait here so just make the raw api call + self.api.post_server_action(server['id'], {'migrate': None}) + # Assert that the dest compute attemptes to call resize_instance + mock_dest_rpcapi_resize.assert_called_once()