From 181352f13a52ea947bf3a45e10462e4f3c3ab4c0 Mon Sep 17 00:00:00 2001 From: Mark Goddard Date: Tue, 6 Apr 2021 17:10:39 +0100 Subject: [PATCH] Ubuntu: support policy-based routing in systemd-networkd This change adds support for policy-based routing via systemd-networkd. Due to differences in the configuration mechanism, routing policy rules are configured via dicts for Ubuntu, while remaining as strings on CentOS. Ideally we would support both formats. Story: 2004960 Task: 42217 Change-Id: I77aec0160eb7e4dd763326bfe6e3d9a44b248108 --- ansible/roles/network-debian/tasks/main.yml | 9 +++ .../configuration/reference/network.rst | 35 +++++++-- kayobe/plugins/filter/networkd.py | 71 +++++++++++++++---- kayobe/plugins/filter/networks.py | 16 +++++ .../unit/plugins/filter/test_networkd.py | 28 ++++++++ .../overrides.yml.j2 | 16 +++++ .../tests/test_overcloud_host_configure.py | 7 ++ 7 files changed, 166 insertions(+), 16 deletions(-) diff --git a/ansible/roles/network-debian/tasks/main.yml b/ansible/roles/network-debian/tasks/main.yml index 76c74635b..22740e0ff 100644 --- a/ansible/roles/network-debian/tasks/main.yml +++ b/ansible/roles/network-debian/tasks/main.yml @@ -4,6 +4,15 @@ when: resolv_is_managed | bool become: True +- name: Ensure IP routing tables are defined for iproute2 + become: true + blockinfile: + dest: /etc/iproute2/rt_tables + block: | + {% for table in network_route_tables %} + {{ table.id }} {{ table.name }} + {% endfor %} + - name: Remove netplan.io packages become: true package: diff --git a/doc/source/configuration/reference/network.rst b/doc/source/configuration/reference/network.rst index e4e685931..3084d021d 100644 --- a/doc/source/configuration/reference/network.rst +++ b/doc/source/configuration/reference/network.rst @@ -67,8 +67,10 @@ supported: table to which the route will be added. ``options`` is a list of option strings to add to the route. ``rules`` - List of IP routing rules. Each item should be an ``iproute2`` IP routing - rule. + List of IP routing rules. + + On CentOS, each item should be a string describing an ``iproute2`` IP + routing rule. ``physical_network`` Name of the physical network on which this network exists. This aligns with the physical network concept in neutron. @@ -258,8 +260,14 @@ Configuring IP Routing Policy Rules ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ IP routing policy rules may be configured by setting the ``rules`` attribute -for a network to a list of rules. The format of a rule is the string which -would be appended to ``ip rule `` to create or delete the rule. +for a network to a list of rules. The format of each rule currently differs +between CentOS and Ubuntu. + +CentOS +"""""" + +The format of a rule is the string which would be appended to ``ip rule +`` to create or delete the rule. To configure a network called ``example`` with an IP routing policy rule to handle traffic from the subnet ``10.1.0.0/24`` using the routing table @@ -273,6 +281,25 @@ handle traffic from the subnet ``10.1.0.0/24`` using the routing table These rules will be configured on all hosts to which the network is mapped. +Ubuntu +"""""" + +The format of a rule is a dictionary with optional items ``from``, ``to``, +``priority``, and ``table``. + +To configure a network called ``example`` with an IP routing policy rule to +handle traffic from the subnet ``10.1.0.0/24`` using the routing table +``exampleroutetable``: + +.. code-block:: yaml + :caption: ``networks.yml`` + + example_rules: + - from: 10.1.0.0/24 + table: exampleroutetable + +These rules will be configured on all hosts to which the network is mapped. + Configuring IP Routes on Specific Tables ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/kayobe/plugins/filter/networkd.py b/kayobe/plugins/filter/networkd.py index 5f0934743..552722472 100644 --- a/kayobe/plugins/filter/networkd.py +++ b/kayobe/plugins/filter/networkd.py @@ -194,6 +194,53 @@ def _veth_netdev(context, veth, inventory_hostname): return _filter_options(config) +def _network_routes(routes, route_tables): + """Return a list of routes for a networkd network. + + :param routes: a list of route dictionaries. + :param route_tables: a dict mapping route table names to IDs. + :returns: a list of routes for a networkd network. + """ + return [ + { + 'Route': [ + # FIXME(mgoddard): No support for 'options'. + {'Destination': route['cidr']}, + {'Gateway': route.get('gateway')}, + {'Table': route_tables.get(route.get('table'), + route.get('table'))}, + ] + } + for route in routes or [] + ] + + +def _network_rules(rules, route_tables): + """Return a list of routing policy rules for a networkd network. + + :param rules: a list of rule dictionaries. + :param route_tables: a dict mapping route table names to IDs. + :returns: a list of rules for a networkd network. + """ + for rule in rules or []: + if not isinstance(rule, dict): + raise errors.AnsibleFilterError( + "Routing policy rules must be defined in dictionary " + "format for systemd-networkd") + return [ + { + 'RoutingPolicyRule': [ + {'From': rule.get("from")}, + {'To': rule.get("to")}, + {'Priority': rule.get("priority")}, + {'Table': route_tables.get(rule.get('table'), + rule.get('table'))}, + ] + } + for rule in rules or [] + ] + + def _network(context, name, inventory_hostname, bridge, bond, vlan_interfaces): """Return a networkd network for an interface. @@ -205,7 +252,7 @@ def _network(context, name, inventory_hostname, bridge, bond, vlan_interfaces): :param bond: Name of a bond of which the interface is a member, or None. :param vlan_interfaces: List of VLAN subinterfaces of the interface. """ - # FIXME(mgoddard): Currently does not support: rules, ethtool_opts, zone, + # FIXME(mgoddard): Currently does not support: ethtool_opts, zone, # allowed_addresses. device = networks.net_interface(context, name, inventory_hostname) ip = networks.net_ip(context, name, inventory_hostname) @@ -223,6 +270,7 @@ def _network(context, name, inventory_hostname, bridge, bond, vlan_interfaces): mtu = networks.net_mtu(context, name, inventory_hostname) routes = networks.net_routes(context, name, inventory_hostname) + rules = networks.net_rules(context, name, inventory_hostname) bootproto = networks.net_bootproto(context, name, inventory_hostname) defroute = networks.net_defroute(context, name, inventory_hostname) if defroute is not None: @@ -256,17 +304,16 @@ def _network(context, name, inventory_hostname, bridge, bond, vlan_interfaces): ] }, ] - if routes: - config += [ - { - 'Route': [ - # FIXME(mgoddard): No support for 'options'. - {'Destination': route['cidr']}, - {'Gateway': route.get('gateway')}, - ] - } - for route in routes or [] - ] + + # NOTE(mgoddard): Systemd-networkd does not support named route tables + # until v248. Until then, translate names to numeric IDs using the + # network_route_tables variable. + route_tables = utils.get_hostvar(context, "network_route_tables", + inventory_hostname) + route_tables = {table["name"]: table["id"] for table in route_tables} + config += _network_routes(routes, route_tables) + config += _network_rules(rules, route_tables) + return _filter_options(config) diff --git a/kayobe/plugins/filter/networks.py b/kayobe/plugins/filter/networks.py index 058fe2224..a6c4b6cf7 100644 --- a/kayobe/plugins/filter/networks.py +++ b/kayobe/plugins/filter/networks.py @@ -307,6 +307,19 @@ def _route_obj(route): return route_obj +def _validate_rules(rules): + """Validate the format of policy-based routing rules. + + :param rules: a list of rules or None. + :raises: AnsibleFilterError if any rule is invalid. + """ + for rule in rules or []: + if not isinstance(rule, str): + raise errors.AnsibleFilterError( + "Routing policy rules must be defined in string format " + "for CentOS") + + @jinja2.contextfilter def net_interface_obj(context, name, inventory_hostname=None): """Return a dict representation of a network interface. @@ -338,6 +351,7 @@ def net_interface_obj(context, name, inventory_hostname=None): zone = net_zone(context, name, inventory_hostname) vip_address = net_vip_address(context, name, inventory_hostname) allowed_addresses = [vip_address] if vip_address else None + _validate_rules(rules) interface = { 'device': device, 'address': ip, @@ -390,6 +404,7 @@ def net_bridge_obj(context, name, inventory_hostname=None): zone = net_zone(context, name, inventory_hostname) vip_address = net_vip_address(context, name, inventory_hostname) allowed_addresses = [vip_address] if vip_address else None + _validate_rules(rules) interface = { 'device': device, 'address': ip, @@ -450,6 +465,7 @@ def net_bond_obj(context, name, inventory_hostname=None): zone = net_zone(context, name, inventory_hostname) vip_address = net_vip_address(context, name, inventory_hostname) allowed_addresses = [vip_address] if vip_address else None + _validate_rules(rules) interface = { 'device': device, 'address': ip, diff --git a/kayobe/tests/unit/plugins/filter/test_networkd.py b/kayobe/tests/unit/plugins/filter/test_networkd.py index 8264301a6..5d17f1cb1 100644 --- a/kayobe/tests/unit/plugins/filter/test_networkd.py +++ b/kayobe/tests/unit/plugins/filter/test_networkd.py @@ -48,6 +48,8 @@ class BaseNetworkdTest(unittest.TestCase): "network_patch_prefix": "p-", "network_patch_suffix_ovs": "-ovs", "network_patch_suffix_phy": "-phy", + # List of route tables. + "network_route_tables": [], } def setUp(self): @@ -317,6 +319,18 @@ class TestNetworkdNetworks(BaseNetworkdTest): }, { "cidr": "1.2.6.0/24", + "table": 42, + }, + ], + "net1_rules": [ + { + "from": "1.2.7.0/24", + "table": 43, + }, + { + "to": "1.2.8.0/24", + "table": 44, + "priority": 1, }, ], "net1_bootproto": "dhcp", @@ -358,6 +372,20 @@ class TestNetworkdNetworks(BaseNetworkdTest): { "Route": [ {"Destination": "1.2.6.0/24"}, + {"Table": 42}, + ] + }, + { + "RoutingPolicyRule": [ + {"From": "1.2.7.0/24"}, + {"Table": 43}, + ] + }, + { + "RoutingPolicyRule": [ + {"To": "1.2.8.0/24"}, + {"Priority": 1}, + {"Table": 44}, ] }, ] diff --git a/playbooks/kayobe-overcloud-host-configure-base/overrides.yml.j2 b/playbooks/kayobe-overcloud-host-configure-base/overrides.yml.j2 index 7ffe1ef65..39b1b6c81 100644 --- a/playbooks/kayobe-overcloud-host-configure-base/overrides.yml.j2 +++ b/playbooks/kayobe-overcloud-host-configure-base/overrides.yml.j2 @@ -19,6 +19,11 @@ controller_extra_network_interfaces: - test_net_bond - test_net_bond_vlan +# Custom IP routing tables. +network_route_tables: + - id: 2 + name: kayobe-test-route-table + # dummy2: Ethernet interface. test_net_eth_cidr: 192.168.34.0/24 test_net_eth_routes: @@ -30,6 +35,17 @@ test_net_eth_interface: dummy2 test_net_eth_vlan_cidr: 192.168.35.0/24 test_net_eth_vlan_interface: "{% raw %}{{ test_net_eth_interface }}.{{ test_net_eth_vlan_vlan }}{% endraw %}" test_net_eth_vlan_vlan: 42 +test_net_eth_vlan_routes: + - cidr: 192.168.40.0/24 + gateway: 192.168.35.254 + table: kayobe-test-route-table +test_net_eth_vlan_rules: +{% if ansible_os_family == 'RedHat' %} + - from 192.168.35.0/24 table kayobe-test-route-table +{% else %} + - from: 192.168.35.0/24 + table: kayobe-test-route-table +{% endif %} # br0: bridge with ports dummy3, dummy4. test_net_bridge_cidr: 192.168.36.0/24 diff --git a/playbooks/kayobe-overcloud-host-configure-base/tests/test_overcloud_host_configure.py b/playbooks/kayobe-overcloud-host-configure-base/tests/test_overcloud_host_configure.py index 6f44c59e8..58654cf90 100644 --- a/playbooks/kayobe-overcloud-host-configure-base/tests/test_overcloud_host_configure.py +++ b/playbooks/kayobe-overcloud-host-configure-base/tests/test_overcloud_host_configure.py @@ -28,6 +28,13 @@ def test_network_ethernet_vlan(host): assert interface.exists assert '192.168.35.1' in interface.addresses assert host.file('/sys/class/net/dummy2.42/lower_dummy2').exists + routes = host.check_output( + '/sbin/ip route show dev dummy2.42 table kayobe-test-route-table') + assert '192.168.40.0/24 via 192.168.35.254' in routes + rules = host.check_output( + '/sbin/ip rule show table kayobe-test-route-table') + expected = 'from 192.168.35.0/24 lookup kayobe-test-route-table' + assert expected in rules def test_network_bridge(host):