diff --git a/doc/source/command-objects/server.rst b/doc/source/command-objects/server.rst index dc78080b6..0287fd310 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 60dc605ca..7ed7c7f6c 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 7d54b6a6a..f4ffe9612 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 9370bf6bd..77e9c3310 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 000000000..bdd6e3f0a --- /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 `_]