Merge "Refactor security group set to use SDK"

openstackclient/compute/v2/security_group.py

@@ -217,47 +217,6 @@ class ListSecurityGroupRule(command.Lister):
                 ) for s in rules))
 
 
-class SetSecurityGroup(command.Command):
-    """Set security group properties"""
-
-    def get_parser(self, prog_name):
-        parser = super(SetSecurityGroup, self).get_parser(prog_name)
-        parser.add_argument(
-            'group',
-            metavar='<group>',
-            help='Security group to modify (name or ID)',
-        )
-        parser.add_argument(
-            '--name',
-            metavar='<new-name>',
-            help='New security group name',
-        )
-        parser.add_argument(
-            "--description",
-            metavar="<description>",
-            help="New security group description",
-        )
-        return parser
-
-    def take_action(self, parsed_args):
-        compute_client = self.app.client_manager.compute
-        data = utils.find_resource(
-            compute_client.security_groups,
-            parsed_args.group,
-        )
-
-        if parsed_args.name:
-            data.name = parsed_args.name
-        if parsed_args.description:
-            data.description = parsed_args.description
-
-        compute_client.security_groups.update(
-            data,
-            data.name,
-            data.description,
-        )
-
-
 class ShowSecurityGroup(command.ShowOne):
     """Display security group details"""
 

openstackclient/network/v2/security_group.py

@@ -88,3 +88,59 @@ class ListSecurityGroup(common.NetworkAndComputeLister):
         data = client.security_groups.list(search_opts=search)
         return self._get_return_data(data,
                                      include_project=parsed_args.all_projects)
+
+
+class SetSecurityGroup(common.NetworkAndComputeCommand):
+    """Set security group properties"""
+
+    def update_parser_common(self, parser):
+        parser.add_argument(
+            'group',
+            metavar='<group>',
+            help='Security group to modify (name or ID)',
+        )
+        parser.add_argument(
+            '--name',
+            metavar='<new-name>',
+            help='New security group name',
+        )
+        parser.add_argument(
+            "--description",
+            metavar="<description>",
+            help="New security group description",
+        )
+        return parser
+
+    def take_action_network(self, client, parsed_args):
+        obj = client.find_security_group(parsed_args.group,
+                                         ignore_missing=False)
+        attrs = {}
+        if parsed_args.name is not None:
+            attrs['name'] = parsed_args.name
+        if parsed_args.description is not None:
+            attrs['description'] = parsed_args.description
+        # NOTE(rtheis): Previous behavior did not raise a CommandError
+        # if there were no updates. Maintain this behavior and issue
+        # the update.
+        client.update_security_group(obj, **attrs)
+        return
+
+    def take_action_compute(self, client, parsed_args):
+        data = utils.find_resource(
+            client.security_groups,
+            parsed_args.group,
+        )
+
+        if parsed_args.name is not None:
+            data.name = parsed_args.name
+        if parsed_args.description is not None:
+            data.description = parsed_args.description
+
+        # NOTE(rtheis): Previous behavior did not raise a CommandError
+        # if there were no updates. Maintain this behavior and issue
+        # the update.
+        client.security_groups.update(
+            data,
+            data.name,
+            data.description,
+        )

openstackclient/tests/network/v2/test_security_group.py

@@ -16,6 +16,7 @@ import mock
 from openstackclient.network.v2 import security_group
 from openstackclient.tests.compute.v2 import fakes as compute_fakes
 from openstackclient.tests.network.v2 import fakes as network_fakes
+from openstackclient.tests import utils as tests_utils
 
 
 class TestSecurityGroupNetwork(network_fakes.TestNetworkV2):
@@ -230,3 +231,135 @@ class TestListSecurityGroupCompute(TestSecurityGroupCompute):
         self.compute.security_groups.list.assert_called_with(**kwargs)
         self.assertEqual(self.expected_columns_all_projects, columns)
         self.assertEqual(self.expected_data_all_projects, tuple(data))
