[ovs-dev] [PATCH ovn v2 2/2] northd: Improve handling of pause and resume
Frode Nordahl
frode.nordahl at canonical.com
Thu Nov 21 16:32:59 UTC 2019
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.nordahl at 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
.
/
More information about the dev
mailing list