On Thu, Feb 12, 2026 at 2:59 PM Dumitru Ceara <[email protected]> wrote:
> Hi Ales, Mark, > Hi Dumitru, thank you for the review. > > 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. > Ack. > > 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. > Makes sense. > > > > >>> +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. > It's the latter, I will rephrase it. > > 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 > > I took care of the other nits, went ahead and merged this into main. Regards, Ales _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
