From e3cb1b4a472e0c2743690af62768a96a1c2b8c00 Mon Sep 17 00:00:00 2001
From: Dmitrii Shcherbakov <Dmitrii.S@protonmail.com>
Date: Thu, 27 Jun 2024 17:17:37 +0300
Subject: [PATCH] Make the local chassis ID configurable

ovn-controller can be configured with a system-id override file or
-n option to use any suffix for OVSDB options. Currently ovn-bgp-agent
just uses a constant "bgp" for this and there is no way to change it.

https://github.com/ovn-org/ovn/commit/48db2a7a353a81aaa1795ef4b35b0fab1f0b0ccc

Let's allow this suffix to be set via a config option instead.

Change-Id: Ida30b46e16ddcdc2d82af980546876f72a845c1d
---
 .../drivers/ovn_bgp_mode_design.rst           | 74 ++++++++++++++++++-
 ovn_bgp_agent/config.py                       |  7 ++
 ovn_bgp_agent/constants.py                    |  1 -
 ovn_bgp_agent/drivers/openstack/utils/wire.py |  4 +-
 .../unit/drivers/openstack/utils/test_wire.py |  4 +-
 ovn_bgp_agent/utils/linux_net.py              |  2 +-
 6 files changed, 83 insertions(+), 9 deletions(-)

diff --git a/doc/source/contributor/drivers/ovn_bgp_mode_design.rst b/doc/source/contributor/drivers/ovn_bgp_mode_design.rst
index dcf8a73e..60c2f83e 100644
--- a/doc/source/contributor/drivers/ovn_bgp_mode_design.rst
+++ b/doc/source/contributor/drivers/ovn_bgp_mode_design.rst
@@ -69,12 +69,15 @@ The OVN routing architecture proposes the following mapping:
   (``br-osp``).
 
 - ``br-osp`` does not have any physical resources attached, just patch
-  ports connecting them to ``br-int`` and ``br-bgp``.
+  ports connecting them to ``br-int`` and ``br-bgp``. The name of this bridge can
+  be arbitrary so long as ovn-bridge-mappings for both OVN clusters (the primary and
+  the extra node-local one) are set accordingly.
 
 - ``br-bgp`` is the integration bridge managed by the extra OVN cluster
   deployed per node. This is where the virtual OVN resources are be created
-  (routers and switches). It creates mappings to ``br-osp`` and ``br-ex``
-  (patch ports).
+  (routers and switches). OVN creates patch ports between ``br-osp`` and ``br-ex``
+  based on ovn-bridge-mappings set for the extra OVN cluster. The name of this bridge
+  is configurable and can be changed by setting a relevant instance-specific OVSDB option.
 
 - ``br-ex`` keeps being the external bridge, where the physical NICs are
   attached (as in default environments without BGP). But instead of being
@@ -236,6 +239,71 @@ range for the provider networks to expose/handle:
     external_nics=eth1,eth2
     peer_ips=100.64.1.5,100.65.1.5
     provider_networks_pool_prefixes=172.16.0.0/16
