Hi Jacob! These changes make sense to me. Thanks for the flaky test fix!
Acked-by: Rosemarie O'Riorden <[email protected]>
One other random minor thing I noticed in the test is that one of the
ovn-sbctl commands is preceded by a double check ("check check") which
doesn't affect anything but maybe removing that could be folded in upon
merge since we might as well clean up a stray typo in this test while
we're at it.
On 10/23/26 1:57 PM, Jacob Tanenbaum via dev wrote:
> Currently AT_CHECK is called inside of OVS_WAIT_UNTIL macro and that
> does not perform as intended. Failing an AT_CHECK exits the test case
> subshell so the OVS_WAIT_UNTIL macro never gets the chance to retry.
>
> remove the AT_CHECK from rbr_match_builtin() and rbr_match_custom() so
> that OVS_WAIT_UNTIL can rety those functions.
>
> Additionally the regex
>
> s/<[[^<>]]>*//g
>
> causes the error
>
> ovn-sbctl: permissions:Service_Monitor0>: trailing garbage ">" in argument
>
> because it matches one character inside of "<" and an arbitrary number
> of ">" exculding "<" and ">". Changing the regex to
>
> s/<[[^<>]]*>//g
>
> so that it will match an arbitrary number of characters inside "<" and
> ">" excluding ">" and "<".
>
> Fixes: e65fb9efe286 ("northd: Fix issues in RBAC tables recovery.")
> Signed-off-by: Jacob Tanenbaum <[email protected]>
> ---
> tests/ovn-northd.at | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index d9682f78e..410730cc1 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -16703,7 +16703,7 @@ AT_SETUP([RBAC -- Recover builtin role and
> permissions])
> ovn_start
>
> wait_row_count RBAC_Role 1 name=ovn-controller
> -RBR_BUILTIN_PNAMES=$(fetch_column RBAC_Role permissions name=ovn-controller
> | uuidfilt | sed -e 's/<[[^<>]]>*//g' | tr -d '=,{}')
> +RBR_BUILTIN_PNAMES=$(fetch_column RBAC_Role permissions name=ovn-controller
> | uuidfilt | sed -e 's/<[[^<>]]*>//g' | tr -d '=,{}')
>
> declare -A RBR_BUILTIN_PERM
>
> @@ -16714,15 +16714,15 @@ do
> done
>
> rbr_match_builtin() {
> - local RBR_REAL_PNAMES=$(fetch_column RBAC_Role permissions
> name=ovn-controller | uuidfilt | sed -e 's/<[[^<>]]>*//g' | tr -d '=,{}')
> + local RBR_REAL_PNAMES=$(fetch_column RBAC_Role permissions
> name=ovn-controller | uuidfilt | sed -e 's/<[[^<>]]*>//g' | tr -d '=,{}')
>
> - AT_CHECK([test "$RBR_BUILTIN_PNAMES" == "$RBR_REAL_PNAMES"])
> + test "$RBR_BUILTIN_PNAMES" == "$RBR_REAL_PNAMES" || return 1
>
> for V in $RBR_BUILTIN_PNAMES
> do
> local _UUID=`ovn-sbctl get RBAC_Role ovn-controller permissions:$V`
> local RBR_REAL_PERM=`ovn-sbctl get RBAC_Permission $_UUID authorization
> insert_delete table update | paste -s -d " "`
> - AT_CHECK([test "${RBR_BUILTIN_PERM[[$V]]}" == "$RBR_REAL_PERM"])
> + test "${RBR_BUILTIN_PERM[[$V]]}" == "$RBR_REAL_PERM" || return 1
> done
> }
>
> @@ -16735,13 +16735,13 @@ rbr_match_custom() {
> [[Load_Balancer]]='[[]] false Load_Balancer [[]]'
> )
>
> - AT_CHECK([test "$RBR_BUILTIN_ROLE" == "$RBR_REAL_ROLE"])
> + test "$RBR_BUILTIN_ROLE" == "$RBR_REAL_ROLE" || return 1
>
> for V in $RBR_BUILTIN_PNAMES
> do
> local _UUID=`ovn-sbctl get RBAC_Role custom-role permissions:$V`
> local _PREAL=`ovn-sbctl get RBAC_Permission $_UUID authorization
> insert_delete table update | paste -s -d " "`
> - AT_CHECK([test "${RBR_BUILTIN_PERM[[$V]]}" == "$_PREAL"])
> + test "${RBR_BUILTIN_PERM[[$V]]}" == "$_PREAL" || return 1
> done
> }
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
