openstack/python-openstackclient
Code Issues Proposed changes

Add a validation about options for server migrate command

Browse Source 
The behavior of server migrate command are different depending on
whether user specify --live option or not.
server migrate command will call live migration API if user specify
--live option.
Ohterwise server migrate command will call migration(cold migration)
API.

Now then, "--block-migraiton" option and "--disk-overcommit" option
only affect live-migration.
But, openstackclient doesn't warn user if user specify these options
without "--live".
But, user can't recognize that specifying options are ignored.

This patch adds a validation that checks whether or not user specify
these options without "--live".

Change-Id: Ifa278abb23ecdba4b13f3742998359ac74eb7ad4
Closes-bug: #1662755
This commit is contained in:
Rikimaru Honjo 2017-02-23 16:59:26 +09:00
parent c8435f2d5a
commit 7063ffb3ca
2 changed files with 207 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -1104,6 +1104,10 @@ class MigrateServer(command.Command):
disk_over_commit=parsed_args.disk_overcommit,
)
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")
server.migrate()
if parsed_args.wait:

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

@ -1185,6 +1185,209 @@ class TestServerLock(TestServer):
self.run_method_with_servers('lock', 3)
class TestServerMigrate(TestServer):
def setUp(self):
super(TestServerMigrate, self).setUp()
methods = {
'migrate': None,
'live_migrate': None,
}
self.server = compute_fakes.FakeServer.create_one_server(
methods=methods)
# This is the return value for utils.find_resource()
self.servers_mock.get.return_value = self.server
self.servers_mock.migrate.return_value = None
self.servers_mock.live_migrate.return_value = None
# Get the command object to test
self.cmd = server.MigrateServer(self.app, None)
def test_server_migrate_no_options(self):
arglist = [
self.server.id,
]
verifylist = [
('live', None),
('block_migration', False),
('disk_overcommit', False),
('wait', False),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
result = self.cmd.take_action(parsed_args)
self.servers_mock.get.assert_called_with(self.server.id)
self.server.migrate.assert_called_with()
self.assertNotCalled(self.servers_mock.live_migrate)
self.assertIsNone(result)
def test_server_migrate_with_block_migration(self):
arglist = [
'--block-migration', self.server.id,
]
verifylist = [
('live', None),
('block_migration', True),
('disk_overcommit', False),
('wait', False),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
self.assertRaises(exceptions.CommandError, self.cmd.take_action,
parsed_args)
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_migrate_with_disk_overcommit(self):
arglist = [
'--disk-overcommit', self.server.id,
]
verifylist = [
('live', None),
('block_migration', False),
('disk_overcommit', True),
('wait', False),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
self.assertRaises(exceptions.CommandError, self.cmd.take_action,
parsed_args)
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'),
('block_migration', False),
('disk_overcommit', False),
('wait', False),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
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_block_live_migrate(self):
arglist = [
'--live', 'fakehost', '--block-migration', self.server.id,
]
verifylist = [
('live', 'fakehost'),
('block_migration', True),
('disk_overcommit', False),
('wait', False),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
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.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,
]
verifylist = [
('live', 'fakehost'),
('block_migration', False),
('disk_overcommit', True),
('wait', False),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
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.assertNotCalled(self.servers_mock.migrate)
self.assertIsNone(result)
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)
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)
@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),
('block_migration', False),
('disk_overcommit', False),
('wait', True),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
result = self.cmd.take_action(parsed_args)
self.servers_mock.get.assert_called_with(self.server.id)
self.server.migrate.assert_called_with()
self.assertNotCalled(self.servers_mock.live_migrate)
self.assertIsNone(result)
@mock.patch.object(common_utils, 'wait_for_status', return_value=False)
def test_server_migrate_with_wait_fails(self, mock_wait_for_status):
arglist = [
'--wait', self.server.id,
]
verifylist = [
('live', None),
('block_migration', False),
('disk_overcommit', False),
('wait', True),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
self.assertRaises(SystemExit, self.cmd.take_action, parsed_args)
self.servers_mock.get.assert_called_with(self.server.id)
self.server.migrate.assert_called_with()
self.assertNotCalled(self.servers_mock.live_migrate)
class TestServerPause(TestServer):
def setUp(self):