openstack/python-neutronclient
Code Issues Proposed changes

SFC plugin: preserve chain order in set/unset commands

Browse Source 
Port pair groups order must be kept so we cannot use a set to filter
duplicates. Else this will change the chain order

Also fixed for flow classifiers: though loosing the order does not
currently trigger a visible change, this restores the neutron CLI
behaviour

Expand help messages to mention the port pair groups order

Change-Id: Ia6e342fda2c739a3553a8736837bcacf00a2d030
Closes-Bug: #1724187
This commit is contained in:
Bernard Cafarelli
2017-10-17 15:04:40 +02:00
parent 6feb30daad
commit 42c204f7cb
2 changed files with 42 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files

61
neutronclient/osc/v2/sfc/sfc_port_chain.py
View File

@@ -75,7 +75,7 @@ class CreateSfcPortChain(command.ShowOne):
dest='port_pair_groups', dest='port_pair_groups',
required=True, required=True,
action='append', action='append',
help=_('Port pair group (name or ID). ' help=_('Add port pair group (name or ID). '
'This option can be repeated.')) 'This option can be repeated.'))
return parser return parser
@@ -160,18 +160,20 @@ class SetSfcPortChain(command.Command):
parser.add_argument( parser.add_argument(
'--no-flow-classifier', '--no-flow-classifier',
action='store_true', action='store_true',
help=_('Associate no flow classifier with the port chain')) help=_('Remove associated flow classifiers from the port chain'))
parser.add_argument( parser.add_argument(
'--port-pair-group', '--port-pair-group',
metavar='<port-pair-group>', metavar='<port-pair-group>',
dest='port_pair_groups', dest='port_pair_groups',
action='append', action='append',
help=_('Add port pair group (name or ID). ' help=_('Add port pair group (name or ID). '
'Current port pair groups order is kept, the added port '
'pair group will be placed at the end of the port chain. '
'This option can be repeated.')) 'This option can be repeated.'))
parser.add_argument( parser.add_argument(
'--no-port-pair-group', '--no-port-pair-group',
action='store_true', action='store_true',
help=_('Remove associated port pair group from the port chain.' help=_('Remove associated port pair groups from the port chain. '
'At least one --port-pair-group must be specified ' 'At least one --port-pair-group must be specified '
'together.')) 'together.'))
parser.add_argument( parser.add_argument(
@@ -195,10 +197,12 @@ class SetSfcPortChain(command.Command):
resource, parsed_args.port_chain, resource, parsed_args.port_chain,
cmd_resource='sfc_port_chain')['flow_classifiers'] cmd_resource='sfc_port_chain')['flow_classifiers']
for fc in parsed_args.flow_classifiers: for fc in parsed_args.flow_classifiers:
fc_list.append(client.find_resource( fc_id = client.find_resource(
'flow_classifier', fc, 'flow_classifier', fc,
cmd_resource='sfc_flow_classifier')['id']) cmd_resource='sfc_flow_classifier')['id']
attrs['flow_classifiers'] = sorted(list(set(fc_list))) if fc_id not in fc_list:
fc_list.append(fc_id)
attrs['flow_classifiers'] = fc_list
if (parsed_args.no_port_pair_group and not if (parsed_args.no_port_pair_group and not
parsed_args.port_pair_groups): parsed_args.port_pair_groups):
message = _('At least one --port-pair-group must be specified.') message = _('At least one --port-pair-group must be specified.')
@@ -206,20 +210,24 @@ class SetSfcPortChain(command.Command):
if parsed_args.no_port_pair_group and parsed_args.port_pair_groups: if parsed_args.no_port_pair_group and parsed_args.port_pair_groups:
ppg_list = [] ppg_list = []
for ppg in parsed_args.port_pair_groups: for ppg in parsed_args.port_pair_groups:
ppg_list.append(client.find_resource( ppg_id = client.find_resource(
'port_pair_group', ppg, 'port_pair_group', ppg,
cmd_resource='sfc_port_pair_group')['id']) cmd_resource='sfc_port_pair_group')['id']
attrs['port_pair_groups'] = sorted(list(set(ppg_list))) if ppg_id not in ppg_list:
ppg_list.append(ppg_id)
attrs['port_pair_groups'] = ppg_list
if (parsed_args.port_pair_groups and if (parsed_args.port_pair_groups and
not parsed_args.no_port_pair_group): not parsed_args.no_port_pair_group):
ppg_list = client.find_resource( ppg_list = client.find_resource(
resource, parsed_args.port_chain, resource, parsed_args.port_chain,
cmd_resource='sfc_port_chain')['port_pair_groups'] cmd_resource='sfc_port_chain')['port_pair_groups']
for ppg in parsed_args.port_pair_groups: for ppg in parsed_args.port_pair_groups:
ppg_list.append(client.find_resource( ppg_id = client.find_resource(
'port_pair_group', ppg, 'port_pair_group', ppg,
cmd_resource='sfc_port_pair_group')['id']) cmd_resource='sfc_port_pair_group')['id']
attrs['port_pair_groups'] = sorted(list(set(ppg_list))) if ppg_id not in ppg_list:
ppg_list.append(ppg_id)
attrs['port_pair_groups'] = ppg_list
body = {resource: attrs} body = {resource: attrs}
try: try:
client.update_sfc_port_chain(pc_id, body) client.update_sfc_port_chain(pc_id, body)
@@ -285,30 +293,33 @@ class UnsetSfcPortChain(command.Command):
pc_id = _get_id(client, parsed_args.port_chain, resource) pc_id = _get_id(client, parsed_args.port_chain, resource)
attrs = {} attrs = {}
if parsed_args.flow_classifiers: if parsed_args.flow_classifiers:
existing = [client.find_resource( fc_list = client.find_resource(
resource, parsed_args.port_chain, resource, parsed_args.port_chain,
cmd_resource='sfc_port_chain')['flow_classifiers']] cmd_resource='sfc_port_chain')['flow_classifiers']
for fc in parsed_args.flow_classifiers: for fc in parsed_args.flow_classifiers:
removed = [client.find_resource( fc_id = client.find_resource(
'flow_classifier', fc, 'flow_classifier', fc,
cmd_resource='sfc_flow_classifier')['id']] cmd_resource='sfc_flow_classifier')['id']
attrs['flow_classifiers'] = list(set(existing) - set(removed)) if fc_id in fc_list:
fc_list.remove(fc_id)
attrs['flow_classifiers'] = fc_list
if parsed_args.all_flow_classifier: if parsed_args.all_flow_classifier:
attrs['flow_classifiers'] = [] attrs['flow_classifiers'] = []
if parsed_args.port_pair_groups: if parsed_args.port_pair_groups:
existing_ppg = [client.find_resource( ppg_list = client.find_resource(
resource, parsed_args.port_chain, resource, parsed_args.port_chain,
cmd_resource='sfc_port_chain')['port_pair_groups']] cmd_resource='sfc_port_chain')['port_pair_groups']
for ppg in parsed_args.port_pair_groups: for ppg in parsed_args.port_pair_groups:
removed_ppg = [client.find_resource( ppg_id = client.find_resource(
'port_pair_group', ppg, 'port_pair_group', ppg,
cmd_resource='sfc_port_pair_group')['id']] cmd_resource='sfc_port_pair_group')['id']
attrs['port_pair_groups'] = list(set(existing_ppg) - if ppg_id in ppg_list:
set(removed_ppg)) ppg_list.remove(ppg_id)
if attrs['port_pair_groups'] == []: if ppg_list == []:
message = _('At least one --port-pair-group must be' message = _('At least one port pair group must be'
' specified.') ' specified.')
raise exceptions.CommandError(message) raise exceptions.CommandError(message)
attrs['port_pair_groups'] = ppg_list
body = {resource: attrs} body = {resource: attrs}
try: try:
client.update_sfc_port_chain(pc_id, body) client.update_sfc_port_chain(pc_id, body)

