On Wed, Nov 20, 2019 at 2:20 PM Numan Siddique <[email protected]> wrote: > > On Wed, Nov 20, 2019 at 3:10 PM Frode Nordahl > <[email protected]> wrote: > > > > Hello Numan, > > > > Have you had a chance to take a look at my updated proposal? > > > > Based on your feedback of `is-active` could be confusing since we have > > `is-paused` which is used for something else, the best option I could > > think of was a generic `status` command. I have made the proposal in > > such a way that we can add more to the status command later if we want > > to. > > > > Thanks for the patch. I applied this to master with one small change
Excellent, thank you Numan. > > I guess we could make the status pick up the paused state too, but > > that would mean moving the paused state into the northd_context struct > > and passing that to the status handler or something similar which > > would be a bit more involved change. > > > > What do you think? > > > > I thought about that. but applied the patch as we can address this > ambiguity in a > separate patch, > > May be status can say - "paused", "active" and "standby" > "paused" - when it is passed > "active" - when it has lock and is not paused > "standby" - When it has no lock. > > What do you think about this ? Yes, I think that is a good idea. I'll take a pass at it in a separate patch. There is a remote corner case here as users in effect can run two ovn-northd processes, pause one while it still has the lock and this status will not be visible. But, as far as I can tell from the documentation this is not the intended use case for the pause command, so it might be OK. Alternatively we can either make that scenario visible with a "paused(active)" and "paused(standby)" status, or change the behaviour of the pause command to also drop the lock. -- Frode Nordahl > Thanks > Numan > > > https://patchwork.ozlabs.org/patch/1196828/ > > > > -- > > Frode Nordahl > > > > > > On Mon, Nov 18, 2019 at 4:44 PM Frode Nordahl > > <[email protected]> wrote: > > > > > > Allow the operator to query whether a ovn-northd process is > > > currently active for the standalone and clustered DB use case. > > > > > > At present this information is only available in the log. > > > > > > Signed-off-by: Frode Nordahl <[email protected]> > > > --- > > > northd/ovn-northd.8.xml | 9 ++++++++- > > > northd/ovn-northd.c | 20 +++++++++++++++++++- > > > tests/ovn-northd.at | 9 +++++++++ > > > 3 files changed, 36 insertions(+), 2 deletions(-) > > > > > > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml > > > index 78b1e84ad..d7833cda7 100644 > > > --- a/northd/ovn-northd.8.xml > > > +++ b/northd/ovn-northd.8.xml > > > @@ -87,13 +87,20 @@ > > > <dd> > > > Returns "true" if ovn-northd is currently paused, "false" > > > otherwise. > > > </dd> > > > + > > > + <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. > > > + </dd> > > > </dl> > > > </p> > > > > > > <h1>Active-Standby for High Availability</h1> > > > <p> > > > You may run <code>ovn-northd</code> more than once in an OVN > > > deployment. > > > - OVN will automatically ensure that only one of them is active at a > > > time. > > > + When connected to a standalone or clustered DB setup, OVN will > > > + automatically ensure that only one of them is active at a time. > > > If multiple instances of <code>ovn-northd</code> are running and > > > the > > > active <code>ovn-northd</code> fails, one of the hot standby > > > instances > > > of <code>ovn-northd</code> will automatically take over. > > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > > > index 41e97f841..83ad6d518 100644 > > > --- a/northd/ovn-northd.c > > > +++ b/northd/ovn-northd.c > > > @@ -55,6 +55,7 @@ static unixctl_cb_func ovn_northd_exit; > > > static unixctl_cb_func ovn_northd_pause; > > > static unixctl_cb_func ovn_northd_resume; > > > static unixctl_cb_func ovn_northd_is_paused; > > > +static unixctl_cb_func ovn_northd_status; > > > > > > struct northd_context { > > > struct ovsdb_idl *ovnnb_idl; > > > @@ -10838,6 +10839,7 @@ main(int argc, char *argv[]) > > > int retval; > > > bool exiting; > > > bool paused; > > > + bool had_lock; > > > > > > fatal_ignore_sigpipe(); > > > ovs_cmdl_proctitle_init(argc, argv); > > > @@ -10863,6 +10865,7 @@ main(int argc, char *argv[]) > > > unixctl_command_register("resume", "", 0, 0, ovn_northd_resume, > > > &paused); > > > unixctl_command_register("is-paused", "", 0, 0, ovn_northd_is_paused, > > > &paused); > > > + unixctl_command_register("status", "", 0, 0, ovn_northd_status, > > > &had_lock); > > > > > > daemonize_complete(); > > > > > > @@ -11068,11 +11071,11 @@ main(int argc, char *argv[]) > > > * 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"); > > > - bool had_lock = false; > > > > > > /* Main loop. */ > > > exiting = false; > > > paused = false; > > > + had_lock = false; > > > while (!exiting) { > > > if (!paused) { > > > struct northd_context ctx = { > > > @@ -11180,3 +11183,18 @@ ovn_northd_is_paused(struct unixctl_conn *conn, > > > int argc OVS_UNUSED, > > > unixctl_command_reply(conn, "false"); > > > } > > > } > > > + > > > +static void > > > +ovn_northd_status(struct unixctl_conn *conn, int argc OVS_UNUSED, > > > + const char *argv[] OVS_UNUSED, void *had_lock_) > > > +{ > > > + bool *had_lock = had_lock_; > > > + /* > > > + * 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"); > > > + unixctl_command_reply(conn, ds_cstr(&s)); > > > + ds_destroy(&s); > > > +} > > > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > > > index da566f900..17e60b051 100644 > > > --- a/tests/ovn-northd.at > > > +++ b/tests/ovn-northd.at > > > @@ -899,6 +899,15 @@ 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 ovs-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 > > > -- > > > 2.24.0 > > > > > > _______________________________________________ > > > dev mailing list > > > [email protected] > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > _______________________________________________ > > dev mailing list > > [email protected] > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
