diff mbox

[ovs-dev] rhel: Environment file option for northd service

Message ID 1479208777-11540-1-git-send-email-bschanmu@redhat.com
State Changes Requested
Delegated to: Russell Bryant
Headers show

Commit Message

Babu Shanmugam Nov. 15, 2016, 11:19 a.m. UTC
From: Babu Shanmugam <bschanmu@redhat.com>

Since the northd service starts the DB servers as well, it will be
better to have an environment file options in the systemd unit file for
northd service.
The environment file is expected to define NORTHD_OPTS
which will have additional parameters to be passed to ovn-ctl script
that starts ovn-northd.

Signed-off-by: Babu Shanmugam <bschanmu@redhat.com>
---
 rhel/usr_lib_systemd_system_ovn-northd.service | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Russell Bryant Nov. 15, 2016, 2:41 p.m. UTC | #1
On Tue, Nov 15, 2016 at 6:19 AM, <bschanmu@redhat.com> wrote:

> From: Babu Shanmugam <bschanmu@redhat.com>
>
> Since the northd service starts the DB servers as well, it will be
> better to have an environment file options in the systemd unit file for
> northd service.
> The environment file is expected to define NORTHD_OPTS
> which will have additional parameters to be passed to ovn-ctl script
> that starts ovn-northd.
>
> Signed-off-by: Babu Shanmugam <bschanmu@redhat.com>
> ---
>  rhel/usr_lib_systemd_system_ovn-northd.service | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/rhel/usr_lib_systemd_system_ovn-northd.service
> b/rhel/usr_lib_systemd_system_ovn-northd.service
> index 5b3b03a..12230e9 100644
> --- a/rhel/usr_lib_systemd_system_ovn-northd.service
> +++ b/rhel/usr_lib_systemd_system_ovn-northd.service
> @@ -7,6 +7,7 @@ After=openvswitch.service
>  [Service]
>  Type=oneshot
>  RemainAfterExit=yes
> +EnvironmentFile=-/etc/sysconfig/ovn-northd
>  Environment=OVS_RUNDIR=%t/openvswitch OVS_DBDIR=/var/lib/openvswitch
> -ExecStart=/usr/share/openvswitch/scripts/ovn-ctl start_northd
> +ExecStart=/usr/share/openvswitch/scripts/ovn-ctl start_northd
> $NORTHD_OPTS
>  ExecStop=/usr/share/openvswitch/scripts/ovn-ctl stop_northd
>

I'm not sure this is necessary.  I believe something close enough is
possible with systemd already.

https://fedoraproject.org/wiki/Systemd#How_do_I_customize_a_unit_file.2F_add_a_custom_unit_file.3F

or:

"Example 2. Overriding vendor settings"
https://www.freedesktop.org/software/systemd/man/systemd.unit.html

For ovn-northd, you would create a directory and config file,
/etc/systemd/system/ovn-northd.d/local.conf, with contents like:

[System]
Environment=MY_ENV_VAR=VALUE "MY_ENV_VAR2=VALUE 2"
Babu Shanmugam Nov. 16, 2016, 6:15 a.m. UTC | #2
On Tuesday 15 November 2016 08:11 PM, Russell Bryant wrote:
>
>
> On Tue, Nov 15, 2016 at 6:19 AM, <bschanmu@redhat.com 
> <mailto:bschanmu@redhat.com>> wrote:
>
>     From: Babu Shanmugam <bschanmu@redhat.com
>     <mailto:bschanmu@redhat.com>>
>
>     Since the northd service starts the DB servers as well, it will be
>     better to have an environment file options in the systemd unit
>     file for
>     northd service.
>     The environment file is expected to define NORTHD_OPTS
>     which will have additional parameters to be passed to ovn-ctl script
>     that starts ovn-northd.
>
>     Signed-off-by: Babu Shanmugam <bschanmu@redhat.com
>     <mailto:bschanmu@redhat.com>>
>     ---
>      rhel/usr_lib_systemd_system_ovn-northd.service | 3 ++-
>      1 file changed, 2 insertions(+), 1 deletion(-)
>
>     diff --git a/rhel/usr_lib_systemd_system_ovn-northd.service
>     b/rhel/usr_lib_systemd_system_ovn-northd.service
>     index 5b3b03a..12230e9 100644
>     --- a/rhel/usr_lib_systemd_system_ovn-northd.service
>     +++ b/rhel/usr_lib_systemd_system_ovn-northd.service
>     @@ -7,6 +7,7 @@ After=openvswitch.service
>      [Service]
>      Type=oneshot
>      RemainAfterExit=yes
>     +EnvironmentFile=-/etc/sysconfig/ovn-northd
>      Environment=OVS_RUNDIR=%t/openvswitch OVS_DBDIR=/var/lib/openvswitch
>     -ExecStart=/usr/share/openvswitch/scripts/ovn-ctl start_northd
>     +ExecStart=/usr/share/openvswitch/scripts/ovn-ctl start_northd
>     $NORTHD_OPTS
>      ExecStop=/usr/share/openvswitch/scripts/ovn-ctl stop_northd
>
>
> I'm not sure this is necessary.  I believe something close enough is 
> possible with systemd already.
>
> https://fedoraproject.org/wiki/Systemd#How_do_I_customize_a_unit_file.2F_add_a_custom_unit_file.3F
>
> or:
>
> "Example 2. Overriding vendor settings"
> https://www.freedesktop.org/software/systemd/man/systemd.unit.html
>
> For ovn-northd, you would create a directory and config file, 
> /etc/systemd/system/ovn-northd.d/local.conf, with contents like:
>
> [System]
> Environment=MY_ENV_VAR=VALUE "MY_ENV_VAR2=VALUE 2"
>

