subnet routes (fixed, completed)

octavia_f5/common/constants.py

@@ -26,7 +26,9 @@
 PREFIX_LOADBALANCER = 'lb_'
 PREFIX_POLICY = 'l7policy_'
 PREFIX_WRAPPER_POLICY = 'wrapper_policy_'
+PREFIX_NETWORK_LEGACY = 'net-'
 PREFIX_NETWORK = 'net_'
+PREFIX_SUBNET = 'sub_'
 PREFIX_IRULE = 'irule_'
 PREFIX_MEMBER = 'member_'
 PREFIX_SECRET = 'secret_'

octavia_f5/controller/worker/controller_worker.py

@@ -107,6 +107,8 @@ def __init__(self):
         super(ControllerWorker, self).__init__()
 
     def as3worker(self):
+        """ AS3 Worker thread, pops tenant to refresh from thread-safe set queue"""
+
         @lockutils.synchronized("f5sync", fair=True)
         def f5sync(network_id, device, *args):
             self._metric_as3worker_queue.labels(octavia_host=CONF.host).set(self.queue.qsize())
@@ -136,7 +138,7 @@ def f5sync(network_id, device, *args):
             for lb in loadbalancers:
                 self._reset_in_use_quota(lb.project_id)
 
-        """ AS3 Worker thread, pops tenant to refresh from thread-safe set queue"""
+        # run sync loop
         while True:
             try:
                 network_id, device = self.queue.get()

octavia_f5/controller/worker/flows/f5_flows.py

@@ -29,14 +29,16 @@ def ensure_l2(self, selfips: [network_models.Port]) -> flow.Flow:
             ensure_selfip_subflow.add(ensure_selfip)
 
         ensure_routedomain = f5_tasks.EnsureRouteDomain()
-        ensure_route = f5_tasks.EnsureRoute()
+        ensure_default_route = f5_tasks.EnsureDefaultRoute()
+        ensure_static_routes = f5_tasks.SyncSubnetRoutes(inject={'selfips': selfips})
         ensure_vlan = f5_tasks.EnsureVLAN()
 
         ensure_l2_flow = linear_flow.Flow('ensure-l2-flow')
         ensure_l2_flow.add(ensure_vlan,
                            ensure_routedomain,
                            ensure_selfip_subflow,
-                           ensure_route)
+                           ensure_default_route,
+                           ensure_static_routes)
         return ensure_l2_flow
 
     def remove_l2(self, selfips: [str]) -> flow.Flow:
@@ -47,12 +49,14 @@ def remove_l2(self, selfips: [str]) -> flow.Flow:
                                                    inject={'port': selfip})
             cleanup_selfip_subflow.add(cleanup_selfip)
 
-        cleanup_route = f5_tasks.CleanupRoute()
+        cleanup_static_routes = f5_tasks.CleanupSubnetRoutes()
+        cleanup_route = f5_tasks.CleanupDefaultRoute()
         cleanup_routedomain = f5_tasks.CleanupRouteDomain()
         cleanup_vlan = f5_tasks.CleanupVLAN()
 
         cleanup_l2_flow = linear_flow.Flow('cleanup-l2-flow')
-        cleanup_l2_flow.add(cleanup_route,
+        cleanup_l2_flow.add(cleanup_static_routes,
+                            cleanup_route,
                             cleanup_selfip_subflow,
                             cleanup_routedomain,
                             cleanup_vlan)

octavia_f5/controller/worker/l2_sync_manager.py

@@ -25,7 +25,9 @@
 from octavia.common.base_taskflow import BaseTaskFlowEngine
 from octavia.network import base
 from octavia.network import data_models as network_models
+from octavia_f5.common import constants
 from octavia_f5.controller.worker.flows import f5_flows
+from octavia_f5.controller.worker.tasks import f5_tasks
 from octavia_f5.restclient.bigip import bigip_auth
 from octavia_f5.restclient.bigip.bigip_restclient import BigIPRestClient
 from octavia_f5.utils import driver_utils, decorators
@@ -121,6 +123,12 @@ def _do_sync_l2_selfips_flow(self, expected_selfips: [network_models.Port], stor
         with tf_logging.LoggingListener(e, log=LOG):
             e.run()
 
+    def _do_sync_l2_static_routes_flow(self, selfips: [network_models.Port], store: dict):
+        ensure_static_routes = f5_tasks.SyncSubnetRoutes(inject={'selfips': selfips})
+        e = self.taskflow_load(ensure_static_routes, store=store)
+        with tf_logging.LoggingListener(e, log=LOG):
+            e.run()
+
     def _do_remove_vcmp_l2_flow(self, store: dict):
         e = self.taskflow_load(self._f5flows.remove_vcmp_l2(), store=store)
         with tf_logging.DynamicLoggingListener(e, log=LOG):
@@ -245,6 +253,8 @@ def sync_l2_selfips_flow(self, selfips: [network_models.Port], network_id: str,
             fs[self.executor.submit(self._do_sync_l2_selfips_flow,
                                     expected_selfips=selfips_for_host,
                                     store=store)] = bigip
+            fs[self.executor.submit(self._do_sync_l2_static_routes_flow,
+                                    selfips=selfips_for_host, store=store)] = bigip
 
         done, not_done = futures.wait(fs, timeout=10)
         for f in done | not_done:
@@ -286,9 +296,18 @@ def full_sync(self, loadbalancers: [octavia_models.LoadBalancer]):
                     continue
 
                 # Skip unmanaged routes
-                if not (route['name'].startswith('net-') or route['name'].startswith('vlan-')):
+                if not (route['name'].startswith(constants.PREFIX_NETWORK_LEGACY)
+                        or route['name'].startswith('vlan-')
+                        or route['name'].startswith(constants.PREFIX_NETWORK)):
                     continue
 
+                # Skip routes for existing subnets
+                for network_id in networks:
+                    for subnet_id in networks[network_id].subnets:
+                        subnet_route_name = f5_tasks.get_subnet_route_name(network_id, subnet_id)
+                        if route['name'] == subnet_route_name:
+                            continue
+
                 # Cleanup
                 path = f"/mgmt/tm/net/route/{route['fullPath'].replace('/', '~')}"
                 fs.append(executor.submit(bigip.delete, path=path))

octavia_f5/controller/worker/tasks/f5_tasks.py

@@ -19,6 +19,7 @@
 from taskflow import task
 
 from octavia.network import data_models as network_models
+from octavia_f5.common import constants
 from octavia_f5.network import data_models as f5_network_models
 from octavia_f5.restclient.bigip import bigip_restclient
 from octavia_f5.utils import driver_utils, decorators
@@ -27,6 +28,11 @@
 CONF = cfg.CONF
 
 
+def get_subnet_route_name(network_id, subnet_id):
+    return "{}{}_{}{}".format(constants.PREFIX_NETWORK, network_id,
+                              constants.PREFIX_SUBNET, subnet_id)
+
+
 class EnsureVLAN(task.Task):
     default_provides = 'device_vlan'
 
@@ -225,7 +231,7 @@ def execute(self, bigip: bigip_restclient.BigIPRestClient,
                 and item['name'].startswith('port-')]
 
 
-class EnsureRoute(task.Task):
+class EnsureDefaultRoute(task.Task):
     default_provides = 'device_route'
 
     """ Task to create or update Route if needed """
@@ -272,10 +278,69 @@ def execute(self, bigip: bigip_restclient.BigIPRestClient,
         return device_route
 
 
+class SyncSubnetRoutes(task.Task):
+    """ Task to create static subnet routes """
+
+    @decorators.RaisesIControlRestError()
+    def execute(self, bigip: bigip_restclient.BigIPRestClient,
+                selfips: [network_models.Port],
+                network: f5_network_models.Network):
+
+        # Skip passive device if route_on_active is enabled
+        if CONF.networking.route_on_active and not bigip.is_active:
+            return None
+
+        def subnet_in_selfips(subnet, selfips):
+            for selfip in selfips:
+                for fixed_ip in selfip.fixed_ips:
+                    if fixed_ip.subnet_id == subnet:
+                        return True
+            return False
+
+        # Fetch existing routes
+        response = bigip.get(path=f"/mgmt/tm/net/route?$filter=partition+eq+Common").json()
+        existing_routes = response.get('items', [])
+
+        # subnet routes that must exist (routes already exist for SelfIPs)
+        subnets_that_need_routes = [subnet for subnet in network.subnets if not subnet_in_selfips(subnet, selfips)]
+
+        # delete existing subnet routes that aren't needed anymore - we'll only provision the missing ones
+        for existing_route in existing_routes:
+            existing_route_name = existing_route['name']
+
+            # ignore routes that are not subnet routes of this network
+            subnet_route_network_part = get_subnet_route_name(network.id, '')
+            if not existing_route_name.startswith(subnet_route_network_part):
+                continue
+
+            if existing_route_name in [get_subnet_route_name(network.id, sn) for sn in subnets_that_need_routes]:
+                # if the existing route is a needed subnet route neither delete nor create it
+                subnets_that_need_routes.remove(existing_route_name[len(subnet_route_network_part):])
+            else:
+                # Delete unneeded route
+                res = bigip.delete(path=f"/mgmt/tm/net/route/~Common~{existing_route_name}")
+                res.raise_for_status()
+
+        # Add missing subnet routes
+        network_driver = driver_utils.get_network_driver()
+        for subnet_id in subnets_that_need_routes:
+            cidr = IPNetwork(network_driver.get_subnet(subnet_id).cidr)
+
+            name = get_subnet_route_name(network.id, subnet_id)
+            vlan = f"/Common/vlan-{network.vlan_id}"
+            net = f"{cidr.ip}%{network.vlan_id}/{cidr.prefixlen}"
+            route = {'name': name, 'tmInterface': vlan, 'network': net}
+
+            res = bigip.post(path='/mgmt/tm/net/route', json=route)
+            res.raise_for_status()
+
+
 """ Cleanup Tasks """
 
 
-class CleanupRoute(task.Task):
+class CleanupDefaultRoute(task.Task):
+
+    @decorators.RaisesIControlRestError()
     def execute(self, network: f5_network_models.Network,
                 bigip: bigip_restclient.BigIPRestClient):
 
@@ -297,6 +362,33 @@ def execute(self, network: f5_network_models.Network,
                         bigip.hostname, network.id, network.vlan_id, res.content)
 
 
+class CleanupSubnetRoutes(task.Task):
+    """ Task to clean up static subnet routes.
+
+    We cannot clean up static subnet routes in SyncSubnetRoutes, because that function does not know whether the
+    network/tenant is to be removed completely, and thus blindly syncs routes for all subnets of the network,
+    even though they have to be deleted.
+    """
+
+    @decorators.RaisesIControlRestError()
+    def execute(self, bigip: bigip_restclient.BigIPRestClient,
+                network: f5_network_models.Network):
+
+        # prefix of subnet routes that belong to this network
+        subnet_route_network_prefix = "{}{}_{}".format(constants.PREFIX_NETWORK, network.id, constants.PREFIX_SUBNET)
+
+        # Fetch existing routes in the partition
+        response = bigip.get(path=f"/mgmt/tm/net/route").json()
+        existing_routes = response.get('items', [])
+
+        # delete routes from this network
+        for existing_route in existing_routes:
+            existing_route_name = existing_route['name']
+            if existing_route_name.startswith(subnet_route_network_prefix):
+                res = bigip.delete(path=f"/mgmt/tm/net/route/~Common~{existing_route_name}")
+                res.raise_for_status()
+
+
 class RemoveSelfIP(task.Task):
     def execute(self, port: network_models.Port,
                 bigip: bigip_restclient.BigIPRestClient):

octavia_f5/tests/unit/controller/worker/flows/test_f5_flows.py

@@ -81,7 +81,10 @@ def test_f5_flow_ensure_l2(self, mock_get_subnet):
         )
 
         mock_bigip = mock.Mock(spec=as3restclient.AS3RestClient)
-        mock_bigip.get.side_effect = empty_response
+        mock_bigip.get.side_effect = [empty_response(), empty_response(),
+                                      empty_response(), empty_response(),
+                                      empty_response(), empty_response(),
+                                      MockResponse({'items':[]}, status_code=200)]
         f5flows = f5_flows.F5Flows()
 
         engines.run(f5flows.ensure_l2([selfip_port]),
@@ -156,7 +159,8 @@ def test_f5_flow_ensure_existing_l2(self, mock_get_subnet):
         mock_bigip.get.side_effect = [mock_vlan_response,
                                       mock_routedomain_response,
                                       mock_selfip_response,
-                                      mock_route_response]
+                                      mock_route_response,
+                                      MockResponse({'items':[]}, status_code=200)]
         f5flows = f5_flows.F5Flows()
 
         engines.run(f5flows.ensure_l2([selfip_port]),

octavia_f5/tests/unit/controller/worker/tasks/test_f5_tasks.py

@@ -17,6 +17,7 @@
 from oslo_config import cfg
 from oslo_config import fixture as oslo_fixture
 from oslo_log import log as logging
+from oslo_utils import uuidutils
 from taskflow import engines
 
 import octavia.tests.unit.base as base
@@ -62,7 +63,7 @@ def test_EnsureRoute(self, mock_get_subnet):
         mock_bigip = mock.Mock(spec=as3restclient.AS3RestClient)
         mock_bigip.get.return_value = mock_route_response
 
-        engines.run(f5_tasks.EnsureRoute(),
+        engines.run(f5_tasks.EnsureDefaultRoute(),
                     store={'network': mock_network,
                            'bigip': mock_bigip,
                            'subnet_id': 'test-subnet-id'})
@@ -96,7 +97,7 @@ def test_EnsureRoute_legacy(self, mock_get_subnet):
         mock_bigip.get.side_effect = [test_f5_flows.MockResponse({}, 404),
                                       mock_route_response]
 
-        engines.run(f5_tasks.EnsureRoute(),
+        engines.run(f5_tasks.EnsureDefaultRoute(),
                     store={'network': mock_network,
                            'bigip': mock_bigip,
                            'subnet_id': 'test-subnet-id'})
@@ -134,7 +135,7 @@ def test_EnsureRoute_legacy_conflict(self, mock_get_subnet):
         # Patch should fail
         mock_bigip.patch.side_effect = test_f5_flows.empty_response
 
-        engines.run(f5_tasks.EnsureRoute(),
+        engines.run(f5_tasks.EnsureDefaultRoute(),
                     store={'network': mock_network,
                            'bigip': mock_bigip,
                            'subnet_id': 'test-subnet-id'})
@@ -150,3 +151,81 @@ def test_EnsureRoute_legacy_conflict(self, mock_get_subnet):
         mock_bigip.post.assert_called_with(json={
             'name': 'vlan-1234', 'gw': '8.8.8.8%1234', 'network': 'default%1234'},
             path='/mgmt/tm/net/route')
+
+    @mock.patch("octavia.network.drivers.noop_driver.driver.NoopManager"
+                ".get_subnet")
+    def test_SyncSubnetRoutes(self, mock_get_subnet):
+        mock_subnets = [
+            network_models.Subnet(
+                id=uuidutils.generate_uuid(), gateway_ip='2.3.4.5',
+                cidr='2.3.4.0/24', network_id='test-network-id'),
+            network_models.Subnet(
+                id=uuidutils.generate_uuid(), gateway_ip='10.0.0.1',
+                cidr='10.0.0.0/24', network_id='test-network-id'),
+        ]
+        mock_get_subnet.side_effect = mock_subnets
+        mock_network = f5_network_models.Network(
+            mtu=9000, id=uuidutils.generate_uuid(),
+            subnets=[subnet.id for subnet in mock_subnets],
+            segments=[{'provider:physical_network': 'physnet',
+                       'provider:segmentation_id': 1234}]
+        )
+
+        # Check that subnet route names always include the network ID as well as the subnet ID
+        subnet_route_name = f5_tasks.get_subnet_route_name(mock_network.id, mock_subnets[0].id)
+        self.assertTrue(mock_network.id in subnet_route_name
+                        or mock_network.id.replace('-', '_') in subnet_route_name)
+        self.assertTrue(mock_subnets[0].id in subnet_route_name
+                        or mock_subnets[0].id.replace('-', '_') in subnet_route_name)
+
+        # No subnet route shall be created when every subnet already has either a SelfIP or a subnet route
+        mock_route_response = test_f5_flows.MockResponse({
+            'items': [
+                {
+                    'name': f5_tasks.get_subnet_route_name(mock_network.id, mock_subnets[1].id),
+                    'tmInterface': 'vlan-1234',
+                    'network': '10.0.0.2%1234/24'
+                }
+            ]
+        }, status_code=200)
+
+        mock_bigip = mock.Mock(spec=as3restclient.AS3RestClient)
+        mock_bigip.get.return_value = mock_route_response
+        mock_selfip = network_models.Port(
+            name=f"local-bigipmockhost-test-subnet-id",
+            fixed_ips=[network_models.FixedIP(
+                ip_address='2.3.4.255',
+                subnet_id=mock_subnets[0].id)
+            ]
+        )
+
+        engines.run(f5_tasks.SyncSubnetRoutes(),
+                    store={'network': mock_network,
+                           'bigip': mock_bigip,
+                           'selfips': [mock_selfip]})
+
+        mock_bigip.get.assert_called_with(
+            path=f"/mgmt/tm/net/route?$filter=partition+eq+Common")
+        mock_bigip.post.assert_not_called()
+        mock_bigip.delete.assert_not_called()
+
+        # Check creating new route
+        mock_bigip = mock.Mock(spec=as3restclient.AS3RestClient)
+        mock_route_response = test_f5_flows.MockResponse({'items': []}, status_code=200)
+        mock_bigip.get.return_value = mock_route_response
+        engines.run(f5_tasks.SyncSubnetRoutes(),
+                    store={'network': mock_network,
+                           'bigip': mock_bigip,
+                           'selfips': [mock_selfip]})
+
+        mock_bigip.get.assert_called_with(
+            path=f"/mgmt/tm/net/route?$filter=partition+eq+Common")
+        mock_bigip.delete.assert_not_called()
+        mock_bigip.post.assert_called_with(
+            path='/mgmt/tm/net/route',
+            json={
+                'name': f5_tasks.get_subnet_route_name(mock_network.id, mock_subnets[1].id),
+                'tmInterface': '/Common/vlan-1234',
+                'network': '2.3.4.0%1234/24'
+            }
+        )