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.

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?

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

Reply via email to