Skip to content

Commit 2bdf34d

Browse files
committed
compute: Auto-configure shared/block live migration
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>
1 parent ace4bfb commit 2bdf34d

3 files changed

Lines changed: 90 additions & 48 deletions

File tree

openstackclient/compute/v2/server.py

Lines changed: 59 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2457,9 +2457,11 @@ def get_parser(self, prog_name):
24572457
'--live-migration',
24582458
dest='live_migration',
24592459
action='store_true',
2460-
help=_('Live migrate the server. Use the ``--host`` option to '
2461-
'specify a target host for the migration which will be '
2462-
'validated by the scheduler.'),
2460+
help=_(
2461+
'Live migrate the server; use the ``--host`` option to '
2462+
'specify a target host for the migration which will be '
2463+
'validated by the scheduler'
2464+
),
24632465
)
24642466
# The --live and --host options are mutually exclusive ways of asking
24652467
# for a target host during a live migration.
@@ -2469,37 +2471,48 @@ def get_parser(self, prog_name):
24692471
host_group.add_argument(
24702472
'--live',
24712473
metavar='<hostname>',
2472-
help=_('**Deprecated** This option is problematic in that it '
2473-
'requires a host and prior to compute API version 2.30, '
2474-
'specifying a host during live migration will bypass '
2475-
'validation by the scheduler which could result in '
2476-
'failures to actually migrate the server to the specified '
2477-
'host or over-subscribe the host. Use the '
2478-
'``--live-migration`` option instead. If both this option '
2479-
'and ``--live-migration`` are used, ``--live-migration`` '
2480-
'takes priority.'),
2474+
help=_(
2475+
'**Deprecated** This option is problematic in that it '
2476+
'requires a host and prior to compute API version 2.30, '
2477+
'specifying a host during live migration will bypass '
2478+
'validation by the scheduler which could result in '
2479+
'failures to actually migrate the server to the specified '
2480+
'host or over-subscribe the host. Use the '
2481+
'``--live-migration`` option instead. If both this option '
2482+
'and ``--live-migration`` are used, ``--live-migration`` '
2483+
'takes priority.'
2484+
),
24812485
)
24822486
host_group.add_argument(
24832487
'--host',
24842488
metavar='<hostname>',
2485-
help=_('Migrate the server to the specified host. Requires '
2486-
'``--os-compute-api-version`` 2.30 or greater when used '
2487-
'with the ``--live-migration`` option, otherwise requires '
2488-
'``--os-compute-api-version`` 2.56 or greater.'),
2489+
help=_(
2490+
'Migrate the server to the specified host. '
2491+
'(supported with --os-compute-api-version 2.30 or above '
2492+
'when used with the --live-migration option) '
2493+
'(supported with --os-compute-api-version 2.56 or above '
2494+
'when used without the --live-migration option)'
2495+
),
24892496
)
24902497
migration_group = parser.add_mutually_exclusive_group()
24912498
migration_group.add_argument(
24922499
'--shared-migration',
24932500
dest='block_migration',
24942501
action='store_false',
2495-
default=False,
2496-
help=_('Perform a shared live migration (default)'),
2502+
default=None,
2503+
help=_(
2504+
'Perform a shared live migration '
2505+
'(default before --os-compute-api-version 2.25, auto after)'
2506+
),
24972507
)
24982508
migration_group.add_argument(
24992509
'--block-migration',
25002510
dest='block_migration',
25012511
action='store_true',
2502-
help=_('Perform a block live migration'),
2512+
help=_(
2513+
'Perform a block live migration '
2514+
'(auto-configured from --os-compute-api-version 2.25)'
2515+
),
25032516
)
25042517
disk_group = parser.add_mutually_exclusive_group()
25052518
disk_group.add_argument(
@@ -2513,8 +2526,9 @@ def get_parser(self, prog_name):
25132526
dest='disk_overcommit',
25142527
action='store_false',
25152528
default=False,
2516-
help=_('Do not over-commit disk on the'
2517-
' destination host (default)'),
2529+
help=_(
2530+
'Do not over-commit disk on the destination host (default)'
2531+
),
25182532
)
25192533
parser.add_argument(
25202534
'--wait',
@@ -2549,9 +2563,21 @@ def _show_progress(progress):
25492563
if parsed_args.live or parsed_args.live_migration:
25502564
# Always log a warning if --live is used.
25512565
self._log_warning_for_live(parsed_args)
2552-
kwargs = {
2553-
'block_migration': parsed_args.block_migration
2554-
}
2566+
2567+
kwargs = {}
2568+
2569+
block_migration = parsed_args.block_migration
2570+
if block_migration is None:
2571+
if (
2572+
compute_client.api_version <
2573+
api_versions.APIVersion('2.25')
2574+
):
2575+
block_migration = False
2576+
else:
2577+
block_migration = 'auto'
2578+
2579+
kwargs['block_migration'] = block_migration
2580+
25552581
# Prefer --live-migration over --live if both are specified.
25562582
if parsed_args.live_migration:
25572583
# Technically we could pass a non-None host with
@@ -2560,19 +2586,24 @@ def _show_progress(progress):
25602586
# want to support, so if the user is using --live-migration
25612587
# and --host, we want to enforce that they are using version
25622588
# 2.30 or greater.
2563-
if (parsed_args.host and
2564-
compute_client.api_version <
2565-
api_versions.APIVersion('2.30')):
2589+
if (
2590+
parsed_args.host and
2591+
compute_client.api_version <
2592+
api_versions.APIVersion('2.30')
2593+
):
25662594
raise exceptions.CommandError(
25672595
'--os-compute-api-version 2.30 or greater is required '
2568-
'when using --host')
2596+
'when using --host'
2597+
)
2598+
25692599
# The host parameter is required in the API even if None.
25702600
kwargs['host'] = parsed_args.host
25712601
else:
25722602
kwargs['host'] = parsed_args.live
25732603

