Merge "Create server with security group ID and name"

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 <security-group-name>
+.. option:: --security-group <security-group>
 
     Security group to assign to this server (name or ID)
     (repeat option to set multiple groups)

openstackclient/compute/v2/server.py

@@ -407,7 +407,7 @@ class CreateServer(command.ShowOne):
         )
         parser.add_argument(
             '--security-group',
-            metavar='<security-group-name>',
+            metavar='<security-group>',
             action='append',
             default=[],
             help=_('Security group to assign to this server (name or ID) '
@@ -695,6 +695,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('=')
@@ -724,7 +740,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,

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

@@ -517,6 +517,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

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

@@ -25,6 +25,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
 
@@ -421,8 +422,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'},
@@ -430,7 +439,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,
@@ -450,6 +459,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',

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 <https://bugs.launchpad.net/python-openstackclient/+bug/1687814>`_]