[ovs-dev] [PATCH ovn v3] northd: Add `status` management command
Frode Nordahl
frode.nordahl at canonical.com
Wed Nov 20 14:32:27 UTC 2019
On Wed, Nov 20, 2019 at 2:20 PM Numan Siddique <numans at ovn.org> wrote:
>
> On Wed, Nov 20, 2019 at 3:10 PM Frode Nordahl
> <frode.nordahl at canonical.com> 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
> > <frode.nordahl at canonical.com> 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 <frode.nordahl at canonical.com>
> > > ---
> > > 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
> > > dev at openvswitch.org
> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
More information about the dev
mailing list