Skip to content

Commit 8868c77

Browse files
committed
compute: Stop silently ignore --(no-)disk-overcommit
These options are not supported from Nova API microversion 2.25 and above. This can be a source of confusion. Start warning, with an eye on erroring out in the future. Change-Id: I53f27eb3e3c1a84d0d77a1672c008d0e8bb8536f Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
1 parent 2bdf34d commit 8868c77

3 files changed

Lines changed: 57 additions & 1 deletion

File tree

openstackclient/compute/v2/server.py

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2519,7 +2519,10 @@ def get_parser(self, prog_name):
25192519
'--disk-overcommit',
25202520
action='store_true',
25212521
default=False,
2522-
help=_('Allow disk over-commit on the destination host'),
2522+
help=_(
2523+
'Allow disk over-commit on the destination host'
2524+
'(supported with --os-compute-api-version 2.24 or below)'
2525+
),
25232526
)
25242527
disk_group.add_argument(
25252528
'--no-disk-overcommit',
@@ -2528,6 +2531,7 @@ def get_parser(self, prog_name):
25282531
default=False,
25292532
help=_(
25302533
'Do not over-commit disk on the destination host (default)'
2534+
'(supported with --os-compute-api-version 2.24 or below)'
25312535
),
25322536
)
25332537
parser.add_argument(
@@ -2603,6 +2607,15 @@ def _show_progress(progress):
26032607

26042608
if compute_client.api_version < api_versions.APIVersion('2.25'):
26052609
kwargs['disk_over_commit'] = parsed_args.disk_overcommit
2610+
elif parsed_args.disk_overcommit is not None:
2611+
# TODO(stephenfin): Raise an error here in OSC 7.0
2612+
msg = _(
2613+
'The --disk-overcommit and --no-disk-overcommit '
2614+
'options are only supported by '
2615+
'--os-compute-api-version 2.24 or below; this will '
2616+
'be an error in a future release'
2617+
)
2618+
self.log.warning(msg)
26062619

26072620
server.live_migrate(**kwargs)
26082621
else:

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4832,6 +4832,40 @@ def test_server_live_migrate_with_disk_overcommit(self):
48324832
self.assertNotCalled(self.servers_mock.migrate)
48334833
self.assertIsNone(result)
48344834

4835+
def test_server_live_migrate_with_disk_overcommit_post_v224(self):
4836+
arglist = [
4837+
'--live-migration',
4838+
'--disk-overcommit',
4839+
self.server.id,
4840+
]
4841+
verifylist = [
4842+
('live', None),
4843+
('live_migration', True),
4844+
('block_migration', None),
4845+
('disk_overcommit', True),
4846+
('wait', False),
4847+
]
4848+
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
4849+
4850+
self.app.client_manager.compute.api_version = \
4851+
api_versions.APIVersion('2.25')
4852+
4853+
with mock.patch.object(self.cmd.log, 'warning') as mock_warning:
4854+
result = self.cmd.take_action(parsed_args)
4855+
4856+
self.servers_mock.get.assert_called_with(self.server.id)
4857+
# There should be no 'disk_over_commit' value present
4858+
self.server.live_migrate.assert_called_with(
4859+
block_migration='auto',
4860+
host=None)
4861+
self.assertNotCalled(self.servers_mock.migrate)
4862+
self.assertIsNone(result)
4863+
# A warning should have been logged for using --disk-overcommit.
4864+
mock_warning.assert_called_once()
4865+
self.assertIn(
4866+
'The --disk-overcommit and --no-disk-overcommit options ',
4867+
str(mock_warning.call_args[0][0]))
4868+
48354869
def test_server_live_migrate_with_false_value_options(self):
48364870
arglist = [
48374871
'--live', 'fakehost', '--no-disk-overcommit',
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
upgrade:
3+
- |
4+
A warning will now be emitted when using the ``--disk-overcommit``
5+
or ``--no-disk-overcommit`` flags of the ``server migrate`` command on
6+
Compute API microversion 2.25 or greater. This feature is only supported
7+
before this microversion and previously the flag was silently ignored on
8+
newer microversions. This warning will become an error in a future
9+
release.

0 commit comments

Comments
 (0)