| Message ID | 20240108145248.1763075-1-i.maximets@ovn.org |
|---|---|
| State | Accepted |
| Delegated to: | Simon Horman |
| Headers | show |
| Series | [ovs-dev] ovsdb-idl.at: Test IDL behavior during database conversion. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
| ovsrobot/intel-ovs-compilation | success | test: success |
On 1/8/24 15:52, Ilya Maximets wrote: > Add new 'monitor' command to test-ovsdb utilities to make them just > run IDL loop infinitely. Other commands can still be placed before > the 'monitor', e.g. setting up conditions, tracking, running a few > transactions. > > Having that, adding a couple test cases for IDL with online database > conversion. Test checks that IDL receives monitor cancellation > notification and successfully re-sends monitor requests. > > While at it, adding debug logging to ovsdb-server processes for easier > debugging. > > While working on a test the issue was discovered that schema for > standalone databases is not getting updated in the _Server database > after conversion. Checking the new schema bits only for clustered > databases for now. > > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > --- > > WARN: The test is going to fail for python IDL implementation without: > https://patchwork.ozlabs.org/project/openvswitch/patch/20231218233124.669221-1-twilson@redhat.com/ > > tests/ovsdb-idl.at | 100 +++++++++++++++++++++++++++++++++++++++++--- > tests/test-ovsdb.c | 7 ++++ > tests/test-ovsdb.py | 27 +++++++----- > 3 files changed, 119 insertions(+), 15 deletions(-) > > diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at > index f17cfdf10..003ba6571 100644 > --- a/tests/ovsdb-idl.at > +++ b/tests/ovsdb-idl.at > @@ -29,8 +29,8 @@ m4_define([OVSDB_START_IDLTEST], > AT_CHECK([ovsdb-tool create db dnl > m4_if([$2], [], [$abs_srcdir/idltest.ovsschema], [$2])]) > PKIDIR=$abs_top_builddir/tests > - AT_CHECK([ovsdb-server -vconsole:warn --log-file --detach --no-chdir dnl > - --pidfile --remote=punix:socket dnl > + AT_CHECK([ovsdb-server -vconsole:warn -vfile:dbg --log-file dnl > + --detach --no-chdir --pidfile --remote=punix:socket dnl > m4_if(m4_substr($1, 0, 5), [pssl:], > [--private-key=$PKIDIR/testpki-privkey2.pem dnl > --certificate=$PKIDIR/testpki-cert2.pem dnl > @@ -57,9 +57,9 @@ m4_define([OVSDB_CLUSTER_START_IDLTEST], > done > on_exit 'kill $(cat s*.pid)' > for i in $(seq $n); do > - AT_CHECK([ovsdb-server -vraft -vconsole:warn --detach --no-chdir \ > - --log-file=s$i.log --pidfile=s$i.pid --unixctl=s$i \ > - --remote=punix:s$i.ovsdb \ > + AT_CHECK([ovsdb-server -vraft -vconsole:warn -vfile:dbg --detach \ > + --no-chdir --log-file=s$i.log --pidfile=s$i.pid \ > + --unixctl=s$i --remote=punix:s$i.ovsdb \ > m4_if([$2], [], [], [--remote=$2]) s$i.db]) > done > > @@ -2756,3 +2756,93 @@ OVSDB_CHECK_IDL_PERS_UUID_INSERT([simple idl, persistent uuid insert], > 011: done > ]], > [['This UUID would duplicate a UUID already present within the table or deleted within the same transaction']]) > + > + > +m4_define([OVSDB_CHECK_IDL_CHANGE_AWARE], > + [AT_SETUP([simple idl, database change aware, online conversion - $1]) > + AT_KEYWORDS([ovsdb server idl db_change_aware conversion $1]) > + > + m4_if([$1], [clustered], > + [OVSDB_CLUSTER_START_IDLTEST([1], [punix:socket])], > + [OVSDB_START_IDLTEST]) > + > + dnl Add some data. > + AT_CHECK([[ovsdb-client transact unix:socket '["idltest", > + {"op": "insert", > + "table": "simple", > + "row": {"i": 1, > + "r": 2.0, > + "b": true, > + "s": "first row", > + "u": ["uuid", "84f5c8f5-ac76-4dbc-a24f-8860eb407fc1"], > + "ia": ["set", [1, 2, 3]], > + "ra": ["set", [-0.5]], > + "ba": ["set", [true]], > + "sa": ["set", ["abc", "def"]], > + "ua": ["set", [["uuid", "69443985-7806-45e2-b35f-574a04e720f9"], > + ["uuid", "aad11ef0-816a-4b01-93e6-03b8b4256b98"]]]}}, > + {"op": "insert", > + "table": "simple", > + "row": {"b": false, "s": "second row"}}, > + {"op": "insert", > + "table": "simple", > + "row": {"b": true, "s": "third row"}} > + ]']], [0], [stdout]) > + > + dnl Create a new schema by adding 'extra_column' to the 'simple' table. > + AT_CHECK([sed 's/"ua": {/"extra_column":{"type": "string"},"ua": {/ > + s/1.2.3/1.2.4/' \ > + $abs_srcdir/idltest.ovsschema > new-idltest.ovsschema]) > + dnl Try "needs-conversion". > + AT_CHECK([ovsdb-client needs-conversion unix:socket $abs_srcdir/idltest.ovsschema], [0], [no > +]) > + AT_CHECK([ovsdb-client needs-conversion unix:socket new-idltest.ovsschema], [0], [yes > +]) > + > + dnl Conditionally exclude the second row from monitoring. > + m4_define([COND], [['condition simple [["b","==",true]]']]) > + > + dnl Start monitoring. > + OVS_DAEMONIZE([test-ovsdb '-vPATTERN:console:test-ovsdb|%c|%m' -vjsonrpc -t30 \ > + idl unix:socket COND monitor \ > + >idl-c.out 2>idl-c.err], [idl-c.pid]) > + AT_CAPTURE_FILE([idl-c.out]) > + AT_CAPTURE_FILE([idl-c.err]) > + > + OVS_DAEMONIZE([$PYTHON3 $srcdir/test-ovsdb.py -t30 \ > + idl $srcdir/idltest.ovsschema unix:socket COND monitor \ > + >idl-python.out 2>idl-python.err], [idl-python.pid]) > + AT_CAPTURE_FILE([idl-python.out]) > + AT_CAPTURE_FILE([idl-python.err]) > + > + dnl Wait for monitors to receive the data. > + OVS_WAIT_UNTIL([grep -q 'third row' idl-c.err]) > + OVS_WAIT_UNTIL([grep -q 'third row' idl-python.err]) > + > + dnl Convert the database. > + AT_CHECK([ovsdb-client convert unix:socket new-idltest.ovsschema]) > + > + dnl Check for the monitor cancellation and the data being requested again. > + m4_foreach([FILE], [[idl-c], [idl-python]], > + [OVS_WAIT_UNTIL([grep -q 'monitor_canceled' FILE.err]) > + OVS_WAIT_UNTIL([test 2 -eq $(grep -c 'send request, method="monitor_cond_since", params=."idltest"' FILE.err)]) > + > + dnl XXX: Checking for a new schema bits conditionally because standalone Nit: s/a new schema/the new schema/ > + dnl databases are not updating the schema in a _Server database properly. Nit: s/in a _Server/in the _Server/ > + m4_if([$1], [clustered], [OVS_WAIT_UNTIL([grep -q 'extra_column' FILE.err])]) > + > + dnl Check that there were no unexpected messages. > + AT_CHECK([! grep 'unexpected' FILE.err]) > + > + dnl Check that the data is received twice and the condition is working. > + AT_CHECK([sort FILE.out | uuidfilt], [0], > +[[000: simple: change conditions > +001: table simple: i=0 r=0 b=true s=third row u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1> > +001: table simple: i=1 r=2 b=true s=first row u=<2> ia=[1 2 3] ra=[-0.5] ba=[true] sa=[abc def] ua=[<3> <4>] uuid=<5> > +002: table simple: i=0 r=0 b=true s=third row u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1> > +002: table simple: i=1 r=2 b=true s=first row u=<2> ia=[1 2 3] ra=[-0.5] ba=[true] sa=[abc def] ua=[<3> <4>] uuid=<5> > +]])]) > + AT_CLEANUP]) > + > +OVSDB_CHECK_IDL_CHANGE_AWARE([standalone]) > +OVSDB_CHECK_IDL_CHANGE_AWARE([clustered]) > diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c > index c761822e6..d6a47de33 100644 > --- a/tests/test-ovsdb.c > +++ b/tests/test-ovsdb.c > @@ -2800,6 +2800,13 @@ do_idl(struct ovs_cmdl_context *ctx) > } else { > print_idl(idl, step++, terse); > } > + > + /* Just run IDL forever for a simple monitoring. */ > + if (!strcmp(arg, "monitor")) { > + seqno = ovsdb_idl_get_seqno(idl); > + i--; > + continue; > + } > } > seqno = ovsdb_idl_get_seqno(idl); > > diff --git a/tests/test-ovsdb.py b/tests/test-ovsdb.py > index 71248854f..48f8ee2d7 100644 > --- a/tests/test-ovsdb.py > +++ b/tests/test-ovsdb.py > @@ -757,16 +757,23 @@ def do_idl(schema_file, remote, *commands): > poller.block() > else: > # Wait for update. > - while idl.change_seqno == seqno and not idl.run(): > - rpc.run() > - > - poller = ovs.poller.Poller() > - idl.wait(poller) > - rpc.wait(poller) > - poller.block() > - > - print_idl(idl, step, terse) > - step += 1 > + while True: > + while idl.change_seqno == seqno and not idl.run(): > + rpc.run() > + > + poller = ovs.poller.Poller() > + idl.wait(poller) > + rpc.wait(poller) > + poller.block() > + > + print_idl(idl, step, terse) > + step += 1 > + > + # Run IDL forever in case of a simple monitor, otherwise > + # break and execute the command. > + seqno = idl.change_seqno > + if command != "monitor": > + break > > seqno = idl.change_seqno > With the minor nits above addressed: Acked-by: Dumitru Ceara <dceara@redhat.com> Thanks, Dumitru
On Mon, Jan 08, 2024 at 03:52:23PM +0100, Ilya Maximets wrote: > Add new 'monitor' command to test-ovsdb utilities to make them just > run IDL loop infinitely. Other commands can still be placed before > the 'monitor', e.g. setting up conditions, tracking, running a few > transactions. > > Having that, adding a couple test cases for IDL with online database > conversion. Test checks that IDL receives monitor cancellation > notification and successfully re-sends monitor requests. > > While at it, adding debug logging to ovsdb-server processes for easier > debugging. > > While working on a test the issue was discovered that schema for > standalone databases is not getting updated in the _Server database > after conversion. Checking the new schema bits only for clustered > databases for now. > > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > --- > > WARN: The test is going to fail for python IDL implementation without: > https://patchwork.ozlabs.org/project/openvswitch/patch/20231218233124.669221-1-twilson@redhat.com/ Accepted as per discussion at the link above. - ovsdb-idl.at: Test IDL behavior during database conversion. https://github.com/openvswitch/ovs/commit/67ee6308781d - python: idl: Handle monitor_canceled. https://github.com/openvswitch/ovs/commit/ac04dfa7ec36 Backport to branch-3.2: - ovsdb-idl.at: Test IDL behavior during database conversion. https://github.com/openvswitch/ovs/commit/b3d094b4fb4e - python: idl: Handle monitor_canceled. https://github.com/openvswitch/ovs/commit/a1935e962876 Backport to branch-3.1 (with dependency): - ovsdb-idl.at: Test IDL behavior during database conversion. https://github.com/openvswitch/ovs/commit/dd3f0626ed16 - tests-ovsdb: Switch OVSDB_START_IDLTEST to macro. https://github.com/openvswitch/ovs/commit/2c6b81ad6e22 - python: idl: Handle monitor_canceled. https://github.com/openvswitch/ovs/commit/e9c5226cc296 Backport to branch-3.2 (with dependency): - ovsdb-idl.at: Test IDL behavior during database conversion. https://github.com/openvswitch/ovs/commit/28926225c422 - tests-ovsdb: Switch OVSDB_START_IDLTEST to macro. https://github.com/openvswitch/ovs/commit/2404ca561e94 - python: idl: Handle monitor_canceled. https://github.com/openvswitch/ovs/commit/219f1ebb0624 Backport to branch-2.17 (with dependencies): - ovsdb-idl.at: Test IDL behavior during database conversion. https://github.com/openvswitch/ovs/commit/9bbc2cf8a89e - tests: Use _DAEMONIZE macro's to start tcpdump. https://github.com/openvswitch/ovs/commit/049189584f5a - tests-ovsdb: Switch OVSDB_START_IDLTEST to macro. https://github.com/openvswitch/ovs/commit/30099c5d9ef7 - python: idl: Handle monitor_canceled. https://github.com/openvswitch/ovs/commit/f4b4d650a1e0
diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at index f17cfdf10..003ba6571 100644 --- a/tests/ovsdb-idl.at +++ b/tests/ovsdb-idl.at @@ -29,8 +29,8 @@ m4_define([OVSDB_START_IDLTEST], AT_CHECK([ovsdb-tool create db dnl m4_if([$2], [], [$abs_srcdir/idltest.ovsschema], [$2])]) PKIDIR=$abs_top_builddir/tests - AT_CHECK([ovsdb-server -vconsole:warn --log-file --detach --no-chdir dnl - --pidfile --remote=punix:socket dnl + AT_CHECK([ovsdb-server -vconsole:warn -vfile:dbg --log-file dnl + --detach --no-chdir --pidfile --remote=punix:socket dnl m4_if(m4_substr($1, 0, 5), [pssl:], [--private-key=$PKIDIR/testpki-privkey2.pem dnl --certificate=$PKIDIR/testpki-cert2.pem dnl @@ -57,9 +57,9 @@ m4_define([OVSDB_CLUSTER_START_IDLTEST], done on_exit 'kill $(cat s*.pid)' for i in $(seq $n); do - AT_CHECK([ovsdb-server -vraft -vconsole:warn --detach --no-chdir \ - --log-file=s$i.log --pidfile=s$i.pid --unixctl=s$i \ - --remote=punix:s$i.ovsdb \ + AT_CHECK([ovsdb-server -vraft -vconsole:warn -vfile:dbg --detach \ + --no-chdir --log-file=s$i.log --pidfile=s$i.pid \ + --unixctl=s$i --remote=punix:s$i.ovsdb \ m4_if([$2], [], [], [--remote=$2]) s$i.db]) done @@ -2756,3 +2756,93 @@ OVSDB_CHECK_IDL_PERS_UUID_INSERT([simple idl, persistent uuid insert], 011: done ]], [['This UUID would duplicate a UUID already present within the table or deleted within the same transaction']]) + + +m4_define([OVSDB_CHECK_IDL_CHANGE_AWARE], + [AT_SETUP([simple idl, database change aware, online conversion - $1]) + AT_KEYWORDS([ovsdb server idl db_change_aware conversion $1]) + + m4_if([$1], [clustered], + [OVSDB_CLUSTER_START_IDLTEST([1], [punix:socket])], + [OVSDB_START_IDLTEST]) + + dnl Add some data. + AT_CHECK([[ovsdb-client transact unix:socket '["idltest", + {"op": "insert", + "table": "simple", + "row": {"i": 1, + "r": 2.0, + "b": true, + "s": "first row", + "u": ["uuid", "84f5c8f5-ac76-4dbc-a24f-8860eb407fc1"], + "ia": ["set", [1, 2, 3]], + "ra": ["set", [-0.5]], + "ba": ["set", [true]], + "sa": ["set", ["abc", "def"]], + "ua": ["set", [["uuid", "69443985-7806-45e2-b35f-574a04e720f9"], + ["uuid", "aad11ef0-816a-4b01-93e6-03b8b4256b98"]]]}}, + {"op": "insert", + "table": "simple", + "row": {"b": false, "s": "second row"}}, + {"op": "insert", + "table": "simple", + "row": {"b": true, "s": "third row"}} + ]']], [0], [stdout]) + + dnl Create a new schema by adding 'extra_column' to the 'simple' table. + AT_CHECK([sed 's/"ua": {/"extra_column":{"type": "string"},"ua": {/ + s/1.2.3/1.2.4/' \ + $abs_srcdir/idltest.ovsschema > new-idltest.ovsschema]) + dnl Try "needs-conversion". + AT_CHECK([ovsdb-client needs-conversion unix:socket $abs_srcdir/idltest.ovsschema], [0], [no +]) + AT_CHECK([ovsdb-client needs-conversion unix:socket new-idltest.ovsschema], [0], [yes +]) + + dnl Conditionally exclude the second row from monitoring. + m4_define([COND], [['condition simple [["b","==",true]]']]) + + dnl Start monitoring. + OVS_DAEMONIZE([test-ovsdb '-vPATTERN:console:test-ovsdb|%c|%m' -vjsonrpc -t30 \ + idl unix:socket COND monitor \ + >idl-c.out 2>idl-c.err], [idl-c.pid]) + AT_CAPTURE_FILE([idl-c.out]) + AT_CAPTURE_FILE([idl-c.err]) + + OVS_DAEMONIZE([$PYTHON3 $srcdir/test-ovsdb.py -t30 \ + idl $srcdir/idltest.ovsschema unix:socket COND monitor \ + >idl-python.out 2>idl-python.err], [idl-python.pid]) + AT_CAPTURE_FILE([idl-python.out]) + AT_CAPTURE_FILE([idl-python.err]) + + dnl Wait for monitors to receive the data. + OVS_WAIT_UNTIL([grep -q 'third row' idl-c.err]) + OVS_WAIT_UNTIL([grep -q 'third row' idl-python.err]) + + dnl Convert the database. + AT_CHECK([ovsdb-client convert unix:socket new-idltest.ovsschema]) + + dnl Check for the monitor cancellation and the data being requested again. + m4_foreach([FILE], [[idl-c], [idl-python]], + [OVS_WAIT_UNTIL([grep -q 'monitor_canceled' FILE.err]) + OVS_WAIT_UNTIL([test 2 -eq $(grep -c 'send request, method="monitor_cond_since", params=."idltest"' FILE.err)]) + + dnl XXX: Checking for a new schema bits conditionally because standalone + dnl databases are not updating the schema in a _Server database properly. + m4_if([$1], [clustered], [OVS_WAIT_UNTIL([grep -q 'extra_column' FILE.err])]) + + dnl Check that there were no unexpected messages. + AT_CHECK([! grep 'unexpected' FILE.err]) + + dnl Check that the data is received twice and the condition is working. + AT_CHECK([sort FILE.out | uuidfilt], [0], +[[000: simple: change conditions +001: table simple: i=0 r=0 b=true s=third row u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1> +001: table simple: i=1 r=2 b=true s=first row u=<2> ia=[1 2 3] ra=[-0.5] ba=[true] sa=[abc def] ua=[<3> <4>] uuid=<5> +002: table simple: i=0 r=0 b=true s=third row u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1> +002: table simple: i=1 r=2 b=true s=first row u=<2> ia=[1 2 3] ra=[-0.5] ba=[true] sa=[abc def] ua=[<3> <4>] uuid=<5> +]])]) + AT_CLEANUP]) + +OVSDB_CHECK_IDL_CHANGE_AWARE([standalone]) +OVSDB_CHECK_IDL_CHANGE_AWARE([clustered]) diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c index c761822e6..d6a47de33 100644 --- a/tests/test-ovsdb.c +++ b/tests/test-ovsdb.c @@ -2800,6 +2800,13 @@ do_idl(struct ovs_cmdl_context *ctx) } else { print_idl(idl, step++, terse); } + + /* Just run IDL forever for a simple monitoring. */ + if (!strcmp(arg, "monitor")) { + seqno = ovsdb_idl_get_seqno(idl); + i--; + continue; + } } seqno = ovsdb_idl_get_seqno(idl); diff --git a/tests/test-ovsdb.py b/tests/test-ovsdb.py index 71248854f..48f8ee2d7 100644 --- a/tests/test-ovsdb.py +++ b/tests/test-ovsdb.py @@ -757,16 +757,23 @@ def do_idl(schema_file, remote, *commands): poller.block() else: # Wait for update. - while idl.change_seqno == seqno and not idl.run(): - rpc.run() - - poller = ovs.poller.Poller() - idl.wait(poller) - rpc.wait(poller) - poller.block() - - print_idl(idl, step, terse) - step += 1 + while True: + while idl.change_seqno == seqno and not idl.run(): + rpc.run() + + poller = ovs.poller.Poller() + idl.wait(poller) + rpc.wait(poller) + poller.block() + + print_idl(idl, step, terse) + step += 1 + + # Run IDL forever in case of a simple monitor, otherwise + # break and execute the command. + seqno = idl.change_seqno + if command != "monitor": + break seqno = idl.change_seqno
Add new 'monitor' command to test-ovsdb utilities to make them just run IDL loop infinitely. Other commands can still be placed before the 'monitor', e.g. setting up conditions, tracking, running a few transactions. Having that, adding a couple test cases for IDL with online database conversion. Test checks that IDL receives monitor cancellation notification and successfully re-sends monitor requests. While at it, adding debug logging to ovsdb-server processes for easier debugging. While working on a test the issue was discovered that schema for standalone databases is not getting updated in the _Server database after conversion. Checking the new schema bits only for clustered databases for now. Signed-off-by: Ilya Maximets <i.maximets@ovn.org> --- WARN: The test is going to fail for python IDL implementation without: https://patchwork.ozlabs.org/project/openvswitch/patch/20231218233124.669221-1-twilson@redhat.com/ tests/ovsdb-idl.at | 100 +++++++++++++++++++++++++++++++++++++++++--- tests/test-ovsdb.c | 7 ++++ tests/test-ovsdb.py | 27 +++++++----- 3 files changed, 119 insertions(+), 15 deletions(-)