Drop env var settings in systemd service file

[Andreas Hasenack]

It looks like we can drop these from the systemd service files:

Environment="KEA_PIDFILE_DIR=/run/kea"
Environment="KEA_LOCKFILE_DIR=/run/lock/kea"

A quick check showed the daemon working just fine and with the locks in the expected places, but a more through tests should be done. In particular, investigate whether there are config file options for the same things, in which case we should definitely drop these vars from the systemd service file to avoid confusion, as they would override the config file settings.

[Paride Legovini (paride)]

I am not sure I agree with dropping those. Rationale:

1. Those do not appear to be configurable via config files. In principle those files could be created *before* the conf file is parsed (especially true for the pidfile, not sure of when the lockfile is created). So no confusion with conf file settings.

2. With those variable set the pid and lock files go in "kea/" directories (/run/kea/ and /run/lock/kea/). I think this is nice given that Kea can have several daemons running at the same time. This is not the default (at least according to the docs).

3. The upstream packaging does set those env vars. We are not bound to follow what upstream does wrt packaging, but I think it's friendly to users not to diverge when possible.

[Andreas Hasenack (ahasenack)]

These are all vars that are set:

# grep Environment /lib/systemd/system/kea-*
/lib/systemd/system/kea-ctrl-agent.service:Environment="KEA_PIDFILE_DIR=/run/kea"
/lib/systemd/system/kea-ctrl-agent.service:Environment="KEA_LOCKFILE_DIR=/run/lock/kea"
/lib/systemd/system/kea-ctrl-agent.service:Environment="KEA_LOGGER_DESTINATION=/var/log/kea"
/lib/systemd/system/kea-dhcp-ddns-server.service:Environment="KEA_PIDFILE_DIR=/run/kea"
/lib/systemd/system/kea-dhcp-ddns-server.service:Environment="KEA_LOCKFILE_DIR=/run/lock/kea"
/lib/systemd/system/kea-dhcp-ddns-server.service:Environment="KEA_LOGGER_DESTINATION=/var/log/kea-ddns"
/lib/systemd/system/kea-dhcp4-server.service:Environment="KEA_PIDFILE_DIR=/run/kea"
/lib/systemd/system/kea-dhcp4-server.service:Environment="KEA_LOCKFILE_DIR=/run/lock/kea"
/lib/systemd/system/kea-dhcp4-server.service:Environment="KEA_LOGGER_DESTINATION=/var/log/kea"
/lib/systemd/system/kea-dhcp6-server.service:Environment="KEA_PIDFILE_DIR=/run/kea"
/lib/systemd/system/kea-dhcp6-server.service:Environment="KEA_LOCKFILE_DIR=/run/lock/kea"
/lib/systemd/system/kea-dhcp6-server.service:Environment="KEA_LOGGER_DESTINATION=/var/log/kea"

a) KEA_LOGGER_DESTINATION is clearly not matching what we have in the config, but there is a twist[2][3] I didn't know of:
"""
There is a small time window after Kea has been started but before it has read its configuration; logging in this short period can be controlled using environment variables. For details, see Logging During Kea Startup.
"""

It looks like this env var will be overridden by what is set in the config file, if anything. If we are defaulting to journal/stdout in the future, maybe we should just make sure that's the compiled-time default, or then keep KEA_LOGGER_DESTINATION but set it to stdout. The only options from [3] are stdout, stderr, a file, and syslog.

b) These vars are meant to be overrides (well, maybe the LOGGER one isn't?). There is no sense in setting them to the default values, other than being a way to tell the user they exist and can be used. To quote the documentation about one of these I picked at random[1]:
"""
runstatedir - is the value as passed into the build configure script; it defaults to “/usr/local/var/run”. Note that this value may be overridden at runtime by setting the environment variable KEA_PIDFILE_DIR. This is intended primarily for testing purposes.
"""

(b) is not so bad, in the end, because there don't seem to be equivalent settings in the config, so there is no room for confusion (set it to one value in config, and another value in env). That leaves just the logger point in (a) to think about.

1, https://kea.readthedocs.io/en/kea-2.2.0/arm/ddns.html?highlight=KEA_PIDFILE_DIR#starting-and-stopping-the-dhcp-ddns-server
2. https://kea.readthedocs.io/en/kea-2.2.0/arm/logging.html?highlight=small#loggers
3. https://kea.readthedocs.io/en/kea-2.2.0/arm/logging.html?highlight=small#logging-during-startup

[Paride Legovini (paride)]

Hi Andreas,

a) We're not setting KEA_LOGGER_DESTINATION anymore, see https://salsa.debian.org/debian/isc-kea/-/merge_requests/12/diffs?commit_id=1c9221b26d88c0250d38bd79078cbb6e4d24268f.

b) I'm +1 with dropping the useless KEA_PIDFILE_DIR override (useless as we're overriding it with its default). I prepared this MR, with a small autopkgtest in case the default changes:

https://salsa.debian.org/debian/isc-kea/-/merge_requests/17

For KEA_LOCKFILE_DIR: that's *not* the default if I'm not mistaken, with the default being

  $(localstatedir)/run/$(PACKAGE_NAME)

(see src/lib/log/interprocess/Makefile.*). So I'd leave KEA_LOCKFILE_DIR set. The location we're using is consistent with hier(7).

[Andreas Hasenack (ahasenack)]

a) ok
b) ok

[Athos Ribeiro (athos-ribeiro)]

This has been fixed in Debian and sync'd into lunar-proposed

[Andreas Hasenack (ahasenack)]

Done in https://launchpad.net/ubuntu/+source/isc-kea/2.2.0-2