[ovs-dev,v3,2/2] rhel: delete transient ports on boot when starting ovsdb-server

Message ID 8b3784fede9de6d2ab745139c2346410d7eed749.1502466863.git.tredaelli@redhat.com
State New
Headers show

Commit Message

Timothy M. Redaelli Aug. 11, 2017, 4:06 p.m.
Use ovs-ctl --delete-transient-ports-on-boot to start ovsdb-server.

This feature can be disabled by appending --no-delete-transient-ports-on-boot
to OPTIONS in /etc/sysconfig/openvswitch

Signed-off-by: Timothy Redaelli <tredaelli@redhat.com>
---
 rhel/usr_lib_systemd_system_ovsdb-server.service | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Aaron Conole Aug. 11, 2017, 5:12 p.m. | #1
Timothy Redaelli <tredaelli@redhat.com> writes:

> Use ovs-ctl --delete-transient-ports-on-boot to start ovsdb-server.
>
> This feature can be disabled by appending --no-delete-transient-ports-on-boot
> to OPTIONS in /etc/sysconfig/openvswitch
>
> Signed-off-by: Timothy Redaelli <tredaelli@redhat.com>
> ---
>  rhel/usr_lib_systemd_system_ovsdb-server.service | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service b/rhel/usr_lib_systemd_system_ovsdb-server.service
> index 7acd25f78..42473161e 100644
> --- a/rhel/usr_lib_systemd_system_ovsdb-server.service
> +++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
> @@ -10,14 +10,14 @@ Type=forking
>  Restart=on-failure
>  EnvironmentFile=/etc/openvswitch/default.conf
>  EnvironmentFile=-/etc/sysconfig/openvswitch
> +ExecStartPre=/usr/bin/mkdir -p /var/run/openvswitch
>  ExecStartPre=/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch
>  ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
>            --no-ovs-vswitchd --no-monitor --system-id=random \
> +          --delete-transient-ports-on-boot \
>            --ovs-user=${OVS_USER_ID} \
>            start $OPTIONS
>  ExecStop=/usr/share/openvswitch/scripts/ovs-ctl --no-ovs-vswitchd stop
>  ExecReload=/usr/share/openvswitch/scripts/ovs-ctl --no-ovs-vswitchd \
>             --ovs-user=${OVS_USER_ID} \
>             --no-monitor restart $OPTIONS
> -RuntimeDirectory=openvswitch
> -RuntimeDirectoryMode=0755

I'm not sure about this change.  One thing that's nice is the way
systemd will cleanup the runtime directories when this is done.  I think
this can leave it around.

I realize that under rhel (and fedora), the /run mountpoint is tmpfs.

Is there another way we could accomodate this?  Does a db flag make
sense?
Timothy M. Redaelli Aug. 28, 2017, 1:21 p.m. | #2
On 08/11/2017 07:12 PM, Aaron Conole wrote:
> Timothy Redaelli <tredaelli@redhat.com> writes:
> 
>> Use ovs-ctl --delete-transient-ports-on-boot to start ovsdb-server.
>>
>> This feature can be disabled by appending --no-delete-transient-ports-on-boot
>> to OPTIONS in /etc/sysconfig/openvswitch
>>
>> Signed-off-by: Timothy Redaelli <tredaelli@redhat.com>
>> ---
>>  rhel/usr_lib_systemd_system_ovsdb-server.service | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service b/rhel/usr_lib_systemd_system_ovsdb-server.service
>> index 7acd25f78..42473161e 100644
>> --- a/rhel/usr_lib_systemd_system_ovsdb-server.service
>> +++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
>> @@ -10,14 +10,14 @@ Type=forking
>>  Restart=on-failure
>>  EnvironmentFile=/etc/openvswitch/default.conf
>>  EnvironmentFile=-/etc/sysconfig/openvswitch
>> +ExecStartPre=/usr/bin/mkdir -p /var/run/openvswitch
>>  ExecStartPre=/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch
>>  ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
>>            --no-ovs-vswitchd --no-monitor --system-id=random \
>> +          --delete-transient-ports-on-boot \
>>            --ovs-user=${OVS_USER_ID} \
>>            start $OPTIONS
>>  ExecStop=/usr/share/openvswitch/scripts/ovs-ctl --no-ovs-vswitchd stop
>>  ExecReload=/usr/share/openvswitch/scripts/ovs-ctl --no-ovs-vswitchd \
>>             --ovs-user=${OVS_USER_ID} \
>>             --no-monitor restart $OPTIONS
>> -RuntimeDirectory=openvswitch
>> -RuntimeDirectoryMode=0755
> 
> I'm not sure about this change.  One thing that's nice is the way
> systemd will cleanup the runtime directories when this is done.  I think
> this can leave it around.
> 
> I realize that under rhel (and fedora), the /run mountpoint is tmpfs.
> 
> Is there another way we could accomodate this?  Does a db flag make
> sense?

