[ovs-dev] [PATCH v3] Dynamically reconnect ovn-controller if ovn-remote value changes

[Lance Richardson]

----- Original Message -----
> From: "Ryan Moats" <rmoats at us.ibm.com>
> To: dev at openvswitch.org
> Sent: Tuesday, April 5, 2016 9:53:44 AM
> Subject: [ovs-dev] [PATCH v3] Dynamically reconnect ovn-controller if	ovn-remote value changes
> 
> From: RYAN D. MOATS <rmoats at us.ibm.com>
> 
> Allows for auto detection and reconnect if the ovn-remote needs
> to change.  Ovn-controller test case updated to include testing
> this code.
> 
> v2->v3:  Addressed Ben's comments from [1]
>     Moved check of remote to head of main loop.
>     Created ovsdb_idl_set_remote to handle change and avoid
>         blocking fetch of initial snapshot
>     Adding on_exit hook and use of
>         OVS_APP_EXIT_AND_WAIT_BY_TARGET in test
> 
> Signed-off-by: RYAN D. MOATS <rmoats at us.ibm.com>

Hi Ryan, two comments below...

> ---
>  lib/ovsdb-idl.c                 |   16 ++++++++++++++++
>  lib/ovsdb-idl.h                 |    1 +
>  ovn/controller/ovn-controller.c |   10 ++++++++++
>  tests/ovn-controller.at         |   16 ++++++++++++++++
>  4 files changed, 43 insertions(+), 0 deletions(-)
> 
> diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
> index 3ab05a3..24c536d 100644
> --- a/lib/ovsdb-idl.c
> +++ b/lib/ovsdb-idl.c
> @@ -272,6 +272,22 @@ ovsdb_idl_create(const char *remote, const struct
> ovsdb_idl_class *class,
>      return idl;
>  }
>  
> +/* chagnes the remote and creates a new session. */

s/chagnes/Changes/

> +
> +void
> +ovsdb_idl_set_remote(struct ovsdb_idl *idl, const char *remote,
> +                     bool retry)
> +{
> +    if (idl) {
> +        ovs_assert(!idl->txn);
> +        ovsdb_idl_clear(idl);
> +        idl->session = jsonrpc_session_open(remote, retry);
> +        idl->state_seqno = UINT_MAX;
> +        idl->request_id = NULL;
> +        idl->schema = NULL;
> +    }
> +}
> +
>  /* Destroys 'idl' and all of the data structures that it manages. */
>  void
>  ovsdb_idl_destroy(struct ovsdb_idl *idl)
> diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
> index e2e2a5e..bad2dc6 100644
> --- a/lib/ovsdb-idl.h
> +++ b/lib/ovsdb-idl.h
> @@ -51,6 +51,7 @@ struct ovsdb_idl *ovsdb_idl_create(const char *remote,
>                                     const struct ovsdb_idl_class *,
>                                     bool monitor_everything_by_default,
>                                     bool retry);
> +void ovsdb_idl_set_remote(struct ovsdb_idl *, const char *, bool);
>  void ovsdb_idl_destroy(struct ovsdb_idl *);
>  
>  void ovsdb_idl_run(struct ovsdb_idl *);
> diff --git a/ovn/controller/ovn-controller.c
> b/ovn/controller/ovn-controller.c
> index 6027011..2ee4589 100644
> --- a/ovn/controller/ovn-controller.c
> +++ b/ovn/controller/ovn-controller.c
> @@ -302,6 +302,16 @@ main(int argc, char *argv[])
>      /* Main loop. */
>      exiting = false;
>      while (!exiting) {
> +        /* Check OVN SB database. */
> +        char *new_ovnsb_remote = get_ovnsb_remote(ovs_idl_loop.idl);
> +        if (strcmp(ovnsb_remote, new_ovnsb_remote)) {
> +            free (ovnsb_remote);
> +            ovnsb_remote = new_ovnsb_remote;
> +            ovsdb_idl_set_remote(ovnsb_idl_loop.idl, ovnsb_remote, true);
> +        } else {
> +            free (new_ovnsb_remote);
> +        }
> +
>          struct controller_ctx ctx = {
>              .ovs_idl = ovs_idl_loop.idl,
>              .ovs_idl_txn = ovsdb_idl_loop_run(&ovs_idl_loop),
> diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
> index 4bee3ca..aa04d4a 100644
> --- a/tests/ovn-controller.at
> +++ b/tests/ovn-controller.at
> @@ -120,6 +120,21 @@ check_patches \
>      'br-int patch-quux-to-baz patch-baz-to-quux' \
>      'br-int patch-baz-to-quux patch-quux-to-baz'
>  
> +# Create an empty database, serve it and switch to it
> +# and verify that the OVS patch ports disappear
> +# then put it back and verify that they reappear
> +
> +#on_exit 'kill `cat $ovs_base/ovn-sb/ovsdb-server-2.pid`'

This line is commented out, should it be deleted? BTW, using
OVS_APP_EXIT_AND_WAIT and friends is the preferred way to clean
up since killing the process will cause code coverage information
to be lost (and worse, coverage data can be corrupted if the process
is killed while in the midst of writing coverage data to disk).

> +
> +ovsdb-tool create $ovs_base/ovn-sb/ovn-sb1.db
> "$abs_top_srcdir"/ovn/ovn-sb.ovsschema
> +as ovn-sb ovsdb-server --detach
> --pidfile=$ovs_base/ovn-sb/ovsdb-server-2.pid
> --remote=punix:$ovs_base/ovn-sb/ovn-sb1.sock $ovs_base/ovn-sb/ovn-sb1.db
> +AT_CHECK([ovs-vsctl -- set Open_vSwitch .
> external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb1.sock])
> +check_patches
> +AT_CHECK([ovs-vsctl -- set Open_vSwitch .
> external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock])
> +check_patches \
> +    'br-int patch-quux-to-baz patch-baz-to-quux' \
> +    'br-int patch-baz-to-quux patch-quux-to-baz'
> +
>  # Change the logical patch ports to VIFs and verify that the OVS patch
>  # ports disappear.
>  AT_CHECK([ovn-sbctl \
> @@ -138,6 +153,7 @@ OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
>  OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>  
>  as ovn-sb
> +OVS_APP_EXIT_AND_WAIT_BY_TARGET([ovsdb-server-2])
>  OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>  
>  AT_CLEANUP
> --
> 1.7.1
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>