#2498 - Provide systemd unit for spawn-fcgi

#2498 - Provide systemd unit for spawn-fcgi

[mathias, 2017-08-01 15:59:32]

To consider this as a solution we would need to ensure to upgrade all remaining
precise systems running spawn-fcgi first. Like the issues.adblockplus.org
backend.

[f.nicolaisen, 2017-08-01 16:11:10]

On 2017/08/01 15:59:32, mathias wrote:
> To consider this as a solution we would need to ensure to upgrade all
remaining
> precise systems running spawn-fcgi first. Like the
http://issues.adblockplus.org
> backend.

Ack. That reminds me, should we consider doing similar fixes for our other init
scripts?

[f.nicolaisen, 2017-08-01 21:48:53]

On 2017/08/01 15:59:32, mathias wrote:
> To consider this as a solution we would need to ensure to upgrade all
remaining
> precise systems running spawn-fcgi first. Like the
http://issues.adblockplus.org
> backend.

This solution is backwards-compatible: Providing a unit file does not prevent
the init script to work the old way on older systems.

(may have to ensure that /etc/systemd/system exists..)

[mathias, 2017-08-01 22:21:18]

On 2017/08/01 21:48:53, f.nicolaisen wrote:
> This solution is backwards-compatible: Providing a unit file does not prevent
> the init script to work the old way on older systems.
> 
> (may have to ensure that /etc/systemd/system exists..)

Doesn't feel quite clean, but well, fair enough. For some reason the systemd
structure is available in those precise setups already, if that is
implicit/consistent you can even save that step.

https://codereview.adblockplus.org/29502643/diff/29502644/modules/spawn_fcgi/...
File modules/spawn_fcgi/files/spawn-fcgi.service (right):

https://codereview.adblockplus.org/29502643/diff/29502644/modules/spawn_fcgi/...
modules/spawn_fcgi/files/spawn-fcgi.service:1: [Unit]
Where does the content of this file come from? How did you come up with the
keys/values here? I guess it's a copy from the auto-generated version on another
system or something, either way it should be documented.

https://codereview.adblockplus.org/29502643/diff/29502644/modules/spawn_fcgi/...
File modules/spawn_fcgi/manifests/init.pp (right):

https://codereview.adblockplus.org/29502643/diff/29502644/modules/spawn_fcgi/...
modules/spawn_fcgi/manifests/init.pp:19: ],
That closing bracket is one indent level too deep.

[f.nicolaisen, 2017-08-02 07:51:48]

> Doesn't feel quite clean, but well, fair enough. For some reason the systemd
> structure is available in those precise setups already,  if that is
> implicit/consistent you can even save that step.

Lots of packages bring these unit files in with them. It would've been better if
the spawn-fcgi package would provide a unit file itself, but I think it will be
too much effort to go that road.

There reason why the directory structure is present on precise can indeed be
that a package provides them, or set them up during their installation.

On issues1 for example, the rsyslog package provided the /etc/systemd/system
dir, and the nginx package also makes use of it (maybe via its postinst script).

https://codereview.adblockplus.org/29502643/diff/29502644/modules/spawn_fcgi/...
File modules/spawn_fcgi/files/spawn-fcgi.service (right):

https://codereview.adblockplus.org/29502643/diff/29502644/modules/spawn_fcgi/...
modules/spawn_fcgi/files/spawn-fcgi.service:1: [Unit]
On 2017/08/01 22:21:18, mathias wrote:
> Where does the content of this file come from? How did you come up with the
> keys/values here? I guess it's a copy from the auto-generated version on
another
> system or something, either way it should be documented.

This is explained in the commit message, but I can add a comment in this file as
well.

https://codereview.adblockplus.org/29502643/diff/29502644/modules/spawn_fcgi/...
File modules/spawn_fcgi/manifests/init.pp (right):

https://codereview.adblockplus.org/29502643/diff/29502644/modules/spawn_fcgi/...
modules/spawn_fcgi/manifests/init.pp:19: ],
On 2017/08/01 22:21:18, mathias wrote:
> That closing bracket is one indent level too deep.

Acknowledged.

[f.nicolaisen, 2017-08-08 07:53:04]

PS2 addresses the latest comments btw.

[mathias, 2017-08-08 10:12:24]

For the sake of consistency please adjust the white-space in your changes to the
four spaces (?) used within these files.

Also, for the record, I want to note that I'd still prefer relying on systemd's
native translation mechanism and just explicitly activate the virtual service
units now - which seems like a way more lightweight approach to me, and it would
ensure the configuration file to be generated by the systemd version that is
running on the local machine. However, I am willing to try this approach and see
how consistent that stuff behaves and get some experience on how much of a
concern version-specific configuration actually is here.

[f.nicolaisen, 2017-08-08 10:32:07]

On 2017/08/08 10:12:24, mathias wrote:
> For the sake of consistency please adjust the white-space in your changes to
the
> four spaces (?) used within these files.

Ack.

> Also, for the record, I want to note that I'd still prefer relying on
systemd's
> native translation mechanism and just explicitly activate the virtual service
> units now - which seems like a way more lightweight approach to me, and it
would
> ensure the configuration file to be generated by the systemd version that is
> running on the local machine. However, I am willing to try this approach and
see
> how consistent that stuff behaves and get some experience on how much of a
> concern version-specific configuration actually is here.

Yeah. This is basically our workaround for a bug in puppet's workaround for a
bug in systemd. We may be able to remove it with systemd 220, which I guess we
get with Stretch, if not before? I'll note that in the comment as well.

[f.nicolaisen, 2017-08-08 10:49:57]

OK, PS5 addresses the latest things now, took me a couple of tries to get the
comment formatting in order.

[mathias, 2017-08-08 11:17:12]

LGTM.