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

@@ -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.'),

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

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}

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:

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

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):
 

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'

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'))

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,

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