Merge ~lucaskanashiro/ubuntu/+source/openvpn:drop-reload into ubuntu/+source/openvpn:ubuntu/devel

Proposed by Lucas Kanashiro on 2020-05-26
Status: Merged
Approved by: Lucas Kanashiro on 2020-05-27
Approved revision: 3ce84bbf5301e107683a57ca53a186c37d24b8fb
Merged at revision: 3ce84bbf5301e107683a57ca53a186c37d24b8fb
Proposed branch: ~lucaskanashiro/ubuntu/+source/openvpn:drop-reload
Merge into: ubuntu/+source/openvpn:ubuntu/devel
Diff against target: 46 lines (+6/-3)
3 files modified
debian/changelog (+6/-0)
debian/openvpn.service (+0/-1)
debian/openvpn@.service (+0/-2)
Reviewer Review Type Date Requested Status
Christian Ehrhardt  2020-05-26 Approve on 2020-05-27
Canonical Server Team 2020-05-26 Pending
Review via email: mp+384585@code.launchpad.net

Description of the change

This MP aims to fix this bug in Groovy:

https://bugs.launchpad.net/ubuntu/+source/openvpn/+bug/1868127

Currently, the ExecReload= tries to send a SIGHUP signal to the process but it does not have the permission to do this. It'd need a new capability (CAP_KILL) or a + prefix to run this command with full privileges. However, after trying the mentioned solutions I noticed that actually the SIGHUP signal handling implementation does something similar to the restart process, not justifying the existence of the reload implementation. Moreover, systemd does not require a reload implementation, so I believe dropping this systemd reload support is the way to go.

I submitted these changes to Debian here:

https://salsa.debian.org/debian/openvpn/-/merge_requests/5

autopkgtest is happy:

autopkgtest [16:59:57]: @@@@@@@@@@@@@@@@@@@@ summary
server-setup-with-ca PASS
server-setup-with-static-key PASS

And here is my PPA with the proposed package:

https://launchpad.net/~lucaskanashiro/+archive/ubuntu/groovy-openvpn-lp1868127/+packages

To post a comment you must log in.
Sergio Durigan Junior (sergiodj) wrote :

Hey, the rationale makes sense to me, and I agree with the proposed solution.

I can't help but wonder why we (and Debian) are carrying our own version of the systemd service file, instead of relying on what upstream provides (and patching/adjusting it, if needed). I tried doing some archaeology on the Debian repo, but could not find the reason for it. Maybe something to keep in mind for a next merge (or a rainy day)...

Christian Ehrhardt  (paelzer) wrote :

On multiple services: IIRC One of them was earlier and now getting rid of either will break users of it on upgrade. But yeah worth some actual digging (instead of assumptions) on a long day.
It certainly was the source of some issues and confusion already.

On the proposed change here +1 on the chosen approach and the MP

Have you submitted to Debian already?
I'd recommend doing so, in case you settle on something else on that discussion it avoids some back and forth.

review: Approve
Lucas Kanashiro (lucaskanashiro) wrote :

I agree about the multiple services, at some point we need to revisit this subject.

And yes Christian, I submitted these changes to Debian as described in the description of this MP, here is the MR:

https://salsa.debian.org/debian/openvpn/-/merge_requests/5

I am going to upload this version.

Lucas Kanashiro (lucaskanashiro) wrote :

$ git push pkg upload/2.4.9-2ubuntu2
Enumerating objects: 15, done.
Counting objects: 100% (15/15), done.
Delta compression using up to 8 threads
Compressing objects: 100% (10/10), done.
Writing objects: 100% (10/10), 1.21 KiB | 248.00 KiB/s, done.
Total 10 (delta 7), reused 0 (delta 0)
To ssh://git.launchpad.net/~usd-import-team/ubuntu/+source/openvpn
 * [new tag] upload/2.4.9-2ubuntu2 -> upload/2.4.9-2ubuntu2

$ dput ubuntu ../openvpn_2.4.9-2ubuntu2_source.changes
Checking signature on .changes
gpg: ../openvpn_2.4.9-2ubuntu2_source.changes: Valid signature from F823A2729883C97C
Checking signature on .dsc
gpg: ../openvpn_2.4.9-2ubuntu2.dsc: Valid signature from F823A2729883C97C
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading openvpn_2.4.9-2ubuntu2.dsc: done.
  Uploading openvpn_2.4.9-2ubuntu2.debian.tar.xz: done.
  Uploading openvpn_2.4.9-2ubuntu2_source.buildinfo: done.
  Uploading openvpn_2.4.9-2ubuntu2_source.changes: done.
Successfully uploaded packages.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index 3ce3b03..430f745 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,9 @@
6+openvpn (2.4.9-2ubuntu2) groovy; urgency=medium
7+
8+ * Drop reload support from systemd unit files (LP: #1868127)
9+
10+ -- Lucas Kanashiro <kanashiro@ubuntu.com> Tue, 26 May 2020 19:04:33 -0300
11+
12 openvpn (2.4.9-2ubuntu1) groovy; urgency=medium
13
14 * Merge with Debian unstable. Remaining changes:
15diff --git a/debian/openvpn.service b/debian/openvpn.service
16index 0075cc4..a4d1149 100644
17--- a/debian/openvpn.service
18+++ b/debian/openvpn.service
19@@ -9,7 +9,6 @@ After=network.target
20 Type=oneshot
21 RemainAfterExit=yes
22 ExecStart=/bin/true
23-ExecReload=/bin/true
24 WorkingDirectory=/etc/openvpn
25
26 [Install]
27diff --git a/debian/openvpn@.service b/debian/openvpn@.service
28index eb4be12..6d59b13 100644
29--- a/debian/openvpn@.service
30+++ b/debian/openvpn@.service
31@@ -1,7 +1,6 @@
32 [Unit]
33 Description=OpenVPN connection to %i
34 PartOf=openvpn.service
35-ReloadPropagatedFrom=openvpn.service
36 Before=systemd-user-sessions.service
37 After=network-online.target
38 Wants=network-online.target
39@@ -16,7 +15,6 @@ WorkingDirectory=/etc/openvpn
40 ExecStart=/usr/sbin/openvpn --daemon ovpn-%i --status /run/openvpn/%i.status 10 --cd /etc/openvpn --script-security 2 --config /etc/openvpn/%i.conf --writepid /run/openvpn/%i.pid
41 PIDFile=/run/openvpn/%i.pid
42 KillMode=process
43-ExecReload=/bin/kill -HUP $MAINPID
44 CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_BIND_SERVICE CAP_NET_RAW CAP_SETGID CAP_SETUID CAP_SYS_CHROOT CAP_DAC_OVERRIDE CAP_AUDIT_WRITE
45 LimitNPROC=100
46 DeviceAllow=/dev/null rw

Subscribers

People subscribed via source and target branches