25742604
if compute_client.api_version < api_versions.APIVersion('2.25'):
25752605
kwargs['disk_over_commit'] = parsed_args.disk_overcommit
2606+
25762607
server.live_migrate(**kwargs)
25772608
else:
25782609
if parsed_args.block_migration or parsed_args.disk_overcommit:

openstackclient/tests/unit/compute/v2/test_server.py

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4524,7 +4524,7 @@ def test_server_migrate_no_options(self):
45244524
]
45254525
verifylist = [
45264526
('live', None),
4527-
('block_migration', False),
4527+
('block_migration', None),
45284528
('disk_overcommit', False),
45294529
('wait', False),
45304530
]
@@ -4547,7 +4547,7 @@ def test_server_migrate_with_host_2_56(self):
45474547
('live', None),
45484548
('live_migration', False),
45494549
('host', 'fakehost'),
4550-
('block_migration', False),
4550+
('block_migration', None),
45514551
('disk_overcommit', False),
45524552
('wait', False),
45534553
]
@@ -4588,7 +4588,7 @@ def test_server_migrate_with_disk_overcommit(self):
45884588
]
45894589
verifylist = [
45904590
('live', None),
4591-
('block_migration', False),
4591+
('block_migration', None),
45924592
('disk_overcommit', True),
45934593
('wait', False),
45944594
]
@@ -4611,7 +4611,7 @@ def test_server_migrate_with_host_pre_2_56(self):
46114611
('live', None),
46124612
('live_migration', False),
46134613
('host', 'fakehost'),
4614-
('block_migration', False),
4614+
('block_migration', None),
46154615
('disk_overcommit', False),
46164616
('wait', False),
46174617
]
@@ -4637,7 +4637,7 @@ def test_server_live_migrate(self):
46374637
('live', 'fakehost'),
46384638
('live_migration', False),
46394639
('host', None),
4640-
('block_migration', False),
4640+
('block_migration', None),
46414641
('disk_overcommit', False),
46424642
('wait', False),
46434643
]
@@ -4670,7 +4670,7 @@ def test_server_live_migrate_host_pre_2_30(self):
46704670
('live', None),
46714671
('live_migration', True),
46724672
('host', 'fakehost'),
4673-
('block_migration', False),
4673+
('block_migration', None),
46744674
('disk_overcommit', False),
46754675
('wait', False),
46764676
]
@@ -4696,7 +4696,7 @@ def test_server_live_migrate_no_host(self):
46964696
('live', None),
46974697
('live_migration', True),
46984698
('host', None),
4699-
('block_migration', False),
4699+
('block_migration', None),
47004700
('disk_overcommit', False),
47014701
('wait', False),
47024702
]
@@ -4724,7 +4724,7 @@ def test_server_live_migrate_with_host(self):
47244724
('live', None),
47254725
('live_migration', True),
47264726
('host', 'fakehost'),
4727-
('block_migration', False),
4727+
('block_migration', None),
47284728
('disk_overcommit', False),
47294729
('wait', False),
47304730
]
@@ -4736,9 +4736,10 @@ def test_server_live_migrate_with_host(self):
47364736
result = self.cmd.take_action(parsed_args)
47374737