Thanks for the information, Russell. This patch can be abandoned.

> -- 
> Russell Bryant
Babu Shanmugam Nov. 16, 2016, 9:02 a.m. UTC | #3
On Wednesday 16 November 2016 11:45 AM, Babu Shanmugam wrote:
>
>
>
> On Tuesday 15 November 2016 08:11 PM, Russell Bryant wrote:
>>
>>
>> On Tue, Nov 15, 2016 at 6:19 AM, <bschanmu@redhat.com 
>> <mailto:bschanmu@redhat.com>> wrote:
>>
>>     From: Babu Shanmugam <bschanmu@redhat.com
>>     <mailto:bschanmu@redhat.com>>
>>
>>     Since the northd service starts the DB servers as well, it will be
>>     better to have an environment file options in the systemd unit
>>     file for
>>     northd service.
>>     The environment file is expected to define NORTHD_OPTS
>>     which will have additional parameters to be passed to ovn-ctl script
>>     that starts ovn-northd.
>>
>>     Signed-off-by: Babu Shanmugam <bschanmu@redhat.com
>>     <mailto:bschanmu@redhat.com>>
>>     ---
>>      rhel/usr_lib_systemd_system_ovn-northd.service | 3 ++-
>>      1 file changed, 2 insertions(+), 1 deletion(-)
>>
>>     diff --git a/rhel/usr_lib_systemd_system_ovn-northd.service
>>     b/rhel/usr_lib_systemd_system_ovn-northd.service
>>     index 5b3b03a..12230e9 100644
>>     --- a/rhel/usr_lib_systemd_system_ovn-northd.service
>>     +++ b/rhel/usr_lib_systemd_system_ovn-northd.service
>>     @@ -7,6 +7,7 @@ After=openvswitch.service
>>      [Service]
>>      Type=oneshot
>>      RemainAfterExit=yes
>>     +EnvironmentFile=-/etc/sysconfig/ovn-northd
>>      Environment=OVS_RUNDIR=%t/openvswitch OVS_DBDIR=/var/lib/openvswitch
>>     -ExecStart=/usr/share/openvswitch/scripts/ovn-ctl start_northd
>>     +ExecStart=/usr/share/openvswitch/scripts/ovn-ctl start_northd
>>     $NORTHD_OPTS
>>      ExecStop=/usr/share/openvswitch/scripts/ovn-ctl stop_northd
>>
>>
>> I'm not sure this is necessary.  I believe something close enough is 
>> possible with systemd already.
>>
>> https://fedoraproject.org/wiki/Systemd#How_do_I_customize_a_unit_file.2F_add_a_custom_unit_file.3F
>>
>> or:
>>
>> "Example 2. Overriding vendor settings"
>> https://www.freedesktop.org/software/systemd/man/systemd.unit.html
>>
>> For ovn-northd, you would create a directory and config file, 
>> /etc/systemd/system/ovn-northd.d/local.conf, with contents like:
>>
>> [System]
>> Environment=MY_ENV_VAR=VALUE "MY_ENV_VAR2=VALUE 2"
>>
>
> Thanks for the information, Russell. This patch can be abandoned.

