[ovs-dev] [PATCH ovn v2 3/3] utilities: add conditional {start/stop}_ic_ovsdb on start_ic command

[Odintsov Vladislav]

Hi,

Sorry for the long reply. There were long NY holidays in Russia.
I do have same thoughts as Han.
Maybe, it's better to introduce separate option --ovn-manage-ic-ovsdb, which can have NO as default value and then just pass --ovn-manage-ic-ovsdb=yes in ovn-ic system-unit?

In such situation current behavior for direct ovn-ctl start_ic call would not be changed and at the same time ovn-ic systemd unit will behave similar to ovn-northd: it will start all necessary services for ovn-ic by invoking: `ovn_ctl start_ic --ovn-manage-ic-ovsdb=yes $OPTS` inside system-unit.

Does that look better?
If yes, I'll update PR. 

Regards,
 
Vladislav Odintsov

On 23.12.2020, 11:24, "dev on behalf of Numan Siddique" <ovs-dev-bounces at openvswitch.org on behalf of numans at ovn.org> wrote:

    On Wed, Dec 23, 2020 at 12:40 PM Han Zhou <hzhou at ovn.org> wrote:
    >
    > On Tue, Dec 22, 2020 at 6:05 AM Numan Siddique <numans at ovn.org> wrote:
    > >
    > > Hi Han,
    > >
    > > Can you please take a look at this patch ? Does this change in
    > > behavior fine with you ?\
    > >
    > > Thanks
    > > Numan
    > >
    >
    > Hi Numan,
    >
    > I am concerned with changing the default behavior. In normal use cases the
    > global IC DBs shouldn't be co-located with ovn-ic, considering that the
    > glocal DBs are used for multiple OVN deployments, and ovn-ic is in each OVN
    > deployment (usually on the central nodes). In production I assume the
    > global DB is managed independently from any of the OVN deployments.
    > If we want some convenience for testing purposes to start the global DBs
    > together with ovn-ic, it is better to be done by explicitly specifying the
    > option --ovn-manage-ovsdb=yes instead of default. In fact, even supporting
    > this with the --ovn-manage-ovsdb=yes may seem confusing, because ovn-ic
    > interacts not only with the IC DBs but also local NB/SB DBs, which is not
    > like ovn-northd, but I am ok if there is a strong need for such
    > convenience. In other words, I'd suggest keeping the current interface for
    > clarity, but I am also ok with starting IC DBs with start_ic provided that
    > it doesn't change the default behavior.
    >
    > Thanks,
    > Han

    Thanks Han. I will let Vladislav comment on this.

    I submitted this patch from the PR - https://github.com/ovn-org/ovn/pull/65

    Numan

    >
    > >
    > > On Tue, Dec 22, 2020 at 1:24 PM <numans at ovn.org> wrote:
    > > >
    > > > From: Vladislav Odintsov <odivlad at gmail.com>
    > > >
    > > > This commit adds capability to start and stop ovsdb-server for OVN
    > interconnection
    > > > when {start,stop}_ic is called.
    > > > It changes previous behavior where {start,stop}_ic only started/stopped
    > ovn-ic daemon without ovn-ic DBs.
    > > >
    > > > If one wants to save previous behavior, --ovn-manage-ovsdb=no must be
    > passed to ovn-ctl script.
    > > >
    > > > Signed-off-by: Vladislav Odintsov <odivlad at gmail.com>
    > > > ---
    > > >  utilities/ovn-ctl | 18 +++++++++++++++++-
    > > >  1 file changed, 17 insertions(+), 1 deletion(-)
    > > >
    > > > diff --git a/utilities/ovn-ctl b/utilities/ovn-ctl
    > > > index 1119308860..178cefaa46 100755
    > > > --- a/utilities/ovn-ctl
    > > > +++ b/utilities/ovn-ctl
    > > > @@ -485,6 +485,19 @@ start_northd () {
    > > >
    > > >  start_ic () {
    > > >      if [ ! -e $ovn_ic_db_conf_file ]; then
    > > > +        if test X"$OVN_MANAGE_OVSDB" = Xyes; then
    > > > +            start_ic_ovsdb
    > > > +
    > > > +            if ! pidfile_is_running $DB_IC_NB_PID; then
    > > > +                log_failure_msg "OVN IC Northbound DB is not running"
    > > > +                exit
    > > > +            fi
    > > > +            if ! pidfile_is_running $DB_IC_SB_PID; then
    > > > +                log_failure_msg "OVN IC Southbound DB is not running"
    > > > +                exit
    > > > +            fi
    > > > +        fi
    > > > +
    > > >          ovn_ic_params="--ovnnb-db=$OVN_NORTHD_NB_DB \
    > > >                         --ovnsb-db=$OVN_NORTHD_SB_DB \
    > > >                         --ic-nb-db=$OVN_IC_NB_DB \
    > > > @@ -581,6 +594,9 @@ stop_northd () {
    > > >  }
    > > >
    > > >  stop_ic () {
    > > > +    if test X"$OVN_MANAGE_OVSDB" = Xyes; then
    > > > +        stop_ic_ovsdb
    > > > +    fi
    > > >      OVS_RUNDIR=${OVS_RUNDIR} stop_ovn_daemon ovn-ic
    > > >  }
    > > >
    > > > @@ -906,7 +922,7 @@ Options:
    > > >    --ovn-northd-ssl-ca-cert=CERT OVN Northd SSL CA certificate file
    > > >    --ovn-manage-ovsdb=yes|no        Whether or not the OVN NB/SB
    > databases should be
    > > >                                     automatically started and stopped
    > along
    > > > -                                   with ovn-northd. The default is
    > "yes". If
    > > > +                                   with ovn-northd and ovn-ic. The
    > default is "yes". If
    > > >                                     this is set to "no", the
    > "start_ovsdb" and
    > > >                                     "stop_ovsdb" commands must be used
    > to start
    > > >                                     and stop the OVN databases.
    > > > --
    > > > 2.29.2
    > > >
    > > > _______________________________________________
    > > > dev mailing list
    > > > dev at openvswitch.org
    > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
    > > >
    > _______________________________________________
    > dev mailing list
    > dev at openvswitch.org
    > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
    >
    _______________________________________________
    dev mailing list
    dev at openvswitch.org
    https://mail.openvswitch.org/mailman/listinfo/ovs-dev