On Wed, Jun 29, 2022 at 2:40 AM Dumitru Ceara <dce...@redhat.com> wrote:
>
> On 6/28/22 17:22, Terry Wilson wrote:
> > On Tue, Jun 28, 2022 at 4:39 AM Dumitru Ceara <dce...@redhat.com> wrote:
> >>
> >> On 6/27/22 18:00, Terry Wilson wrote:
> >>> It is common to pass ovn-ctl options via an /etc/sysconfig file.
> >>> It would be useful to be able to pass custom --remote options or
> >>> additional DBs to listen to via this file. This would give end
> >>> users the ability to have more fine-grained control without
> >>> having to modify ovn-ctl.
> >>>
> >>> Signed-off-by: Terry Wilson <twil...@redhat.com>
> >>> ---
> >>
> >> Hi Terry,
> >>
> >> For the change itself:
> >>
> >> Acked-by: Dumitru Ceara <dce...@redhat.com>
> >
> > Thanks!
> >
> >> While we can make it work for now with this new arbitrary args feature,
> >> for the actual goal, of passing a custom --remote option and additional
> >> DBs to listen to I think we need a follow up.  Currently, when starting
> >> OVN databases, if the database doesn't exist ovn-ctl takes care of
> >> creating it:
> >>
> >> https://github.com/ovn-org/ovn/blob/86684ad759b4f517b8f34fbf7ba8bb8ed77bb575/utilities/ovn-ctl#L246
> >> (raft)
> >>
> >> https://github.com/ovn-org/ovn/blob/86684ad759b4f517b8f34fbf7ba8bb8ed77bb575/utilities/ovn-ctl#L249
> >> (standalone)
> >>
> >> In the future, after [0] is accepted, can we add a patch that changes
> >> ovn-ctl to check if the local_config.ovsschema file is installed?  If it
> >> is and if DB_*_USE_REMOTE_IN_DB=yes it could then create the
> >> corresponding local_config database if not already found in $ovn_dbdir
> >> and automatically add the --remote and new db file args.
> >>
> >> What do you think?
> >
> > Something like that sounds good to me. With this I was looking for
> > something that would not specifically require the ovs change, but
> > would still let me solve the problem if I needed to pass the
> > schema/create the db locally. And in general it's useful to be able to
> > pass through some options.
> >
>
> Agreed.
>
> > I hadn't originally planned to use Local_Config just because it
> > existed--I can imagine situations where you have a manually added
> > Connection or custom set things like inactivity_probe set up in the
> > main DB, and if ovn-ctl now notices you have a Local_Config schema, it
> > stops using the original table and you lose whatever was in the
> > original db. So I was thinking of an additional --use-local-config
> > kind of option to opt into using it.
> >
>
> Also fine, in my opinion.  Will you be working on adding this support to
> ovn-ctl?

Yeah, I can do that.