+    # This will be used as a suffix for options relevant to the node-local OVN.
+    bgp_chassis_id = bgp
+
+
+Multiple OVN Controllers
+++++++++++++++++++++++++
+
+This mode relies on running two ovn-controllers on the same host. However, a single
+OVSDB is shared for both controllers. To achieve that, OVN supports having option
+suffixes for options stored in OVSDB that look like this: ``<option>-<system-id>``.
+
+One of the ``ovn-controller`` instances will need to have ``-n <system-id-override>``
+passed in via command-line arguments to the daemon. Alternatively, if the second
+ovn-controller is run in a container, ``/etc/<ovn-config-dir>/system-id-override``
+can be provided to override the system-id.
+
+``ovn-bgp-agent`` itself needs to parse bridge-mappings related to the local OVN instance
+and by default uses ``bgp_chassis_id`` config option set to `bgp` making it look for
+bridge mappings in the ``ovn-bridge-mappings-bgp`` option. Make sure to set this option
+correctly, otherwise, ``ovn-bgp-agent`` will not create the necessary local ``ovn-nb`` state
+and, as a result, no patch ports will be created between ``br-bgp`` and ``br-ex``.
+
+An example of how to set relevant OVSDB options for both ``ovn-controller``s via ``ovs-vsctl``:
+
+  .. code-block:: bash
+
+    # Set the hostname that will be used by both ovn-controllers.
+    ovs-vsctl set open . external-ids:hostname=<desired-hostname>
+
+    # This is optional as it matches the default integration bridge
+    # name in OVN but present here to clarify the difference with the
+    # extra OVN cluster config.
+    ovs-vsctl set open . external-ids:ovn-bridge=br-int
+
+    # Bridge mappings for the primary OVN cluster's ovn-controller.
+    ovs-vsctl set open . external-ids:ovn-bridge-mappings=provider:br-osp
+    # Set the IP to be used for a tunnel endpoint.
+    ovs-vsctl set open . external-ids:ovn-encap-ip=<desired-vtep-ip>
+    # Set the desired encapsulation (will apply to both ovn-controllers as there's no
+    # suffixed override):
+    ovs-vsctl set open . external-ids:ovn-encap-type=geneve
+    # Assuming the primary OVN deployment has a clustered ovn-sb setup with 3 IPs
+    # and listening on port 6642:
+    ovs-vsctl set open . external-ids:ovn-remote="ssl:<primary-ovn-sb-ip-1>:6642,ssl:<primary-ovn-sb-ip-2>:6642,ssl:<primary-ovn-sb-ip-3>:6642"
+
+    # Set the integation bridge name for the extra OVN deployment
+    # (this overrides the default br-int):
+    ovs-vsctl set open . external-ids:ovn-bridge-bgp=br-bgp
+
+    # Set the bridge mappings for the extra OVN's ovn-controller instance. Note that
+    # there will be localnet ports on both the northbound and southbound side of br-bgp
+    # as a result.
+    ovs-vsctl set open . external-ids:ovn-bridge-mappings-bgp=dcfabric:br-ex,local:br-osp
+
+    # Make sure that the local ovn-controller speaks to the local ovn-sb.
+    ovs-vsctl set open . external-ids:ovn-remote-bgp=unix:/var/run/ovn/ovnsb_db.sock
+
+    # Have to set both ovn-encap-ip (taken from the option without suffix) and ovn-encap-ip
+    # in order for ovn-controller to start successfully.
+    # Since we only use localnet ports for the extra cluster, we can set this IP to the localhost IP.
+    ovs-vsctl set open . external-ids:ovn-encap-ip-bgp=127.0.0.1
+
+    # Enable hardware offload if your hardware supports it which will apply to the state created
+    # by both ovn-controllers.
+    ovs-vsctl set open . other-config:hw-offload=true
 
 
 Limitations
diff --git a/ovn_bgp_agent/config.py b/ovn_bgp_agent/config.py
index b2a3fced..63a04f65 100644
--- a/ovn_bgp_agent/config.py
+++ b/ovn_bgp_agent/config.py
@@ -231,6 +231,13 @@ local_ovn_cluster_opts = [
     cfg.ListOpt('provider_networks_pool_prefixes',
                 default=['192.168.0.0/16'],
                 help='List of prefixes for provider networks'),
+    cfg.ListOpt('bgp_chassis_id',
+                default='bgp',
+                help='The chassis_id used for the ovn-controller instance'
+                     ' related to the node-local OVN instance. Used as a'
+                     ' suffix for getting instance-specific options'
+                     ' from OVSDB. This option has effect only when the OVN'
+                     ' NB driver is used.'),
 ]
 
 CONF = cfg.CONF
diff --git a/ovn_bgp_agent/constants.py b/ovn_bgp_agent/constants.py
index 944cfd39..e6fa72db 100644
--- a/ovn_bgp_agent/constants.py
+++ b/ovn_bgp_agent/constants.py
@@ -133,7 +133,6 @@ ADVERTISEMENT_METHOD_HOST = 'host'
 ADVERTISEMENT_METHOD_SUBNET = 'subnet'
 
 # OVN Cluster related constants
