| Message ID | d7ca67ed74fb08f425f74d919b47f8710e492372.1613636533.git.frode.nordahl@canonical.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Backport rollup | expand |
On Thu, Feb 18, 2021 at 2:21 PM Frode Nordahl <frode.nordahl@canonical.com> 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 > Acked-by: Mark Michelson <mmichels@redhat.com> > Signed-off-by: Numan Siddique <numans@ovn.org> > (cherry picked from commit 67e2f386cc838d0b0f9b4b5da7fe611e1113b70c) > Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com> Hi Frode, Thanks for the backports to branch-20.03 and branch-20.06 I was able to apply all the patches and backport them to respective branches. Few of the patches failed while applying. So I cherry-picked those patches manually. I also backported the recent bug fixes related to ofctrl code. Please check out the branches 20.03 and 20.06 and see if they are fine. All the test cases have passed for me. Thanks Numan > --- > utilities/ovn-ctl | 20 +++++++++++++++++++- > 1 file changed, 19 insertions(+), 1 deletion(-) > > diff --git a/utilities/ovn-ctl b/utilities/ovn-ctl > index c7cb42bc1..7285c0533 100755 > --- a/utilities/ovn-ctl > +++ b/utilities/ovn-ctl > @@ -284,7 +284,21 @@ $cluster_remote_port > set "$@" --sync-from=`cat $active_conf_file` > fi > > - "$@" "$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 running standalone, > # active-passive, or is the first server in a cluster. > @@ -295,6 +309,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() { > -- > 2.30.0 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Tue, Mar 2, 2021 at 7:35 PM Numan Siddique <numans@ovn.org> wrote: > > On Thu, Feb 18, 2021 at 2:21 PM Frode Nordahl > <frode.nordahl@canonical.com> 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 > > Acked-by: Mark Michelson <mmichels@redhat.com> > > Signed-off-by: Numan Siddique <numans@ovn.org> > > (cherry picked from commit 67e2f386cc838d0b0f9b4b5da7fe611e1113b70c) > > Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com> > > Hi Frode, > > Thanks for the backports to branch-20.03 and branch-20.06 > > I was able to apply all the patches and backport them to respective branches. > Few of the patches failed while applying. So I cherry-picked those > patches manually. > > I also backported the recent bug fixes related to ofctrl code. > > Please check out the branches 20.03 and 20.06 and see if they are fine. > > All the test cases have passed for me. Numan, Thank you so much for this, as I've alluded to in other conversations we have had these running on top of 20.03 (sans the two most recent ofrctrl bug fixes) in the wild for a while, and I'll take some extra rounds of validation of the final product as presented in the branches now and report back. We would probably want the missing RBAC rule fix I have in flight too, and as a token of our appreciation I'm currently working on patches that would enable us to run the OVN testsuite with SSL+RBAC enabled by default to go along with it. I'll propose something soon. -- Frode Nordahl > Thanks > Numan > > > --- > > utilities/ovn-ctl | 20 +++++++++++++++++++- > > 1 file changed, 19 insertions(+), 1 deletion(-) > > > > diff --git a/utilities/ovn-ctl b/utilities/ovn-ctl > > index c7cb42bc1..7285c0533 100755 > > --- a/utilities/ovn-ctl > > +++ b/utilities/ovn-ctl > > @@ -284,7 +284,21 @@ $cluster_remote_port > > set "$@" --sync-from=`cat $active_conf_file` > > fi > > > > - "$@" "$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 running standalone, > > # active-passive, or is the first server in a cluster. > > @@ -295,6 +309,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() { > > -- > > 2.30.0 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > -- Frode Nordahl
On 3/3/21 2:19 PM, Frode Nordahl wrote: <snip> >> >>> --- >>> utilities/ovn-ctl | 20 +++++++++++++++++++- >>> 1 file changed, 19 insertions(+), 1 deletion(-) >>> >>> diff --git a/utilities/ovn-ctl b/utilities/ovn-ctl >>> index c7cb42bc1..7285c0533 100755 >>> --- a/utilities/ovn-ctl >>> +++ b/utilities/ovn-ctl >>> @@ -284,7 +284,21 @@ $cluster_remote_port >>> set "$@" --sync-from=`cat $active_conf_file` >>> fi >>> >>> - "$@" "$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" This actually broke ovn-ctl on branches 20.03 and 20.06. There is no such function as start_wrapped_daemon on these branches. >>> + fi >>> >>> # Initialize the database if it's running standalone, >>> # active-passive, or is the first server in a cluster. >>> @@ -295,6 +309,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, Mar 11, 2021 at 1:14 PM Ilya Maximets <i.maximets@ovn.org> wrote: > > On 3/3/21 2:19 PM, Frode Nordahl wrote: > > <snip> > > >> > >>> --- > >>> utilities/ovn-ctl | 20 +++++++++++++++++++- > >>> 1 file changed, 19 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/utilities/ovn-ctl b/utilities/ovn-ctl > >>> index c7cb42bc1..7285c0533 100755 > >>> --- a/utilities/ovn-ctl > >>> +++ b/utilities/ovn-ctl > >>> @@ -284,7 +284,21 @@ $cluster_remote_port > >>> set "$@" --sync-from=`cat $active_conf_file` > >>> fi > >>> > >>> - "$@" "$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" > > This actually broke ovn-ctl on branches 20.03 and 20.06. > There is no such function as start_wrapped_daemon on these branches. Oh shoot, that adaption got lost in the upstreaming of the backport. I'll submit revert+correct version momentarily.
On Thu, Mar 11, 2021 at 4:12 PM Frode Nordahl <frode.nordahl@canonical.com> wrote: > > On Thu, Mar 11, 2021 at 1:14 PM Ilya Maximets <i.maximets@ovn.org> wrote: > > > > On 3/3/21 2:19 PM, Frode Nordahl wrote: > > > > <snip> > > > > >> > > >>> --- > > >>> utilities/ovn-ctl | 20 +++++++++++++++++++- > > >>> 1 file changed, 19 insertions(+), 1 deletion(-) > > >>> > > >>> diff --git a/utilities/ovn-ctl b/utilities/ovn-ctl > > >>> index c7cb42bc1..7285c0533 100755 > > >>> --- a/utilities/ovn-ctl > > >>> +++ b/utilities/ovn-ctl > > >>> @@ -284,7 +284,21 @@ $cluster_remote_port > > >>> set "$@" --sync-from=`cat $active_conf_file` > > >>> fi > > >>> > > >>> - "$@" "$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" > > > > This actually broke ovn-ctl on branches 20.03 and 20.06. > > There is no such function as start_wrapped_daemon on these branches. > > Oh shoot, that adaption got lost in the upstreaming of the backport. > I'll submit revert+correct version momentarily. I have raised these series to address the issue, apologies for the breakage and thank you for raising it to my attention: https://patchwork.ozlabs.org/project/ovn/list/?series=233453 https://patchwork.ozlabs.org/project/ovn/list/?series=233454 -- Frode Nordahl > -- > Frode Nordahl > > > >>> + fi > > >>> > > >>> # Initialize the database if it's running standalone, > > >>> # active-passive, or is the first server in a cluster. > > >>> @@ -295,6 +309,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() {
diff --git a/utilities/ovn-ctl b/utilities/ovn-ctl index c7cb42bc1..7285c0533 100755 --- a/utilities/ovn-ctl +++ b/utilities/ovn-ctl @@ -284,7 +284,21 @@ $cluster_remote_port set "$@" --sync-from=`cat $active_conf_file` fi - "$@" "$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 running standalone, # active-passive, or is the first server in a cluster. @@ -295,6 +309,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() {