I was trying to use it like you said. But, I had to override the 
ExecStart as not all the options are read from the environment variable 
in the ovn-ctl script.
Do you think its better to add an EnvironmentFile option as proposed in 
this patch or to override ExecStart using the .conf file?

>
>> -- 
>> Russell Bryant
>
Russell Bryant Nov. 16, 2016, 4:37 p.m. UTC | #4
On Wed, Nov 16, 2016 at 4:02 AM, Babu Shanmugam <bschanmu@redhat.com> wrote:

>
>
> On Wednesday 16 November 2016 11:45 AM, Babu Shanmugam wrote:
>
>
>
> On Tuesday 15 November 2016 08:11 PM, Russell Bryant wrote:
>
>
>
> On Tue, Nov 15, 2016 at 6:19 AM, <bschanmu@redhat.com> wrote:
>
>> From: Babu Shanmugam <bschanmu@redhat.com>
>>
>> Since the northd service starts the DB servers as well, it will be
>> better to have an environment file options in the systemd unit file for
>> northd service.
>> The environment file is expected to define NORTHD_OPTS
>> which will have additional parameters to be passed to ovn-ctl script
>> that starts ovn-northd.
>>
>> Signed-off-by: Babu Shanmugam <bschanmu@redhat.com>
>> ---
>>  rhel/usr_lib_systemd_system_ovn-northd.service | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/rhel/usr_lib_systemd_system_ovn-northd.service
>> b/rhel/usr_lib_systemd_system_ovn-northd.service
>> index 5b3b03a..12230e9 100644
>> --- a/rhel/usr_lib_systemd_system_ovn-northd.service
>> +++ b/rhel/usr_lib_systemd_system_ovn-northd.service
>> @@ -7,6 +7,7 @@ After=openvswitch.service
>>  [Service]
>>  Type=oneshot
>>  RemainAfterExit=yes
>> +EnvironmentFile=-/etc/sysconfig/ovn-northd
>>  Environment=OVS_RUNDIR=%t/openvswitch OVS_DBDIR=/var/lib/openvswitch
>> -ExecStart=/usr/share/openvswitch/scripts/ovn-ctl start_northd
>> +ExecStart=/usr/share/openvswitch/scripts/ovn-ctl start_northd
>> $NORTHD_OPTS
>>  ExecStop=/usr/share/openvswitch/scripts/ovn-ctl stop_northd
>>
>
> I'm not sure this is necessary.  I believe something close enough is
> possible with systemd already.
>
> https://fedoraproject.org/wiki/Systemd#How_do_I_
> customize_a_unit_file.2F_add_a_custom_unit_file.3F
>
> or:
>
> "Example 2. Overriding vendor settings"
> https://www.freedesktop.org/software/systemd/man/systemd.unit.html
>
> For ovn-northd, you would create a directory and config file,
> /etc/systemd/system/ovn-northd.d/local.conf, with contents like:
>
> [System]
> Environment=MY_ENV_VAR=VALUE "MY_ENV_VAR2=VALUE 2"
>
>
> Thanks for the information, Russell. This patch can be abandoned.
>
>
> I was trying to use it like you said. But, I had to override the ExecStart
> as not all the options are read from the environment variable in the
> ovn-ctl script.
> Do you think its better to add an EnvironmentFile option as proposed in
> this patch or to override ExecStart using the .conf file?
>

OK, I didn't realize that most options couldn't be specified with
environment variables.

