[ovs-dev] [PATCH V3] Separating ovn nb&sb databases

Russell Bryant russell at ovn.org
Fri Feb 12 19:29:07 UTC 2016


On 02/10/2016 09:56 AM, Michael wrote:
> Added a check on pid in start_northd
> Added ssl params option cloning the conf db of normal openvswitch db (in this way ssl should follow exactly the same behaviour as in normal ovsdb)
> Added configuration options for logging

Please line wrap the commit message at 79 characters.

I'm also not able to apply this patch.  It seems to be because of how
you've sent it using Apple Mail.  There seems to be regular problems
like this unless you use the "git send-email" utility, instead.

If you have trouble getting "git send-email" to work, feel free to just
submit a github pull request, instead.

> 
> Signed-off-by: Michael Arnaldi <mike at mymoneyex.com>
> ---
> NEWS                      |   2 +
> ovn/northd/ovn-northd.c   |  33 ++++++---
> ovn/utilities/ovn-ctl     | 172 ++++++++++++++++++++++++++++++++++++++--------
> ovn/utilities/ovn-nbctl.c |   2 +-
> ovn/utilities/ovn-sbctl.c |   3 +-
> 5 files changed, 171 insertions(+), 41 deletions(-)
> 
> diff --git a/NEWS b/NEWS
> index 3e33871..aa53e8c 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,5 +1,7 @@
> Post-v2.5.0
> ---------------------
> +   - ovn:
> +     * Separation of NB & SB db

I would expand on this a bit.  I would say that ovn-ctl now launches
independent ovsdb-server instances for both the northbound and
southbound databases.  I would also note that this is not backwards
compatible with systems using ovn-ctl prior to this change.

