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. > > 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 <mj.ponso...@canonical.com> >> --- >> 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 d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