+
+
+class TestSetSecurityGroupNetwork(TestSecurityGroupNetwork):
+
+    # The security group to be set.
+    _security_group = \
+        network_fakes.FakeSecurityGroup.create_one_security_group()
+
+    def setUp(self):
+        super(TestSetSecurityGroupNetwork, self).setUp()
+
+        self.network.update_security_group = mock.Mock(return_value=None)
+
+        self.network.find_security_group = mock.Mock(
+            return_value=self._security_group)
+
+        # Get the command object to test
+        self.cmd = security_group.SetSecurityGroup(self.app, self.namespace)
+
+    def test_set_no_options(self):
+        self.assertRaises(tests_utils.ParserException,
+                          self.check_parser, self.cmd, [], [])
+
+    def test_set_no_updates(self):
+        arglist = [
+            self._security_group.name,
+        ]
+        verifylist = [
+            ('group', self._security_group.name),
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        result = self.cmd.take_action(parsed_args)
+
+        self.network.update_security_group.assert_called_once_with(
+            self._security_group,
+            **{}
+        )
+        self.assertIsNone(result)
+
+    def test_set_all_options(self):
+        new_name = 'new-' + self._security_group.name
+        new_description = 'new-' + self._security_group.description
+        arglist = [
+            '--name', new_name,
+            '--description', new_description,
+            self._security_group.name,
+        ]
+        verifylist = [
+            ('description', new_description),
+            ('group', self._security_group.name),
+            ('name', new_name),
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        result = self.cmd.take_action(parsed_args)
+
+        attrs = {
+            'description': new_description,
+            'name': new_name,
+        }
+        self.network.update_security_group.assert_called_once_with(
+            self._security_group,
+            **attrs
+        )
+        self.assertIsNone(result)
+
+
+class TestSetSecurityGroupCompute(TestSecurityGroupCompute):
+
+    # The security group to be set.
+    _security_group = \
+        compute_fakes.FakeSecurityGroup.create_one_security_group()
+
+    def setUp(self):
+        super(TestSetSecurityGroupCompute, self).setUp()
+
+        self.app.client_manager.network_endpoint_enabled = False
+
+        self.compute.security_groups.update = mock.Mock(return_value=None)
+
+        self.compute.security_groups.get = mock.Mock(
+            return_value=self._security_group)
+
+        # Get the command object to test
+        self.cmd = security_group.SetSecurityGroup(self.app, None)
+
+    def test_set_no_options(self):
+        self.assertRaises(tests_utils.ParserException,
+                          self.check_parser, self.cmd, [], [])
+
+    def test_set_no_updates(self):
+        arglist = [
+            self._security_group.name,
+        ]
+        verifylist = [
+            ('group', self._security_group.name),
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        result = self.cmd.take_action(parsed_args)
+
+        self.compute.security_groups.update.assert_called_once_with(
+            self._security_group,
+            self._security_group.name,
+            self._security_group.description
+        )
+        self.assertIsNone(result)
+
+    def test_set_all_options(self):
+        new_name = 'new-' + self._security_group.name
+        new_description = 'new-' + self._security_group.description
+        arglist = [
+            '--name', new_name,
+            '--description', new_description,
+            self._security_group.name,
+        ]
+        verifylist = [
+            ('description', new_description),
+            ('group', self._security_group.name),
+            ('name', new_name),
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        result = self.cmd.take_action(parsed_args)
+
+        self.compute.security_groups.update.assert_called_once_with(
+            self._security_group,
+            new_name,
+            new_description
+        )
+        self.assertIsNone(result)

setup.cfg

@@ -100,7 +100,6 @@ openstack.compute.v2 =
     keypair_show = openstackclient.compute.v2.keypair:ShowKeypair
 
     security_group_create = openstackclient.compute.v2.security_group:CreateSecurityGroup
-    security_group_set = openstackclient.compute.v2.security_group:SetSecurityGroup
     security_group_show = openstackclient.compute.v2.security_group:ShowSecurityGroup
     security_group_rule_create = openstackclient.compute.v2.security_group:CreateSecurityGroupRule
     security_group_rule_list = openstackclient.compute.v2.security_group:ListSecurityGroupRule
@@ -342,6 +341,7 @@ openstack.network.v2 =
     router_show = openstackclient.network.v2.router:ShowRouter
     security_group_delete = openstackclient.network.v2.security_group:DeleteSecurityGroup
     security_group_list = openstackclient.network.v2.security_group:ListSecurityGroup
+    security_group_set = openstackclient.network.v2.security_group:SetSecurityGroup
     security_group_rule_delete = openstackclient.network.v2.security_group_rule:DeleteSecurityGroupRule
     security_group_rule_show = openstackclient.network.v2.security_group_rule:ShowSecurityGroupRule
     subnet_delete = openstackclient.network.v2.subnet:DeleteSubnet