From c9041d6979928c0690f8dd913facab8ae3dcc0fd Mon Sep 17 00:00:00 2001 From: Antoni Segura Puimedon Date: Fri, 16 Feb 2018 16:09:56 +0100 Subject: [PATCH] Services: Set missing SGs for haproxy provider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since we started using Octavia we never got around to setting the security groups for the legacy haproxy provider. This only affects when using the native firewall as otherwise the haproxy internal ovs port bypasses the SGs Change-Id: Ie4a53dedf54472394f92fdfacddf0632e33f1f5b Closes-Bug: 1749968 Co-Authored-By: Michał Dulko Signed-off-by: Antoni Segura Puimedon --- kuryr_kubernetes/constants.py | 1 + .../controller/drivers/lbaasv2.py | 21 ++++- kuryr_kubernetes/objects/lbaas.py | 5 +- .../unit/controller/drivers/test_lbaasv2.py | 88 +++++++++++++++++-- 4 files changed, 105 insertions(+), 10 deletions(-) diff --git a/kuryr_kubernetes/constants.py b/kuryr_kubernetes/constants.py index d40cea940..a5e414063 100644 --- a/kuryr_kubernetes/constants.py +++ b/kuryr_kubernetes/constants.py @@ -37,6 +37,7 @@ KURYR_PORT_NAME = 'kuryr-pool-port' OCTAVIA_L2_MEMBER_MODE = "L2" OCTAVIA_L3_MEMBER_MODE = "L3" +NEUTRON_LBAAS_HAPROXY_PROVIDER = 'haproxy' VIF_POOL_POPULATE = '/populatePool' VIF_POOL_FREE = '/freePool' diff --git a/kuryr_kubernetes/controller/drivers/lbaasv2.py b/kuryr_kubernetes/controller/drivers/lbaasv2.py index 00aa5e35d..a7562439e 100644 --- a/kuryr_kubernetes/controller/drivers/lbaasv2.py +++ b/kuryr_kubernetes/controller/drivers/lbaasv2.py @@ -22,6 +22,7 @@ from oslo_utils import excutils from oslo_utils import timeutils from kuryr_kubernetes import clients +from kuryr_kubernetes import constants as const from kuryr_kubernetes.controller.drivers import base from kuryr_kubernetes import exceptions as k_exc from kuryr_kubernetes.objects import lbaas as obj_lbaas @@ -48,8 +49,22 @@ class LBaaSv2Driver(base.LBaaSDriver): # deleted externally between 'create' and 'find' raise k_exc.ResourceNotReady(request) - # TODO(ivc): handle security groups - + # We only handle SGs for legacy LBaaSv2, Octavia handles it dynamically + # according to listener ports. + if response.provider == const.NEUTRON_LBAAS_HAPROXY_PROVIDER: + vip_port_id = response.port_id + neutron = clients.get_neutron_client() + try: + neutron.update_port( + vip_port_id, + {'port': {'security_groups': security_groups_ids}}) + except n_exc.NeutronClientException: + LOG.exception('Failed to set SG for LBaaS v2 VIP port %s.', + vip_port_id) + # NOTE(dulek): `endpoints` arguments on release_loadbalancer() + # is ignored for some reason, so just pass None. + self.release_loadbalancer(None, response) + raise return response def release_loadbalancer(self, endpoints, loadbalancer): @@ -136,6 +151,7 @@ class LBaaSv2Driver(base.LBaaSDriver): 'vip_subnet_id': loadbalancer.subnet_id}}) loadbalancer.id = response['loadbalancer']['id'] loadbalancer.port_id = self._get_vip_port_id(loadbalancer) + loadbalancer.provider = response['loadbalancer']['provider'] return loadbalancer def _find_loadbalancer(self, loadbalancer): @@ -150,6 +166,7 @@ class LBaaSv2Driver(base.LBaaSDriver): try: loadbalancer.id = response['loadbalancers'][0]['id'] loadbalancer.port_id = self._get_vip_port_id(loadbalancer) + loadbalancer.provider = response['loadbalancers'][0]['provider'] except (KeyError, IndexError): return None diff --git a/kuryr_kubernetes/objects/lbaas.py b/kuryr_kubernetes/objects/lbaas.py index 29a73f4cf..bd12fd7ee 100644 --- a/kuryr_kubernetes/objects/lbaas.py +++ b/kuryr_kubernetes/objects/lbaas.py @@ -22,7 +22,9 @@ from kuryr_kubernetes.objects import fields as k_fields @obj_base.VersionedObjectRegistry.register class LBaaSLoadBalancer(k_obj.KuryrK8sObjectBase): - VERSION = '1.0' + # Version 1.0: Initial version + # Version 1.1: Added provider field + VERSION = '1.1' fields = { 'id': obj_fields.UUIDField(), @@ -31,6 +33,7 @@ class LBaaSLoadBalancer(k_obj.KuryrK8sObjectBase): 'ip': obj_fields.IPAddressField(), 'subnet_id': obj_fields.UUIDField(), 'port_id': obj_fields.UUIDField(), + 'provider': obj_fields.StringField(), } diff --git a/kuryr_kubernetes/tests/unit/controller/drivers/test_lbaasv2.py b/kuryr_kubernetes/tests/unit/controller/drivers/test_lbaasv2.py index 6542b4335..0170feb20 100644 --- a/kuryr_kubernetes/tests/unit/controller/drivers/test_lbaasv2.py +++ b/kuryr_kubernetes/tests/unit/controller/drivers/test_lbaasv2.py @@ -17,6 +17,7 @@ import mock from neutronclient.common import exceptions as n_exc +from kuryr_kubernetes import constants as const from kuryr_kubernetes.controller.drivers import lbaasv2 as d_lbaasv2 from kuryr_kubernetes import exceptions as k_exc from kuryr_kubernetes.objects import lbaas as obj_lbaas @@ -26,19 +27,21 @@ from kuryr_kubernetes.tests.unit import kuryr_fixtures as k_fix class TestLBaaSv2Driver(test_base.TestCase): def test_ensure_loadbalancer(self): + neutron = self.useFixture(k_fix.MockNeutronClient()).client cls = d_lbaasv2.LBaaSv2Driver m_driver = mock.Mock(spec=d_lbaasv2.LBaaSv2Driver) - expected_resp = mock.sentinel.expected_resp + expected_resp = obj_lbaas.LBaaSLoadBalancer( + provider='octavia', port_id='D3FA400A-F543-4B91-9CD3-047AF0CE42E2') namespace = 'TEST_NAMESPACE' name = 'TEST_NAME' project_id = 'TEST_PROJECT' subnet_id = 'D3FA400A-F543-4B91-9CD3-047AF0CE42D1' ip = '1.2.3.4' - # TODO(ivc): handle security groups - sg_ids = [] + sg_ids = ['foo', 'bar'] endpoints = {'metadata': {'namespace': namespace, 'name': name}} m_driver._ensure.return_value = expected_resp + neutron.update_port = mock.Mock() resp = cls.ensure_loadbalancer(m_driver, endpoints, project_id, subnet_id, ip, sg_ids) m_driver._ensure.assert_called_once_with(mock.ANY, @@ -50,6 +53,74 @@ class TestLBaaSv2Driver(test_base.TestCase): self.assertEqual(subnet_id, req.subnet_id) self.assertEqual(ip, str(req.ip)) self.assertEqual(expected_resp, resp) + neutron.update_port.assert_not_called() + + def test_ensure_loadbalancer_sg_updated(self): + neutron = self.useFixture(k_fix.MockNeutronClient()).client + cls = d_lbaasv2.LBaaSv2Driver + m_driver = mock.Mock(spec=d_lbaasv2.LBaaSv2Driver) + expected_resp = obj_lbaas.LBaaSLoadBalancer( + provider=const.NEUTRON_LBAAS_HAPROXY_PROVIDER, + port_id='D3FA400A-F543-4B91-9CD3-047AF0CE42E2') + namespace = 'TEST_NAMESPACE' + name = 'TEST_NAME' + project_id = 'TEST_PROJECT' + subnet_id = 'D3FA400A-F543-4B91-9CD3-047AF0CE42D1' + ip = '1.2.3.4' + sg_ids = ['foo', 'bar'] + endpoints = {'metadata': {'namespace': namespace, 'name': name}} + + m_driver._ensure.return_value = expected_resp + neutron.update_port = mock.Mock() + resp = cls.ensure_loadbalancer(m_driver, endpoints, project_id, + subnet_id, ip, sg_ids) + m_driver._ensure.assert_called_once_with(mock.ANY, + m_driver._create_loadbalancer, + m_driver._find_loadbalancer) + req = m_driver._ensure.call_args[0][0] + self.assertEqual("%s/%s" % (namespace, name), req.name) + self.assertEqual(project_id, req.project_id) + self.assertEqual(subnet_id, req.subnet_id) + self.assertEqual(ip, str(req.ip)) + self.assertEqual(expected_resp, resp) + neutron.update_port.assert_called_once_with( + 'D3FA400A-F543-4B91-9CD3-047AF0CE42E2', + {'port': {'security_groups': ['foo', 'bar']}}) + + def test_ensure_loadbalancer_neutron_error(self): + neutron = self.useFixture(k_fix.MockNeutronClient()).client + cls = d_lbaasv2.LBaaSv2Driver + m_driver = mock.Mock(spec=d_lbaasv2.LBaaSv2Driver) + expected_resp = obj_lbaas.LBaaSLoadBalancer( + provider=const.NEUTRON_LBAAS_HAPROXY_PROVIDER, + port_id='D3FA400A-F543-4B91-9CD3-047AF0CE42E2') + namespace = 'TEST_NAMESPACE' + name = 'TEST_NAME' + project_id = 'TEST_PROJECT' + subnet_id = 'D3FA400A-F543-4B91-9CD3-047AF0CE42D1' + ip = '1.2.3.4' + sg_ids = ['foo', 'bar'] + endpoints = {'metadata': {'namespace': namespace, 'name': name}} + + m_driver._ensure.return_value = expected_resp + neutron.update_port = mock.Mock( + side_effect=n_exc.NeutronClientException) + self.assertRaises(n_exc.NeutronClientException, + cls.ensure_loadbalancer, m_driver, endpoints, + project_id, subnet_id, ip, sg_ids) + m_driver._ensure.assert_called_once_with(mock.ANY, + m_driver._create_loadbalancer, + m_driver._find_loadbalancer) + req = m_driver._ensure.call_args[0][0] + self.assertEqual("%s/%s" % (namespace, name), req.name) + self.assertEqual(project_id, req.project_id) + self.assertEqual(subnet_id, req.subnet_id) + self.assertEqual(ip, str(req.ip)) + neutron.update_port.assert_called_once_with( + 'D3FA400A-F543-4B91-9CD3-047AF0CE42E2', + {'port': {'security_groups': ['foo', 'bar']}}) + m_driver.release_loadbalancer.assert_called_once_with(None, + expected_resp) def test_ensure_loadbalancer_not_ready(self): cls = d_lbaasv2.LBaaSv2Driver @@ -227,8 +298,9 @@ class TestLBaaSv2Driver(test_base.TestCase): 'project_id': loadbalancer.project_id, 'tenant_id': loadbalancer.project_id, 'vip_address': str(loadbalancer.ip), - 'vip_subnet_id': loadbalancer.subnet_id}} - resp = {'loadbalancer': {'id': loadbalancer_id}} + 'vip_subnet_id': loadbalancer.subnet_id, + }} + resp = {'loadbalancer': {'id': loadbalancer_id, 'provider': 'haproxy'}} neutron.create_loadbalancer.return_value = resp ret = cls._create_loadbalancer(m_driver, loadbalancer) @@ -244,9 +316,11 @@ class TestLBaaSv2Driver(test_base.TestCase): m_driver = mock.Mock(spec=d_lbaasv2.LBaaSv2Driver) loadbalancer = obj_lbaas.LBaaSLoadBalancer( name='TEST_NAME', project_id='TEST_PROJECT', ip='1.2.3.4', - subnet_id='D3FA400A-F543-4B91-9CD3-047AF0CE42D1') + subnet_id='D3FA400A-F543-4B91-9CD3-047AF0CE42D1', + provider='haproxy') loadbalancer_id = '00EE9E11-91C2-41CF-8FD4-7970579E5C4C' - resp = {'loadbalancers': [{'id': loadbalancer_id}]} + resp = {'loadbalancers': [{'id': loadbalancer_id, + 'provider': 'haproxy'}]} neutron.list_loadbalancers.return_value = resp ret = cls._find_loadbalancer(m_driver, loadbalancer)