On Tue, 2024-01-09 at 08:23 +0100, Frode Nordahl wrote:
> Hello, Martin,
> 
> Thank you for working on this!
> 
> While the automatic conversion is nice when it works, it is a source
> of problems when it does not, so I welcome this change.
> 
> An additional argument for this change is that the existing approach
> executes asynchronously, so there is no way for the human or machine
> operator to get the result of the `ovsdb-client` call.
> 
> On Mon, Jan 8, 2024 at 10:15 AM Martin Kalcok
> <[email protected]> wrote:
> > 
> > ovn-ctl script currently automatically attempts to perform
> > clustered
> > database schema upgrade when starting OVN SB or NB clustered
> > database. To provide more controll over this procees a
> 
> nit: a couple of typos above.

Thanks I'll fix these.

> 
> > `--db-cluster-skip-upgrade` option is added that allows skipping
> > the upgrade.
> 
> There is a precedent in the `ovn-ctl` and `ovs-ctl` scripts to use
> options in the form of `--no-something` for optional negation of some
> feature (`--no-leader-only` `--no-record-hostname` etc).
> 
> What do you think about making your new option conforming with this?

Yup, make's more sense. I'll flip the wording to fit '--flag'/'--no-
flag' pattern.
> 
> > Default value for this option is `no`, to preserve current default
> > behavior.
> > 
> > Signed-off-by: Martin Kalcok <[email protected]>
> > ---
> >  utilities/ovn-ctl       |  7 ++++++-
> >  utilities/ovn-ctl.8.xml | 11 +++++++++++
> >  2 files changed, 17 insertions(+), 1 deletion(-)
> > 
> > diff --git a/utilities/ovn-ctl b/utilities/ovn-ctl
> > index 876565c80..011ef9c0c 100755
> > --- a/utilities/ovn-ctl
> > +++ b/utilities/ovn-ctl
> > @@ -184,6 +184,7 @@ start_ovsdb__() {
> >      local ovn_db_ssl_cacert
> >      local ovn_db_election_timer
> >      local relay_mode
> > +    local skip_cluster_db_upgrade
> >      eval db_pid_file=\$DB_${DB}_PIDFILE
> >      eval cluster_local_addr=\$DB_${DB}_CLUSTER_LOCAL_ADDR
> >      eval cluster_local_port=\$DB_${DB}_CLUSTER_LOCAL_PORT
> > @@ -212,6 +213,7 @@ start_ovsdb__() {
> >      eval ovn_db_election_timer=\$DB_${DB}_ELECTION_TIMER
> >      eval relay_mode=\$RELAY_MODE
> >      eval relay_remote=\$DB_${DB}_REMOTE
> > +    eval skip_cluster_db_upgrade=\$DB_CLUSTER_SKIP_UPGRADE
> > 
> >      ovn_install_dir "$OVN_RUNDIR"
> >      ovn_install_dir "$ovn_logdir"
> > @@ -347,7 +349,7 @@ $cluster_remote_port
> >          $(echo ovn-${db}ctl | tr _ -) --no-leader-only --
> > db="unix:$sock" init
> >      fi
> > 
> > -    if test $mode = cluster; then
> > +    if test $mode = cluster && test X"$skip_cluster_db_upgrade" =
> > Xno; then
> >          upgrade_cluster "$schema" "unix:$sock"
> >      fi
> > 
> > @@ -898,6 +900,8 @@ set_defaults () {
> >      OVN_SB_RELAY_DB_SSL_CERT=""
> >      OVN_SB_RELAY_DB_SSL_CA_CERT=""
> >      DB_SB_RELAY_USE_REMOTE_IN_DB="yes"
> > +
> > +    DB_CLUSTER_SKIP_UPGRADE="no"
> >  }
> > 
> >  set_option () {
> > @@ -1148,6 +1152,7 @@ File location options:
> >    --ovn-sb-relay-db-ssl-key=KEY OVN_Southbound DB relay SSL
> > private key file
> >    --ovn-sb-relay-db-ssl-cert=CERT OVN_Southbound DB relay SSL
> > certificate file
> >    --ovn-sb-relay-db-ssl-ca-cert=CERT OVN OVN_Southbound DB relay
> > SSL CA certificate file
> > +  --db-cluster-skip-upgrade=yes|no (default:
> > $DB_CLUSTER_SKIP_UPGRADE)
> > 
> >  Default directories with "configure" option and environment
> > variable override:
> >    logs: /usr/local/var/log/ovn (--with-logdir, OVN_LOGDIR)
> > diff --git a/utilities/ovn-ctl.8.xml b/utilities/ovn-ctl.8.xml
> > index 01f4aa26b..5762fd3a4 100644
> > --- a/utilities/ovn-ctl.8.xml
> > +++ b/utilities/ovn-ctl.8.xml
> > @@ -156,6 +156,7 @@
> >      <p><code>--db-ic-sb-cluster-remote-addr=<var>IP
> > ADDRESS</var></code></p>
> >      <p><code>--db-ic-sb-cluster-remote-port=<var>PORT
> > NUMBER</var></code></p>
> >      <p><code>--db-ic-sb-cluster-remote-proto=<var>PROTO
> > (tcp/ssl)</var></code></p>
> > +    <p><code>--db-cluster-skip-
> > upgrade=<var>yes|no</var></code></p>
> > 
> >      <h1> Probe interval options </h1>
> >      <p><code>--db-nb-probe-interval-to-active=<var>Time in
> > milliseconds</var></code></p>
> > @@ -324,4 +325,14 @@
> >             start_northd
> >        </code>
> >      </p>
> > +    <h2>Avoiding automatic clustered OVN database schema
> > upgrade</h2>
> > +    <p>
> > +      If you desire more controll over clustered DB schema
> > upgrade, you can
> 
> s/controll/control

Thanks.

> 
> > +      opt-out of automatic on-start upgrade attempts with
> > +      <code>--db-cluster-skip-upgrade</code>.
> > +    </p>
> > +    <h3>Start OVN NB (or SB) clustered database on host with IP
> > x.x.x.x without schema upgrade</h3>
> > +    <p><code># ovn-ctl start_nb_ovsdb --db-nb-cluster-local-
> > addr=x.x.x.x --db-cluster-skip-upgrade=yes</code></p>
> > +    <h3>Trigger clustered DB schema upgrade manually</h3>
> > +    <p><code># ovsdb-client -t 30 convert
> > unix:/var/run/ovn/ovnnb_db.sock /usr/local/share/ovn/ovn-
> > nb.ovsschema</code></p>
> 
> I'd drop the `-t 30` in this example.
> 
> For completeness, should this section also list the command to
> convert
> the southbound database?
> 

I intentionally provided only one example for the sake of keeping the
example short, but I have no strong opinion on this. I'll add the SB
example as well.

Thanks for the review Frode, I'll update the proposal shortly.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to