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>

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?

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