> - ovsdb-server:
>   * New "monitor2" and "update2" extensions to RFC 7047.
> - OpenFlow:
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index e6271cf..051c75a 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -52,7 +52,8 @@ struct northd_context {
> static const char *ovnnb_db;
> static const char *ovnsb_db;
> 
> -static const char *default_db(void);
> +static const char *default_nb_db(void);
> +static const char *default_sb_db(void);
> 
> /* Pipeline stages. */
> 
> @@ -167,7 +168,7 @@ Options:\n\
> -h, --help                display this help message\n\
> -o, --options             list available options\n\
> -V, --version             display version information\n\
> -", program_name, program_name, default_db(), default_db());
> +", program_name, program_name, default_nb_db(), default_sb_db());
>  daemon_usage();
>  vlog_usage();
>  stream_usage("database", true, true, false);
> @@ -1786,15 +1787,26 @@ ovnsb_db_run(struct northd_context *ctx)
> }
> 
> 
> -static char *default_db_;
> +static char *default_nb_db_;
> 
> static const char *
> -default_db(void)
> +default_nb_db(void)
> {
> -    if (!default_db_) {
> -        default_db_ = xasprintf("unix:%s/db.sock", ovs_rundir());
> +    if (!default_nb_db_) {
> +        default_nb_db_ = xasprintf("unix:%s/ovnnb_db.sock", ovs_rundir());
>  }
> -    return default_db_;
> +    return default_nb_db_;
> +}
> +
> +static char *default_sb_db_;
> +
> +static const char *
> +default_sb_db(void)
> +{
> +    if (!default_sb_db_) {
> +        default_sb_db_ = xasprintf("unix:%s/ovnsb_db.sock", ovs_rundir());
> +    }
> +    return default_sb_db_;
> }
> 
> static void
> @@ -1856,11 +1868,11 @@ parse_options(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
>  }
> 
>  if (!ovnsb_db) {
> -        ovnsb_db = default_db();
> +        ovnsb_db = default_sb_db();
>  }
> 
>  if (!ovnnb_db) {
> -        ovnnb_db = default_db();
> +        ovnnb_db = default_nb_db();
>  }
> 
>  free(short_options);
> @@ -1976,7 +1988,8 @@ main(int argc, char *argv[])
>  ovsdb_idl_loop_destroy(&ovnsb_idl_loop);
>  service_stop();
> 
> -    free(default_db_);
> +    free(default_nb_db_);
> +    free(default_sb_db_);
>  exit(res);
> }
> 
> diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl
> index b171934..4563279 100755
> --- a/ovn/utilities/ovn-ctl
> +++ b/ovn/utilities/ovn-ctl
> @@ -30,37 +30,102 @@ done
> ## start ##
> ## ----- ##
> 
> -upgrade_ovn_dbs () {
> -    ovn_dbs=$(ovs-appctl -t ovsdb-server ovsdb-server/list-dbs 2>/dev/null)
> -    for db in $ovn_dbs; do
> -        case $db in
> -            OVN*)
> -                action "Removing $db from ovsdb-server" \
> -                    ovs-appctl -t ovsdb-server ovsdb-server/remove-db $db
> -                ;;
> -        esac
> -    done
> -    upgrade_db "$DB_NB_FILE" "$DB_NB_SCHEMA"
> -    upgrade_db "$DB_SB_FILE" "$DB_SB_SCHEMA"
> -    for db in $DB_NB_FILE $DB_SB_FILE; do
> -        action "Adding $db to ovsdb-server" \
> -            ovs-appctl -t ovsdb-server ovsdb-server/add-db $db || exit 1
> -    done
> +pidfile_is_running () {
> +    pidfile=$1
> +    test -e "$pidfile" && pid=`cat "$pidfile"` && pid_exists "$pid"
> +} >/dev/null 2>&1
> +
> +stop_ovsdb () {
> +    if pidfile_is_running $DB_NB_PID; then
> +        kill -9 $(cat $DB_NB_PID) 1>/dev/null 2>/dev/null
> +        rm -f $DB_NB_PID 1>/dev/null 2>/dev/null
> +    fi
> +
> +    if pidfile_is_running $DB_SB_PID; then
> +        kill -9 $(cat $DB_SB_PID) 1>/dev/null 2>/dev/null
> +        rm -f $DB_SB_PID 1>/dev/null 2>/dev/null
> +    fi
> +}
> +
> +start_ovsdb () {
> +    # Check and eventually start ovsdb-server for Northbound DB
> +    if ! pidfile_is_running $DB_NB_PID; then
> +        upgrade_db "$DB_NB_FILE" "$DB_NB_SCHEMA" 1>/dev/null 2>/dev/null
> +
> +        if [ ! -f $DB_NB_CONF_FILE ]; then
> +            cp -f $DB_CONF_FILE $DB_NB_CONF_FILE
> +        fi

Copying an existing Open_vSwitch database could be really confusing.
We'll be running with an arbitrary snapshot of a db potentially in use
for real ovs things on the system already.

I think at this point we should just drop trying to retain SSL support
and look at adding it later, probably without a dependency on the
Open_vSwitch schema.

> +
> +        set ovsdb-server
> +
> +        set "$@" --detach $OVN_OVSDB_LOG --remote=punix:$DB_NB_SOCK --remote=ptcp:$DB_NB_PORT --pidfile=$DB_NB_PID
> +        set "$@" --private-key=db:Open_vSwitch,SSL,private_key
> +        set "$@" --certificate=db:Open_vSwitch,SSL,certificate
> +        set "$@" --bootstrap-ca-cert=db:Open_vSwitch,SSL,ca_cert
> +
> +        $@ $DB_NB_FILE $DB_NB_CONF_FILE
> +    fi
> +
> +    # Check and eventually start ovsdb-server for Southbound DB
> +    if ! pidfile_is_running $DB_SB_PID; then
> +        upgrade_db "$DB_SB_FILE" "$DB_SB_SCHEMA" 1>/dev/null 2>/dev/null
> +
> +        if [ ! -f $DB_SB_CONF_FILE ]; then
> +            cp -f $DB_CONF_FILE $DB_SB_CONF_FILE
> +        fi
> +
> +        set ovsdb-server
> +
> +        set "$@" --detach $OVN_OVSDB_LOG --remote=punix:$DB_SB_SOCK --remote=ptcp:$DB_SB_PORT --pidfile=$DB_SB_PID
> +        set "$@" --private-key=db:Open_vSwitch,SSL,private_key
> +        set "$@" --certificate=db:Open_vSwitch,SSL,certificate
> +        set "$@" --bootstrap-ca-cert=db:Open_vSwitch,SSL,ca_cert
> +
> +        $@ $DB_SB_FILE $DB_SB_CONF_FILE
> +    fi
> +}
> +
> +status_ovsdb () {
> +  if ! pidfile_is_running $DB_NB_PID; then
> +      log_success_msg "OVN Northbound DB is not running"
> +    else
> +        log_success_msg "OVN Northbound DB is running"
> +  fi
> +  if ! pidfile_is_running $DB_SB_PID; then
> +      log_success_msg "OVN Southbound DB is not running"
> +  else
> +      log_success_msg "OVN Southbound DB is running"
> +  fi
> }
> 
> start_northd () {
>  # We expect ovn-northd to be co-located with ovsdb-server handling both the
>  # OVN_Northbound and OVN_Southbound dbs.
> -    upgrade_ovn_dbs
> +    if test X"$OVN_MANAGE_OVSDB" = Xyes; then
> +        start_ovsdb
> +    fi
> 
> -    set ovn-northd
> -    set "$@" -vconsole:emer -vsyslog:err -vfile:info
> -    OVS_RUNDIR=${OVN_RUNDIR} start_daemon "$OVN_NORTHD_PRIORITY" "$OVN_NORTHD_WRAPPER" "$@"
> +    if ! pidfile_is_running $DB_NB_PID; then
> +        log_failure_msg "OVN Northbound DB is not running"
> +        exit
> +    fi
> +    if ! pidfile_is_running $DB_SB_PID; then
> +        log_failure_msg "OVN Southbound DB is not running"
> +        exit
> +    fi
> +
> +    if daemon_is_running ovn-northd; then
> +        log_success_msg "OVN Northbound is already running"
> +    else
> +        set ovn-northd
> +        set "$@" $OVN_NORTHD_LOG --ovnnb-db=unix:$DB_NB_SOCK --ovnsb-db=unix:$DB_SB_SOCK
> +        OVS_RUNDIR=${OVN_RUNDIR} start_daemon "$OVN_NORTHD_PRIORITY" "$OVN_NORTHD_WRAPPER" "$@"
> +    fi
> }
> 
> start_controller () {
>  set ovn-controller "unix:$DB_SOCK"
> -    set "$@" -vconsole:emer -vsyslog:err -vfile:info
> +    set "$@" $OVN_CONTROLLER_LOG
>  OVS_RUNDIR=${OVN_RUNDIR} start_daemon "$OVN_CONTROLLER_PRIORITY" "$OVN_CONTROLLER_WRAPPER" "$@"
> }
> 
> @@ -70,6 +135,10 @@ start_controller () {
> 
> stop_northd () {
>  OVS_RUNDIR=${OVN_RUNDIR} stop_daemon ovn-northd
> +
> +    if test X"$OVN_MANAGE_OVSDB" = Xyes; then
> +        stop_ovsdb
> +    fi
> }
> 
> stop_controller () {
> @@ -90,17 +159,38 @@ restart_controller () {
>  start_controller
> }
> 
> +restart_ovsdb () {
> +    stop_ovsdb
> +    start_ovsdb
> +}
> +
> ## ---- ##
> ## main ##
> ## ---- ##
> 
> set_defaults () {
> -    DB_SOCK=$rundir/db.sock
> -    DB_NB_FILE=$dbdir/ovnnb.db
> -    DB_SB_FILE=$dbdir/ovnsb.db
> +    OVN_DIR=$rundir
> +    OVN_MANAGE_OVSDB=yes
> +
> +    DB_NB_SOCK=$OVN_DIR/ovnnb_db.sock
> +    DB_NB_PID=$OVN_DIR/ovnnb_db.pid
> +    DB_NB_FILE=$OVN_DIR/ovnnb_db.db
> +    DB_NB_PORT=6641
> +
> +    DB_SB_SOCK=$OVN_DIR/ovnsb_db.sock
> +    DB_SB_PID=$OVN_DIR/ovnsb_db.pid
> +    DB_SB_FILE=$OVN_DIR/ovnsb_db.db
> +    DB_SB_PORT=6642
> +
>  DB_NB_SCHEMA=$datadir/ovn-nb.ovsschema
>  DB_SB_SCHEMA=$datadir/ovn-sb.ovsschema
> 
> +    DB_SOCK=$rundir/db.sock
> +    DB_CONF_FILE=$dbdir/conf.db
> +
> +    DB_NB_CONF_FILE=$OVN_DIR/ovnnb_conf.db
> +    DB_SB_CONF_FILE=$OVN_DIR/ovnsb_conf.db
> +
>  OVN_NORTHD_PRIORITY=-10
>  OVN_NORTHD_WRAPPER=
>  OVN_CONTROLLER_PRIORITY=-10
> @@ -108,6 +198,10 @@ set_defaults () {
> 
>  OVS_RUNDIR=${OVS_RUNDIR:-${rundir}}
>  OVN_RUNDIR=${OVN_RUNDIR:-${OVS_RUNDIR}}
> +
> +    OVN_CONTROLLER_LOG="-vconsole:emer -vsyslog:err -vfile:info"
> +    OVN_NORHD_LOG="-vconsole:emer -vsyslog:err -vfile:info"
> +    OVN_OVSDB_LOG="-vconsole:off"
> }
> 
> set_option () {
> @@ -134,18 +228,25 @@ startup scripts.  System administrators should not normally invoke it directly.
> 
> Commands:
> start_northd           start ovn-northd
> +  start_ovsdb            start ovn related ovsdb-server processes
> start_controller       start ovn-controller
> stop_northd            stop ovn-northd
> +  stop_ovsdb             stop ovn related ovsdb-server processes
> stop_controller        stop ovn-controller
> restart_northd         restart ovn-northd
> +  restart_ovsdb          restart ovn related ovsdb-server processes
> restart_controller     restart ovn-controller
> 
> Options:
> -  --ovn-northd-priority=NICE     set ovn-northd's niceness (default: $OVN_NORTHD_PRIORITY)
> -  --ovn-northd-wrapper=WRAPPER   run with a wrapper like valgrind for debugging
> +  --ovn-northd-priority=NICE         set ovn-northd's niceness (default: $OVN_NORTHD_PRIORITY)
> +  --ovn-northd-wrapper=WRAPPER       run with a wrapper like valgrind for debugging

Please try to split out unrelated formatting changes into another patch.
 Cleanups are great, but it's helpful to separate them so that the code
review can focus on the real code changes.

> --ovn-controller-priority=NICE     set ovn-northd's niceness (default: $OVN_CONTROLLER_PRIORITY)
> --ovn-controller-wrapper=WRAPPER   run with a wrapper like valgrind for debugging
> -  -h, --help                     display this help message
> +  --ovn-manage-ovsdb=no              manage ovsdb separately from start_northd and stop_northd
> +  --ovn-controller-log=STRING        ovn controller process logging params (default: $OVN_CONTROLLER_LOG)
> +  --ovn-northd-log=STRING            ovn northd process logging params (default: $OVN_NORTHD_LOG)
> +  --ovn-ovsdb-log=STRING             ovn ovsdb-server processes logging params (default: $OVN_OVSDB_LOG)
> +  -h, --help                         display this help message
> 
> File location options:
> --db-sock=SOCKET     JSON-RPC socket name (default: $DB_SOCK)
> @@ -153,12 +254,13 @@ File location options:
> --db-sb-file=FILE    OVN_Southbound db file (default: $DB_SB_FILE)
> --db-nb-schema=FILE  OVN_Northbound db file (default: $DB_NB_SCHEMA)
> --db-sb-schema=FILE  OVN_Southbound db file (default: $DB_SB_SCHEMA)
> +  --db-nb-port=PORT    OVN Northbound db ptcp port (default: $DB_NB_PORT)
> +  --db-sb-port=PORT    OVN Southbound db ptcp port (default: $DB_SB_PORT)
> +  --ovn-dir=FILE       OVN Databases directory (default: $OVN_DIR)
> 
> Default directories with "configure" option and environment variable override:
> logs: /usr/local/var/log/openvswitch (--with-logdir, OVS_LOGDIR)
> pidfiles and sockets: /usr/local/var/run/openvswitch (--with-rundir, OVS_RUNDIR)
> -  ovn-nb.db: /usr/local/etc/openvswitch (--with-dbdir, OVS_DBDIR)
> -  ovn-sb.db: /usr/local/etc/openvswitch (--with-dbdir, OVS_DBDIR)
> system configuration: /usr/local/etc (--sysconfdir, OVS_SYSCONFDIR)
> data files: /usr/local/share/openvswitch (--pkgdatadir, OVS_PKGDATADIR)
> user binaries: /usr/local/bin (--bindir, OVS_BINDIR)
> @@ -210,24 +312,36 @@ case $command in
>  start_northd)
>      start_northd
>      ;;
> +    start_ovsdb)
> +        start_ovsdb
> +        ;;
>  start_controller)
>      start_controller
>      ;;
>  stop_northd)
>      stop_northd
>      ;;
> +    stop_ovsdb)
> +        stop_ovsdb
> +        ;;
>  stop_controller)
>      stop_controller
>      ;;
>  restart_northd)
>      restart_northd
>      ;;
> +    restart_ovsdb)
> +        restart_ovsdb
> +        ;;
>  restart_controller)
>      restart_controller
>      ;;
>  status_northd)
>      daemon_status ovn-northd || exit 1
>      ;;
> +    status_ovsdb)
> +        status_ovsdb
> +        ;;
>  status_controller)
>      daemon_status ovn-controller || exit 1
>      ;;
> diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
> index 324a0a4..9ce65b7 100644
> --- a/ovn/utilities/ovn-nbctl.c
> +++ b/ovn/utilities/ovn-nbctl.c
> @@ -142,7 +142,7 @@ nbctl_default_db(void)
>  if (!def) {
>      def = getenv("OVN_NB_DB");
>      if (!def) {
> -            def = ctl_default_db();
> +            def = xasprintf("unix:%s/ovnnb_db.sock", ovs_rundir());
>      }
>  }
>  return def;
> diff --git a/ovn/utilities/ovn-sbctl.c b/ovn/utilities/ovn-sbctl.c
> index b428a35..8bbfdfd 100644
> --- a/ovn/utilities/ovn-sbctl.c
> +++ b/ovn/utilities/ovn-sbctl.c
> @@ -28,6 +28,7 @@
> #include <unistd.h>
> 
> #include "db-ctl-base.h"
> +#include "dirs.h"
> 
> #include "command-line.h"
> #include "compiler.h"
> @@ -154,7 +155,7 @@ sbctl_default_db(void)
>  if (!def) {
>      def = getenv("OVN_SB_DB");
>      if (!def) {
> -            def = ctl_default_db();
> +            def = xasprintf("unix:%s/ovnsb_db.sock", ovs_rundir());
>      }
>  }
>  return def;
> 


-- 
Russell Bryant



More information about the dev mailing list