I don't think we can use db since we need that flags to be reset on each
reboot (that's the reason I use /run).

In the previous patchset this run directory was created by using
tmpfiles.d, but unlucky tmpfiles.d cannot use environment files, so you
can't use ${OVS_USER_ID}.

If cleanup is needed, but imho it should be done by ovs-ctl or
ovs-vswitchd/ovsdb-server directly, we may do the cleanup on ExecStopPost.
Aaron Conole Aug. 28, 2017, 9:01 p.m. | #3
"Timothy M. Redaelli" <tredaelli@redhat.com> writes:

> On 08/11/2017 07:12 PM, Aaron Conole wrote:
>> Timothy Redaelli <tredaelli@redhat.com> writes:
>> 
>>> Use ovs-ctl --delete-transient-ports-on-boot to start ovsdb-server.
>>>
>>> This feature can be disabled by appending --no-delete-transient-ports-on-boot
>>> to OPTIONS in /etc/sysconfig/openvswitch
>>>
>>> Signed-off-by: Timothy Redaelli <tredaelli@redhat.com>
>>> ---
>>>  rhel/usr_lib_systemd_system_ovsdb-server.service | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service b/rhel/usr_lib_systemd_system_ovsdb-server.service
>>> index 7acd25f78..42473161e 100644
>>> --- a/rhel/usr_lib_systemd_system_ovsdb-server.service
>>> +++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
>>> @@ -10,14 +10,14 @@ Type=forking
>>>  Restart=on-failure
>>>  EnvironmentFile=/etc/openvswitch/default.conf
>>>  EnvironmentFile=-/etc/sysconfig/openvswitch
>>> +ExecStartPre=/usr/bin/mkdir -p /var/run/openvswitch
>>>  ExecStartPre=/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch
>>>  ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
>>>            --no-ovs-vswitchd --no-monitor --system-id=random \
>>> +          --delete-transient-ports-on-boot \
>>>            --ovs-user=${OVS_USER_ID} \
>>>            start $OPTIONS
>>>  ExecStop=/usr/share/openvswitch/scripts/ovs-ctl --no-ovs-vswitchd stop
>>>  ExecReload=/usr/share/openvswitch/scripts/ovs-ctl --no-ovs-vswitchd \
>>>             --ovs-user=${OVS_USER_ID} \
>>>             --no-monitor restart $OPTIONS
>>> -RuntimeDirectory=openvswitch
>>> -RuntimeDirectoryMode=0755
>> 
>> I'm not sure about this change.  One thing that's nice is the way
>> systemd will cleanup the runtime directories when this is done.  I think
>> this can leave it around.
>> 
>> I realize that under rhel (and fedora), the /run mountpoint is tmpfs.
>> 
>> Is there another way we could accomodate this?  Does a db flag make
>> sense?
>
> I don't think we can use db since we need that flags to be reset on each
> reboot (that's the reason I use /run).

I *think* systemd has a way of tracking how many starts the service has
had since boot time.  If not, that might actually be a good feature to
add to systemd, and then pull in this service file (because then we
don't need a flag in the DB, just query the environment variable for the
counter and if it is the very first start, delete - otherwise don't).

I'm not sure.  I asked in the #systemd chat on freenode, and the answer
was something like it exists, but might not be exposed via environment
variable.

I realize that would make the approach take longer to implement, but it
also does feel like it puts the concerns in the correct layers (ie: the
thing that can track restarts does so and informs the thing that cares
about them).

What do you think?

> In the previous patchset this run directory was created by using
> tmpfiles.d, but unlucky tmpfiles.d cannot use environment files, so you
> can't use ${OVS_USER_ID}.

That's kind of a shame, but I understand.

> If cleanup is needed, but imho it should be done by ovs-ctl or
> ovs-vswitchd/ovsdb-server directly, we may do the cleanup on ExecStopPost.

Patch

diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service b/rhel/usr_lib_systemd_system_ovsdb-server.service
index 7acd25f78..42473161e 100644
--- a/rhel/usr_lib_systemd_system_ovsdb-server.service
+++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
@@ -10,14 +10,14 @@  Type=forking
 Restart=on-failure
 EnvironmentFile=/etc/openvswitch/default.conf
 EnvironmentFile=-/etc/sysconfig/openvswitch
+ExecStartPre=/usr/bin/mkdir -p /var/run/openvswitch
 ExecStartPre=/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch
 ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
           --no-ovs-vswitchd --no-monitor --system-id=random \
+          --delete-transient-ports-on-boot \
           --ovs-user=${OVS_USER_ID} \
           start $OPTIONS
 ExecStop=/usr/share/openvswitch/scripts/ovs-ctl --no-ovs-vswitchd stop
 ExecReload=/usr/share/openvswitch/scripts/ovs-ctl --no-ovs-vswitchd \
            --ovs-user=${OVS_USER_ID} \
            --no-monitor restart $OPTIONS
-RuntimeDirectory=openvswitch
-RuntimeDirectoryMode=0755