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
This commit is contained in:
Richard Theis
parent
f9add0559c
commit
bac9fb18c1
openstackclient
setup.cfg@ -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"""
|
||||
|
||||
|
||||
@ -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,
|
||||
)
|
||||
|
||||
@ -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)
|
||||
|
||||
@ -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
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user