On 3/17/25 3:59 PM, Dumitru Ceara wrote: > On 3/14/25 7:04 PM, Frode Nordahl wrote: >> Hello, MJ, >> > > Hi, MJ, Frode, > >> Thank you for your work on this! >> >> Note that this is not a full review, I spotted that there are some >> assumptions for Ubuntu image in the code and wanted to shoot in a quick >> comment to help anyone attempting to review it on other images. >> > > Thanks for the patch and for the initial review! > >> On 13.03.2025 17:43, MJ Ponsonby wrote: >>> Requires frr to be installed on ovn-gw-1 and ovn-gw-2 pending the change >>> to the multinode framework for the CI. > > While we're at it, I think the ovn-fake-multinode PR that adds frr to > the list of packages to be installed needs a few small changes: > > https://github.com/ovn-org/ovn-fake-multinode/pull/97 > > Once those are addressed I can merge it and I can tag a new > ovn-fake-multinode version, v0.3. Then I can take care of updating the > existing system multinode OVN tests to use the new version. The > multinode.at suite runs periodically (daily) on the OVN GitHub repo so > the new BGP tests would also be exercised there. >
MJ, I just merged your updated ovn-fake-multinode PR and tagged ovn-fake-multinode v0.3 (cc Numan). As discussed during the community meeting earlier, the next step would be to update the existing OVN multinode tests and make sure they work with ovn-fake-multinode v0.3. Please let me know if you need help with that. Regards, Dumitru >> >> nit: The above paragraph is to support reviewers, and is likely not >> suitable for forever storage in the actual commit message. While >> putting this information here is very useful, you can put it below the >> `---` to get it on the mailing list but not in the commit message. >> >>> This tests OVN BGP capabilities in a multinode environment by setting up >>> two sets of a ToR switch, connected to an OVN node with BGP set up. The >>> pair then form a connection and then the ToR switch can connect to a >>> guest-vm which is accessible via a distributed gateway logical router >>> port. >>> >>> Signed-off-by: MJ Ponsonby <[email protected]> >>> --- >>> tests/multinode.at | 228 +++++++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 228 insertions(+) >>> >>> diff --git a/tests/multinode.at b/tests/multinode.at >>> index 68c9eba22..c3e73443a 100644 >>> --- a/tests/multinode.at >>> +++ b/tests/multinode.at >>> @@ -3030,4 +3030,232 @@ m_as ovn-chassis-3 killall tcpdump >>> >>> AT_CLEANUP >>> >>> +AT_SETUP([ovn multinode bgp unnumbered]) >>> >>> +check_fake_multinode_setup >>> +cleanup_multinode_resources >>> + >>> + >>> +setup_frr() { >>> + container_prefix=$1 >>> + container_number=$2 >>> + br_name=br-$container_prefix-$container_number >>> + >>> + check m_as $container_prefix-$container_number ovs-vsctl add-br >>> $br_name >>> + on_exit "m_as $container_prefix-$container_number ovs-vsctl del- >>> br br-$container_prefix-$container_number" >>> + check m_as $container_prefix-$container_number ip netns add frr-ns >>> + on_exit "m_as $container_prefix-$container_number ip netns del >>> frr-ns" >>> + check m_as $container_prefix-$container_number ip netns exec frr- >>> ns ip link set lo up >>> + m_as $container_prefix-$container_number ovs-vsctl add-port >>> $br_name ext0 -- set interface ext0 type=internal >>> + m_as $container_prefix-$container_number ovs-vsctl add-port >>> $br_name ext1 -- set interface ext1 type=internal >>> + >>> + m_as $container_prefix-$container_number ip link set ext1 netns >>> frr-ns >>> + m_as $container_prefix-$container_number ip netns exec frr-ns ip >>> link set ext1 up >>> + m_as $container_prefix-$container_number ip netns exec frr-ns ip >>> addr add >>> 4$container_number.4$container_number.4$container_number.4$container_number >>> dev ext1 >>> + m_as $container_prefix-$container_number ip link set ext0 up >>> + >>> + check m_as $container_prefix-$container_number sed -i 's/bgpd=no/ >>> bgpd=yes/g' /etc/frr/daemons >>> + check m_as $container_prefix-$container_number sed -i 's/ >>> StartLimitBurst=.*/StartLimitBurst=100/g' /usr/lib/systemd/system/ >>> frr.service >>> + check m_as $container_prefix-$container_number systemctl daemon- >>> reload >>> + >>> + check m_as $container_prefix-$container_number mkdir -p /etc/frr/ >>> frr-ns >>> + check m_as $container_prefix-$container_number cp -r /etc/frr/ >>> daemons /etc/frr/frr.conf /etc/frr/support_bundle_commands.conf /etc/ >>> frr/frr-ns/ >>> + on_exit "m_as $container_prefix-$container_number rm -rf /etc/ >>> frr/frr-ns" >>> + check m_as $container_prefix-$container_number rm -rf /etc/frr/ >>> frr-ns/vtysh.conf >>> + check m_as $container_prefix-$container_number touch /etc/frr/ >>> frr-ns/vtysh.conf >>> + >>> + check m_as $container_prefix-$container_number systemctl stop frr >>> + >>> + m_as $container_prefix-$container_number ip netns exec frr-ns / >>> usr/lib/frr/frrinit.sh start frr-ns >>> + on_exit "m_as $container_prefix-$container_number ip netns exec >>> frr-ns /usr/lib/frr/frrinit.sh stop frr-ns" >>> + on_exit "m_as $container_prefix-$container_number systemctl stop >>> frr" >>> + >>> + check m_as $container_prefix-$container_number systemctl start frr >> >> Note that the above may not work on the fedora image used in the CI, >> something like the diff below would make it work there: >> >> diff --git a/tests/multinode.at b/tests/multinode.at >> index 23ded1236..8626f2fd4 100644 >> --- a/tests/multinode.at >> +++ b/tests/multinode.at >> @@ -23,13 +23,16 @@ setup_frr() { >> check m_as $container_prefix-$container_number systemctl daemon-reload >> >> check m_as $container_prefix-$container_number mkdir -p /etc/frr/ >> frr-ns >> - check m_as $container_prefix-$container_number cp -r /etc/frr/ >> daemons /etc/frr/frr.conf /etc/frr/support_bundle_commands.conf /etc/ >> frr/frr-ns/ >> + check m_as $container_prefix-$container_number chown -R frr:frr / >> etc/frr/frr-ns >> + check m_as $container_prefix-$container_number mkdir -p /run/frr/ >> frr-ns >> + check m_as $container_prefix-$container_number chown -R frr:frr / >> run/frr/frr-ns >> + check m_as $container_prefix-$container_number cp -r /etc/frr/ >> daemons /etc/frr/frr.conf /etc/frr/frr-ns/ >> check m_as $container_prefix-$container_number rm -rf /etc/frr/frr- >> ns/vtysh.conf >> check m_as $container_prefix-$container_number touch /etc/frr/frr- >> ns/vtysh.conf >> >> check m_as $container_prefix-$container_number systemctl stop frr >> >> - m_as $container_prefix-$container_number ip netns exec frr-ns /usr/ >> lib/frr/frrinit.sh start frr-ns >> + m_as $container_prefix-$container_number ip netns exec frr-ns /usr/ >> libexec/frr/frrinit.sh start frr-ns >> >> check m_as $container_prefix-$container_number systemctl start frr >> >> @@ -50,7 +53,7 @@ setup_frr() { >> neighbor ext1 soft-reconfiguration inbound >> neighbor ext1 activate >> exit-address-family >> - !" | podman exec -i $container_prefix-$container_number vtysh -N >> frr-ns >> + !" | podman exec -i $container_prefix-$container_number vtysh -- >> vty_socket /run/frr/frr-ns >> } >> >> setup_ovn_bgp() { >> --- >> >> We of course have an interest in running this on multiple container >> images. Is there any way we could make the code conditionally handle >> this at runtime? >> > > Frode, I'm assuming you were thinking of distro-specific wrappers in the > test suite for some of the parts that require different paths on > different distros, right? Something along the lines of what we do in > ovn-heater maybe? > > https://github.com/ovn-org/ovn-heater/blob/5dc43e47ae3d030f4625da7714f0f26b0177ff76/do.sh#L53-L87 > > Regards, > Dumitru > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
