From 45496feee6ae781dc0c1df4a0e5bd71b05653748 Mon Sep 17 00:00:00 2001 From: Rui Chen Date: Wed, 3 May 2017 15:17:10 +0800 Subject: [PATCH] Create server with security group ID and name Both resource ID and name are supported to identify an object in openstackclient to make user easy to input, for security group, nova only support security group name in API when launch a new server, this patch convert ID to name, then pass name to nova API, and check the security group exist before creating server. Change-Id: I1ed4a967fb9de3f91c8945a1ef63f6c7b6b2dfb2 Closes-Bug: #1687814 --- doc/source/command-objects/server.rst | 2 +- openstackclient/compute/v2/server.py | 20 +++- .../functional/compute/v2/test_server.py | 46 +++++++++ .../tests/unit/compute/v2/test_server.py | 97 ++++++++++++++++++- .../notes/bug-1687814-743ad8418923d5e3.yaml | 7 ++ 5 files changed, 168 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/bug-1687814-743ad8418923d5e3.yaml diff --git a/doc/source/command-objects/server.rst b/doc/source/command-objects/server.rst index dc78080b6d..0287fd3104 100644 --- a/doc/source/command-objects/server.rst +++ b/doc/source/command-objects/server.rst @@ -164,7 +164,7 @@ Create a new server Create server with this flavor (name or ID) -.. option:: --security-group +.. option:: --security-group Security group to assign to this server (name or ID) (repeat option to set multiple groups) diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index 60dc605caf..7ed7c7f6c2 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -407,7 +407,7 @@ class CreateServer(command.ShowOne): ) parser.add_argument( '--security-group', - metavar='', + metavar='', action='append', default=[], help=_('Security group to assign to this server (name or ID) ' @@ -677,6 +677,22 @@ class CreateServer(command.ShowOne): # decide the default behavior. nics = [] + # Check security group exist and convert ID to name + security_group_names = [] + if self.app.client_manager.is_network_endpoint_enabled(): + network_client = self.app.client_manager.network + for each_sg in parsed_args.security_group: + sg = network_client.find_security_group(each_sg, + ignore_missing=False) + # Use security group ID to avoid multiple security group have + # same name in neutron networking backend + security_group_names.append(sg.id) + else: + # Handle nova-network case + for each_sg in parsed_args.security_group: + sg = compute_client.api.security_group_find(each_sg) + security_group_names.append(sg['name']) + hints = {} for hint in parsed_args.hint: key, _sep, value = hint.partition('=') @@ -706,7 +722,7 @@ class CreateServer(command.ShowOne): reservation_id=None, min_count=parsed_args.min, max_count=parsed_args.max, - security_groups=parsed_args.security_group, + security_groups=security_group_names, userdata=userdata, key_name=parsed_args.key_name, availability_zone=parsed_args.availability_zone, diff --git a/openstackclient/tests/functional/compute/v2/test_server.py b/openstackclient/tests/functional/compute/v2/test_server.py index 7d54b6a6a5..f4ffe9612e 100644 --- a/openstackclient/tests/functional/compute/v2/test_server.py +++ b/openstackclient/tests/functional/compute/v2/test_server.py @@ -410,6 +410,52 @@ class ServerTests(common.ComputeTestCase): self.assertIsNotNone(server['addresses']) self.assertEqual('', server['addresses']) + def test_server_create_with_security_group(self): + """Test server create with security group ID and name""" + if not self.haz_network: + # NOTE(dtroyer): As of Ocata release Nova forces nova-network to + # run in a cells v1 configuration. Security group + # and network functions currently do not work in + # the gate jobs so we have to skip this. It is + # known to work tested against a Mitaka nova-net + # DevStack without cells. + self.skipTest("No Network service present") + # Create two security group, use name and ID to create server + sg_name1 = uuid.uuid4().hex + security_group1 = json.loads(self.openstack( + 'security group create -f json ' + sg_name1 + )) + self.addCleanup(self.openstack, 'security group delete ' + sg_name1) + sg_name2 = uuid.uuid4().hex + security_group2 = json.loads(self.openstack( + 'security group create -f json ' + sg_name2 + )) + self.addCleanup(self.openstack, 'security group delete ' + sg_name2) + + server_name = uuid.uuid4().hex + server = json.loads(self.openstack( + 'server create -f json ' + + '--flavor ' + self.flavor_name + ' ' + + '--image ' + self.image_name + ' ' + + # Security group id is integer in nova-network, convert to string + '--security-group ' + str(security_group1['id']) + ' ' + + '--security-group ' + security_group2['name'] + ' ' + + self.network_arg + ' ' + + server_name + )) + self.addCleanup(self.openstack, 'server delete --wait ' + server_name) + + self.assertIsNotNone(server['id']) + self.assertEqual(server_name, server['name']) + self.assertIn(str(security_group1['id']), server['security_groups']) + self.assertIn(str(security_group2['id']), server['security_groups']) + self.wait_for_status(server_name, 'ACTIVE') + server = json.loads(self.openstack( + 'server show -f json ' + server_name + )) + self.assertIn(sg_name1, server['security_groups']) + self.assertIn(sg_name2, server['security_groups']) + def test_server_create_with_empty_network_option_latest(self): """Test server create with empty network option in nova 2.latest.""" server_name = uuid.uuid4().hex diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index 9370bf6bd1..77e9c33106 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -24,6 +24,7 @@ from oslo_utils import timeutils from openstackclient.compute.v2 import server from openstackclient.tests.unit.compute.v2 import fakes as compute_fakes from openstackclient.tests.unit.image.v2 import fakes as image_fakes +from openstackclient.tests.unit.network.v2 import fakes as network_fakes from openstackclient.tests.unit import utils from openstackclient.tests.unit.volume.v2 import fakes as volume_fakes @@ -414,8 +415,16 @@ class TestServerCreate(TestServer): # In base command class ShowOne in cliff, abstract method take_action() # returns a two-part tuple with a tuple of column names and a tuple of # data to be shown. + fake_sg = network_fakes.FakeSecurityGroup.create_security_groups() + mock_find_sg = ( + network_fakes.FakeSecurityGroup.get_security_groups(fake_sg) + ) + self.app.client_manager.network.find_security_group = mock_find_sg + columns, data = self.cmd.take_action(parsed_args) + mock_find_sg.assert_called_once_with('securitygroup', + ignore_missing=False) # Set expected values kwargs = dict( meta={'Beta': 'b'}, @@ -423,7 +432,7 @@ class TestServerCreate(TestServer): reservation_id=None, min_count=1, max_count=1, - security_groups=['securitygroup'], + security_groups=[fake_sg[0].id], userdata=None, key_name='keyname', availability_zone=None, @@ -443,6 +452,92 @@ class TestServerCreate(TestServer): self.assertEqual(self.columns, columns) self.assertEqual(self.datalist(), data) + def test_server_create_with_not_exist_security_group(self): + arglist = [ + '--image', 'image1', + '--flavor', 'flavor1', + '--key-name', 'keyname', + '--security-group', 'securitygroup', + '--security-group', 'not_exist_sg', + self.new_server.name, + ] + verifylist = [ + ('image', 'image1'), + ('flavor', 'flavor1'), + ('key_name', 'keyname'), + ('security_group', ['securitygroup', 'not_exist_sg']), + ('server_name', self.new_server.name), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + fake_sg = network_fakes.FakeSecurityGroup.create_security_groups( + count=1) + fake_sg.append(exceptions.NotFound(code=404)) + mock_find_sg = ( + network_fakes.FakeSecurityGroup.get_security_groups(fake_sg) + ) + self.app.client_manager.network.find_security_group = mock_find_sg + + self.assertRaises(exceptions.NotFound, + self.cmd.take_action, + parsed_args) + mock_find_sg.assert_called_with('not_exist_sg', + ignore_missing=False) + + def test_server_create_with_security_group_in_nova_network(self): + arglist = [ + '--image', 'image1', + '--flavor', 'flavor1', + '--key-name', 'keyname', + '--security-group', 'securitygroup', + self.new_server.name, + ] + verifylist = [ + ('image', 'image1'), + ('flavor', 'flavor1'), + ('key_name', 'keyname'), + ('security_group', ['securitygroup']), + ('server_name', self.new_server.name), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + with mock.patch.object(self.app.client_manager, + 'is_network_endpoint_enabled', + return_value=False): + with mock.patch.object(self.app.client_manager.compute.api, + 'security_group_find', + return_value={'name': 'fake_sg'} + ) as mock_find: + columns, data = self.cmd.take_action(parsed_args) + mock_find.assert_called_once_with('securitygroup') + + # Set expected values + kwargs = dict( + meta=None, + files={}, + reservation_id=None, + min_count=1, + max_count=1, + security_groups=['fake_sg'], + userdata=None, + key_name='keyname', + availability_zone=None, + block_device_mapping_v2=[], + nics=[], + 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_network(self): arglist = [ '--image', 'image1', diff --git a/releasenotes/notes/bug-1687814-743ad8418923d5e3.yaml b/releasenotes/notes/bug-1687814-743ad8418923d5e3.yaml new file mode 100644 index 0000000000..bdd6e3f0a0 --- /dev/null +++ b/releasenotes/notes/bug-1687814-743ad8418923d5e3.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Allow security groups in ``server create`` command to be specified by name or ID with + the ``--security-group`` option. This also checks that the security group exist before + creating the server. + [Bug `1687814 `_]