openstack/python-openstackclient
Code Issues Proposed changes

compute: Auto-configure shared/block live migration

Browse Source 
API microversion 2.25 introduced the 'block_migration=auto' value for
the os-migrateLive server action. This is a sensible default that we
should use, allowing users to avoid stating one of the
'--block-migration' or '--shared-migration' parameters explicitly.

While we're here, we take the opportunity to fix up some formatting in
the function, which is really rather messy.

Change-Id: Ieedc77d6dc3d4a3cd93b29672faa97dd4e8c1185
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
This commit is contained in:
Stephen Finucane 2021-01-21 11:25:08 +00:00
parent ace4bfb640
commit 2bdf34dcc3
3 changed files with 90 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

69
openstackclient/compute/v2/server.py
View File

@ -2457,9 +2457,11 @@ revert to release the new server and restart the old one.""")
'--live-migration',
dest='live_migration',
action='store_true',
help=_('Live migrate the server. Use the ``--host`` option to '
help=_(
'Live migrate the server; use the ``--host`` option to '
'specify a target host for the migration which will be '
'validated by the scheduler.'),
'validated by the scheduler'
),
)
# The --live and --host options are mutually exclusive ways of asking
# for a target host during a live migration.
@ -2469,7 +2471,8 @@ revert to release the new server and restart the old one.""")
host_group.add_argument(
'--live',
metavar='<hostname>',
help=_('**Deprecated** This option is problematic in that it '
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 '
@ -2477,29 +2480,39 @@ revert to release the new server and restart the old one.""")
'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.'),
'takes priority.'
),
)
host_group.add_argument(
'--host',
metavar='<hostname>',
help=_('Migrate the server to the specified host. Requires '
'``--os-compute-api-version`` 2.30 or greater when used '
'with the ``--live-migration`` option, otherwise requires '
'``--os-compute-api-version`` 2.56 or greater.'),
help=_(
'Migrate the server to the specified host. '
'(supported with --os-compute-api-version 2.30 or above '
'when used with the --live-migration option) '
'(supported with --os-compute-api-version 2.56 or above '
'when used without the --live-migration option)'
),
)
migration_group = parser.add_mutually_exclusive_group()
migration_group.add_argument(
'--shared-migration',
dest='block_migration',
action='store_false',
default=False,
help=_('Perform a shared live migration (default)'),
default=None,
help=_(
'Perform a shared live migration '
'(default before --os-compute-api-version 2.25, auto after)'
),
)
migration_group.add_argument(
'--block-migration',
dest='block_migration',
action='store_true',
help=_('Perform a block live migration'),
help=_(
'Perform a block live migration '
'(auto-configured from --os-compute-api-version 2.25)'
),
)
disk_group = parser.add_mutually_exclusive_group()
disk_group.add_argument(
@ -2513,8 +2526,9 @@ revert to release the new server and restart the old one.""")
dest='disk_overcommit',
action='store_false',
default=False,
help=_('Do not over-commit disk on the'
' destination host (default)'),
help=_(
'Do not over-commit disk on the destination host (default)'
),
)
parser.add_argument(
'--wait',
@ -2549,9 +2563,21 @@ revert to release the new server and restart the old one.""")
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 = {
'block_migration': parsed_args.block_migration
}
kwargs = {}
block_migration = parsed_args.block_migration
if block_migration is None:
if (
compute_client.api_version <
api_versions.APIVersion('2.25')
):
block_migration = False
else:
block_migration = 'auto'
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
@ -2560,12 +2586,16 @@ revert to release the new server and restart the old one.""")
# 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
if (
parsed_args.host and
compute_client.api_version <
api_versions.APIVersion('2.30')):
api_versions.APIVersion('2.30')
):
raise exceptions.CommandError(
'--os-compute-api-version 2.30 or greater is required '
'when using --host')
'when using --host'
)
# The host parameter is required in the API even if None.
kwargs['host'] = parsed_args.host
else:
@ -2573,6 +2603,7 @@ revert to release the new server and restart the old one.""")
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:

41
openstackclient/tests/unit/compute/v2/test_server.py
View File

@ -4524,7 +4524,7 @@ class TestServerMigrate(TestServer):
]
verifylist = [
('live', None),
('block_migration', False),
('block_migration', None),
('disk_overcommit', False),
('wait', False),
]
@ -4547,7 +4547,7 @@ class TestServerMigrate(TestServer):
('live', None),
('live_migration', False),
('host', 'fakehost'),
('block_migration', False),
('block_migration', None),
('disk_overcommit', False),
('wait', False),
]
@ -4588,7 +4588,7 @@ class TestServerMigrate(TestServer):
]
verifylist = [
('live', None),
('block_migration', False),
('block_migration', None),
('disk_overcommit', True),
('wait', False),
]
@ -4611,7 +4611,7 @@ class TestServerMigrate(TestServer):
('live', None),
('live_migration', False),
('host', 'fakehost'),
('block_migration', False),
('block_migration', None),
('disk_overcommit', False),
('wait', False),
]
@ -4637,7 +4637,7 @@ class TestServerMigrate(TestServer):
('live', 'fakehost'),
('live_migration', False),
('host', None),
('block_migration', False),
('block_migration', None),
('disk_overcommit', False),
('wait', False),
]
@ -4670,7 +4670,7 @@ class TestServerMigrate(TestServer):
('live', None),
('live_migration', True),
('host', 'fakehost'),
('block_migration', False),
('block_migration', None),
('disk_overcommit', False),
('wait', False),
]
@ -4696,7 +4696,7 @@ class TestServerMigrate(TestServer):
('live', None),
('live_migration', True),
('host', None),
('block_migration', False),
('block_migration', None),
('disk_overcommit', False),
('wait', False),
]
@ -4724,7 +4724,7 @@ class TestServerMigrate(TestServer):
('live', None),
('live_migration', True),
('host', 'fakehost'),
('block_migration', False),
('block_migration', None),
('disk_overcommit', False),
('wait', False),
]
@ -4736,9 +4736,10 @@ class TestServerMigrate(TestServer):
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')
# 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)
@ -4753,7 +4754,7 @@ class TestServerMigrate(TestServer):
('live', 'fakehost'),
('live_migration', True),
('host', None),
('block_migration', False),
('block_migration', None),
('disk_overcommit', False),
('wait', False),
]
@ -4779,7 +4780,8 @@ class TestServerMigrate(TestServer):
arglist = [
'--live', 'fakehost', '--host', 'fakehost', self.server.id,
]
self.assertRaises(utils.ParserException,
self.assertRaises(
utils.ParserException,
self.check_parser, self.cmd, arglist, verify_args=[])
def test_server_block_live_migrate(self):
@ -4812,7 +4814,7 @@ class TestServerMigrate(TestServer):
]
verifylist = [
('live', 'fakehost'),
('block_migration', False),
('block_migration', None),
('disk_overcommit', True),
('wait', False),
]
@ -4861,7 +4863,7 @@ class TestServerMigrate(TestServer):
]
verifylist = [
('live', 'fakehost'),
('block_migration', False),
('block_migration', None),
('wait', False),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@ -4872,7 +4874,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,
# 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)
@ -4884,7 +4889,7 @@ class TestServerMigrate(TestServer):
]
verifylist = [
('live', None),
('block_migration', False),
('block_migration', None),
('disk_overcommit', False),
('wait', True),
]
@ -4904,7 +4909,7 @@ class TestServerMigrate(TestServer):
]
verifylist = [
('live', None),
('block_migration', False),
('block_migration', None),
('disk_overcommit', False),
('wait', True),
]

6
releasenotes/notes/auto-configure-block-live-migration-437d461c914f8f2f.yaml Normal file
View File

@ -0,0 +1,6 @@
---
features:
- |
The ``server migrate`` command will now automatically determine whether
to use block or shared migration during a live migration operation. This
requires Compute API microversion 2.25 or greater.