| Message ID | 20220627160042.115781-1-twilson@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,v3,1/1] Allow arbitrary args to be passed to called binary | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
| ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
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 <twilson@redhat.com> > --- Hi Terry, For the change itself: Acked-by: Dumitru Ceara <dceara@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> >
On Tue, Jun 28, 2022 at 4:39 AM Dumitru Ceara <dceara@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 <twilson@redhat.com> > > --- > > Hi Terry, > > For the change itself: > > Acked-by: Dumitru Ceara <dceara@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. 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. > 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> > > >
On 6/28/22 17:22, Terry Wilson wrote: > On Tue, Jun 28, 2022 at 4:39 AM Dumitru Ceara <dceara@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 <twilson@redhat.com> >>> --- >> >> Hi Terry, >> >> For the change itself: >> >> Acked-by: Dumitru Ceara <dceara@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? 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> >>> >> >
On 6/29/22 03:40, Dumitru Ceara wrote: > On 6/28/22 17:22, Terry Wilson wrote: >> On Tue, Jun 28, 2022 at 4:39 AM Dumitru Ceara <dceara@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 <twilson@redhat.com> >>>> --- >>> >>> Hi Terry, >>> >>> For the change itself: >>> >>> Acked-by: Dumitru Ceara <dceara@redhat.com> >> >> Thanks! I've merged the change to main, branch-22.06 and branch-22.03. >> >>> 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? > > 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 > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Wed, Jun 29, 2022 at 2:40 AM Dumitru Ceara <dceara@redhat.com> wrote: > > On 6/28/22 17:22, Terry Wilson wrote: > > On Tue, Jun 28, 2022 at 4:39 AM Dumitru Ceara <dceara@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 <twilson@redhat.com> > >>> --- > >> > >> Hi Terry, > >> > >> For the change itself: > >> > >> Acked-by: Dumitru Ceara <dceara@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> > >>> > >> > > >
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>
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 <twilson@redhat.com> --- utilities/ovn-ctl | 22 ++++++++++++++++++++++ utilities/ovn-ctl.8.xml | 14 +++++++++++++- 2 files changed, 35 insertions(+), 1 deletion(-)