From cae194972a8f82a3c9029db71b2ec428992f848c Mon Sep 17 00:00:00 2001 From: Roman Dobosz Date: Thu, 9 Jul 2020 11:41:24 +0200 Subject: [PATCH] Fix SG rules to be created twice for the services. Repeated rules are being created for the service while not needed, but on the downside it cause increased number of calls to the Neutron. In this patch we're being polite to Neutron by not creating them. Closes-Bug: 1888407 Change-Id: I4e64fb00666f0d8ebcb757d77b5cbc81bd69f9d3 --- .../controller/drivers/network_policy.py | 13 ------------- .../controller/drivers/test_network_policy.py | 17 +++-------------- 2 files changed, 3 insertions(+), 27 deletions(-) diff --git a/kuryr_kubernetes/controller/drivers/network_policy.py b/kuryr_kubernetes/controller/drivers/network_policy.py index ce2f74b33..3a2ef3265 100644 --- a/kuryr_kubernetes/controller/drivers/network_policy.py +++ b/kuryr_kubernetes/controller/drivers/network_policy.py @@ -341,7 +341,6 @@ class NetworkPolicyDriver(base.NetworkPolicyDriver): crd_rules, direction, port, pod_selector, policy_namespace) if allow_all: - container_port = None for container_port, pods in matched_pods.items(): for ethertype in (constants.IPv4, constants.IPv6): sg_rule = driver_utils.create_security_group_rule_body( @@ -350,10 +349,6 @@ class NetworkPolicyDriver(base.NetworkPolicyDriver): ethertype=ethertype, pods=pods) crd_rules.append(sg_rule) - if direction == 'egress': - self._create_svc_egress_sg_rule( - policy_namespace, crd_rules, - port=container_port, protocol=port.get('protocol')) def _create_sg_rule_on_number_port(self, allowed_resources, direction, port, sg_rule_body_list, @@ -395,11 +390,6 @@ class NetworkPolicyDriver(base.NetworkPolicyDriver): ethertype=ethertype, protocol=port.get('protocol'))) sg_rule_body_list.append(sg_rule) - if direction == 'egress': - self._create_svc_egress_sg_rule( - policy_namespace, sg_rule_body_list, - port=port.get('port'), - protocol=port.get('protocol')) def _create_default_sg_rule(self, direction, sg_rule_body_list): for ethertype in (constants.IPv4, constants.IPv6): @@ -532,9 +522,6 @@ class NetworkPolicyDriver(base.NetworkPolicyDriver): port_range_max=65535, ethertype=ethertype) sg_rule_body_list.append(rule) - if direction == 'egress': - self._create_svc_egress_sg_rule(policy_namespace, - sg_rule_body_list) else: LOG.debug('This network policy specifies no %(direction)s ' '%(rule_direction)s and no ports: %(policy)s', diff --git a/kuryr_kubernetes/tests/unit/controller/drivers/test_network_policy.py b/kuryr_kubernetes/tests/unit/controller/drivers/test_network_policy.py index 2232f5104..2fd1d7e8c 100644 --- a/kuryr_kubernetes/tests/unit/controller/drivers/test_network_policy.py +++ b/kuryr_kubernetes/tests/unit/controller/drivers/test_network_policy.py @@ -524,15 +524,12 @@ class TestNetworkPolicyDriver(test_base.TestCase): m_get_ports.assert_called_with(resources[0], port) - @mock.patch('kuryr_kubernetes.controller.drivers.utils.' - 'create_security_group_rule_body') @mock.patch.object(network_policy.NetworkPolicyDriver, '_create_sg_rules_with_container_ports') @mock.patch('kuryr_kubernetes.controller.drivers.utils.get_ports') def test__create_sg_rule_body_on_text_port_egress_all(self, m_get_ports, - m_create_sgr_cont, - m_create_sgr): + m_create_sgr_cont): port = {'protocol': 'TCP', 'port': 22} container_ports = mock.sentinel.ports resources = [{'spec': 'foo'}] @@ -552,9 +549,7 @@ class TestNetworkPolicyDriver(test_base.TestCase): allow_all=True) m_get_ports.assert_called_with(resources[0], port) - m_create_sgr.assert_called_once_with('egress', None, cidr=mock.ANY, - protocol='TCP') - self.assertEqual(len(crd_rules), 1) + self.assertEqual(len(crd_rules), 0) @mock.patch('kuryr_kubernetes.utils.get_subnet_cidr') @mock.patch('kuryr_kubernetes.controller.drivers.utils.' @@ -600,14 +595,8 @@ class TestNetworkPolicyDriver(test_base.TestCase): calls = [mock.call(direction, container_ports[0][1], protocol=port['protocol'], ethertype=e, pods='foo') for e in ('IPv4', 'IPv6')] - calls.append(mock.call(direction, container_ports[0][1], - protocol=port['protocol'], - cidr='10.0.0.128/26')) m_create_sgr.assert_has_calls(calls) - - # NOTE(gryf): there are 3 rules created in case of egress direction, - # since additional one is created for specific cidr in service subnet. - self.assertEqual(len(crd_rules), 3) + self.assertEqual(len(crd_rules), 2) def test__create_all_pods_sg_rules(self): port = {'protocol': 'TCP', 'port': 22}