From abc39b0e68b6ad8dd49edf354076d7cfb7a09bed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Dulko?= Date: Wed, 23 Dec 2020 17:39:11 +0100 Subject: [PATCH] Introduce NodesSubnetsDriver In order to have more control over the nodes subnets we expect instead of relying on static configuration option it's better to have flexibility. This commit introduces NodesSubnetsDriver model that will allow writing more complicated drivers providing the worker_nodes_subnets setting. A use case in mind is to use OpenShift Machine Custom Resources in order to discover subnets the nodes are using. Change-Id: I0eb5d9ad50895151967c23d3ad6d1237cc4d9667 --- kuryr_kubernetes/config.py | 3 + kuryr_kubernetes/controller/drivers/base.py | 32 ++++++++++ .../controller/drivers/nested_vif.py | 11 ++-- .../controller/drivers/network_policy.py | 3 +- .../controller/drivers/node_subnets.py | 43 +++++++++++++ .../controller/handlers/loadbalancer.py | 17 ++++-- .../controller/drivers/test_nested_vif.py | 13 ++-- .../controller/drivers/test_node_subnets.py | 61 +++++++++++++++++++ .../controller/handlers/test_loadbalancer.py | 32 ++++++++-- setup.cfg | 3 + 10 files changed, 197 insertions(+), 21 deletions(-) create mode 100644 kuryr_kubernetes/controller/drivers/node_subnets.py create mode 100644 kuryr_kubernetes/tests/unit/controller/drivers/test_node_subnets.py diff --git a/kuryr_kubernetes/config.py b/kuryr_kubernetes/config.py index cd3d34aec..7a22915a9 100644 --- a/kuryr_kubernetes/config.py +++ b/kuryr_kubernetes/config.py @@ -142,6 +142,9 @@ k8s_opts = [ help=_("The driver that manages VIFs pools for " "Kubernetes Pods"), default='noop'), + cfg.StrOpt('nodes_subnets_driver', + help=_("The driver that manages listing K8s nodes subnet_ids."), + default='config'), cfg.BoolOpt('port_debug', help=_('Enable port debug to force kuryr port names to be ' 'set to their corresponding pod names.'), diff --git a/kuryr_kubernetes/controller/drivers/base.py b/kuryr_kubernetes/controller/drivers/base.py index 455d746c9..c83f44529 100644 --- a/kuryr_kubernetes/controller/drivers/base.py +++ b/kuryr_kubernetes/controller/drivers/base.py @@ -751,3 +751,35 @@ class NetworkPolicyProjectDriver(DriverBase, metaclass=abc.ABCMeta): :returns: OpenStack project_id """ raise NotImplementedError() + + +class NodesSubnetsDriver(DriverBase, metaclass=abc.ABCMeta): + """Keeps list of subnet_ids of the OpenShift Nodes.""" + + ALIAS = 'nodes_subnets' + + @abc.abstractmethod + def get_nodes_subnets(self, raise_on_empty=False): + """Gets list of subnet_ids of OpenShift Nodes. + + :param raise_on_empty: whether it should raise if list is empty. + :return: list of subnets + """ + + raise NotImplementedError() + + @abc.abstractmethod + def add_node(self, node): + """Handles node addition. + + :param node: Node object + """ + pass + + @abc.abstractmethod + def delete_node(self, node): + """Handles node removal + + :param node: Node object + """ + pass diff --git a/kuryr_kubernetes/controller/drivers/nested_vif.py b/kuryr_kubernetes/controller/drivers/nested_vif.py index 7e5205d06..b6c3c9c88 100644 --- a/kuryr_kubernetes/controller/drivers/nested_vif.py +++ b/kuryr_kubernetes/controller/drivers/nested_vif.py @@ -20,6 +20,7 @@ from oslo_config import cfg as oslo_cfg from oslo_log import log as logging from kuryr_kubernetes import clients +from kuryr_kubernetes.controller.drivers import base from kuryr_kubernetes.controller.drivers import neutron_vif @@ -31,12 +32,14 @@ class NestedPodVIFDriver(neutron_vif.NeutronPodVIFDriver, metaclass=abc.ABCMeta): """Skeletal handler driver for VIFs for Nested Pods.""" + def __init__(self): + super().__init__() + self.nodes_subnets_driver = base.NodesSubnetsDriver.get_instance() + def _get_parent_port_by_host_ip(self, node_fixed_ip): os_net = clients.get_network_client() - node_subnet_ids = oslo_cfg.CONF.pod_vif_nested.worker_nodes_subnets - if not node_subnet_ids: - raise oslo_cfg.RequiredOptError( - 'worker_nodes_subnets', oslo_cfg.OptGroup('pod_vif_nested')) + node_subnet_ids = self.nodes_subnets_driver.get_nodes_subnets( + raise_on_empty=True) fixed_ips = ['ip_address=%s' % str(node_fixed_ip)] filters = {'fixed_ips': fixed_ips} diff --git a/kuryr_kubernetes/controller/drivers/network_policy.py b/kuryr_kubernetes/controller/drivers/network_policy.py index ee9d8eb9d..769470773 100644 --- a/kuryr_kubernetes/controller/drivers/network_policy.py +++ b/kuryr_kubernetes/controller/drivers/network_policy.py @@ -37,6 +37,7 @@ class NetworkPolicyDriver(base.NetworkPolicyDriver): def __init__(self): self.os_net = clients.get_network_client() self.kubernetes = clients.get_kubernetes_client() + self.nodes_subnets_driver = base.NodesSubnetsDriver.get_instance() def ensure_network_policy(self, policy): """Create security group rules out of network policies @@ -147,7 +148,7 @@ class NetworkPolicyDriver(base.NetworkPolicyDriver): if CONF.octavia_defaults.enforce_sg_rules: default_cidrs.append(utils.get_subnet_cidr( CONF.neutron_defaults.service_subnet)) - worker_subnet_ids = CONF.pod_vif_nested.worker_nodes_subnets + worker_subnet_ids = self.nodes_subnets_driver.get_nodes_subnets() default_cidrs.extend(utils.get_subnets_cidrs(worker_subnet_ids)) for cidr in default_cidrs: diff --git a/kuryr_kubernetes/controller/drivers/node_subnets.py b/kuryr_kubernetes/controller/drivers/node_subnets.py new file mode 100644 index 000000000..09738f7cf --- /dev/null +++ b/kuryr_kubernetes/controller/drivers/node_subnets.py @@ -0,0 +1,43 @@ +# Copyright 2020 Red Hat, Inc. +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from oslo_config import cfg +from oslo_log import log as logging + +from kuryr_kubernetes.controller.drivers import base + +CONF = cfg.CONF +LOG = logging.getLogger(__name__) + + +class ConfigNodesSubnets(base.NodesSubnetsDriver): + """Provides list of nodes subnets from configuration.""" + + def get_nodes_subnets(self, raise_on_empty=False): + node_subnet_ids = CONF.pod_vif_nested.worker_nodes_subnets + if not node_subnet_ids: + if raise_on_empty: + raise cfg.RequiredOptError( + 'worker_nodes_subnets', cfg.OptGroup('pod_vif_nested')) + else: + return [] + + return node_subnet_ids + + def add_node(self, node): + return False + + def delete_node(self, node): + return False diff --git a/kuryr_kubernetes/controller/handlers/loadbalancer.py b/kuryr_kubernetes/controller/handlers/loadbalancer.py index e230f98af..e430af640 100644 --- a/kuryr_kubernetes/controller/handlers/loadbalancer.py +++ b/kuryr_kubernetes/controller/handlers/loadbalancer.py @@ -50,8 +50,11 @@ class KuryrLoadBalancerHandler(k8s_base.ResourceEventHandler): self._drv_service_pub_ip = drv_base.ServicePubIpDriver.get_instance() self._drv_svc_project = drv_base.ServiceProjectDriver.get_instance() self._drv_sg = drv_base.ServiceSecurityGroupsDriver.get_instance() - self._nodes_subnets = utils.get_subnets_id_cidrs( - CONF.pod_vif_nested.worker_nodes_subnets) + self._drv_nodes_subnets = drv_base.NodesSubnetsDriver.get_instance() + + def _get_nodes_subnets(self): + return utils.get_subnets_id_cidrs( + self._drv_nodes_subnets.get_nodes_subnets()) def on_present(self, loadbalancer_crd): if self._should_ignore(loadbalancer_crd): @@ -262,7 +265,8 @@ class KuryrLoadBalancerHandler(k8s_base.ResourceEventHandler): # Avoid to point to a Pod on hostNetwork # that isn't the one to be added as Member. if not target_ref and utils.get_subnet_by_ip( - self._nodes_subnets, target_ip): + self._get_nodes_subnets(), + target_ip): target_pod = {} else: target_pod = utils.get_pod_by_ip( @@ -359,7 +363,8 @@ class KuryrLoadBalancerHandler(k8s_base.ResourceEventHandler): if target_pod: subnet_id = self._get_pod_subnet(target_pod, target_ip) else: - subnet = utils.get_subnet_by_ip(self._nodes_subnets, target_ip) + subnet = utils.get_subnet_by_ip( + self._drv_nodes_subnets.get_nodes_subnets(), target_ip) if subnet: subnet_id = subnet[0] else: @@ -381,13 +386,13 @@ class KuryrLoadBalancerHandler(k8s_base.ResourceEventHandler): # NOTE(ltomasbo): We are assuming that if IP is not on the # pod subnet it's because the member is using hostNetworking. In # this case we look for the IP in worker_nodes_subnets. - subnet = utils.get_subnet_by_ip(self._nodes_subnets, ip) + subnet = utils.get_subnet_by_ip(self._get_nodes_subnets(), ip) if subnet: return subnet[0] else: # This shouldn't ever happen but let's return just the first # worker_nodes_subnet id. - return self._nodes_subnets[0][0] + return self._get_nodes_subnets()[0][0] def _get_port_in_pool(self, pool, loadbalancer_crd): diff --git a/kuryr_kubernetes/tests/unit/controller/drivers/test_nested_vif.py b/kuryr_kubernetes/tests/unit/controller/drivers/test_nested_vif.py index 35419c1c7..9ed004d27 100644 --- a/kuryr_kubernetes/tests/unit/controller/drivers/test_nested_vif.py +++ b/kuryr_kubernetes/tests/unit/controller/drivers/test_nested_vif.py @@ -16,6 +16,7 @@ from kuryr.lib import exceptions as kl_exc from oslo_config import cfg as oslo_cfg from kuryr_kubernetes.controller.drivers import nested_vif +from kuryr_kubernetes.controller.drivers import node_subnets from kuryr_kubernetes.tests import base as test_base from kuryr_kubernetes.tests.unit import kuryr_fixtures as k_fix @@ -42,7 +43,8 @@ class TestNestedPodVIFDriver(test_base.TestCase): def test_get_parent_port_by_host_ip(self): cls = nested_vif.NestedPodVIFDriver - m_driver = mock.Mock(spec=cls) + m_driver = mock.Mock( + spec=cls, nodes_subnets_driver=node_subnets.ConfigNodesSubnets()) os_net = self.useFixture(k_fix.MockNetworkClient()).client node_subnet_id1 = 'node_subnet_id1' @@ -66,7 +68,8 @@ class TestNestedPodVIFDriver(test_base.TestCase): def test_get_parent_port_by_host_ip_multiple(self): cls = nested_vif.NestedPodVIFDriver - m_driver = mock.Mock(spec=cls) + m_driver = mock.Mock( + spec=cls, nodes_subnets_driver=node_subnets.ConfigNodesSubnets()) os_net = self.useFixture(k_fix.MockNetworkClient()).client node_subnet_id1 = 'node_subnet_id1' @@ -91,7 +94,8 @@ class TestNestedPodVIFDriver(test_base.TestCase): def test_get_parent_port_by_host_ip_subnet_id_not_configured(self): cls = nested_vif.NestedPodVIFDriver - m_driver = mock.Mock(spec=cls) + m_driver = mock.Mock( + spec=cls, nodes_subnets_driver=node_subnets.ConfigNodesSubnets()) self.useFixture(k_fix.MockNetworkClient()).client oslo_cfg.CONF.set_override('worker_nodes_subnets', '', @@ -103,7 +107,8 @@ class TestNestedPodVIFDriver(test_base.TestCase): def test_get_parent_port_by_host_ip_trunk_not_found(self): cls = nested_vif.NestedPodVIFDriver - m_driver = mock.Mock(spec=cls) + m_driver = mock.Mock( + spec=cls, nodes_subnets_driver=node_subnets.ConfigNodesSubnets()) os_net = self.useFixture(k_fix.MockNetworkClient()).client node_subnet_id = 'node_subnet_id' diff --git a/kuryr_kubernetes/tests/unit/controller/drivers/test_node_subnets.py b/kuryr_kubernetes/tests/unit/controller/drivers/test_node_subnets.py new file mode 100644 index 000000000..7e65da0db --- /dev/null +++ b/kuryr_kubernetes/tests/unit/controller/drivers/test_node_subnets.py @@ -0,0 +1,61 @@ +# Copyright 2020 Red Hat, Inc. +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from oslo_config import cfg + +from kuryr_kubernetes.controller.drivers import node_subnets +from kuryr_kubernetes.tests import base as test_base + + +class TestConfigNodesSubnetsDriver(test_base.TestCase): + + def test_get_nodes_subnets(self): + subnets = ['subnet1', 'subnet2'] + cfg.CONF.set_override('worker_nodes_subnets', subnets, + group='pod_vif_nested') + driver = node_subnets.ConfigNodesSubnets() + + self.assertEqual(subnets, driver.get_nodes_subnets()) + + def test_get_nodes_subnets_alias(self): + subnet = 'subnet1' + cfg.CONF.set_override('worker_nodes_subnet', subnet, + group='pod_vif_nested') + driver = node_subnets.ConfigNodesSubnets() + + self.assertEqual([subnet], driver.get_nodes_subnets()) + + def test_get_project_not_set_raise(self): + cfg.CONF.set_override('worker_nodes_subnets', None, + group='pod_vif_nested') + driver = node_subnets.ConfigNodesSubnets() + + self.assertRaises(cfg.RequiredOptError, driver.get_nodes_subnets, + raise_on_empty=True) + + def test_get_project_not_set(self): + cfg.CONF.set_override('worker_nodes_subnets', None, + group='pod_vif_nested') + driver = node_subnets.ConfigNodesSubnets() + + self.assertEqual([], driver.get_nodes_subnets()) + + def test_add_node(self): + driver = node_subnets.ConfigNodesSubnets() + self.assertFalse(driver.add_node('node')) + + def test_delete_node(self): + driver = node_subnets.ConfigNodesSubnets() + self.assertFalse(driver.delete_node('node')) diff --git a/kuryr_kubernetes/tests/unit/controller/handlers/test_loadbalancer.py b/kuryr_kubernetes/tests/unit/controller/handlers/test_loadbalancer.py index 59705dbd1..304c10d3f 100644 --- a/kuryr_kubernetes/tests/unit/controller/handlers/test_loadbalancer.py +++ b/kuryr_kubernetes/tests/unit/controller/handlers/test_loadbalancer.py @@ -207,9 +207,12 @@ class TestKuryrLoadBalancerHandler(test_base.TestCase): '.PodProjectDriver.get_instance') @mock.patch('kuryr_kubernetes.controller.drivers.base' '.LBaaSDriver.get_instance') - def test_init(self, m_get_drv_lbaas, m_get_drv_project, - m_get_drv_subnets, m_get_drv_service_pub_ip, m_cfg, - m_get_svc_sg_drv, m_get_svc_drv_project, m_get_cidr): + @mock.patch('kuryr_kubernetes.controller.drivers.base' + '.NodesSubnetsDriver.get_instance') + def test_init(self, m_get_drv_node_subnets, m_get_drv_lbaas, + m_get_drv_project, m_get_drv_subnets, + m_get_drv_service_pub_ip, m_cfg, m_get_svc_sg_drv, + m_get_svc_drv_project, m_get_cidr): m_get_drv_lbaas.return_value = mock.sentinel.drv_lbaas m_get_drv_project.return_value = mock.sentinel.drv_project m_get_drv_subnets.return_value = mock.sentinel.drv_subnets @@ -217,12 +220,15 @@ class TestKuryrLoadBalancerHandler(test_base.TestCase): m_get_drv_service_pub_ip.return_value = mock.sentinel.drv_lb_ip m_get_svc_drv_project.return_value = mock.sentinel.drv_svc_project m_get_svc_sg_drv.return_value = mock.sentinel.drv_sg + m_get_drv_node_subnets.return_value = mock.sentinel.drv_node_subnets handler = h_lb.KuryrLoadBalancerHandler() self.assertEqual(mock.sentinel.drv_lbaas, handler._drv_lbaas) self.assertEqual(mock.sentinel.drv_project, handler._drv_pod_project) self.assertEqual(mock.sentinel.drv_subnets, handler._drv_pod_subnets) self.assertEqual(mock.sentinel.drv_lb_ip, handler._drv_service_pub_ip) + self.assertEqual(mock.sentinel.drv_node_subnets, + handler._drv_nodes_subnets) @mock.patch('kuryr_kubernetes.utils.get_subnet_cidr') @mock.patch('kuryr_kubernetes.controller.drivers.base.' @@ -238,9 +244,12 @@ class TestKuryrLoadBalancerHandler(test_base.TestCase): '.PodProjectDriver.get_instance') @mock.patch('kuryr_kubernetes.controller.drivers.base' '.LBaaSDriver.get_instance') - def test_init_provider_ovn(self, m_get_drv_lbaas, m_get_drv_project, - m_get_drv_subnets, m_get_drv_service_pub_ip, - m_cfg, m_get_svc_sg_drv, m_get_svc_drv_project, + @mock.patch('kuryr_kubernetes.controller.drivers.base' + '.NodesSubnetsDriver.get_instance') + def test_init_provider_ovn(self, m_get_drv_node_subnets, m_get_drv_lbaas, + m_get_drv_project, m_get_drv_subnets, + m_get_drv_service_pub_ip, m_cfg, + m_get_svc_sg_drv, m_get_svc_drv_project, m_get_cidr): m_get_cidr.return_value = '10.0.0.128/26' m_get_drv_lbaas.return_value = mock.sentinel.drv_lbaas @@ -249,12 +258,15 @@ class TestKuryrLoadBalancerHandler(test_base.TestCase): m_get_drv_service_pub_ip.return_value = mock.sentinel.drv_lb_ip m_get_svc_drv_project.return_value = mock.sentinel.drv_svc_project m_get_svc_sg_drv.return_value = mock.sentinel.drv_sg + m_get_drv_node_subnets.return_value = mock.sentinel.drv_node_subnets handler = h_lb .KuryrLoadBalancerHandler() self.assertEqual(mock.sentinel.drv_lbaas, handler._drv_lbaas) self.assertEqual(mock.sentinel.drv_project, handler._drv_pod_project) self.assertEqual(mock.sentinel.drv_subnets, handler._drv_pod_subnets) self.assertEqual(mock.sentinel.drv_lb_ip, handler._drv_service_pub_ip) + self.assertEqual(mock.sentinel.drv_node_subnets, + handler._drv_nodes_subnets) def test_on_present(self): m_drv_service_pub_ip = mock.Mock() @@ -439,6 +451,8 @@ class TestKuryrLoadBalancerHandler(test_base.TestCase): '.PodProjectDriver.get_instance') @mock.patch('kuryr_kubernetes.controller.drivers.base' '.LBaaSDriver.get_instance') + @mock.patch('kuryr_kubernetes.controller.drivers.base' + '.NodesSubnetsDriver.get_instance', mock.Mock()) def test_sync_lbaas_members(self, m_get_drv_lbaas, m_get_drv_project, m_get_drv_subnets, m_k8s, m_svc_project_drv, m_svc_sg_drv, m_get_cidr): @@ -472,6 +486,8 @@ class TestKuryrLoadBalancerHandler(test_base.TestCase): '.PodProjectDriver.get_instance') @mock.patch('kuryr_kubernetes.controller.drivers.base' '.LBaaSDriver.get_instance') + @mock.patch('kuryr_kubernetes.controller.drivers.base' + '.NodesSubnetsDriver.get_instance', mock.Mock()) def test_sync_lbaas_members_udp(self, m_get_drv_lbaas, m_get_drv_project, m_get_drv_subnets, m_k8s, m_svc_project_drv, m_svc_sg_drv, @@ -507,6 +523,8 @@ class TestKuryrLoadBalancerHandler(test_base.TestCase): '.PodProjectDriver.get_instance') @mock.patch('kuryr_kubernetes.controller.drivers.base' '.LBaaSDriver.get_instance') + @mock.patch('kuryr_kubernetes.controller.drivers.base' + '.NodesSubnetsDriver.get_instance', mock.Mock()) def test_sync_lbaas_members_svc_listener_port_edit( self, m_get_drv_lbaas, m_get_drv_project, m_get_drv_subnets, m_k8s, m_svc_project_drv, m_svc_sg_drv, m_get_cidr): @@ -547,6 +565,8 @@ class TestKuryrLoadBalancerHandler(test_base.TestCase): '.PodProjectDriver.get_instance') @mock.patch('kuryr_kubernetes.controller.drivers.base' '.LBaaSDriver.get_instance') + @mock.patch('kuryr_kubernetes.controller.drivers.base' + '.NodesSubnetsDriver.get_instance', mock.Mock()) def test_add_new_members_udp(self, m_get_drv_lbaas, m_get_drv_project, m_get_drv_subnets, m_k8s, m_svc_project_drv, diff --git a/setup.cfg b/setup.cfg index bb7071b1e..ee3e67243 100644 --- a/setup.cfg +++ b/setup.cfg @@ -96,6 +96,9 @@ kuryr_kubernetes.controller.drivers.vif_pool = nested = kuryr_kubernetes.controller.drivers.vif_pool:NestedVIFPool multi_pool = kuryr_kubernetes.controller.drivers.vif_pool:MultiVIFPool +kuryr_kubernetes.controller.drivers.nodes_subnets = + config = kuryr_kubernetes.controller.drivers.node_subnets:ConfigNodesSubnets + kuryr_kubernetes.controller.handlers = vif = kuryr_kubernetes.controller.handlers.vif:VIFHandler service = kuryr_kubernetes.controller.handlers.lbaas:ServiceHandler