> Thanks!
>
> >> Thanks,
> >> Dumitru
> >>
> >>>  utilities/ovn-ctl       | 22 ++++++++++++++++++++++
> >>>  utilities/ovn-ctl.8.xml | 14 +++++++++++++-
> >>>  2 files changed, 35 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/utilities/ovn-ctl b/utilities/ovn-ctl
> >>> index 23d4d7f8c..93be9b84b 100755
> >>> --- a/utilities/ovn-ctl
> >>> +++ b/utilities/ovn-ctl
> >>> @@ -311,6 +311,10 @@ $cluster_remote_port
> >>>          set "$@" --sync-from=`cat $active_conf_file`
> >>>      fi
> >>>
> >>> +    if test X"$extra_args" != X; then
> >>> +        set "$@" $extra_args
> >>> +    fi
> >>> +
> >>>      local run_ovsdb_in_bg="no"
> >>>      local process_id=
> >>>      if test X$detach = Xno && test $mode = cluster && test -z 
> >>> "$cluster_remote_addr" ; then
> >>> @@ -541,6 +545,10 @@ start_ic () {
> >>>
> >>>          set "$@" $OVN_IC_LOG $ovn_ic_params
> >>>
> >>> +        if test X"$extra_args" != X; then
> >>> +            set "$@" $extra_args
> >>> +        fi
> >>> +
> >>>          OVS_RUNDIR=${OVS_RUNDIR} start_ovn_daemon "$OVN_IC_PRIORITY" 
> >>> "$OVN_IC_WRAPPER" "$@"
> >>>      fi
> >>>  }
> >>> @@ -563,6 +571,10 @@ start_controller () {
> >>>
> >>>      [ "$OVN_USER" != "" ] && set "$@" --user "$OVN_USER"
> >>>
> >>> +    if test X"$extra_args" != X; then
> >>> +        set "$@" $extra_args
> >>> +    fi
> >>> +
> >>>      OVS_RUNDIR=${OVS_RUNDIR} start_ovn_daemon "$OVN_CONTROLLER_PRIORITY" 
> >>> "$OVN_CONTROLLER_WRAPPER" "$@"
> >>>  }
> >>>
> >>> @@ -590,6 +602,10 @@ start_controller_vtep () {
> >>>
> >>>      [ "$OVN_USER" != "" ] && set "$@" --user "$OVN_USER"
> >>>
> >>> +    if test X"$extra_args" != X; then
> >>> +        set "$@" $extra_args
> >>> +    fi
> >>> +
> >>>      OVS_RUNDIR=${OVS_RUNDIR} start_ovn_daemon "$OVN_CONTROLLER_PRIORITY" 
> >>> "$OVN_CONTROLLER_WRAPPER" "$@"
> >>>  }
> >>>
> >>> @@ -1106,8 +1122,10 @@ EOF
> >>>
> >>>  set_defaults
> >>>  command=
> >>> +extra_args=
> >>>  for arg
> >>>  do
> >>> +    shift
> >>>      case $arg in
> >>>          -h | --help)
> >>>              usage
> >>> @@ -1130,6 +1148,10 @@ do
> >>>              type=bool
> >>>              set_option
> >>>              ;;
> >>> +        --)
> >>> +            extra_args=$@
> >>> +            break
> >>> +            ;;
> >>>          -*)
> >>>              echo >&2 "$0: unknown option \"$arg\" (use --help for help)"
> >>>              exit 1
> >>> diff --git a/utilities/ovn-ctl.8.xml b/utilities/ovn-ctl.8.xml
> >>> index a1d39b22b..42d16fabc 100644
> >>> --- a/utilities/ovn-ctl.8.xml
> >>> +++ b/utilities/ovn-ctl.8.xml
> >>> @@ -4,7 +4,10 @@
> >>>      <p>ovn-ctl -- Open Virtual Network northbound daemon lifecycle 
> >>> utility</p>
> >>>
> >>>      <h1>Synopsis</h1>
> >>> -    <p><code>ovn-ctl</code> [<var>options</var>] <var>command</var></p>
> >>> +    <p>
> >>> +      <code>ovn-ctl</code> [<var>options</var>] <var>command</var>
> >>> +      [--- <var>extra_args</var>]
> >>> +    </p>
> >>>
> >>>      <h1>Description</h1>
> >>>      <p>This program is intended to be invoked internally by Open Virtual 
> >>> Network
> >>> @@ -156,6 +159,15 @@
> >>>      <p><code>--db-nb-probe-interval-to-active=<var>Time in 
> >>> milliseconds</var></code></p>
> >>>      <p><code>--db-sb-probe-interval-to-active=<var>Time in 
> >>> milliseconds</var></code></p>
> >>>
> >>> +    <h1> Extra Options </h1>
> >>> +    <p>
> >>> +      Any options after '--' will be passed on to the binary run by
> >>> +      <var>command</var> with the exception of start_northd, which can 
> >>> have
> >>> +      options specified in ovn-northd-db-params.conf. Any 
> >>> <var>extra_args</var>
> >>> +      passed to start_northd will be passed to the ovsdb-servers if
> >>> +      <code>--ovn-manage-ovsdb=yes</code>
> >>> +    </p>
> >>> +
> >>>      <h1>Configuration files</h1>
> >>>      <p>Following are the optional configuration files. If present, it 
> >>> should be located in the etc dir</p>
> >>>
> >>
> >
>

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to