Move paused state to ``struct northd_context`` and pass the context to paused and status command handlers.
On pause release the OVSDB lock on SB DB. Re-instante the lock on resume. Status command will now provide accurate information for 'paused', 'active' and 'standby' states. Merge separate status command test into the pause and resume tests. Signed-off-by: Frode Nordahl <frode.nord...@canonical.com> --- northd/ovn-northd.8.xml | 9 +++-- northd/ovn-northd.c | 87 +++++++++++++++++++++++++++-------------- tests/ovn-northd.at | 24 +++++++----- 3 files changed, 79 insertions(+), 41 deletions(-) diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml index 9734e79e6..c6d5d96b9 100644 --- a/northd/ovn-northd.8.xml +++ b/northd/ovn-northd.8.xml @@ -74,13 +74,15 @@ <dt><code>pause</code></dt> <dd> Pauses the ovn-northd operation from processing any Northbound and - Southbound database changes. + Southbound database changes. This will also instruct ovn-northd to + drop any lock on SB DB. </dd> <dt><code>resume</code></dt> <dd> Resumes the ovn-northd operation to process Northbound and - Southbound database contents and generate logical flows. + Southbound database contents and generate logical flows. This will + also instruct ovn-northd to aspire for the lock on SB DB. </dd> <dt><code>is-paused</code></dt> @@ -91,7 +93,8 @@ <dt><code>status</code></dt> <dd> Prints this server's status. Status will be "active" if ovn-northd has - acquired OVSDB lock on NB DB, "standby" otherwise. + acquired OVSDB lock on SB DB, "standby" if it has not or "paused" if + this instance is paused. </dd> </dl> </p> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index a943e1037..e19515d14 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -65,6 +65,7 @@ struct northd_context { struct ovsdb_idl_index *sbrec_ha_chassis_grp_by_name; struct ovsdb_idl_index *sbrec_mcast_group_by_name_dp; struct ovsdb_idl_index *sbrec_ip_mcast_by_dp; + bool paused; }; /* An IPv4 or IPv6 address */ @@ -10836,6 +10837,8 @@ add_column_noalert(struct ovsdb_idl *idl, ovsdb_idl_omit_alert(idl, column); } +#define OVN_NORTHD_SB_DB_LOCK_NAME "ovn_northd" + int main(int argc, char *argv[]) { @@ -10843,8 +10846,10 @@ main(int argc, char *argv[]) struct unixctl_server *unixctl; int retval; bool exiting; - bool paused; bool had_lock; + struct northd_context ctx; + + memset(&ctx, 0, sizeof(ctx)); fatal_ignore_sigpipe(); ovs_cmdl_proctitle_init(argc, argv); @@ -10866,11 +10871,11 @@ main(int argc, char *argv[]) exit(EXIT_FAILURE); } unixctl_command_register("exit", "", 0, 0, ovn_northd_exit, &exiting); - unixctl_command_register("pause", "", 0, 0, ovn_northd_pause, &paused); - unixctl_command_register("resume", "", 0, 0, ovn_northd_resume, &paused); + unixctl_command_register("pause", "", 0, 0, ovn_northd_pause, &ctx); + unixctl_command_register("resume", "", 0, 0, ovn_northd_resume, &ctx); unixctl_command_register("is-paused", "", 0, 0, ovn_northd_is_paused, - &paused); - unixctl_command_register("status", "", 0, 0, ovn_northd_status, &had_lock); + &ctx); + unixctl_command_register("status", "", 0, 0, ovn_northd_status, &ctx); daemonize_complete(); @@ -11075,23 +11080,21 @@ main(int argc, char *argv[]) /* Ensure that only a single ovn-northd is active in the deployment by * acquiring a lock called "ovn_northd" on the southbound database * and then only performing DB transactions if the lock is held. */ - ovsdb_idl_set_lock(ovnsb_idl_loop.idl, "ovn_northd"); + ovsdb_idl_set_lock(ovnsb_idl_loop.idl, OVN_NORTHD_SB_DB_LOCK_NAME); /* Main loop. */ exiting = false; - paused = false; + ctx.paused = false; had_lock = false; while (!exiting) { - if (!paused) { - struct northd_context ctx = { - .ovnnb_idl = ovnnb_idl_loop.idl, - .ovnnb_txn = ovsdb_idl_loop_run(&ovnnb_idl_loop), - .ovnsb_idl = ovnsb_idl_loop.idl, - .ovnsb_txn = ovsdb_idl_loop_run(&ovnsb_idl_loop), - .sbrec_ha_chassis_grp_by_name = sbrec_ha_chassis_grp_by_name, - .sbrec_mcast_group_by_name_dp = sbrec_mcast_group_by_name_dp, - .sbrec_ip_mcast_by_dp = sbrec_ip_mcast_by_dp, - }; + if (!ctx.paused) { + ctx.ovnnb_idl = ovnnb_idl_loop.idl; + ctx.ovnnb_txn = ovsdb_idl_loop_run(&ovnnb_idl_loop); + ctx.ovnsb_idl = ovnsb_idl_loop.idl; + ctx.ovnsb_txn = ovsdb_idl_loop_run(&ovnsb_idl_loop); + ctx.sbrec_ha_chassis_grp_by_name = sbrec_ha_chassis_grp_by_name; + ctx.sbrec_mcast_group_by_name_dp = sbrec_mcast_group_by_name_dp; + ctx.sbrec_ip_mcast_by_dp = sbrec_ip_mcast_by_dp; if (!had_lock && ovsdb_idl_has_lock(ovnsb_idl_loop.idl)) { VLOG_INFO("ovn-northd lock acquired. " @@ -11125,6 +11128,11 @@ main(int argc, char *argv[]) ovsdb_idl_run(ovnsb_idl_loop.idl); ovsdb_idl_wait(ovnnb_idl_loop.idl); ovsdb_idl_wait(ovnsb_idl_loop.idl); + /* + * the lock is dropped on pause, avoid incorrect message logged + * about lock lost when resumed. + */ + had_lock = false; } unixctl_server_run(unixctl); @@ -11159,30 +11167,41 @@ ovn_northd_exit(struct unixctl_conn *conn, int argc OVS_UNUSED, static void ovn_northd_pause(struct unixctl_conn *conn, int argc OVS_UNUSED, - const char *argv[] OVS_UNUSED, void *pause_) + const char *argv[] OVS_UNUSED, void *ctx_) { - bool *pause = pause_; - *pause = true; + struct northd_context *ctx = ctx_; + + if (!ctx->paused && ctx->ovnsb_idl != NULL) { + /* Drop our aspiration for the lock while paused */ + ovsdb_idl_set_lock(ctx->ovnsb_idl, NULL); + ctx->paused = true; + VLOG_INFO("This ovn-northd instance is now paused."); + } unixctl_command_reply(conn, NULL); } static void ovn_northd_resume(struct unixctl_conn *conn, int argc OVS_UNUSED, - const char *argv[] OVS_UNUSED, void *pause_) + const char *argv[] OVS_UNUSED, void *ctx_) { - bool *pause = pause_; - *pause = false; + struct northd_context *ctx = ctx_; + + if (ctx->paused && ctx->ovnsb_idl != NULL) { + /* Reinstate our aspiration for lock */ + ovsdb_idl_set_lock(ctx->ovnsb_idl, OVN_NORTHD_SB_DB_LOCK_NAME); + ctx->paused = false; + } unixctl_command_reply(conn, NULL); } static void ovn_northd_is_paused(struct unixctl_conn *conn, int argc OVS_UNUSED, - const char *argv[] OVS_UNUSED, void *paused_) + const char *argv[] OVS_UNUSED, void *ctx_) { - bool *paused = paused_; - if (*paused) { + struct northd_context *ctx = ctx_; + if (ctx->paused) { unixctl_command_reply(conn, "true"); } else { unixctl_command_reply(conn, "false"); @@ -11191,15 +11210,25 @@ ovn_northd_is_paused(struct unixctl_conn *conn, int argc OVS_UNUSED, static void ovn_northd_status(struct unixctl_conn *conn, int argc OVS_UNUSED, - const char *argv[] OVS_UNUSED, void *had_lock_) + const char *argv[] OVS_UNUSED, void *ctx_) { - bool *had_lock = had_lock_; + struct northd_context *ctx = ctx_; + char *status; + + if (ctx->paused) { + status = "paused"; + } else if (ctx->ovnsb_idl != NULL) { + status = ovsdb_idl_has_lock(ctx->ovnsb_idl) ? "active" : "standby"; + } else { + status = "unknown"; + } + /* * Use a labelled formatted output so we can add more to the status command * later without breaking any consuming scripts */ struct ds s = DS_EMPTY_INITIALIZER; - ds_put_format(&s, "Status: %s\n", *had_lock ? "active" : "standby"); + ds_put_format(&s, "Status: %s\n", status); unixctl_command_reply(conn, ds_cstr(&s)); ds_destroy(&s); } diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 1c94f2f65..d73a22f68 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -899,22 +899,18 @@ OVS_APP_EXIT_AND_WAIT([ovn-northd]) AT_CLEANUP -AT_SETUP([ovn -- ovn-northd status]) -AT_SKIP_IF([test $HAVE_PYTHON = no]) -ovn_start - -AT_CHECK([as northd ovn-appctl -t ovn-northd status], [0], [Status: active -]) - -AT_CLEANUP - AT_SETUP([ovn -- ovn-northd pause and resume]) AT_SKIP_IF([test $HAVE_PYTHON = no]) ovn_start AT_CHECK([test xfalse = x`as northd ovn-appctl -t ovn-northd is-paused`]) +AT_CHECK([as northd ovn-appctl -t ovn-northd status], [0], [Status: active +]) AT_CHECK([test xfalse = x`as northd-backup ovn-appctl -t ovn-northd \ is-paused`]) +AT_CHECK([as northd-backup ovn-appctl -t ovn-northd status], [0], +[Status: standby +]) ovn-nbctl ls-add sw0 @@ -931,7 +927,12 @@ OVS_WAIT_UNTIL([ as northd ovs-appctl -t ovn-northd pause as northd-backup ovs-appctl -t ovn-northd pause AT_CHECK([test xtrue = x`as northd ovn-appctl -t ovn-northd is-paused`]) +AT_CHECK([as northd ovn-appctl -t ovn-northd status], [0], [Status: paused +]) AT_CHECK([test xtrue = x`as northd-backup ovn-appctl -t ovn-northd is-paused`]) +AT_CHECK([as northd-backup ovn-appctl -t ovn-northd status], [0], +[Status: paused +]) ovn-nbctl ls-add sw0 @@ -944,8 +945,13 @@ OVS_WAIT_UNTIL([ as northd ovs-appctl -t ovn-northd resume as northd-backup ovs-appctl -t ovn-northd resume AT_CHECK([test xfalse = x`as northd ovn-appctl -t ovn-northd is-paused`]) +AT_CHECK([as northd ovn-appctl -t ovn-northd status], [0], [Status: active +]) AT_CHECK([test xfalse = x`as northd-backup ovn-appctl -t ovn-northd \ is-paused`]) +AT_CHECK([as northd-backup ovn-appctl -t ovn-northd status], [0], +[Status: standby +]) OVS_WAIT_UNTIL([ ovn-sbctl lflow-list sw0 -- 2.24.0 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev