From c051c5f090fa6729a005c9d462afd8a75fc1b40f Mon Sep 17 00:00:00 2001 From: Rui Chen Date: Mon, 13 Feb 2017 15:31:23 +0800 Subject: [PATCH] Fix "server create" command failed when --nic auto or none "auto" and "none" options was added into --nic argument of server create command in patch https://review.openstack.org/#/c/412698/ , but that don't work and raise internal error when execute command. The patch fix that issue and add unit and functional tests. Change-Id: Ia718c3bac0a5172a0cdbe9f0d97972a9346c1172 Co-Authored-By: Kevin_Zheng Closes-Bug: #1663520 --- doc/source/command-objects/server.rst | 2 + openstackclient/compute/v2/server.py | 36 ++++- .../functional/compute/v2/test_server.py | 53 ++++++- .../tests/unit/compute/v2/test_server.py | 146 ++++++++++++++++++ .../notes/bug-1663520-d880bfa51ca7b798.yaml | 8 + 5 files changed, 232 insertions(+), 13 deletions(-) create mode 100644 releasenotes/notes/bug-1663520-d880bfa51ca7b798.yaml diff --git a/doc/source/command-objects/server.rst b/doc/source/command-objects/server.rst index a5d22f81c2..b2ae965a7b 100644 --- a/doc/source/command-objects/server.rst +++ b/doc/source/command-objects/server.rst @@ -173,6 +173,8 @@ Create a new server v6-fixed-ip: IPv6 fixed address for NIC (optional). none: (v2.37+) no network is attached. auto: (v2.37+) the compute service will automatically allocate a network. + Specifying a --nic of auto or none cannot be used with any other + --nic value. .. option:: --hint diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index f9f0df4f2f..d33c631a17 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -402,7 +402,7 @@ class CreateServer(command.ShowOne): parser.add_argument( '--nic', metavar="", + "port-id=port-uuid,auto,none>", action='append', default=[], help=_("Create a NIC on the server. " @@ -414,7 +414,8 @@ class CreateServer(command.ShowOne): "v6-fixed-ip: IPv6 fixed address for NIC (optional), " "none: (v2.37+) no network is attached, " "auto: (v2.37+) the compute service will automatically " - "allocate a network."), + "allocate a network. Specifying a --nic of auto or none " + "cannot be used with any other --nic value."), ) parser.add_argument( '--hint', @@ -547,14 +548,21 @@ class CreateServer(command.ShowOne): block_device_mapping_v2.append(mapping) nics = [] - if parsed_args.nic in ('auto', 'none'): - nics = [parsed_args.nic] - else: - for nic_str in parsed_args.nic: + auto_or_none = False + for nic_str in parsed_args.nic: + # Handle the special auto/none cases + if nic_str in ('auto', 'none'): + auto_or_none = True + nics.append(nic_str) + else: nic_info = {"net-id": "", "v4-fixed-ip": "", "v6-fixed-ip": "", "port-id": ""} - nic_info.update(dict(kv_str.split("=", 1) - for kv_str in nic_str.split(","))) + try: + nic_info.update(dict(kv_str.split("=", 1) + for kv_str in nic_str.split(","))) + except ValueError: + msg = _('Invalid --nic argument %s.') % nic_str + raise exceptions.CommandError(msg) if bool(nic_info["net-id"]) == bool(nic_info["port-id"]): msg = _("either net-id or port-id should be specified " "but not both") @@ -581,6 +589,18 @@ class CreateServer(command.ShowOne): raise exceptions.CommandError(msg) nics.append(nic_info) + if nics: + if auto_or_none: + if len(nics) > 1: + msg = _('Specifying a --nic of auto or none cannot ' + 'be used with any other --nic value.') + raise exceptions.CommandError(msg) + nics = nics[0] + else: + # Default to empty list if nothing was specified, let nova side to + # decide the default behavior. + nics = [] + hints = {} for hint in parsed_args.hint: key, _sep, value = hint.partition('=') diff --git a/openstackclient/tests/functional/compute/v2/test_server.py b/openstackclient/tests/functional/compute/v2/test_server.py index 119ef05c5d..6ecb325564 100644 --- a/openstackclient/tests/functional/compute/v2/test_server.py +++ b/openstackclient/tests/functional/compute/v2/test_server.py @@ -14,10 +14,10 @@ import json import time from tempest.lib.common.utils import data_utils +from tempest.lib import exceptions from openstackclient.tests.functional import base from openstackclient.tests.functional.volume.v2 import test_volume -from tempest.lib import exceptions class ServerTests(base.TestCase): @@ -319,7 +319,7 @@ class ServerTests(base.TestCase): self.assertEqual("", raw_output) self.wait_for_status("ACTIVE") - def test_server_boot_from_volume(self): + def test_server_create_from_volume(self): """Test server create from volume, server delete Test steps: @@ -437,14 +437,57 @@ class ServerTests(base.TestCase): cmd_output['status'], ) - def wait_for_status(self, expected_status='ACTIVE', wait=900, interval=30): + def test_server_create_with_none_network(self): + """Test server create with none network option.""" + server_name = data_utils.rand_name('TestServer') + server = json.loads(self.openstack( + # auto/none enable in nova micro version (v2.37+) + '--os-compute-api-version 2.latest ' + + 'server create -f json ' + + '--flavor ' + self.flavor_name + ' ' + + '--image ' + self.image_name + ' ' + + '--nic none ' + + server_name + )) + self.assertIsNotNone(server["id"]) + self.addCleanup(self.openstack, 'server delete --wait ' + server_name) + self.assertEqual(server_name, server['name']) + self.wait_for_status(server_name=server_name) + server = json.loads(self.openstack( + 'server show -f json ' + server_name + )) + self.assertIsNotNone(server['addresses']) + self.assertEqual('', server['addresses']) + + def test_server_create_with_empty_network_option_latest(self): + """Test server create with empty network option in nova 2.latest.""" + server_name = data_utils.rand_name('TestServer') + try: + self.openstack( + # auto/none enable in nova micro version (v2.37+) + '--os-compute-api-version 2.latest ' + + 'server create -f json ' + + '--flavor ' + self.flavor_name + ' ' + + '--image ' + self.image_name + ' ' + + server_name + ) + except exceptions.CommandFailed as e: + self.assertIn('nics are required after microversion 2.36', + e.stderr) + else: + self.fail('CommandFailed should be raised.') + + def wait_for_status(self, expected_status='ACTIVE', + wait=900, interval=30, server_name=None): """Wait until server reaches expected status.""" # TODO(thowe): Add a server wait command to osc failures = ['ERROR'] total_sleep = 0 opts = self.get_opts(['status']) + if not server_name: + server_name = self.NAME while total_sleep < wait: - status = self.openstack('server show ' + self.NAME + opts) + status = self.openstack('server show ' + server_name + opts) status = status.rstrip() print('Waiting for {} current status: {}'.format(expected_status, status)) @@ -454,7 +497,7 @@ class ServerTests(base.TestCase): time.sleep(interval) total_sleep += interval - status = self.openstack('server show ' + self.NAME + opts) + status = self.openstack('server show ' + server_name + opts) status = status.rstrip() self.assertEqual(status, expected_status) # give it a little bit more time diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index 4cac990ec6..249902bca4 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -467,6 +467,152 @@ class TestServerCreate(TestServer): self.assertEqual(self.columns, columns) self.assertEqual(self.datalist(), data) + def test_server_create_with_auto_network(self): + arglist = [ + '--image', 'image1', + '--flavor', 'flavor1', + '--nic', 'auto', + self.new_server.name, + ] + verifylist = [ + ('image', 'image1'), + ('flavor', 'flavor1'), + ('nic', ['auto']), + ('config_drive', False), + ('server_name', self.new_server.name), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + columns, data = self.cmd.take_action(parsed_args) + + # Set expected values + kwargs = dict( + meta=None, + files={}, + reservation_id=None, + min_count=1, + max_count=1, + security_groups=[], + userdata=None, + key_name=None, + availability_zone=None, + block_device_mapping_v2=[], + nics='auto', + scheduler_hints={}, + config_drive=None, + ) + # ServerManager.create(name, image, flavor, **kwargs) + self.servers_mock.create.assert_called_with( + self.new_server.name, + self.image, + self.flavor, + **kwargs + ) + + self.assertEqual(self.columns, columns) + self.assertEqual(self.datalist(), data) + + def test_server_create_with_none_network(self): + arglist = [ + '--image', 'image1', + '--flavor', 'flavor1', + '--nic', 'none', + self.new_server.name, + ] + verifylist = [ + ('image', 'image1'), + ('flavor', 'flavor1'), + ('nic', ['none']), + ('config_drive', False), + ('server_name', self.new_server.name), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + columns, data = self.cmd.take_action(parsed_args) + + # Set expected values + kwargs = dict( + meta=None, + files={}, + reservation_id=None, + min_count=1, + max_count=1, + security_groups=[], + userdata=None, + key_name=None, + availability_zone=None, + block_device_mapping_v2=[], + nics='none', + scheduler_hints={}, + config_drive=None, + ) + # ServerManager.create(name, image, flavor, **kwargs) + self.servers_mock.create.assert_called_with( + self.new_server.name, + self.image, + self.flavor, + **kwargs + ) + + self.assertEqual(self.columns, columns) + self.assertEqual(self.datalist(), data) + + def test_server_create_with_conflict_network_options(self): + arglist = [ + '--image', 'image1', + '--flavor', 'flavor1', + '--nic', 'none', + '--nic', 'auto', + '--nic', 'port-id=port1', + self.new_server.name, + ] + verifylist = [ + ('image', 'image1'), + ('flavor', 'flavor1'), + ('nic', ['none', 'auto', 'port-id=port1']), + ('config_drive', False), + ('server_name', self.new_server.name), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + get_endpoints = mock.Mock() + get_endpoints.return_value = {'network': []} + self.app.client_manager.auth_ref = mock.Mock() + self.app.client_manager.auth_ref.service_catalog = mock.Mock() + self.app.client_manager.auth_ref.service_catalog.get_endpoints = ( + get_endpoints) + + find_port = mock.Mock() + network_client = self.app.client_manager.network + network_client.find_port = find_port + port_resource = mock.Mock() + port_resource.id = 'port1_uuid' + find_port.return_value = port_resource + + self.assertRaises(exceptions.CommandError, + self.cmd.take_action, parsed_args) + self.assertNotCalled(self.servers_mock.create) + + def test_server_create_with_invalid_network_options(self): + arglist = [ + '--image', 'image1', + '--flavor', 'flavor1', + '--nic', 'abcdefgh', + self.new_server.name, + ] + verifylist = [ + ('image', 'image1'), + ('flavor', 'flavor1'), + ('nic', ['abcdefgh']), + ('config_drive', False), + ('server_name', self.new_server.name), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + self.assertRaises(exceptions.CommandError, + self.cmd.take_action, parsed_args) + self.assertNotCalled(self.servers_mock.create) + @mock.patch.object(common_utils, 'wait_for_status', return_value=True) def test_server_create_with_wait_ok(self, mock_wait_for_status): arglist = [ diff --git a/releasenotes/notes/bug-1663520-d880bfa51ca7b798.yaml b/releasenotes/notes/bug-1663520-d880bfa51ca7b798.yaml new file mode 100644 index 0000000000..4c4d7e3b03 --- /dev/null +++ b/releasenotes/notes/bug-1663520-d880bfa51ca7b798.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + Fix ``server create`` command failed when ``--nic`` auto or none. + ``auto`` and ``none`` options was added into --nic argument of server + create command, but that don't work and raise internal error when execute + command. The patch fix that issue. + [Bug `1663520 `_]