12
neutronclient/tests/unit/osc/v2/sfc/test_port_chain.py
View File

@@ -297,7 +297,7 @@ class TestSetSfcPortChain(fakes.TestNeutronClientOSCV2):
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
result = self.cmd.take_action(parsed_args) result = self.cmd.take_action(parsed_args)
expect = {'flow_classifiers': sorted([self.pc_fc, fc1, fc2])} expect = {'flow_classifiers': [self.pc_fc, fc1, fc2]}
self.mocked.assert_called_once_with(target, {self.res: expect}) self.mocked.assert_called_once_with(target, {self.res: expect})
self.assertEqual(3, self.neutronclient.find_resource.call_count) self.assertEqual(3, self.neutronclient.find_resource.call_count)
self.assertIsNone(result) self.assertIsNone(result)
@@ -356,7 +356,7 @@ class TestSetSfcPortChain(fakes.TestNeutronClientOSCV2):
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
result = self.cmd.take_action(parsed_args) result = self.cmd.take_action(parsed_args)
expect = {'port_pair_groups': sorted([existing_ppg, ppg1, ppg2])} expect = {'port_pair_groups': [existing_ppg, ppg1, ppg2]}
self.mocked.assert_called_once_with(target, {self.res: expect}) self.mocked.assert_called_once_with(target, {self.res: expect})
self.assertEqual(3, self.neutronclient.find_resource.call_count) self.assertEqual(3, self.neutronclient.find_resource.call_count)
self.assertIsNone(result) self.assertIsNone(result)
@@ -484,7 +484,7 @@ class TestUnsetSfcPortChain(fakes.TestNeutronClientOSCV2):
if self.neutronclient.find_resource.call_count == 1: if self.neutronclient.find_resource.call_count == 1:
self.neutronclient.find_resource.assert_called_with( self.neutronclient.find_resource.assert_called_with(
self.res, target, cmd_resource='sfc_port_chain') self.res, target, cmd_resource='sfc_port_chain')
return {'port_pair_groups': self.pc_ppg} return {'port_pair_groups': [self.pc_ppg]}
if self.neutronclient.find_resource.call_count == 2: if self.neutronclient.find_resource.call_count == 2:
self.neutronclient.find_resource.assert_called_with( self.neutronclient.find_resource.assert_called_with(
@@ -507,7 +507,7 @@ class TestUnsetSfcPortChain(fakes.TestNeutronClientOSCV2):
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
result = self.cmd.take_action(parsed_args) result = self.cmd.take_action(parsed_args)
expect = {'port_pair_groups': sorted([self.pc_ppg])} expect = {'port_pair_groups': [self.pc_ppg]}
self.mocked.assert_called_once_with(target, {self.res: expect}) self.mocked.assert_called_once_with(target, {self.res: expect})
self.assertIsNone(result) self.assertIsNone(result)
@@ -519,7 +519,7 @@ class TestUnsetSfcPortChain(fakes.TestNeutronClientOSCV2):
if self.neutronclient.find_resource.call_count == 1: if self.neutronclient.find_resource.call_count == 1:
self.neutronclient.find_resource.assert_called_with( self.neutronclient.find_resource.assert_called_with(
self.res, target, cmd_resource='sfc_port_chain') self.res, target, cmd_resource='sfc_port_chain')
return {'flow_classifiers': self.pc_fc} return {'flow_classifiers': [self.pc_fc]}
if self.neutronclient.find_resource.call_count == 2: if self.neutronclient.find_resource.call_count == 2:
self.neutronclient.find_resource.assert_called_with( self.neutronclient.find_resource.assert_called_with(
@@ -537,7 +537,7 @@ class TestUnsetSfcPortChain(fakes.TestNeutronClientOSCV2):
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
result = self.cmd.take_action(parsed_args) result = self.cmd.take_action(parsed_args)
expect = {'flow_classifiers': sorted([self.pc_fc])} expect = {'flow_classifiers': [self.pc_fc]}
self.mocked.assert_called_once_with(target, {self.res: expect}) self.mocked.assert_called_once_with(target, {self.res: expect})
self.assertIsNone(result) self.assertIsNone(result)