-OVN_CLUSTER_BRIDGE = 'bgp'
 OVN_CLUSTER_ROUTER = 'bgp-router'
 OVN_CLUSTER_ROUTER_INTERNAL_MAC = '40:44:00:00:00:06'
 
diff --git a/ovn_bgp_agent/drivers/openstack/utils/wire.py b/ovn_bgp_agent/drivers/openstack/utils/wire.py
index 69f39791..8fd5b916 100644
--- a/ovn_bgp_agent/drivers/openstack/utils/wire.py
+++ b/ovn_bgp_agent/drivers/openstack/utils/wire.py
@@ -239,7 +239,7 @@ def _ensure_base_wiring_config_ovn(ovs_idl, ovn_idl):
 
     # LS
     bgp_bridge_mappings = ovs_idl.get_ovn_bridge_mappings(
-        bridge=constants.OVN_CLUSTER_BRIDGE)
+        bridge=CONF.local_ovn_cluster.bgp_chassis_id)
     for bridge_mapping in bgp_bridge_mappings:
         network, bridge = helpers.parse_bridge_mapping(bridge_mapping)
         if not network:
@@ -347,7 +347,7 @@ def _ensure_ovn_network_link(ovn_idl, switch_name, direction,
         # bind to local chassis
         # ovn-nbctl lrp-set-gateway-chassis  bgp-router-public bgp 1
         cmds.append(ovn_idl.lrp_set_gateway_chassis(
-            r_port_name, constants.OVN_CLUSTER_BRIDGE, 1))
+            r_port_name, CONF.local_ovn_cluster.bgp_chassis_id, 1))
     else:  # direction == 'external'
         # Connect BGP router to the external logical switch
         r_port_name = "{}-{}".format(constants.OVN_CLUSTER_ROUTER, switch_name)
diff --git a/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_wire.py b/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_wire.py
index fd82b1d6..2b6fe5b9 100644
--- a/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_wire.py
+++ b/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_wire.py
@@ -180,7 +180,7 @@ class TestWire(test_base.TestCase):
         m_ensure_lsp.assert_called_once_with(
             self.nb_idl, mock.ANY, switch_name, 'router', 'router', **options)
         self.nb_idl.lrp_set_gateway_chassis.assert_called_once_with(
-            r_port_name, constants.OVN_CLUSTER_BRIDGE, 1)
+            r_port_name, CONF.local_ovn_cluster.bgp_chassis_id, 1)
 
     @mock.patch.object(wire, '_execute_commands')
     @mock.patch.object(wire, '_ensure_lsp_cmds')
@@ -205,7 +205,7 @@ class TestWire(test_base.TestCase):
         m_ensure_lsp.assert_called_once_with(
             self.nb_idl, mock.ANY, switch_name, 'router', 'router', **options)
         self.nb_idl.lrp_set_gateway_chassis.assert_called_once_with(
-            r_port_name, constants.OVN_CLUSTER_BRIDGE, 1)
+            r_port_name, CONF.local_ovn_cluster.bgp_chassis_id, 1)
 
     @mock.patch.object(wire, '_ensure_lsp_cmds')
     def test__ensure_ovn_network_link_external(self, m_ensure_lsp):
diff --git a/ovn_bgp_agent/utils/linux_net.py b/ovn_bgp_agent/utils/linux_net.py
index 4dc25740..e396a5d6 100644
--- a/ovn_bgp_agent/utils/linux_net.py
+++ b/ovn_bgp_agent/utils/linux_net.py
@@ -179,7 +179,7 @@ def ensure_anycast_mac_for_interface(intf, offset):
         # Also update the 'scope link' address on the interface.
         ll = lladdr.replace(':', '')
         ll_ip_address = ipaddress.IPv6Address(
-            f'fe80::{ll[0:4]}:{ll[4:8]}:{ll[8:12]}')
+            f'fe80::{ll[0:4]}:{ll[4:8]}:{ll[8:12]}')  # noqa: E231
         ll_net = ipaddress.IPv6Network('fe80::/10')
 
         # Fetch all ipv6 addresses and check if we already configured the