| Message ID | 20250502161632.27412-1-fnordahl@ubuntu.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] tests: Don't assume first bucket will be chosen. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
On Fri, May 2, 2025 at 6:16 PM Frode Nordahl <fnordahl@ubuntu.com> wrote: > The commit in the fixes tag introduced a system test that configure > load balancers with multiple backends. > > Load balancers are implemented with the group action with equally > weighted buckets. > > One of the assertions in the added system test are on there being > exactly one backends being utilized, and this assertion assumes > that the first bucket of the group will be used. > > As demonstrated by the reported bug, this is not a correct > assumption, and factors such as system architecture may play a > role in which bucket will be selected. > > Since the purpose of the assertion is to confirm only one backend > is utilized, we do not need to match on the exact IP address. > > Clear the reply src value, allowing us to assert on there being one > data path flow, regardless of which bucket is chosen. > > Reported-at: https://launchpad.net/bugs/2104227 > Fixes: 15ab046e12cd ("northd: Add ipv6_{src, dst} to selection_fields > column in the NB db.") > Signed-off-by: Frode Nordahl <fnordahl@ubuntu.com> > --- > tests/system-ovn.at | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > A successufl test run on s390x architecture can be viewed here: > https://github.com/canonical/ubuntu-ovn-robot/actions/runs/14798878414 > > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > index 6e71286ad..b3990d848 100644 > --- a/tests/system-ovn.at > +++ b/tests/system-ovn.at > @@ -2024,8 +2024,9 @@ done > > # Only one backend should be chosen. > AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(30.0.0.1) | \ > -sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl > > -tcp,orig=(src=192.168.1.2,dst=30.0.0.1,sport=<cleared>,dport=<cleared>),reply=(src=172.16.1.2,dst=192.168.1.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,mark=2,protoinfo=(state=<cleared>) > +sed -e 's/zone=[[0-9]]*/zone=<cleared>/' \ > + -e 's/src=172.16.1.[[0-9]]*/src=<cleared>/'], [0], [dnl > > +tcp,orig=(src=192.168.1.2,dst=30.0.0.1,sport=<cleared>,dport=<cleared>),reply=(src=<cleared>,dst=192.168.1.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,mark=2,protoinfo=(state=<cleared>) > ]) > > for i in $(seq 1 20); do > @@ -2035,8 +2036,9 @@ done > > # Only one backend should be chosen. > AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(fd03::2) | \ > -sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl > > -tcp,orig=(src=fd01::2,dst=fd03::2,sport=<cleared>,dport=<cleared>),reply=(src=fd02::2,dst=fd01::2,sport=<cleared>,dport=<cleared>),zone=<cleared>,mark=2,protoinfo=(state=<cleared>) > +sed -e 's/zone=[[0-9]]*/zone=<cleared>/' \ > + -e 's/src=fd02::[[0-f]]*/src=<cleared>/'], [0], [dnl > > +tcp,orig=(src=fd01::2,dst=fd03::2,sport=<cleared>,dport=<cleared>),reply=(src=<cleared>,dst=fd01::2,sport=<cleared>,dport=<cleared>),zone=<cleared>,mark=2,protoinfo=(state=<cleared>) > ]) > > # Test load-balancing using IP src and IP dst. > -- > 2.43.0 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Thank you Frode, I went ahead and merged this into and backported to 25.03. Regards, Ales
diff --git a/tests/system-ovn.at b/tests/system-ovn.at index 6e71286ad..b3990d848 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -2024,8 +2024,9 @@ done # Only one backend should be chosen. AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(30.0.0.1) | \ -sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl -tcp,orig=(src=192.168.1.2,dst=30.0.0.1,sport=<cleared>,dport=<cleared>),reply=(src=172.16.1.2,dst=192.168.1.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,mark=2,protoinfo=(state=<cleared>) +sed -e 's/zone=[[0-9]]*/zone=<cleared>/' \ + -e 's/src=172.16.1.[[0-9]]*/src=<cleared>/'], [0], [dnl +tcp,orig=(src=192.168.1.2,dst=30.0.0.1,sport=<cleared>,dport=<cleared>),reply=(src=<cleared>,dst=192.168.1.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,mark=2,protoinfo=(state=<cleared>) ]) for i in $(seq 1 20); do @@ -2035,8 +2036,9 @@ done # Only one backend should be chosen. AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(fd03::2) | \ -sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl -tcp,orig=(src=fd01::2,dst=fd03::2,sport=<cleared>,dport=<cleared>),reply=(src=fd02::2,dst=fd01::2,sport=<cleared>,dport=<cleared>),zone=<cleared>,mark=2,protoinfo=(state=<cleared>) +sed -e 's/zone=[[0-9]]*/zone=<cleared>/' \ + -e 's/src=fd02::[[0-f]]*/src=<cleared>/'], [0], [dnl +tcp,orig=(src=fd01::2,dst=fd03::2,sport=<cleared>,dport=<cleared>),reply=(src=<cleared>,dst=fd01::2,sport=<cleared>,dport=<cleared>),zone=<cleared>,mark=2,protoinfo=(state=<cleared>) ]) # Test load-balancing using IP src and IP dst.
The commit in the fixes tag introduced a system test that configure load balancers with multiple backends. Load balancers are implemented with the group action with equally weighted buckets. One of the assertions in the added system test are on there being exactly one backends being utilized, and this assertion assumes that the first bucket of the group will be used. As demonstrated by the reported bug, this is not a correct assumption, and factors such as system architecture may play a role in which bucket will be selected. Since the purpose of the assertion is to confirm only one backend is utilized, we do not need to match on the exact IP address. Clear the reply src value, allowing us to assert on there being one data path flow, regardless of which bucket is chosen. Reported-at: https://launchpad.net/bugs/2104227 Fixes: 15ab046e12cd ("northd: Add ipv6_{src, dst} to selection_fields column in the NB db.") Signed-off-by: Frode Nordahl <fnordahl@ubuntu.com> --- tests/system-ovn.at | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) A successufl test run on s390x architecture can be viewed here: https://github.com/canonical/ubuntu-ovn-robot/actions/runs/14798878414