47384738
self.servers_mock.get.assert_called_with(self.server.id)
4739-
# No disk_overcommit with microversion >= 2.25.
4740-
self.server.live_migrate.assert_called_with(block_migration=False,
4741-
host='fakehost')
4739+
# No disk_overcommit and block_migration defaults to auto with
4740+
# microversion >= 2.25
4741+
self.server.live_migrate.assert_called_with(
4742+
block_migration='auto', host='fakehost')
47424743
self.assertNotCalled(self.servers_mock.migrate)
47434744
self.assertIsNone(result)
47444745

@@ -4753,7 +4754,7 @@ def test_server_live_migrate_without_host_override_live(self):
47534754
('live', 'fakehost'),
47544755
('live_migration', True),
47554756
('host', None),
4756-
('block_migration', False),
4757+
('block_migration', None),
47574758
('disk_overcommit', False),
47584759
('wait', False),
47594760
]
@@ -4779,8 +4780,9 @@ def test_server_live_migrate_live_and_host_mutex(self):
47794780
arglist = [
47804781
'--live', 'fakehost', '--host', 'fakehost', self.server.id,
47814782
]
4782-
self.assertRaises(utils.ParserException,
4783-
self.check_parser, self.cmd, arglist, verify_args=[])
4783+
self.assertRaises(
4784+
utils.ParserException,
4785+
self.check_parser, self.cmd, arglist, verify_args=[])
47844786

47854787
def test_server_block_live_migrate(self):
47864788
arglist = [
@@ -4812,7 +4814,7 @@ def test_server_live_migrate_with_disk_overcommit(self):
48124814
]
48134815
verifylist = [
48144816
('live', 'fakehost'),
4815-
('block_migration', False),
4817+
('block_migration', None),
48164818
('disk_overcommit', True),
48174819
('wait', False),
48184820
]
@@ -4861,7 +4863,7 @@ def test_server_live_migrate_225(self):
48614863
]
48624864
verifylist = [
48634865
('live', 'fakehost'),
4864-
('block_migration', False),
4866+
('block_migration', None),
48654867
('wait', False),
48664868
]
48674869
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -4872,8 +4874,11 @@ def test_server_live_migrate_225(self):
48724874
result = self.cmd.take_action(parsed_args)
48734875

48744876
self.servers_mock.get.assert_called_with(self.server.id)
4875-
self.server.live_migrate.assert_called_with(block_migration=False,
4876-
host='fakehost')
4877+
# No disk_overcommit and block_migration defaults to auto with
4878+
# microversion >= 2.25
4879+
self.server.live_migrate.assert_called_with(
4880+
block_migration='auto',
4881+
host='fakehost')
48774882
self.assertNotCalled(self.servers_mock.migrate)
48784883
self.assertIsNone(result)
48794884

@@ -4884,7 +4889,7 @@ def test_server_migrate_with_wait(self, mock_wait_for_status):
48844889
]
48854890
verifylist = [
48864891
('live', None),
4887-
('block_migration', False),
4892+
('block_migration', None),
48884893
('disk_overcommit', False),
48894894
('wait', True),
48904895
]
@@ -4904,7 +4909,7 @@ def test_server_migrate_with_wait_fails(self, mock_wait_for_status):
49044909
]
49054910
verifylist = [
49064911
('live', None),
4907-
('block_migration', False),
4912+
('block_migration', None),
49084913
('disk_overcommit', False),
49094914
('wait', True),
49104915
]
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
features:
3+
- |
4+
The ``server migrate`` command will now automatically determine whether
5+
to use block or shared migration during a live migration operation. This
6+
requires Compute API microversion 2.25 or greater.

0 commit comments

Comments
 (0)