What if we only changed the ExecStart line and then used the approach I
described to set $OVN_NORTHD_OPTS as an environment variables instead of
adding the EnvironmentFile option?
Babu Shanmugam Nov. 17, 2016, 11:42 a.m. UTC | #5
On Wednesday 16 November 2016 10:07 PM, Russell Bryant wrote:
>
>
> On Wed, Nov 16, 2016 at 4:02 AM, Babu Shanmugam <bschanmu@redhat.com 
> <mailto:bschanmu@redhat.com>> wrote:
>
>
>
>     On Wednesday 16 November 2016 11:45 AM, Babu Shanmugam wrote:
>>
>>
>>
>>     On Tuesday 15 November 2016 08:11 PM, Russell Bryant wrote:
>>>
>>>
>>>     On Tue, Nov 15, 2016 at 6:19 AM, <bschanmu@redhat.com
>>>     <mailto:bschanmu@redhat.com>> wrote:
>>>
>>>         From: Babu Shanmugam <bschanmu@redhat.com
>>>         <mailto:bschanmu@redhat.com>>
>>>
>>>         Since the northd service starts the DB servers as well, it
>>>         will be
>>>         better to have an environment file options in the systemd
>>>         unit file for
>>>         northd service.
>>>         The environment file is expected to define NORTHD_OPTS
>>>         which will have additional parameters to be passed to
>>>         ovn-ctl script
>>>         that starts ovn-northd.
>>>
>>>         Signed-off-by: Babu Shanmugam <bschanmu@redhat.com
>>>         <mailto:bschanmu@redhat.com>>
>>>         ---
>>>          rhel/usr_lib_systemd_system_ovn-northd.service | 3 ++-
>>>          1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>>         diff --git a/rhel/usr_lib_systemd_system_ovn-northd.service
>>>         b/rhel/usr_lib_systemd_system_ovn-northd.service
>>>         index 5b3b03a..12230e9 100644
>>>         --- a/rhel/usr_lib_systemd_system_ovn-northd.service
>>>         +++ b/rhel/usr_lib_systemd_system_ovn-northd.service
>>>         @@ -7,6 +7,7 @@ After=openvswitch.service
>>>          [Service]
>>>          Type=oneshot
>>>          RemainAfterExit=yes
>>>         +EnvironmentFile=-/etc/sysconfig/ovn-northd
>>>          Environment=OVS_RUNDIR=%t/openvswitch
>>>         OVS_DBDIR=/var/lib/openvswitch
>>>         -ExecStart=/usr/share/openvswitch/scripts/ovn-ctl start_northd
>>>         +ExecStart=/usr/share/openvswitch/scripts/ovn-ctl
>>>         start_northd $NORTHD_OPTS
>>>          ExecStop=/usr/share/openvswitch/scripts/ovn-ctl stop_northd
>>>
>>>
>>>     I'm not sure this is necessary.  I believe something close
>>>     enough is possible with systemd already.
>>>
>>>     https://fedoraproject.org/wiki/Systemd#How_do_I_customize_a_unit_file.2F_add_a_custom_unit_file.3F
>>>     <https://fedoraproject.org/wiki/Systemd#How_do_I_customize_a_unit_file.2F_add_a_custom_unit_file.3F>
>>>
>>>     or:
>>>
>>>     "Example 2. Overriding vendor settings"
>>>     https://www.freedesktop.org/software/systemd/man/systemd.unit.html
>>>     <https://www.freedesktop.org/software/systemd/man/systemd.unit.html>
>>>
>>>     For ovn-northd, you would create a directory and config file,
>>>     /etc/systemd/system/ovn-northd.d/local.conf, with contents like:
>>>
>>>     [System]
>>>     Environment=MY_ENV_VAR=VALUE "MY_ENV_VAR2=VALUE 2"
>>>
>>
>>     Thanks for the information, Russell. This patch can be abandoned.
>
>     I was trying to use it like you said. But, I had to override the
>     ExecStart as not all the options are read from the environment
>     variable in the ovn-ctl script.
>     Do you think its better to add an EnvironmentFile option as
>     proposed in this patch or to override ExecStart using the .conf file?
>
>
> OK, I didn't realize that most options couldn't be specified with 
> environment variables.
>
> What if we only changed the ExecStart line and then used the approach 
> I described to set $OVN_NORTHD_OPTS as an environment variables 
> instead of adding the EnvironmentFile option?
>

I tried your suggestion and it works, Russell. Thank you.

> -- 
> Russell Bryant
Russell Bryant Nov. 17, 2016, 1:41 p.m. UTC | #6
On Thu, Nov 17, 2016 at 6:42 AM, Babu Shanmugam <bschanmu@redhat.com> wrote:

