[ovs-dev,v2] ovn-ctl: Handle cluster db upgrades for run_(nb/sb)_ovsdb

From: Numan Siddique <numans@ovn.org>

when ovn-ctl run_(nb_sb)_ovsdb is called, the ovsdb-server is started without
passing --detach and --monoitor and the process is exec'd.

For cluster mode, upgrade_cluster is never called and hence the dbs are not upraded
to new schema. CMS has to handle the db upgrade separately.

This patch handles the db upgrade by starting ovsdb-server in background and then
waits on ovsdb-server to complete.

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1868392
Signed-off-by: Numan Siddique <numans@ovn.org>
---
 v1 -> v2
 -----
  * Rebased to resovle the conflicts.

 utilities/ovn-ctl | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

Bleep bloop.  Greetings Numan Siddique, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 91 characters long (recommended limit is 79)
#32 FILE: utilities/ovn-ctl:293:
    if test X$detach = Xno && test $mode = cluster && test -z "$cluster_remote_addr" ; then

Lines checked: 61, Warnings: 1, Errors: 0


Please check this out.  If you feel there has been an error, please email aconole@redhat.com

Thanks,
0-day Robot

Acked-by: Mark Michelson <mmichels@redhat.com>

On 9/9/20 3:19 AM, numans@ovn.org wrote:
> From: Numan Siddique <numans@ovn.org>
> 
> when ovn-ctl run_(nb_sb)_ovsdb is called, the ovsdb-server is started without
> passing --detach and --monoitor and the process is exec'd.
> 
> For cluster mode, upgrade_cluster is never called and hence the dbs are not upraded
> to new schema. CMS has to handle the db upgrade separately.
> 
> This patch handles the db upgrade by starting ovsdb-server in background and then
> waits on ovsdb-server to complete.
> 
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1868392
> Signed-off-by: Numan Siddique <numans@ovn.org>
> ---
>   v1 -> v2
>   -----
>    * Rebased to resovle the conflicts.
> 
>   utilities/ovn-ctl | 20 +++++++++++++++++++-
>   1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/utilities/ovn-ctl b/utilities/ovn-ctl
> index af095ea1bd..c44201ccfb 100755
> --- a/utilities/ovn-ctl
> +++ b/utilities/ovn-ctl
> @@ -288,7 +288,21 @@ $cluster_remote_port
>           set "$@" --sync-from=`cat $active_conf_file`
>       fi
>   
> -    start_wrapped_daemon "$wrapper" ovsdb-$db "" "$@" "$file"
> +    local run_ovsdb_in_bg="no"
> +    local process_id=
> +    if test X$detach = Xno && test $mode = cluster && test -z "$cluster_remote_addr" ; then
> +        # When detach is no (for run_nb_ovsdb/run_sb_ovsdb commands)
> +        # we want to run ovsdb-server in background rather than running it in
> +        # foreground so that the OVN dbs are upgraded for the cluster mode.
> +        # Otherwise, CMS has to take the responsibility of upgrading the dbs.
> +        # Note: We run only the ovsdb-server in backgroud which created the
> +        # cluster (i.e cluster_remote_addr is not set.).
> +        run_ovsdb_in_bg="yes"
> +        "$@" $file &
> +        process_id=$!
> +    else
> +        start_wrapped_daemon "$wrapper" ovsdb-$db "" "$@" "$file"
> +    fi
>   
>       # Initialize the database if it's NOT joining a cluster.
>       if test -z "$cluster_remote_addr"; then
> @@ -298,6 +312,10 @@ $cluster_remote_port
>       if test $mode = cluster; then
>           upgrade_cluster "$schema" "unix:$sock"
>       fi
> +
> +    if test $run_ovsdb_in_bg = yes; then
> +        wait $process_id
> +    fi
>   }
>   
>   start_nb_ovsdb() {
>

On Thu, Sep 10, 2020 at 11:34 PM Mark Michelson <mmichels@redhat.com> wrote:
>
> Acked-by: Mark Michelson <mmichels@redhat.com>

Thanks Mark. I applied this patch to master.

Numan

>
> On 9/9/20 3:19 AM, numans@ovn.org wrote:
> > From: Numan Siddique <numans@ovn.org>
> >
> > when ovn-ctl run_(nb_sb)_ovsdb is called, the ovsdb-server is started without
> > passing --detach and --monoitor and the process is exec'd.
> >
> > For cluster mode, upgrade_cluster is never called and hence the dbs are not upraded
> > to new schema. CMS has to handle the db upgrade separately.
> >
> > This patch handles the db upgrade by starting ovsdb-server in background and then
> > waits on ovsdb-server to complete.
> >
> > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1868392
> > Signed-off-by: Numan Siddique <numans@ovn.org>
> > ---
> >   v1 -> v2
> >   -----
> >    * Rebased to resovle the conflicts.
> >
> >   utilities/ovn-ctl | 20 +++++++++++++++++++-
> >   1 file changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/utilities/ovn-ctl b/utilities/ovn-ctl
> > index af095ea1bd..c44201ccfb 100755
> > --- a/utilities/ovn-ctl
> > +++ b/utilities/ovn-ctl
> > @@ -288,7 +288,21 @@ $cluster_remote_port
> >           set "$@" --sync-from=`cat $active_conf_file`
> >       fi
> >
> > -    start_wrapped_daemon "$wrapper" ovsdb-$db "" "$@" "$file"
> > +    local run_ovsdb_in_bg="no"
> > +    local process_id=
> > +    if test X$detach = Xno && test $mode = cluster && test -z "$cluster_remote_addr" ; then
> > +        # When detach is no (for run_nb_ovsdb/run_sb_ovsdb commands)
> > +        # we want to run ovsdb-server in background rather than running it in
> > +        # foreground so that the OVN dbs are upgraded for the cluster mode.
> > +        # Otherwise, CMS has to take the responsibility of upgrading the dbs.
> > +        # Note: We run only the ovsdb-server in backgroud which created the
> > +        # cluster (i.e cluster_remote_addr is not set.).
> > +        run_ovsdb_in_bg="yes"
> > +        "$@" $file &
> > +        process_id=$!
> > +    else
> > +        start_wrapped_daemon "$wrapper" ovsdb-$db "" "$@" "$file"
> > +    fi
> >
> >       # Initialize the database if it's NOT joining a cluster.
> >       if test -z "$cluster_remote_addr"; then
> > @@ -298,6 +312,10 @@ $cluster_remote_port
> >       if test $mode = cluster; then
> >           upgrade_cluster "$schema" "unix:$sock"
> >       fi
> > +
> > +    if test $run_ovsdb_in_bg = yes; then
> > +        wait $process_id
> > +    fi
> >   }
> >
> >   start_nb_ovsdb() {
> >
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>