diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py
index 48f5b7cf16..9838ed5480 100644
--- a/openstackclient/compute/v2/server.py
+++ b/openstackclient/compute/v2/server.py
@@ -2473,27 +2473,7 @@ revert to release the new server and restart the old one.""")
'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=_(
- '**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.'
- ),
- )
- host_group.add_argument(
+ parser.add_argument(
'--host',
metavar='<hostname>',
help=_(
@@ -2551,15 +2531,6 @@ revert to release the new server and restart the old one.""")
)
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):
@@ -2573,11 +2544,8 @@ revert to release the new server and restart the old one.""")
compute_client.servers,
parsed_args.server,
)
- # 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)
+ if parsed_args.live_migration:
kwargs = {}
block_migration = parsed_args.block_migration
@@ -2592,28 +2560,23 @@ revert to release the new server and restart the old one.""")
kwargs['block_migration'] = 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'
- )
+ # 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
+ # The host parameter is required in the API even if None.
+ kwargs['host'] = parsed_args.host
if compute_client.api_version < api_versions.APIVersion('2.25'):
kwargs['disk_over_commit'] = parsed_args.disk_overcommit
@@ -2628,7 +2591,7 @@ revert to release the new server and restart the old one.""")
self.log.warning(msg)
server.live_migrate(**kwargs)
- else:
+ else: # cold migration
if parsed_args.block_migration or parsed_args.disk_overcommit:
raise exceptions.CommandError(
"--live-migration must be specified if "
diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py
index 9afc4cebe8..ced5d458c2 100644
--- a/openstackclient/tests/unit/compute/v2/test_server.py
+++ b/openstackclient/tests/unit/compute/v2/test_server.py
@@ -4523,7 +4523,7 @@ class TestServerMigrate(TestServer):
self.server.id,
]
verifylist = [
- ('live', None),
+ ('live_migration', False),
('block_migration', None),
('disk_overcommit', False),
('wait', False),
@@ -4544,7 +4544,6 @@ class TestServerMigrate(TestServer):
'--host', 'fakehost', self.server.id,
]
verifylist = [
- ('live', None),
('live_migration', False),
('host', 'fakehost'),
('block_migration', None),
@@ -4568,7 +4567,7 @@ class TestServerMigrate(TestServer):
'--block-migration', self.server.id,
]
verifylist = [
- ('live', None),
+ ('live_migration', False),
('block_migration', True),
('disk_overcommit', False),
('wait', False),
@@ -4587,7 +4586,7 @@ class TestServerMigrate(TestServer):
'--disk-overcommit', self.server.id,
]
verifylist = [
- ('live', None),
+ ('live_migration', False),
('block_migration', None),
('disk_overcommit', True),
('wait', False),
@@ -4608,7 +4607,6 @@ class TestServerMigrate(TestServer):
'--host', 'fakehost', self.server.id,
]
verifylist = [
- ('live', None),
('live_migration', False),
('host', 'fakehost'),
('block_migration', None),
@@ -4630,70 +4628,11 @@ class TestServerMigrate(TestServer):
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', None),
- ('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.24')
-
- 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='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.',
- str(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', None),
- ('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', str(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', None),
@@ -4702,26 +4641,22 @@ class TestServerMigrate(TestServer):
]
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)
+ 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.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', None),
@@ -4743,53 +4678,42 @@ class TestServerMigrate(TestServer):
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.
+ def test_server_live_migrate_with_host_pre_v230(self):
+ # Tests that the --host option is not supported for --live-migration
+ # before microversion 2.30 (the test defaults to 2.1).
arglist = [
- '--live', 'fakehost', '--live-migration', self.server.id,
+ '--live-migration', '--host', 'fakehost', self.server.id,
]
verifylist = [
- ('live', 'fakehost'),
('live_migration', True),
- ('host', None),
+ ('host', 'fakehost'),
('block_migration', None),
('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)
+ 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', str(ex))
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.live_migrate)
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.',
- str(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 = [
- '--live', 'fakehost', '--block-migration', self.server.id,
+ '--live-migration',
+ '--block-migration',
+ self.server.id,
]
verifylist = [
- ('live', 'fakehost'),
+ ('live_migration', True),
('block_migration', True),
('disk_overcommit', False),
('wait', False),
@@ -4802,18 +4726,21 @@ class TestServerMigrate(TestServer):
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=True,
- disk_over_commit=False,
- host='fakehost')
+ self.server.live_migrate.assert_called_with(
+ block_migration=True,
+ disk_over_commit=False,
+ host=None)
self.assertNotCalled(self.servers_mock.migrate)
self.assertIsNone(result)
def test_server_live_migrate_with_disk_overcommit(self):
arglist = [
- '--live', 'fakehost', '--disk-overcommit', self.server.id,
+ '--live-migration',
+ '--disk-overcommit',
+ self.server.id,
]
verifylist = [
- ('live', 'fakehost'),
+ ('live_migration', True),
('block_migration', None),
('disk_overcommit', True),
('wait', False),
@@ -4826,9 +4753,10 @@ class TestServerMigrate(TestServer):
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=True,
- host='fakehost')
+ self.server.live_migrate.assert_called_with(
+ block_migration=False,
+ disk_over_commit=True,
+ host=None)
self.assertNotCalled(self.servers_mock.migrate)
self.assertIsNone(result)
@@ -4839,7 +4767,6 @@ class TestServerMigrate(TestServer):
self.server.id,
]
verifylist = [
- ('live', None),
('live_migration', True),
('block_migration', None),
('disk_overcommit', True),
@@ -4866,63 +4793,13 @@ class TestServerMigrate(TestServer):
'The --disk-overcommit and --no-disk-overcommit options ',
str(mock_warning.call_args[0][0]))
- def test_server_live_migrate_with_false_value_options(self):
- arglist = [
- '--live', 'fakehost', '--no-disk-overcommit',
- '--shared-migration', self.server.id,
- ]
- verifylist = [
- ('live', '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.24')
-
- 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='fakehost')
- self.assertNotCalled(self.servers_mock.migrate)
- self.assertIsNone(result)
-
- def test_server_live_migrate_225(self):
- arglist = [
- '--live', 'fakehost', self.server.id,
- ]
- verifylist = [
- ('live', 'fakehost'),
- ('block_migration', None),
- ('wait', False),
- ]
- parsed_args = self.check_parser(self.cmd, arglist, verifylist)
-
- self.app.client_manager.compute.api_version = \
- api_versions.APIVersion('2.25')
-
- result = self.cmd.take_action(parsed_args)
-
- self.servers_mock.get.assert_called_with(self.server.id)
- # No disk_overcommit and block_migration defaults to auto with
- # microversion >= 2.25
- self.server.live_migrate.assert_called_with(
- block_migration='auto',
- host='fakehost')
- self.assertNotCalled(self.servers_mock.migrate)
- self.assertIsNone(result)
-
@mock.patch.object(common_utils, 'wait_for_status', return_value=True)
def test_server_migrate_with_wait(self, mock_wait_for_status):
arglist = [
'--wait', self.server.id,
]
verifylist = [
- ('live', None),
+ ('live_migration', False),
('block_migration', None),
('disk_overcommit', False),
('wait', True),
@@ -4942,7 +4819,7 @@ class TestServerMigrate(TestServer):
'--wait', self.server.id,
]
verifylist = [
- ('live', None),
+ ('live_migration', False),
('block_migration', None),
('disk_overcommit', False),
('wait', True),
diff --git a/releasenotes/notes/remove-deprecated-server-migrate-live-option-28ec43ee210124dc.yaml b/releasenotes/notes/remove-deprecated-server-migrate-live-option-28ec43ee210124dc.yaml
new file mode 100644
index 0000000000..bc31aa2699
--- /dev/null
+++ b/releasenotes/notes/remove-deprecated-server-migrate-live-option-28ec43ee210124dc.yaml
@@ -0,0 +1,8 @@
+---
+upgrade:
+ - |
+ The deprecated ``--live`` option of the ``server migrate`` command has
+ been removed. This was problematic as it required a host argument and
+ would result in a forced live migration to a host, bypassing the
+ scheduler. It has been replaced by a ``--live-migration`` option and
+ optional ``--host`` option.