From 29821a63ea17887400c48894087d5d0bd3f7190c Mon Sep 17 00:00:00 2001 From: Yossi Boaron Date: Sun, 4 Mar 2018 22:53:39 +0200 Subject: [PATCH] Services: Gracefully ignore exposed UDP ports Since LBaaSv2 doesn't support UDP load balancing, Kuryr should ignore exposed UDP ports in K8S service. This patch updates Kuryr to gracefully ignore UDP exposed ports and updates the documentation with this info. Closes-Bug: 1736060 Change-Id: I03f6d95a2d855cbd8954018c930e283a46763655 (cherry picked from commit d5e5d1537bc911a893f068d094f057c45444fc40) --- doc/source/installation/services.rst | 4 +- .../controller/drivers/lbaasv2.py | 5 + kuryr_kubernetes/controller/handlers/lbaas.py | 19 ++- .../unit/controller/handlers/test_lbaas.py | 115 ++++++++++++++---- 4 files changed, 114 insertions(+), 29 deletions(-) diff --git a/doc/source/installation/services.rst b/doc/source/installation/services.rst index a914f49d8..14fb3f552 100644 --- a/doc/source/installation/services.rst +++ b/doc/source/installation/services.rst @@ -24,9 +24,11 @@ be implemented in the following way: default configuration (bottom) If you are paying attention and are familiar with the `LBaaS API`_ you probably -noticed that we have separate pools for each exposed pool in a service. This is +noticed that we have separate pools for each exposed port in a service. This is probably not optimal and we would probably benefit from keeping a single Neutron pool that lists each of the per port listeners. +Since `LBaaS API`_ doesn't support UDP load balancing, service exported UDP +ports will be ignored. When installing you can decide to use the legacy Neutron HAProxy driver for LBaaSv2 or install and configure OpenStack Octavia, which as of Pike implements diff --git a/kuryr_kubernetes/controller/drivers/lbaasv2.py b/kuryr_kubernetes/controller/drivers/lbaasv2.py index e8643c9ba..c7504969e 100644 --- a/kuryr_kubernetes/controller/drivers/lbaasv2.py +++ b/kuryr_kubernetes/controller/drivers/lbaasv2.py @@ -31,6 +31,7 @@ from kuryr_kubernetes.objects import lbaas as obj_lbaas LOG = logging.getLogger(__name__) _ACTIVATION_TIMEOUT = 300 +_SUPPORTED_LISTENER_PROT = ('HTTP', 'HTTPS', 'TCP') class LBaaSv2Driver(base.LBaaSDriver): @@ -128,6 +129,10 @@ class LBaaSv2Driver(base.LBaaSDriver): 'for listener %s.', listener.name) def ensure_listener(self, endpoints, loadbalancer, protocol, port): + if protocol not in _SUPPORTED_LISTENER_PROT: + LOG.info("Protocol: %(prot)s: is not supported by LBaaSV2", { + 'prot': protocol}) + return None name = "%(namespace)s/%(name)s" % endpoints['metadata'] name += ":%s:%s" % (protocol, port) listener = obj_lbaas.LBaaSListener(name=name, diff --git a/kuryr_kubernetes/controller/handlers/lbaas.py b/kuryr_kubernetes/controller/handlers/lbaas.py index 882a139a6..2f579ac16 100644 --- a/kuryr_kubernetes/controller/handlers/lbaas.py +++ b/kuryr_kubernetes/controller/handlers/lbaas.py @@ -301,8 +301,18 @@ class LoadBalancerHandler(k8s_base.ResourceEventHandler): pool_by_lsnr_port = {(lsnr_by_id[p.listener_id].protocol, lsnr_by_id[p.listener_id].port): p for p in lbaas_state.pools} - pool_by_tgt_name = {p.name: pool_by_lsnr_port[p.protocol, p.port] - for p in lbaas_spec.ports} + # NOTE(yboaron): Since LBaaSv2 doesn't support UDP load balancing, + # the LBaaS driver will return 'None' in case of UDP port + # listener creation. + # we should consider the case in which + # 'pool_by_lsnr_port[p.protocol, p.port]' is missing + pool_by_tgt_name = {} + for p in lbaas_spec.ports: + try: + pool_by_tgt_name[p.name] = pool_by_lsnr_port[p.protocol, + p.port] + except KeyError: + continue current_targets = {(str(m.ip), m.port) for m in lbaas_state.members} for subset in endpoints.get('subsets', []): @@ -454,8 +464,9 @@ class LoadBalancerHandler(k8s_base.ResourceEventHandler): loadbalancer=lbaas_state.loadbalancer, protocol=protocol, port=port) - lbaas_state.listeners.append(listener) - changed = True + if listener is not None: + lbaas_state.listeners.append(listener) + changed = True return changed def _remove_unused_listeners(self, endpoints, lbaas_state, lbaas_spec): diff --git a/kuryr_kubernetes/tests/unit/controller/handlers/test_lbaas.py b/kuryr_kubernetes/tests/unit/controller/handlers/test_lbaas.py index a40dba22a..d23088bc0 100644 --- a/kuryr_kubernetes/tests/unit/controller/handlers/test_lbaas.py +++ b/kuryr_kubernetes/tests/unit/controller/handlers/test_lbaas.py @@ -26,6 +26,8 @@ from kuryr_kubernetes import exceptions as k_exc from kuryr_kubernetes.objects import lbaas as obj_lbaas from kuryr_kubernetes.tests import base as test_base +_SUPPORTED_LISTENER_PROT = ('HTTP', 'HTTPS', 'TCP') + class TestLBaaSSpecHandler(test_base.TestCase): @@ -304,6 +306,23 @@ class TestLBaaSSpecHandler(test_base.TestCase): m_handler._get_service_ports.assert_called_once_with( mock.sentinel.service) + def test_generate_lbaas_port_specs_udp(self): + m_handler = mock.Mock(spec=h_lbaas.LBaaSSpecHandler) + m_handler._get_service_ports.return_value = [ + {'port': 1, 'name': 'X', 'protocol': 'TCP'}, + {'port': 2, 'name': 'Y', 'protocol': 'UDP'} + ] + expected_ports = [ + obj_lbaas.LBaaSPortSpec(name='X', protocol='TCP', port=1), + obj_lbaas.LBaaSPortSpec(name='Y', protocol='UDP', port=2), + ] + + ret = h_lbaas.LBaaSSpecHandler._generate_lbaas_port_specs( + m_handler, mock.sentinel.service) + self.assertEqual(expected_ports, ret) + m_handler._get_service_ports.assert_called_once_with( + mock.sentinel.service) + def test_get_endpoints_link(self): m_handler = mock.Mock(spec=h_lbaas.LBaaSSpecHandler) service = {'metadata': { @@ -330,6 +349,7 @@ class TestLBaaSSpecHandler(test_base.TestCase): class FakeLBaaSDriver(drv_base.LBaaSDriver): + def ensure_loadbalancer(self, endpoints, project_id, subnet_id, ip, security_groups_ids, service_type): name = str(ip) @@ -340,6 +360,9 @@ class FakeLBaaSDriver(drv_base.LBaaSDriver): id=uuidutils.generate_uuid()) def ensure_listener(self, endpoints, loadbalancer, protocol, port): + if protocol not in _SUPPORTED_LISTENER_PROT: + return None + name = "%s:%s:%s" % (loadbalancer.name, protocol, port) return obj_lbaas.LBaaSListener(name=name, project_id=loadbalancer.project_id, @@ -529,13 +552,14 @@ class TestLoadBalancerHandler(test_base.TestCase): pools=list(pools.values()), members=list(members.values())) - def _generate_lbaas_spec(self, vip, targets, project_id, subnet_id): + def _generate_lbaas_spec(self, vip, targets, project_id, + subnet_id, prot='TCP'): return obj_lbaas.LBaaSServiceSpec( ip=vip, project_id=project_id, subnet_id=subnet_id, ports=[obj_lbaas.LBaaSPortSpec(name=str(port), - protocol='TCP', + protocol=prot, port=port) for port in set(t[0] for t in targets.values())]) @@ -567,6 +591,34 @@ class TestLoadBalancerHandler(test_base.TestCase): ] } + def _sync_lbaas_members_impl(self, m_get_drv_lbaas, m_get_drv_project, + m_get_drv_subnets, subnet_id, project_id, + endpoints, state, spec): + m_drv_lbaas = mock.Mock(wraps=FakeLBaaSDriver()) + m_drv_project = mock.Mock() + m_drv_project.get_project.return_value = project_id + m_drv_subnets = mock.Mock() + m_drv_subnets.get_subnets.return_value = { + subnet_id: mock.sentinel.subnet} + m_get_drv_lbaas.return_value = m_drv_lbaas + m_get_drv_project.return_value = m_drv_project + m_get_drv_subnets.return_value = m_drv_subnets + + handler = h_lbaas.LoadBalancerHandler() + + with mock.patch.object(handler, '_get_pod_subnet') as m_get_pod_subnet: + m_get_pod_subnet.return_value = subnet_id + handler._sync_lbaas_members(endpoints, state, spec) + + lsnrs = {lsnr.id: lsnr for lsnr in state.listeners} + pools = {pool.id: pool for pool in state.pools} + observed_targets = sorted( + (str(member.ip), ( + lsnrs[pools[member.pool_id].listener_id].port, + member.port)) + for member in state.members) + return observed_targets + @mock.patch('kuryr_kubernetes.controller.drivers.base' '.PodSubnetsDriver.get_instance') @mock.patch('kuryr_kubernetes.controller.drivers.base' @@ -594,32 +646,47 @@ class TestLoadBalancerHandler(test_base.TestCase): spec = self._generate_lbaas_spec(expected_ip, expected_targets, project_id, subnet_id) - m_drv_lbaas = mock.Mock(wraps=FakeLBaaSDriver()) - m_drv_project = mock.Mock() - m_drv_project.get_project.return_value = project_id - m_drv_subnets = mock.Mock() - m_drv_subnets.get_subnets.return_value = { - subnet_id: mock.sentinel.subnet} - m_get_drv_lbaas.return_value = m_drv_lbaas - m_get_drv_project.return_value = m_drv_project - m_get_drv_subnets.return_value = m_drv_subnets + observed_targets = self._sync_lbaas_members_impl( + m_get_drv_lbaas, m_get_drv_project, m_get_drv_subnets, + subnet_id, project_id, endpoints, state, spec) - handler = h_lbaas.LoadBalancerHandler() - - with mock.patch.object(handler, '_get_pod_subnet') as m_get_pod_subnet: - m_get_pod_subnet.return_value = subnet_id - handler._sync_lbaas_members(endpoints, state, spec) - - lsnrs = {lsnr.id: lsnr for lsnr in state.listeners} - pools = {pool.id: pool for pool in state.pools} - observed_targets = sorted( - (str(member.ip), ( - lsnrs[pools[member.pool_id].listener_id].port, - member.port)) - for member in state.members) self.assertEqual(sorted(expected_targets.items()), observed_targets) self.assertEqual(expected_ip, str(state.loadbalancer.ip)) + @mock.patch('kuryr_kubernetes.controller.drivers.base' + '.PodSubnetsDriver.get_instance') + @mock.patch('kuryr_kubernetes.controller.drivers.base' + '.PodProjectDriver.get_instance') + @mock.patch('kuryr_kubernetes.controller.drivers.base' + '.LBaaSDriver.get_instance') + def test_sync_lbaas_members_udp(self, m_get_drv_lbaas, + m_get_drv_project, m_get_drv_subnets): + # REVISIT(ivc): test methods separately and verify ensure/release + project_id = uuidutils.generate_uuid() + subnet_id = uuidutils.generate_uuid() + current_ip = '1.1.1.1' + current_targets = { + '1.1.1.101': (1001, 10001), + '1.1.1.111': (1001, 10001), + '1.1.1.201': (2001, 20001)} + expected_ip = '2.2.2.2' + expected_targets = { + '2.2.2.101': (1201, 12001), + '2.2.2.111': (1201, 12001), + '2.2.2.201': (2201, 22001)} + endpoints = self._generate_endpoints(expected_targets) + state = self._generate_lbaas_state( + current_ip, current_targets, project_id, subnet_id) + spec = self._generate_lbaas_spec(expected_ip, expected_targets, + project_id, subnet_id, 'UDP') + + observed_targets = self._sync_lbaas_members_impl( + m_get_drv_lbaas, m_get_drv_project, m_get_drv_subnets, + subnet_id, project_id, endpoints, state, spec) + + self.assertEqual([], observed_targets) + self.assertEqual(expected_ip, str(state.loadbalancer.ip)) + def test_get_lbaas_spec(self): self.skipTest("skipping until generalised annotation handling is " "implemented")