Thanks for the patch set Xavier.

With the exception of patch 13:

Acked-by: Mark Michelson <mmich...@redhat.com>

I'm acking the patch set, but the nature of these changes worries me. I could easily see another patch series like this needed in the future because it's too easy to write flaky tests. Here are some high-level ideas I have based on changes I see in these patches.

1) From patches 12 and 15: Should we change ovn_start() so that by default it does not start a backup northd process? I suspect the number of tests that rely on a backup northd process are very small.

2) From patch 5 and possibly patch 14: Should we add a new --wait option to ovn-nbctl (e.g. "--wait=ovs") that waits for OVS to provide a confirmation that flows from this ovn-nbctl call have been installed?

3) From patch 4: Would it be possible to provide a "blocking" version of `ovn-appctl -t ovn-controller exit` ? In other words, could we make a version that will not return control to the shell until the process has exited? Barring that, we could write a macro for ovn-macros.at that will call `ovn-appctl -t ovn-controller exit` and then block until the process has exited. We could then always call that macro when we want to shut ovn-controller down.

On 9/18/23 12:46, Xavier Simonart wrote:
Xavier Simonart (15):
   tests: fixed multiple ovn-ic tests
   tests: fixed "Logical router policy packet marking"
   tests: fixed "options:requested-chassis for logical port"
   tests: tests fixed "Encaps tunnel cleanup ..." and "ovn-controller
     exit"
   tests: wait for all flows to be installed before sending packets
   tests: fixed multiple tests not      properly waiting for packets to
     be received
   tests: fixed multiple tests missing ovn-nbctl "wait"
   tests: fixed "L2 Drop and Allow ACL w/ Stateful ACL"
   tests: skip test "MAC binding aging" if scapy not available.
   tests: move trim_zeros() to ovn-macros
   tests: fixed "send gratuitous ARP for NAT rules on HA distributed
     router"
   tests: fixed "SCTP Load balancer health checks"a
   tests: fixed "LSP incremental processing"
   tests: fixed "ipsec -- basic configuration"
   tests: do not start northd-backup for northd tests querying northd

  tests/ovn-ic.at     |  14 +-
  tests/ovn-ipsec.at  |   4 +-
  tests/ovn-macros.at |   4 +
  tests/ovn-northd.at |  27 ++-
  tests/ovn.at        | 484 ++++++++++++++++----------------------------
  5 files changed, 207 insertions(+), 326 deletions(-)


_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to