| Message ID | 20231006151258.983906-2-aconole@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | selftests: openvswitch: Minor fixes for some systems | expand |
On Fri, 2023-10-06 at 11:12 -0400, Aaron Conole wrote: > Paolo Abeni reports that on some systems the pyroute2 version isn't > new enough to run the test suite. Ensure that we support a minimum > version of 0.6 for all cases (which does include the existing ones). > The 0.6.1 version was released in May of 2021, so should be > propagated to most installations at this point. > > The alternative that Paolo proposed was to only skip when the > add-flow is being run. This would be okay for most cases, except > if a future test case is added that needs to do flow dump without > an associated add (just guessing). In that case, it could also be > broken and we would need additional skip logic anyway. Just draw > a line in the sand now. > > Fixes: 25f16c873fb1 ("selftests: add openvswitch selftest suite") > Reported-by: Paolo Abeni <pabeni@redhat.com> > Closes: https://lore.kernel.org/lkml/8470c431e0930d2ea204a9363a60937289b7fdbe.camel@redhat.com/ > Signed-off-by: Aaron Conole <aconole@redhat.com> > --- > tools/testing/selftests/net/openvswitch/openvswitch.sh | 2 +- > tools/testing/selftests/net/openvswitch/ovs-dpctl.py | 8 ++++++++ > 2 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh > index 9c2012d70b08e..220c3356901ef 100755 > --- a/tools/testing/selftests/net/openvswitch/openvswitch.sh > +++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh > @@ -525,7 +525,7 @@ run_test() { > fi > > if python3 ovs-dpctl.py -h 2>&1 | \ > - grep "Need to install the python" >/dev/null 2>&1; then > + grep -E "Need to (install|upgrade) the python" >/dev/null 2>&1; then > stdbuf -o0 printf "TEST: %-60s [PYLIB]\n" "${tdesc}" > return $ksft_skip > fi > diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py > index 912dc8c490858..9686ca30d516d 100644 > --- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py > +++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py > @@ -28,6 +28,8 @@ try: > from pyroute2.netlink import nlmsg_atoms > from pyroute2.netlink.exceptions import NetlinkError > from pyroute2.netlink.generic import GenericNetlinkSocket > + import pyroute2 > + > except ModuleNotFoundError: > print("Need to install the python pyroute2 package.") > sys.exit(0) > @@ -1998,6 +2000,12 @@ def main(argv): > nlmsg_atoms.ovskey = ovskey > nlmsg_atoms.ovsactions = ovsactions > > + # version check for pyroute2 > + prverscheck = pyroute2.__version__.split(".") > + if int(prverscheck[0]) == 0 and int(prverscheck[1]) < 6: > + print("Need to upgrade the python pyroute2 package.") I think it would be better to propagate/print also the minimum version required, so that the user should not have to resort looking at the self-test sources to learn the required minimum version. Cheers, Paolo
Paolo Abeni <pabeni@redhat.com> writes: > On Fri, 2023-10-06 at 11:12 -0400, Aaron Conole wrote: >> Paolo Abeni reports that on some systems the pyroute2 version isn't >> new enough to run the test suite. Ensure that we support a minimum >> version of 0.6 for all cases (which does include the existing ones). >> The 0.6.1 version was released in May of 2021, so should be >> propagated to most installations at this point. >> >> The alternative that Paolo proposed was to only skip when the >> add-flow is being run. This would be okay for most cases, except >> if a future test case is added that needs to do flow dump without >> an associated add (just guessing). In that case, it could also be >> broken and we would need additional skip logic anyway. Just draw >> a line in the sand now. >> >> Fixes: 25f16c873fb1 ("selftests: add openvswitch selftest suite") >> Reported-by: Paolo Abeni <pabeni@redhat.com> >> Closes: https://lore.kernel.org/lkml/8470c431e0930d2ea204a9363a60937289b7fdbe.camel@redhat.com/ >> Signed-off-by: Aaron Conole <aconole@redhat.com> >> --- >> tools/testing/selftests/net/openvswitch/openvswitch.sh | 2 +- >> tools/testing/selftests/net/openvswitch/ovs-dpctl.py | 8 ++++++++ >> 2 files changed, 9 insertions(+), 1 deletion(-) >> >> diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh >> index 9c2012d70b08e..220c3356901ef 100755 >> --- a/tools/testing/selftests/net/openvswitch/openvswitch.sh >> +++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh >> @@ -525,7 +525,7 @@ run_test() { >> fi >> >> if python3 ovs-dpctl.py -h 2>&1 | \ >> - grep "Need to install the python" >/dev/null 2>&1; then >> + grep -E "Need to (install|upgrade) the python" >/dev/null 2>&1; then >> stdbuf -o0 printf "TEST: %-60s [PYLIB]\n" "${tdesc}" >> return $ksft_skip >> fi >> diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py >> index 912dc8c490858..9686ca30d516d 100644 >> --- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py >> +++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py >> @@ -28,6 +28,8 @@ try: >> from pyroute2.netlink import nlmsg_atoms >> from pyroute2.netlink.exceptions import NetlinkError >> from pyroute2.netlink.generic import GenericNetlinkSocket >> + import pyroute2 >> + >> except ModuleNotFoundError: >> print("Need to install the python pyroute2 package.") >> sys.exit(0) >> @@ -1998,6 +2000,12 @@ def main(argv): >> nlmsg_atoms.ovskey = ovskey >> nlmsg_atoms.ovsactions = ovsactions >> >> + # version check for pyroute2 >> + prverscheck = pyroute2.__version__.split(".") >> + if int(prverscheck[0]) == 0 and int(prverscheck[1]) < 6: >> + print("Need to upgrade the python pyroute2 package.") > > I think it would be better to propagate/print also the minimum version > required, so that the user should not have to resort looking at the > self-test sources to learn the required minimum version. ACK - makes sense to me. > Cheers, > > Paolo
diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh index 9c2012d70b08e..220c3356901ef 100755 --- a/tools/testing/selftests/net/openvswitch/openvswitch.sh +++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh @@ -525,7 +525,7 @@ run_test() { fi if python3 ovs-dpctl.py -h 2>&1 | \ - grep "Need to install the python" >/dev/null 2>&1; then + grep -E "Need to (install|upgrade) the python" >/dev/null 2>&1; then stdbuf -o0 printf "TEST: %-60s [PYLIB]\n" "${tdesc}" return $ksft_skip fi diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py index 912dc8c490858..9686ca30d516d 100644 --- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py +++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py @@ -28,6 +28,8 @@ try: from pyroute2.netlink import nlmsg_atoms from pyroute2.netlink.exceptions import NetlinkError from pyroute2.netlink.generic import GenericNetlinkSocket + import pyroute2 + except ModuleNotFoundError: print("Need to install the python pyroute2 package.") sys.exit(0) @@ -1998,6 +2000,12 @@ def main(argv): nlmsg_atoms.ovskey = ovskey nlmsg_atoms.ovsactions = ovsactions + # version check for pyroute2 + prverscheck = pyroute2.__version__.split(".") + if int(prverscheck[0]) == 0 and int(prverscheck[1]) < 6: + print("Need to upgrade the python pyroute2 package.") + sys.exit(0) + parser = argparse.ArgumentParser() parser.add_argument( "-v",
Paolo Abeni reports that on some systems the pyroute2 version isn't new enough to run the test suite. Ensure that we support a minimum version of 0.6 for all cases (which does include the existing ones). The 0.6.1 version was released in May of 2021, so should be propagated to most installations at this point. The alternative that Paolo proposed was to only skip when the add-flow is being run. This would be okay for most cases, except if a future test case is added that needs to do flow dump without an associated add (just guessing). In that case, it could also be broken and we would need additional skip logic anyway. Just draw a line in the sand now. Fixes: 25f16c873fb1 ("selftests: add openvswitch selftest suite") Reported-by: Paolo Abeni <pabeni@redhat.com> Closes: https://lore.kernel.org/lkml/8470c431e0930d2ea204a9363a60937289b7fdbe.camel@redhat.com/ Signed-off-by: Aaron Conole <aconole@redhat.com> --- tools/testing/selftests/net/openvswitch/openvswitch.sh | 2 +- tools/testing/selftests/net/openvswitch/ovs-dpctl.py | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-)