Place systemd units in /usr/lib/systemd/system

[behrmann]

Using /usr/lib/systemd/system allows local sysadmins to overwrite the unit file by placing one of the same name in /etc/systemd/system.

Fixes: #259

[coveralls]

Coverage decreased (-0.03%) to 12.733% when pulling e8fc22a on behrmann:unitseachpath into a8dda22 on kardianos:master.

[behrmann]

Just noticed, that in the meantime a merge conflict has occurred, so I rebased against current master.

[kardianos]

Thanks. Also, this is a fine change, but for some operations, we need to check both the old and new path in sequence to not break existing programs.

[behrmann]

Which operations do you have in mind?

When writing a new file to /usr/lib instead of /etc the latter will take precedence and as long as how the service is to be invoked it will keep in working. In most cases the same file will be recreated in /usr/lib. Sure, as long as there is a file in /etc all newly generated files won't take effect, but I don't think that is something this module can do something about, because the only way out is deleting that and that's generally not a good idea, because it could be a user-edited/user-supplied file.

I could imagine writing a warning when a service file is generated, that is shadowed by one in /etc.

[kardianos]

1. Please reference documentation as to the different locations.
2. For checking existence or for removal, both locations should be checked.
3. For creating a new service instance, only the new location should be used.

[behrmann]

1. Please reference documentation as to the different locations.

Where exactly should I reference documentation?

2. For checking existence or for removal, both locations should be checked.

I'm not sure I agree.

Removing files under /etc is just not a good idea. This module should never touch /etc. Managing files under /etc is the prerogative of the user/sysadmin or at the least the package manger, but daemons shouldn't touch it, since the risk of inadvertently deleting a file the user doesn't want to lose is too high.

Checking for existence of a file under /etc for the installation shouldn't really matter all that much, since it will generally mean that a user supplied their own service file. Supplying a vendor service file in /usr/lib still makes sense, so it might be prudent to just get rid of the check

    _, err = os.Stat(confPath)
    if err == nil {
        return fmt.Errorf("Init already exists: %s", confPath)
    }

in Install and always clobber the file under /usr/lib.

3. For creating a new service instance, only the new location should be used.

Ok, I think that would be the case of the PR in its current state

[kardianos]

1. Just add a comment near the referenced /usr/lib location.
2. Well, we are writing and removing now. So we need to continue looking for files there, at least for now, or bump to v2, and I don't think I want to bump to v2. We can add an option to opt out of looking at etc, so a new author can opt to use that.

[behrmann]

1. Just add a comment near the referenced /usr/lib location.

Done.

2. Well, we are writing and removing now. So we need to continue looking for files there, at least for now, or bump to v2, and I don't think I want to bump to v2. We can add an option to opt out of looking at etc, so a new author can opt to use that.

That doesn't make it a good idea. Example: A binary I use currently uses this module and places its service file in /etc. The point of this PR is to not have to replace that file all the time anymore. If that binary now started removing that very same file again would make this change a bit pointless. :)

I consider writing to /etc a bug. The proper state of a service file under /etc is unknowable from the point of this module and I think it is definitely better to be safe and not to guess.

[gregharvey]

Cross-posting discussion in the GitLab issue queue:

• https://gitlab.com/gitlab-org/gitlab-runner/-/issues/2422

As a gitlab-runner user this is pretty frustrating - every time they push an upgrade everyone with custom overrides finds their runners break until they fix the unit file. What do we need to do to get this over the line? Can I help? I've never really used Go, but I don't mind having a crack if there's a spec the maintainer would be happy with. 😄

[gregharvey]

For anyone who finds this while the PR is in limbo, workarounds are possible - see use of override configs in https://askubuntu.com/a/659268

[behrmann]

Having just remembered this PR, I want to caution against this workaround.

Adding /etc/systemd/system/foo..service will totally overwrite /usr/lib/systemd/system/foo.service so that the latter is ignored and only the content of the former counts. The drop in configuration for /etc/systemd/system/foo.service via /etc/systemd/system/foo.service.d/*.conf will extend the former.

While one can overwrite each and every config item, this becomes tedious and is not safe if new config items were added from systemd's side. Shipping the unit file in /etc/, which is the wrong place—the highest—in the hierarchy of unit files, and maybe overwriting admin config while doing so, is wrong behaviour.