diff options
Diffstat (limited to 'openstackclient')
| -rw-r--r-- | openstackclient/compute/v2/server.py | 78 | ||||
| -rw-r--r-- | openstackclient/tests/unit/compute/v2/test_server.py | 158 |
2 files changed, 228 insertions, 8 deletions
diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index af1bcc9d..3e1deed5 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -1445,9 +1445,38 @@ class MigrateServer(command.Command): help=_('Server (name or ID)'), ) parser.add_argument( + '--live-migration', + dest='live_migration', + action='store_true', + help=_('Live migrate the server. Use the ``--host`` option to ' + 'specify a target host for the migration which will be ' + 'validated by the scheduler.'), + ) + # The --live and --host options are mutually exclusive ways of asking + # for a target host during a live migration. + host_group = parser.add_mutually_exclusive_group() + # TODO(mriedem): Remove --live in the next major version bump after + # the Train release. + host_group.add_argument( '--live', metavar='<hostname>', - help=_('Target hostname'), + help=_('**Deprecated** This option is problematic in that it ' + 'requires a host and prior to compute API version 2.30, ' + 'specifying a host during live migration will bypass ' + 'validation by the scheduler which could result in ' + 'failures to actually migrate the server to the specified ' + 'host or over-subscribe the host. Use the ' + '``--live-migration`` option instead. If both this option ' + 'and ``--live-migration`` are used, ``--live-migration`` ' + 'takes priority.'), + ) + # TODO(mriedem): Add support for --os-compute-api-version >= 2.56 where + # you can cold migrate to a specified target host. + host_group.add_argument( + '--host', + metavar='<hostname>', + help=_('Live migrate the server to the specified host. Requires ' + '``--os-compute-api-version`` 2.30 or greater.'), ) migration_group = parser.add_mutually_exclusive_group() migration_group.add_argument( @@ -1485,6 +1514,15 @@ class MigrateServer(command.Command): ) return parser + def _log_warning_for_live(self, parsed_args): + if parsed_args.live: + # NOTE(mriedem): The --live option requires a host and if + # --os-compute-api-version is less than 2.30 it will forcefully + # bypass the scheduler which is dangerous. + self.log.warning(_( + 'The --live option has been deprecated. Please use the ' + '--live-migration option instead.')) + def take_action(self, parsed_args): def _show_progress(progress): @@ -1498,19 +1536,45 @@ class MigrateServer(command.Command): compute_client.servers, parsed_args.server, ) - if parsed_args.live: + # Check for live migration. + if parsed_args.live or parsed_args.live_migration: + # Always log a warning if --live is used. + self._log_warning_for_live(parsed_args) kwargs = { - 'host': parsed_args.live, 'block_migration': parsed_args.block_migration } + # Prefer --live-migration over --live if both are specified. + if parsed_args.live_migration: + # Technically we could pass a non-None host with + # --os-compute-api-version < 2.30 but that is the same thing + # as the --live option bypassing the scheduler which we don't + # want to support, so if the user is using --live-migration + # and --host, we want to enforce that they are using version + # 2.30 or greater. + if (parsed_args.host and + compute_client.api_version < + api_versions.APIVersion('2.30')): + raise exceptions.CommandError( + '--os-compute-api-version 2.30 or greater is required ' + 'when using --host') + # The host parameter is required in the API even if None. + kwargs['host'] = parsed_args.host + else: + kwargs['host'] = parsed_args.live + if compute_client.api_version < api_versions.APIVersion('2.25'): kwargs['disk_over_commit'] = parsed_args.disk_overcommit server.live_migrate(**kwargs) else: - if parsed_args.block_migration or parsed_args.disk_overcommit: - raise exceptions.CommandError("--live must be specified if " - "--block-migration or " - "--disk-overcommit is specified") + if (parsed_args.block_migration or parsed_args.disk_overcommit or + parsed_args.host): + # TODO(mriedem): Allow --host for cold migration if + # --os-compute-api-version >= 2.56. + raise exceptions.CommandError( + "--live-migration must be specified if " + "--block-migration, --disk-overcommit or --host is " + "specified") + server.migrate() if parsed_args.wait: diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index 8ddfe6d5..4a37a812 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -23,6 +23,7 @@ from openstack import exceptions as sdk_exceptions from osc_lib import exceptions from osc_lib import utils as common_utils from oslo_utils import timeutils +import six from openstackclient.compute.v2 import server from openstackclient.tests.unit.compute.v2 import fakes as compute_fakes @@ -2565,12 +2566,40 @@ class TestServerMigrate(TestServer): self.assertNotCalled(self.servers_mock.live_migrate) self.assertNotCalled(self.servers_mock.migrate) + def test_server_migrate_with_host(self): + # Tests that --host is not allowed for a cold migration. + arglist = [ + '--host', 'fakehost', self.server.id, + ] + verifylist = [ + ('live', None), + ('live_migration', False), + ('host', 'fakehost'), + ('block_migration', False), + ('disk_overcommit', False), + ('wait', False), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + ex = self.assertRaises(exceptions.CommandError, self.cmd.take_action, + parsed_args) + + # Make sure it's the error we expect. + self.assertIn("--live-migration must be specified if " + "--block-migration, --disk-overcommit or --host is " + "specified", six.text_type(ex)) + self.servers_mock.get.assert_called_with(self.server.id) + self.assertNotCalled(self.servers_mock.live_migrate) + self.assertNotCalled(self.servers_mock.migrate) + def test_server_live_migrate(self): arglist = [ '--live', 'fakehost', self.server.id, ] verifylist = [ ('live', 'fakehost'), + ('live_migration', False), + ('host', None), ('block_migration', False), ('disk_overcommit', False), ('wait', False), @@ -2580,7 +2609,8 @@ class TestServerMigrate(TestServer): self.app.client_manager.compute.api_version = \ api_versions.APIVersion('2.24') - result = self.cmd.take_action(parsed_args) + with mock.patch.object(self.cmd.log, 'warning') as mock_warning: + result = self.cmd.take_action(parsed_args) self.servers_mock.get.assert_called_with(self.server.id) self.server.live_migrate.assert_called_with(block_migration=False, @@ -2588,6 +2618,132 @@ class TestServerMigrate(TestServer): host='fakehost') self.assertNotCalled(self.servers_mock.migrate) self.assertIsNone(result) + # A warning should have been logged for using --live. + mock_warning.assert_called_once() + self.assertIn('The --live option has been deprecated.', + six.text_type(mock_warning.call_args[0][0])) + + def test_server_live_migrate_host_pre_2_30(self): + # Tests that the --host option is not supported for --live-migration + # before microversion 2.30 (the test defaults to 2.1). + arglist = [ + '--live-migration', '--host', 'fakehost', self.server.id, + ] + verifylist = [ + ('live', None), + ('live_migration', True), + ('host', 'fakehost'), + ('block_migration', False), + ('disk_overcommit', False), + ('wait', False), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + ex = self.assertRaises(exceptions.CommandError, self.cmd.take_action, + parsed_args) + + # Make sure it's the error we expect. + self.assertIn('--os-compute-api-version 2.30 or greater is required ' + 'when using --host', six.text_type(ex)) + + self.servers_mock.get.assert_called_with(self.server.id) + self.assertNotCalled(self.servers_mock.live_migrate) + self.assertNotCalled(self.servers_mock.migrate) + + def test_server_live_migrate_no_host(self): + # Tests the --live-migration option without --host or --live. + arglist = [ + '--live-migration', self.server.id, + ] + verifylist = [ + ('live', None), + ('live_migration', True), + ('host', None), + ('block_migration', False), + ('disk_overcommit', False), + ('wait', False), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + with mock.patch.object(self.cmd.log, 'warning') as mock_warning: + result = self.cmd.take_action(parsed_args) + + self.servers_mock.get.assert_called_with(self.server.id) + self.server.live_migrate.assert_called_with(block_migration=False, + disk_over_commit=False, + host=None) + self.assertNotCalled(self.servers_mock.migrate) + self.assertIsNone(result) + # Since --live wasn't used a warning shouldn't have been logged. + mock_warning.assert_not_called() + + def test_server_live_migrate_with_host(self): + # Tests the --live-migration option with --host but no --live. + # This requires --os-compute-api-version >= 2.30 so the test uses 2.30. + arglist = [ + '--live-migration', '--host', 'fakehost', self.server.id, + ] + verifylist = [ + ('live', None), + ('live_migration', True), + ('host', 'fakehost'), + ('block_migration', False), + ('disk_overcommit', False), + ('wait', False), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + self.app.client_manager.compute.api_version = \ + api_versions.APIVersion('2.30') + + result = self.cmd.take_action(parsed_args) + + self.servers_mock.get.assert_called_with(self.server.id) + # No disk_overcommit with microversion >= 2.25. + self.server.live_migrate.assert_called_with(block_migration=False, + host='fakehost') + self.assertNotCalled(self.servers_mock.migrate) + self.assertIsNone(result) + + def test_server_live_migrate_without_host_override_live(self): + # Tests the --live-migration option without --host and with --live. + # The --live-migration option will take precedence and a warning is + # logged for using --live. + arglist = [ + '--live', 'fakehost', '--live-migration', self.server.id, + ] + verifylist = [ + ('live', 'fakehost'), + ('live_migration', True), + ('host', None), + ('block_migration', False), + ('disk_overcommit', False), + ('wait', False), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + with mock.patch.object(self.cmd.log, 'warning') as mock_warning: + result = self.cmd.take_action(parsed_args) + + self.servers_mock.get.assert_called_with(self.server.id) + self.server.live_migrate.assert_called_with(block_migration=False, + disk_over_commit=False, + host=None) + self.assertNotCalled(self.servers_mock.migrate) + self.assertIsNone(result) + # A warning should have been logged for using --live. + mock_warning.assert_called_once() + self.assertIn('The --live option has been deprecated.', + six.text_type(mock_warning.call_args[0][0])) + + def test_server_live_migrate_live_and_host_mutex(self): + # Tests specifying both the --live and --host options which are in a + # mutex group so argparse should fail. + arglist = [ + '--live', 'fakehost', '--host', 'fakehost', self.server.id, + ] + self.assertRaises(utils.ParserException, + self.check_parser, self.cmd, arglist, verify_args=[]) def test_server_block_live_migrate(self): arglist = [ |