Hi Ales, Mark, Nit: I'd re-title the commit to "test: Test co-hosted ovn-controllers with different CT range." to match how we advertise the feature in the NEWS file.
On 2/10/26 7:42 AM, Ales Musil via dev wrote: > On Mon, Feb 9, 2026 at 4:44 PM Mark Michelson <[email protected]> wrote: > >> Hi Ales, thanks for the test. >> > > Hi Mark, > > thank you for the review. > > >> >> On Thu, Feb 5, 2026 at 7:12 AM Ales Musil via dev >> <[email protected]> wrote: >>> >>> Add a test that should ensure that two controllers running next to >>> each other do not touch zones from different range then the one >> >> s/then/than/ >> >>> assigned to them. >>> >>> Reported-at: https://issues.redhat.com/browse/FDP-2084 >>> Signed-off-by: Ales Musil <[email protected]> >>> --- >>> tests/ovn.at | 153 +++++++++++++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 153 insertions(+) >>> >>> diff --git a/tests/ovn.at b/tests/ovn.at >>> index d7f173c90..1c33cae57 100644 >>> --- a/tests/ovn.at >>> +++ b/tests/ovn.at >>> @@ -44562,3 +44562,156 @@ check ovn-nbctl --wait=hv lsp-set-type >> down_ext localnet >>> OVN_CLEANUP([hv1],[hv2]) >>> AT_CLEANUP >>> ]) >>> + >>> +# NOTE: This test case runs two ovn-controllers inside the same sandbox >> (hv1). >>> +# Each controller uses a unique chassis name - hv1 and hv2 - and manage >>> +# different bridges with different ports. This is why all 'as' commands >> below >>> +# are executed from the same - hv1 - sandbox, regardless of whether they >>> +# logically belong to ports of chassis named hv1 or hv2. >> >> Is it possible to give the sandbox a name that is independent from the >> two chassis? Maybe call the sandbox "hv1" and the two ovn-controller >> chassis "virt-chassis-1" and "virt-chassis-2"? >> > > I'm afraid not, at least not without rewriting the ovn_attach > (ovn_az_attach) helpers. Which I'm not sure is worth doing for > 4 tests that use this technique. > I think we can follow up if needed, we already had the "multiple controllers on the same host can talk to each other" test that did the same thing. I wouldn't block the current patch on this. > >>> +OVN_FOR_EACH_NORTHD([ >>> +AT_SETUP([multiple controllers on the same host doesn't have CT >> conflict]) Maybe "multiple controllers on the same host non-overlapping CT zone ranges"? But I won't insist, it's ok as is too. >>> +ovn_start >>> +net_add n1 >>> + >>> +sim_add hv1 >>> +as hv1 >>> +ovs-vsctl add-br br-phys-1 >>> +ovn_attach n1 br-phys-1 192.168.0.1 24 >>> + >>> +ovs-vsctl add-br br-phys-2 >>> + >>> +ovn-appctl vlog/set vconn:dbg >>> + >>> +check ovs-vsctl \ >>> + -- set Open_vSwitch . external-ids:ct-zone-range-hv1=100-199 \ >>> + -- set Open_vSwitch . external-ids:ct-zone-range-hv2=300-399 >>> + >>> +check ovn-nbctl ls-add ls >>> + >>> +check ovn-nbctl lsp-add ls lsp1 >>> +check ovn-nbctl lsp-add ls lsp2 >>> + >>> +check ovn-nbctl lsp-set-addresses lsp1 "00:00:00:00:00:10 10.0.0.10" \ >>> + -- lsp-set-options lsp1 requested-chassis=hv1 >>> +check ovn-nbctl lsp-set-addresses lsp2 "00:00:00:00:00:20 10.0.0.20" \ >>> + -- lsp-set-options lsp2 requested-chassis=hv2 >>> + >>> +check ovn-nbctl lr-add lr1 -- set Logical_Router lr1 options:chassis=hv1 >>> +check ovn-nbctl lrp-add lr1 lr1-ls 00:00:00:00:00:01 10.0.0.1/24 >>> +check ovn-nbctl lsp-add-router-port ls ls-lr1 lr1-ls >>> + >>> +check ovn-nbctl lr-add lr2 -- set Logical_Router lr2 options:chassis=hv2 >>> +check ovn-nbctl lrp-add lr2 lr2-ls 00:00:00:00:00:01 10.0.0.1/24 >>> +check ovn-nbctl lsp-add-router-port ls ls-lr2 lr2-ls >>> + >>> +check ovs-vsctl -- add-port br-int vif1 -- \ >>> + set interface vif1 external-ids:iface-id=lsp1 >>> + >>> +wait_for_ports_up lsp1 >>> +check ovn-nbctl --wait=hv sync >>> + >>> +# the file is read once at startup so it's safe to write it >>> +# here after the first ovn-controller has started Nit: I know this comment was copied verbatim from the already existing co-hosted ovn-controller test but I'd change this to a sentence (start with capital and end with period). >>> +echo hv2 > ${OVN_SYSCONFDIR}/system-id-override >>> + >>> +# for some reason SSL/TLS ovsdb configuration overrides CLI, so >>> +# delete ssl config from ovsdb to give CLI arguments priority Nit: same here. >>> +ovs-vsctl del-ssl check? >>> + >>> +start_virtual_controller n1 br-phys-2 br-int-2 192.168.2.1 24 geneve >> hv2 \ >>> + --pidfile=${OVS_RUNDIR}/ovn-controller-2.pid \ >>> + --log-file=${OVS_RUNDIR}/ovn-controller-2.log \ >>> + -p $PKIDIR/testpki-hv2-privkey.pem \ >>> + -c $PKIDIR/testpki-hv2-cert.pem \ >>> + -C $PKIDIR/testpki-cacert.pem \ >>> + -vvconn:dbg >>> +pidfile="$OVS_RUNDIR"/ovn-controller-2.pid >>> +on_exit "test -e \"$pidfile\" && kill \`cat \"$pidfile\"\`" >>> + >>> +check ovs-vsctl -- add-port br-int-2 vif2 -- \ >>> + set interface vif2 external-ids:iface-id=lsp2 >>> + >>> +wait_for_ports_up lsp2 >>> +check ovn-nbctl --wait=hv sync >>> +OVN_POPULATE_ARP >>> + >>> +check_zone_allocation() { >>> + local ctl=$1 >>> + local prefix=$2 >>> + >>> + AT_CHECK_UNQUOTED([ovn-appctl -t $ctl ct-zone-list | cut -d ' ' -f2 >> | grep -vE "$prefix[[0-9]]{2}"], [1]) Nit: we don't need extended regex I guess. This could just be: grep -v "$prefix[[0-9]][[0-9]]" >>> +} >>> + >>> +check_zone_flush() { >>> + local file=$1 >>> + local prefix=$2 >>> + >>> + AT_CHECK_UNQUOTED([grep NXT_CT_FLUSH_ZONE ${OVS_RUNDIR}/$file | >> grep -vE "zone_id=$prefix[[0-9]]{2}"], [1]) Similar comment here, this could just be: grep -v "zone_id=$prefix[[0-9]][[0-9]]" >>> +} >>> + >>> +hv1_ctl="${OVS_RUNDIR}/ovn-controller.$(cat >> ${OVS_RUNDIR}/ovn-controller.pid).ctl" >>> +hv2_ctl="${OVS_RUNDIR}/ovn-controller.$(cat >> ${OVS_RUNDIR}/ovn-controller-2.pid).ctl" >>> + >>> +check_zone_allocation $hv1_ctl 1 >>> +check_zone_flush ovn-controller.log 1 >>> +check_zone_allocation $hv2_ctl 3 >>> +check_zone_flush ovn-controller-2.log 3 >>> + >>> +# Re-bind both LSPs Nit: missing period at end of sentence. >>> +check ovs-vsctl set interface vif1 external-ids:iface-id=lsp2 \ >>> + -- set interface vif2 external-ids:iface-id=lsp1 >>> +check ovn-nbctl lsp-set-options lsp1 requested-chassis=hv2 \ >>> + -- lsp-set-options lsp2 requested-chassis=hv1 >>> +wait_for_ports_up >>> +check ovn-nbctl --wait=hv sync >>> + >>> +check_zone_allocation $hv1_ctl 1 >>> +check_zone_flush ovn-controller.log 1 >>> +check_zone_allocation $hv2_ctl 3 >>> +check_zone_flush ovn-controller-2.log 3 >>> + >>> +# Swap GW routers. >>> +check ovn-nbctl set Logical_Router lr1 options:chassis=hv2 \ >>> + -- set Logical_Router lr2 options:chassis=hv1 >>> +check ovn-nbctl --wait=hv sync >>> + >>> +# Do recompute for both controller to make that datapath swap effective. Is this a bug we should follow up on? Or do you mean to say that ovn-controller-1 will still consider LR1 as "local datapath" until recompute? The latter is indeed a known issue. If that's what you meant, I'd rephrase this comment to: # Datapaths become non-local to ovn-controllers when runtime-data is # recomputed. Trigger that. >>> +check ovn-appctl -t $hv1_ctl inc-engine/recompute >>> +check ovn-appctl -t $hv2_ctl inc-engine/recompute >>> + >>> +check_zone_allocation $hv1_ctl 1 >>> +check_zone_flush ovn-controller.log 1 >>> +check_zone_allocation $hv2_ctl 3 >>> +check_zone_flush ovn-controller-2.log 3 >>> + >>> +# Remove LSPs. >>> +check ovn-nbctl --wait=hv lsp-del lsp1 >>> + >>> +check_zone_allocation $hv1_ctl 1 >>> +check_zone_flush ovn-controller.log 1 >>> +check_zone_allocation $hv2_ctl 3 >>> +check_zone_flush ovn-controller-2.log 3 >>> + >>> +check ovn-nbctl --wait=hv lsp-del lsp2 >>> +check_zone_allocation $hv1_ctl 1 >>> +check_zone_flush ovn-controller.log 1 >>> +check_zone_allocation $hv2_ctl 3 >>> +check_zone_flush ovn-controller-2.log 3 >>> + >>> +# Make the routers distributed again. >>> +check ovn-nbctl remove Logical_Router lr1 options chassis >>> +check ovn-nbctl remove Logical_Router lr2 options chassis >>> +check ovn-nbctl --wait=hv sync >>> + >>> +# Do recompute for both controller to make that datapath remove >> effective. Same here. >>> +check ovn-appctl -t $hv1_ctl inc-engine/recompute >>> +check ovn-appctl -t $hv2_ctl inc-engine/recompute >>> + >>> +check_zone_allocation $hv1_ctl 1 >>> +check_zone_flush ovn-controller.log 1 >>> +check_zone_allocation $hv2_ctl 3 >>> +check_zone_flush ovn-controller-2.log 3 >>> + >>> +OVN_CLEANUP([hv1]) >>> +AT_CLEANUP >>> +]) >>> -- >>> 2.52.0 >>> >>> _______________________________________________ >>> dev mailing list >>> [email protected] >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>> >> >> > Regards, > Ales If you want to squash in fixes for the minor comments I had above that's fine with me. V2 is also fine, if you prefer that. In any case, if the only changes are the ones listed above then: Acked-by: Dumitru Ceara <[email protected]> Thanks, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
