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

****
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 17e60b051..c73fd9003 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -903,7 +903,7 @@ 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_CHECK([as northd ovn-appctl -t ovn-northd status], [0], [Status: active
 ])

 AT_CLEANUP
****

> 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 ?

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

Reply via email to