From 29652e0aff760bf445fd7ffef8eeede61a6710e3 Mon Sep 17 00:00:00 2001 From: Brian Haley Date: Fri, 24 Mar 2017 17:41:17 -0400 Subject: [PATCH] Verify metering label exists before applying rule If the metering-agent receives a label rule before it has added the label, it will fail to update the iptables rules as there are no existing chains. When the action is "create", check if there is an existing label, and create one and the corresponding iptables chains, before trying to add the rule. Closes-Bug: #1617248 Change-Id: Ia0ec1361188cca53023667d249c2b1e10bc22089 --- .../drivers/iptables/iptables_driver.py | 31 +++++++++---- .../metering/drivers/test_iptables.py | 44 +++++++++++++++++++ 2 files changed, 67 insertions(+), 8 deletions(-) diff --git a/neutron/services/metering/drivers/iptables/iptables_driver.py b/neutron/services/metering/drivers/iptables/iptables_driver.py index 3c4971b43fb..57c7cfab982 100644 --- a/neutron/services/metering/drivers/iptables/iptables_driver.py +++ b/neutron/services/metering/drivers/iptables/iptables_driver.py @@ -233,24 +233,22 @@ class IptablesMeteringDriver(abstract_driver.MeteringAbstractDriver): label_chain = iptables_manager.get_chain_name( WRAP_NAME + LABEL + label_id, wrap=False) - im.ipv4['filter'].add_chain(label_chain, wrap=False) rules_chain = iptables_manager.get_chain_name( WRAP_NAME + RULE + label_id, wrap=False) - im.ipv4['filter'].add_chain(rules_chain, wrap=False) - im.ipv4['filter'].add_rule( - TOP_CHAIN, '-j ' + rules_chain, wrap=False) - im.ipv4['filter'].add_rule( - label_chain, '', wrap=False) + exists = rm.metering_labels.get(label_id) + if not exists: + self._create_metering_label_chain(rm, + label_chain, + rules_chain) + rm.metering_labels[label_id] = label rules = label.get('rules') if rules: self._process_metering_label_rules( rules, label_chain, rules_chain, ext_dev, im) - rm.metering_labels[label_id] = label - def _process_associate_metering_label(self, router): self._update_router(router) rm = self.routers.get(router['id']) @@ -319,9 +317,18 @@ class IptablesMeteringDriver(abstract_driver.MeteringAbstractDriver): def _remove_metering_label_rule(self, router): self._process_metering_rule_action(router, 'delete') + def _create_metering_label_chain(self, rm, label_chain, rules_chain): + rm.iptables_manager.ipv4['filter'].add_chain(label_chain, wrap=False) + rm.iptables_manager.ipv4['filter'].add_chain(rules_chain, wrap=False) + rm.iptables_manager.ipv4['filter'].add_rule( + TOP_CHAIN, '-j ' + rules_chain, wrap=False) + rm.iptables_manager.ipv4['filter'].add_rule( + label_chain, '', wrap=False) + def _process_metering_rule_action_based_on_ns( self, router, action, ext_dev, im): '''Process metering rule actions based specific namespaces.''' + rm = self.routers.get(router['id']) with IptablesManagerTransaction(im): labels = router.get(constants.METERING_LABEL_KEY, []) for label in labels: @@ -331,6 +338,14 @@ class IptablesMeteringDriver(abstract_driver.MeteringAbstractDriver): rules_chain = iptables_manager.get_chain_name( WRAP_NAME + RULE + label_id, wrap=False) + + exists = rm.metering_labels.get(label_id) + if action == 'create' and not exists: + self._create_metering_label_chain(rm, + label_chain, + rules_chain) + rm.metering_labels[label_id] = label + rule = label.get('rule') if rule: if action == 'create': diff --git a/neutron/tests/unit/services/metering/drivers/test_iptables.py b/neutron/tests/unit/services/metering/drivers/test_iptables.py index 3044a921581..3ea0eb48464 100644 --- a/neutron/tests/unit/services/metering/drivers/test_iptables.py +++ b/neutron/tests/unit/services/metering/drivers/test_iptables.py @@ -104,6 +104,22 @@ TEST_ROUTERS_WITH_ONE_RULE = [ 'tenant_id': '6c5f5d2a1fa2441e88e35422926f48e8'}, ] +TEST_ROUTERS_WITH_NEW_LABEL = [ + {'_metering_labels': [ + {'id': 'e27fe2df-376e-4ac7-ae13-92f050a21f84', + 'rule': { + 'direction': 'ingress', + 'excluded': False, + 'id': '7f1a261f-2489-4ed1-870c-a62754501379', + 'metering_label_id': 'e27fe2df-376e-4ac7-ae13-92f050a21f84', + 'remote_ip_prefix': '50.0.0.0/24'}}], + 'admin_state_up': True, + 'gw_port_id': '6d411f48-ecc7-45e0-9ece-3b5bdb54fcee', + 'id': '473ec392-1711-44e3-b008-3251ccfc5099', + 'name': 'router1', + 'status': 'ACTIVE', + 'tenant_id': '6c5f5d2a1fa2441e88e35422926f48e8'}] + class IptablesDriverTestCase(base.BaseTestCase): def setUp(self): @@ -440,7 +456,35 @@ class IptablesDriverTestCase(base.BaseTestCase): '-d 40.0.0.0/24 -o qg-7d411f48-ec' ' -j neutron-meter-l-eeef45da-c60', wrap=False, top=False), + ] + self.v4filter_inst.assert_has_calls(calls) + def test_add_metering_label_rule_without_label(self): + new_routers_rules = TEST_ROUTERS_WITH_NEW_LABEL + # clear all the metering labels + for r in TEST_ROUTERS: + rm = iptables_driver.RouterWithMetering(self.metering.conf, r) + rm.metering_labels = {} + + self.metering.update_routers(None, TEST_ROUTERS) + self.metering.add_metering_label_rule(None, new_routers_rules) + calls = [ + mock.call.add_chain('neutron-meter-l-e27fe2df-376', + wrap=False), + mock.call.add_chain('neutron-meter-r-e27fe2df-376', + wrap=False), + mock.call.add_rule('neutron-meter-FORWARD', + '-j neutron-meter-r-e27fe2df-376', + wrap=False), + mock.call.add_rule('neutron-meter-l-e27fe2df-376', + '', + wrap=False), + mock.call.add_rule('neutron-meter-r-e27fe2df-376', + '-s 50.0.0.0/24 ' + '-i qg-6d411f48-ec ' + '-j neutron-meter-l-e27fe2df-376', + top=False, + wrap=False) ] self.v4filter_inst.assert_has_calls(calls)