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