[ovs-dev] rhel: let *-ctl handle runtime directory
diff mbox series

Message ID 20190610135531.14010-1-jcaamano@suse.com
State New
Headers show
Series
  • [ovs-dev] rhel: let *-ctl handle runtime directory
Related show

Commit Message

Jaime Caamaño Ruiz June 10, 2019, 1:55 p.m. UTC
Recent versions of systemd restores RuntimeDirectory ownership to the
unit's User in between execution of *Exec directives (see [1]). Using
ExecStartPre to reset RuntimeDirectory ownership to OVS_USER no longer
works as expected.

The ctl scripts already handle creation of the runtime directory with
correct ownership and permissions so we can basically remove
RuntimeDirectory from systemd unit file. There is still need to handle
ownsership to cover some upgrade scenarios, but success of that will be
optional as the directory itself wont exist at first time run.

[1] https://github.com/systemd/systemd/issues/12713

Signed-off-by: Jaime Caamaño Ruiz <jcaamano@suse.com>
---
 rhel/usr_lib_systemd_system_ovsdb-server.service | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

0-day Robot June 10, 2019, 3:03 p.m. UTC | #1
Bleep bloop.  Greetings Jaime Caamaño Ruiz, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 85 characters long (recommended limit is 79)
#33 FILE: rhel/usr_lib_systemd_system_ovsdb-server.service:14:
ExecStartPre=-/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch /var/log/openvswitch

Lines checked: 45, Warnings: 1, Errors: 0


Please check this out.  If you feel there has been an error, please email aconole@bytheb.org

Thanks,
0-day Robot
Ben Pfaff June 10, 2019, 8:51 p.m. UTC | #2
On Mon, Jun 10, 2019 at 03:55:31PM +0200, Jaime Caamaño Ruiz wrote:
> Recent versions of systemd restores RuntimeDirectory ownership to the
> unit's User in between execution of *Exec directives (see [1]). Using
> ExecStartPre to reset RuntimeDirectory ownership to OVS_USER no longer
> works as expected.
> 
> The ctl scripts already handle creation of the runtime directory with
> correct ownership and permissions so we can basically remove
> RuntimeDirectory from systemd unit file. There is still need to handle
> ownsership to cover some upgrade scenarios, but success of that will be
> optional as the directory itself wont exist at first time run.
> 
> [1] https://github.com/systemd/systemd/issues/12713
> 
> Signed-off-by: Jaime Caamaño Ruiz <jcaamano@suse.com>

Thanks, I applied this to master.

Patch
diff mbox series

diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service b/rhel/usr_lib_systemd_system_ovsdb-server.service
index 9bb37fd06..a6de4d3c1 100644
--- a/rhel/usr_lib_systemd_system_ovsdb-server.service
+++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
@@ -11,7 +11,7 @@  PIDFile=/var/run/openvswitch/ovsdb-server.pid
 Restart=on-failure
 EnvironmentFile=/etc/openvswitch/default.conf
 EnvironmentFile=-/etc/sysconfig/openvswitch
-ExecStartPre=/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch /var/log/openvswitch
+ExecStartPre=-/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch /var/log/openvswitch
 ExecStartPre=/bin/sh -c 'rm -f /run/openvswitch.useropts; /usr/bin/echo "OVS_USER_ID=${OVS_USER_ID}" > /run/openvswitch.useropts'
 ExecStartPre=/bin/sh -c 'if [ "$${OVS_USER_ID/:*/}" != "root" ]; then /usr/bin/echo "OVS_USER_OPT=--ovs-user=${OVS_USER_ID}" >> /run/openvswitch.useropts; fi'
 EnvironmentFile=/run/openvswitch.useropts
@@ -23,5 +23,3 @@  ExecStop=/usr/share/openvswitch/scripts/ovs-ctl --no-ovs-vswitchd stop
 ExecReload=/usr/share/openvswitch/scripts/ovs-ctl --no-ovs-vswitchd \
            ${OVS_USER_OPT} \
            --no-monitor restart $OPTIONS
-RuntimeDirectory=openvswitch
-RuntimeDirectoryMode=0755