[ovs-dev] [PATCH ovn v2] northd: Add `is-active` management command

[Numan Siddique]

On Sat, Nov 16, 2019 at 2:46 AM Frode Nordahl
<frode.nordahl at canonical.com> wrote:
>
> When ovn-northd is connected to clustered OVN DB servers a OVSDB
> locking feature is used to ensure only one ovn-northd process is
> active at a time.
>
> This patch adds a `is-active` management command that allows the
> operator to query whether a ovn-northd process is currently active
> or not.
>
> At present this information is only available in the log.
>
> Signed-off-by: Frode Nordahl <frode.nordahl at canonical.com>

Thanks for the patch. This command would be useful.
HA for ovn-northd (active/standby) is supported using the lock
mechanism and it doesn't
matter if ovn-northd connects to the clustered db or standalone. This
patch assumes
that locking feature is used only for clustered deployment which is wrong.

Also we have commands - "is-paused", "pause" and "resume". "is-active"
command could confuse
the user.

So can you please update the documentation properly ?

Thanks
Numan


> ---
>  northd/ovn-northd.8.xml | 19 +++++++++++++++++++
>  northd/ovn-northd.c     | 18 +++++++++++++++++-
>  2 files changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> index 344cc0dbf..e712000f3 100644
> --- a/northd/ovn-northd.8.xml
> +++ b/northd/ovn-northd.8.xml
> @@ -87,6 +87,12 @@
>        <dd>
>          Returns "true" if ovn-northd is currently paused, "false" otherwise.
>        </dd>
> +
> +      <dt><code>is-active</code></dt>
> +      <dd>
> +        When ovn-northd is connected to clustered OVN DB servers, this returns
> +        "true" if ovn-northd is currently active, "false" otherwise.

You could probably say  something like -
"Returns true if ovn-northd has acquired OVSDB lock and is currently
active, false otherwise.

> +      </dd>
>        </dl>
>      </p>
>
> @@ -130,6 +136,19 @@
>        DB changes.
>      </p>
>
> +    <h2> Active-Standby with clustered OVN DB servers</h2>
> +    <p>
> +      When <code>ovn-northd</code> is connected to clustered OVN DB servers a
> +      OVSDB locking feature will be used to ensure only one
> +      <code>ovn-northd</code> process is active at a time.
> +    </p>
> +
> +    <p>
> +      The <code>ovn-northd</code> daemon will write an entry in its log when
> +      it becomes active.  You may query the active status at any time with
> +      the <code>is-active</code> management command.
> +    </p>

We probably don't need this section as the documentation here [1]
already mentions about it.

[1] - https://github.com/ovn-org/ovn/blob/master/northd/ovn-northd.8.xml#L95

Thanks
Numan


> +
>      <h1>Logical Flow Table Structure</h1>
>
>      <p>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 6742bc002..31a744f4d 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_is_active;
>
>  struct northd_context {
>      struct ovsdb_idl *ovnnb_idl;
> @@ -10425,6 +10426,7 @@ main(int argc, char *argv[])
>      int retval;
>      bool exiting;
>      bool paused;
> +    bool had_lock;
>
>      fatal_ignore_sigpipe();
>      ovs_cmdl_proctitle_init(argc, argv);
> @@ -10450,6 +10452,8 @@ 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("is-active", "", 0, 0, ovn_northd_is_active,
> +                             &had_lock);
>
>      daemonize_complete();
>
> @@ -10636,11 +10640,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 = {
> @@ -10748,3 +10752,15 @@ ovn_northd_is_paused(struct unixctl_conn *conn, int argc OVS_UNUSED,
>          unixctl_command_reply(conn, "false");
>      }
>  }
> +
> +static void
> +ovn_northd_is_active(struct unixctl_conn *conn, int argc OVS_UNUSED,
> +             const char *argv[] OVS_UNUSED, void *had_lock_)
> +{
> +    bool *had_lock = had_lock_;
> +    if (*had_lock) {
> +        unixctl_command_reply(conn, "true");
> +    } else {
> +        unixctl_command_reply(conn, "false");
> +    }
> +}
> --
> 2.20.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>