From 42c204f7cbef8f28356c84a1fc3dfdc0575d50eb Mon Sep 17 00:00:00 2001 From: Bernard Cafarelli Date: Tue, 17 Oct 2017 15:04:40 +0200 Subject: [PATCH] SFC plugin: preserve chain order in set/unset commands 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 --- neutronclient/osc/v2/sfc/sfc_port_chain.py | 61 +++++++++++-------- .../tests/unit/osc/v2/sfc/test_port_chain.py | 12 ++-- 2 files changed, 42 insertions(+), 31 deletions(-) diff --git a/neutronclient/osc/v2/sfc/sfc_port_chain.py b/neutronclient/osc/v2/sfc/sfc_port_chain.py index e5e9f7743..1044c2bd3 100755 --- a/neutronclient/osc/v2/sfc/sfc_port_chain.py +++ b/neutronclient/osc/v2/sfc/sfc_port_chain.py @@ -75,7 +75,7 @@ class CreateSfcPortChain(command.ShowOne): dest='port_pair_groups', required=True, action='append', - help=_('Port pair group (name or ID). ' + help=_('Add port pair group (name or ID). ' 'This option can be repeated.')) return parser @@ -160,18 +160,20 @@ class SetSfcPortChain(command.Command): parser.add_argument( '--no-flow-classifier', action='store_true', - help=_('Associate no flow classifier with the port chain')) + help=_('Remove associated flow classifiers from the port chain')) parser.add_argument( '--port-pair-group', metavar='', dest='port_pair_groups', action='append', 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.')) parser.add_argument( '--no-port-pair-group', 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 ' 'together.')) parser.add_argument( @@ -195,10 +197,12 @@ class SetSfcPortChain(command.Command): resource, parsed_args.port_chain, cmd_resource='sfc_port_chain')['flow_classifiers'] for fc in parsed_args.flow_classifiers: - fc_list.append(client.find_resource( + fc_id = client.find_resource( 'flow_classifier', fc, - cmd_resource='sfc_flow_classifier')['id']) - attrs['flow_classifiers'] = sorted(list(set(fc_list))) + cmd_resource='sfc_flow_classifier')['id'] + 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 parsed_args.port_pair_groups): 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: ppg_list = [] for ppg in parsed_args.port_pair_groups: - ppg_list.append(client.find_resource( + ppg_id = client.find_resource( 'port_pair_group', ppg, - cmd_resource='sfc_port_pair_group')['id']) - attrs['port_pair_groups'] = sorted(list(set(ppg_list))) + cmd_resource='sfc_port_pair_group')['id'] + 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 not parsed_args.no_port_pair_group): ppg_list = client.find_resource( resource, parsed_args.port_chain, cmd_resource='sfc_port_chain')['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, - cmd_resource='sfc_port_pair_group')['id']) - attrs['port_pair_groups'] = sorted(list(set(ppg_list))) + cmd_resource='sfc_port_pair_group')['id'] + if ppg_id not in ppg_list: + ppg_list.append(ppg_id) + attrs['port_pair_groups'] = ppg_list body = {resource: attrs} try: 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) attrs = {} if parsed_args.flow_classifiers: - existing = [client.find_resource( + fc_list = client.find_resource( 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: - removed = [client.find_resource( + fc_id = client.find_resource( 'flow_classifier', fc, - cmd_resource='sfc_flow_classifier')['id']] - attrs['flow_classifiers'] = list(set(existing) - set(removed)) + cmd_resource='sfc_flow_classifier')['id'] + if fc_id in fc_list: + fc_list.remove(fc_id) + attrs['flow_classifiers'] = fc_list if parsed_args.all_flow_classifier: attrs['flow_classifiers'] = [] if parsed_args.port_pair_groups: - existing_ppg = [client.find_resource( + ppg_list = client.find_resource( 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: - removed_ppg = [client.find_resource( + ppg_id = client.find_resource( 'port_pair_group', ppg, - cmd_resource='sfc_port_pair_group')['id']] - attrs['port_pair_groups'] = list(set(existing_ppg) - - set(removed_ppg)) - if attrs['port_pair_groups'] == []: - message = _('At least one --port-pair-group must be' + cmd_resource='sfc_port_pair_group')['id'] + if ppg_id in ppg_list: + ppg_list.remove(ppg_id) + if ppg_list == []: + message = _('At least one port pair group must be' ' specified.') raise exceptions.CommandError(message) + attrs['port_pair_groups'] = ppg_list body = {resource: attrs} try: client.update_sfc_port_chain(pc_id, body) diff --git a/neutronclient/tests/unit/osc/v2/sfc/test_port_chain.py b/neutronclient/tests/unit/osc/v2/sfc/test_port_chain.py index 1782be927..fbf60303d 100755 --- a/neutronclient/tests/unit/osc/v2/sfc/test_port_chain.py +++ b/neutronclient/tests/unit/osc/v2/sfc/test_port_chain.py @@ -297,7 +297,7 @@ class TestSetSfcPortChain(fakes.TestNeutronClientOSCV2): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) 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.assertEqual(3, self.neutronclient.find_resource.call_count) self.assertIsNone(result) @@ -356,7 +356,7 @@ class TestSetSfcPortChain(fakes.TestNeutronClientOSCV2): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) 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.assertEqual(3, self.neutronclient.find_resource.call_count) self.assertIsNone(result) @@ -484,7 +484,7 @@ class TestUnsetSfcPortChain(fakes.TestNeutronClientOSCV2): if self.neutronclient.find_resource.call_count == 1: self.neutronclient.find_resource.assert_called_with( 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: self.neutronclient.find_resource.assert_called_with( @@ -507,7 +507,7 @@ class TestUnsetSfcPortChain(fakes.TestNeutronClientOSCV2): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) 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.assertIsNone(result) @@ -519,7 +519,7 @@ class TestUnsetSfcPortChain(fakes.TestNeutronClientOSCV2): if self.neutronclient.find_resource.call_count == 1: self.neutronclient.find_resource.assert_called_with( 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: self.neutronclient.find_resource.assert_called_with( @@ -537,7 +537,7 @@ class TestUnsetSfcPortChain(fakes.TestNeutronClientOSCV2): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) 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.assertIsNone(result)