>
>
> On Wednesday 16 November 2016 10:07 PM, Russell Bryant wrote:
>
>
>
> On Wed, Nov 16, 2016 at 4:02 AM, Babu Shanmugam <bschanmu@redhat.com>
> wrote:
>
>>
>>
>> On Wednesday 16 November 2016 11:45 AM, Babu Shanmugam wrote:
>>
>>
>>
>> On Tuesday 15 November 2016 08:11 PM, Russell Bryant wrote:
>>
>>
>>
>> On Tue, Nov 15, 2016 at 6:19 AM, <bschanmu@redhat.com> wrote:
>>
>>> From: Babu Shanmugam <bschanmu@redhat.com>
>>>
>>> Since the northd service starts the DB servers as well, it will be
>>> better to have an environment file options in the systemd unit file for
>>> northd service.
>>> The environment file is expected to define NORTHD_OPTS
>>> which will have additional parameters to be passed to ovn-ctl script
>>> that starts ovn-northd.
>>>
>>> Signed-off-by: Babu Shanmugam <bschanmu@redhat.com>
>>> ---
>>>  rhel/usr_lib_systemd_system_ovn-northd.service | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/rhel/usr_lib_systemd_system_ovn-northd.service
>>> b/rhel/usr_lib_systemd_system_ovn-northd.service
>>> index 5b3b03a..12230e9 100644
>>> --- a/rhel/usr_lib_systemd_system_ovn-northd.service
>>> +++ b/rhel/usr_lib_systemd_system_ovn-northd.service
>>> @@ -7,6 +7,7 @@ After=openvswitch.service
>>>  [Service]
>>>  Type=oneshot
>>>  RemainAfterExit=yes
>>> +EnvironmentFile=-/etc/sysconfig/ovn-northd
>>>  Environment=OVS_RUNDIR=%t/openvswitch OVS_DBDIR=/var/lib/openvswitch
>>> -ExecStart=/usr/share/openvswitch/scripts/ovn-ctl start_northd
>>> +ExecStart=/usr/share/openvswitch/scripts/ovn-ctl start_northd
>>> $NORTHD_OPTS
>>>  ExecStop=/usr/share/openvswitch/scripts/ovn-ctl stop_northd
>>>
>>
>> I'm not sure this is necessary.  I believe something close enough is
>> possible with systemd already.
>>
>> https://fedoraproject.org/wiki/Systemd#How_do_I_customize_a_
>> unit_file.2F_add_a_custom_unit_file.3F
>>
>> or:
>>
>> "Example 2. Overriding vendor settings"
>> https://www.freedesktop.org/software/systemd/man/systemd.unit.html
>>
>> For ovn-northd, you would create a directory and config file,
>> /etc/systemd/system/ovn-northd.d/local.conf, with contents like:
>>
>> [System]
>> Environment=MY_ENV_VAR=VALUE "MY_ENV_VAR2=VALUE 2"
>>
>>
>> Thanks for the information, Russell. This patch can be abandoned.
>>
>>
>> I was trying to use it like you said. But, I had to override the
>> ExecStart as not all the options are read from the environment variable in
>> the ovn-ctl script.
>> Do you think its better to add an EnvironmentFile option as proposed in
>> this patch or to override ExecStart using the .conf file?
>>
>
> OK, I didn't realize that most options couldn't be specified with
> environment variables.
>
> What if we only changed the ExecStart line and then used the approach I
> described to set $OVN_NORTHD_OPTS as an environment variables instead of
> adding the EnvironmentFile option?
>
>
> I tried your suggestion and it works, Russell. Thank you.
>

Great, thanks.  I updated this patch with some documentation and posted an
updated version here:

https://patchwork.ozlabs.org/patch/696112/
diff mbox

Patch

diff --git a/rhel/usr_lib_systemd_system_ovn-northd.service b/rhel/usr_lib_systemd_system_ovn-northd.service
index 5b3b03a..12230e9 100644
--- a/rhel/usr_lib_systemd_system_ovn-northd.service
+++ b/rhel/usr_lib_systemd_system_ovn-northd.service
@@ -7,6 +7,7 @@  After=openvswitch.service
 [Service]
 Type=oneshot
 RemainAfterExit=yes
+EnvironmentFile=-/etc/sysconfig/ovn-northd
 Environment=OVS_RUNDIR=%t/openvswitch OVS_DBDIR=/var/lib/openvswitch
-ExecStart=/usr/share/openvswitch/scripts/ovn-ctl start_northd
+ExecStart=/usr/share/openvswitch/scripts/ovn-ctl start_northd $NORTHD_OPTS
 ExecStop=/usr/share/openvswitch/scripts/ovn-ctl stop_northd