From bac9fb18c1455f6a309e7acff9230a8d6bf7079b Mon Sep 17 00:00:00 2001
From: Richard Theis <rtheis@us.ibm.com>
Date: Wed, 2 Mar 2016 15:45:01 -0600
Subject: [PATCH] Refactor security group set to use SDK
Refactored the 'os security group set' command to use the SDK
when neutron is enabled, but continue to use the nova client
when nova network is enabled.
This patch set also fixes a compute bug which ignores name
and description when set to an empty value.
Change-Id: I4225179dca4aedf799e1656ec49236bdedc5e9bd
Partial-Bug: #1519511
Implements: blueprint neutron-client
---
openstackclient/compute/v2/security_group.py | 41 ------
openstackclient/network/v2/security_group.py | 56 ++++++++
.../tests/network/v2/test_security_group.py | 133 ++++++++++++++++++
setup.cfg | 2 +-
4 files changed, 190 insertions(+), 42 deletions(-)
diff --git a/openstackclient/compute/v2/security_group.py b/openstackclient/compute/v2/security_group.py
index 907175f7d0..f378af14cb 100644
--- a/openstackclient/compute/v2/security_group.py
+++ b/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"""
diff --git a/openstackclient/network/v2/security_group.py b/openstackclient/network/v2/security_group.py
index 29d82b08d3..9cefb42066 100644
--- a/openstackclient/network/v2/security_group.py
+++ b/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,
+ )
diff --git a/openstackclient/tests/network/v2/test_security_group.py b/openstackclient/tests/network/v2/test_security_group.py
index ea96442584..b8114cbcf0 100644
--- a/openstackclient/tests/network/v2/test_security_group.py
+++ b/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)
diff --git a/setup.cfg b/setup.cfg
index 284e6dec12..8cf1dd082a 100644
--- a/setup.cfg
+++ b/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
@@ -340,6 +339,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