diff mbox series

[ovs-dev] ovn.at: Make some of the tests more predictable.

Message ID 1604929423-13031-1-git-send-email-dceara@redhat.com
State Changes Requested
Headers show
Series [ovs-dev] ovn.at: Make some of the tests more predictable. | expand

Commit Message

Dumitru Ceara Nov. 9, 2020, 1:43 p.m. UTC 
Fix some of the race conditions that are present in the current OVN test
suite.

Signed-off-by: Dumitru Ceara <dceara@redhat.com>
---
 tests/ovn.at | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

Comments

Ben Pfaff Nov. 10, 2020, 12:47 a.m. UTC | #1 
On Mon, Nov 09, 2020 at 02:43:43PM +0100, Dumitru Ceara wrote:
> Fix some of the race conditions that are present in the current OVN test
> suite.
> 
> Signed-off-by: Dumitru Ceara <dceara@redhat.com>

Thanks!

I think I see some problems (I did not test this).  First, I'm surprised
this works, since I would expect m4 to drop the [], so that they'd need
to be doubled into [[]]:

    sed 's/conjunction([0-9]\+,[0-9]\+\/[0-9]\+)/conjunction()/g'

Second, \+ is a GNU sed extension.  Usually we avoid GNU extensions
since sometimes people want to run OVS (and maybe OVN?) with BSD.  The
portable way to write it is \{1,\}.

Maybe it would be easier to write s/conjunction([^)]*)/conjunction()/g,
doubling the [] if necessary.

Thanks,

Ben.
Dumitru Ceara Nov. 10, 2020, 8:39 a.m. UTC | #2 
On 11/10/20 1:47 AM, Ben Pfaff wrote:
> On Mon, Nov 09, 2020 at 02:43:43PM +0100, Dumitru Ceara wrote:
>> Fix some of the race conditions that are present in the current OVN test
>> suite.
>>
>> Signed-off-by: Dumitru Ceara <dceara@redhat.com>
> 
> Thanks!
> 
> I think I see some problems (I did not test this).  First, I'm surprised
> this works, since I would expect m4 to drop the [], so that they'd need
> to be doubled into [[]]:
> 
>     sed 's/conjunction([0-9]\+,[0-9]\+\/[0-9]\+)/conjunction()/g'
> 
> Second, \+ is a GNU sed extension.  Usually we avoid GNU extensions
> since sometimes people want to run OVS (and maybe OVN?) with BSD.  The
> portable way to write it is \{1,\}.
> 

You're right, sorry, I didn't test this properly.  sed wasn't matching
anything with my change.

> Maybe it would be easier to write s/conjunction([^)]*)/conjunction()/g,
> doubling the [] if necessary.
> 

This is way better indeed.  I'll use it in v2.

Thanks,
Dumitru
diff mbox series

Patch

diff --git a/tests/ovn.at b/tests/ovn.at
index f154e3d..7ecdfab 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -4114,6 +4114,7 @@  for i in 1 2 3; do
             ovn-nbctl lsp-set-addresses lp$i$j "f0:00:00:00:00:$i$j 192.168.0.$i$j" "$extra_addr"
             ovn-nbctl lsp-set-port-security lp$i$j "f0:00:00:00:00:$i$j 192.168.0.$i$j" "$extra_addr"
         fi
+        OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up lp$i$j` = xup])
     done
 done
 
@@ -13579,7 +13580,8 @@  ovn-nbctl --wait=hv sync
 
 # Check OVS flows, the less restrictive flows should have been installed.
 AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \
-    grep "priority=1003" | awk '{print $7 " " $8}' | sort], [0], [dnl
+    grep "priority=1003" | awk '{print $7 " " $8}' | \
+    sed 's/conjunction([0-9]\+,[0-9]\+\/[0-9]\+)/conjunction()/g' | sort], [0], [dnl
 priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46)
 priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46)
 priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(2,1/2),conjunction(3,1/2)
@@ -13624,7 +13626,8 @@  ovn-nbctl --wait=hv sync
 
 # Check OVS flows, the second less restrictive allow ACL should have been installed.
 AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \
-    grep "priority=1003" | awk '{print $7 " " $8}' | sort], [0], [dnl
+    grep "priority=1003" | awk '{print $7 " " $8}' | \
+    sed 's/conjunction([0-9]\+,[0-9]\+\/[0-9]\+)/conjunction()/g' | sort], [0], [dnl
 priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46)
 priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46)
 priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(2,1/2),conjunction(3,1/2)
@@ -13640,7 +13643,8 @@  ovn-nbctl --wait=hv sync
 
 # Check OVS flows, the 10.0.0.1 conjunction should have been reinstalled.
 AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \
-    grep "priority=1003" | awk '{print $7 " " $8}' | sort], [0], [dnl
+    grep "priority=1003" | awk '{print $7 " " $8}' | \
+    sed 's/conjunction([0-9]\+,[0-9]\+\/[0-9]\+)/conjunction()/g' | sort], [0], [dnl
 priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46)
 priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46)
 priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(2,1/2),conjunction(3,1/2)
@@ -13678,7 +13682,8 @@  ovn-nbctl --wait=hv sync
 
 # Check OVS flows, the less restrictive flows should have been installed.
 AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \
-   grep "priority=1003" | awk '{print $7 " " $8}' | sort], [0], [dnl
+   grep "priority=1003" | awk '{print $7 " " $8}' | \
+   sed 's/conjunction([0-9]\+,[0-9]\+\/[0-9]\+)/conjunction()/g' | sort], [0], [dnl
 priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46)
 priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46)
 priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(2,1/2),conjunction(3,1/2)
@@ -14275,6 +14280,7 @@  ovn-nbctl set Logical_Switch_Port ls1-lr0 type=router \
 # Create HA chassis group
 ovn-nbctl ha-chassis-group-add hagrp1
 ovn-nbctl ha-chassis-group-add-chassis hagrp1 hv1 30
+ovn-nbctl --wait=sb sync
 
 hagrp1_uuid=`ovn-nbctl --bare --columns _uuid find ha_chassis_group name="hagrp1"`