FS#65027 - [netdata] Improve netdata package service file and configuration
Attached to Project:
Community Packages
Opened by AMM (amish) - Friday, 03 January 2020, 08:42 GMT
Last edited by Sven-Hendrik Haase (Svenstaro) - Saturday, 21 March 2020, 07:04 GMT
Opened by AMM (amish) - Friday, 03 January 2020, 08:42 GMT
Last edited by Sven-Hendrik Haase (Svenstaro) - Saturday, 21 March 2020, 07:04 GMT
|
Details
Description:
I would like to propose following improvements to netdata package Improve service file - /usr/lib/systemd/system/netdata.service 1a) Remove four ExecStartPre lines from service file, which creates 'run' and 'cache' directories and changes the ownership recursively. 1b) Instead let systemd create them when necessary. Add these lines to netdata.service file: CacheDirectory=netdata StateDirectory=netdata LogsDirectory=netdata StateDirectoryMode=0750 CacheDirectoryMode=0750 LogsDirectoryMode=0750 1c) File /usr/lib/tmpfiles.d/netdata.conf is not required anymore. As all necessary directories will be created automatically by systemd when required. 2a) Dont set configuration options in ExecStart but put them in netdata.conf (which is by default shipped as empty file). i.e. remove all -W options from ExecStart of netdata.service file. New ExecStart will look like this: ExecStart=/usr/bin/netdata -P /run/netdata/netdata.pid -D 2b) Add following to /etc/netdata/netdata.conf file (by default it is empty) [global] process scheduling policy = keep OOM score = keep 3) Systemd recommends to use /run instead of /var/run (it logs a warning if you use /var/run). ExecStart=/usr/bin/netdata -P /run/netdata/netdata.pid -D PIDFile=/run/netdata/netdata.pid 4) PermissionsStartOnly is deprecated by systemd and not required anyway, if above changes are implemented. Additional info: * package version 1.19.0-1 * config and/or log files etc. /usr/lib/systemd/system/netdata.service /usr/lib/tmpfiles.d/netdata.conf /etc/netdata/netdata.conf |
This task depends upon
Closed by Sven-Hendrik Haase (Svenstaro)
Saturday, 21 March 2020, 07:04 GMT
Reason for closing: Fixed
Saturday, 21 March 2020, 07:04 GMT
Reason for closing: Fixed
https://github.com/netdata/netdata/pull/7790
Initial response from one of the developer is that they want to support older systemd. And older systemd does not support those directives.
They have not rejected the PR or reviewed it thoroughly yet but Arch being bleeding edge, I believe we should go ahead and alter the service file to use the latest features available with systemd, instead of many ExecStartPre lines which look more like a hack.
If you want I can provide a patch file for netdata.service file.
About my point number 3), the directives can be changed to these:
PIDFile=netdata/netdata.pid
ExecStart=/usr/bin/netdata -P $PIDFILE -D
Systemd will automatically add /run and pass PIDFILE to ExecStart.
https://github.com/netdata/netdata/commit/2979efe03161499e9a53f76d843c2fd7e5006a92
I expect it to be in next release. There will be a new service file named netdata.service.v235 which uses additional features available after systemd v235+.
So PKGBUILD should copy that file instead of netdata.service file.(unless they make changes to Makefile to automatically create appropriate service file by checking systemd version)
Afterwards there will be no need for systemd.tmpfiles that is shipped.
Additionally, earlier netdata.conf was shipped as empty file, but now it requires some lines. So it would be better to copy upstream netdata.conf file.
So right now we need to wait, till the next version is released.
New version of netdata (v 1.20.0) ships with new service file (along with old).
But this is still not used by default because upstream still wants to support older distributions (where new service file will not work).
So I would suggest the following changes to PKGBUILD so that Arch linux uses new service file.
prepare() {
cd "$pkgname-$pkgver"
# We are Arch! We use advanced service file
mv system/netdata.service.v235.in system/netdata.service.in
}
package() {
...
- touch "$pkgdir/etc/netdata/netdata.conf"
+ # use netdata.conf file provided by upstream
+ install -Dm0644 "system/netdata.conf" "$pkgdir/etc/netdata/netdata.conf"
...
}
Please also refer to my comments above for other changes which will cleanup some no-more-required files.
Thank you.
Ownership was netdata:netdata before this too. (check tmpfiles file as well as PKGBUILD setting ownership to 134:134)
https://git.archlinux.org/svntogit/community.git/tree/trunk/netdata.tmpfiles?h=packages/netdata
https://git.archlinux.org/svntogit/community.git/tree/trunk/PKGBUILD?h=packages/netdata
Also files are 0644 i.e permission to read for all. So I am not sure why dashboard would not load for you.
It is working for me.
If you still think it is a bug, then please file another bug report.
It's not a permission to read issue. But netdata checks if file ownership is correct and issues an error and stops otherwise. The error is logged as "netdata ERROR : WEB_SERVER[static4] : 2: File '/usr/share/netdata/web//index.html' is owned by user 134 (expected user 0). Access Denied." in /var/log/netdata/error.log and "Access to file is not permitted: /usr/share/netdata/web//index.html" at http://localhost:19999. The mechanics is explained in the github issue I shared above.
It's probably working for you because you use your own netdata.conf (with "web files owner = netdata" or unset), not the one installed by this package.
So to maintain current behaviour (of working package), PKGBUILD needs one more line after netdata.conf file is copied: (after line 59 in current PKGBUILD)
sed -i '/web files owner/ s/root/netdata/g' "$pkgdir"/etc/netdata/netdata.conf
Upstream recommendation about ownership of web files, needs another bug report. (if at all required)
Arch should probably do as the others, i.e., change "chown -R 134:134 "$pkgdir"/usr/share/netdata/web" to "chown -R 0:134 "$pkgdir"/usr/share/netdata/web" in the PKGBUILD.
Two details:
* netdata.tmpfiles can be removed.
* I don't know who should own /etc/netdata and if "chown -R 0:134 "$pkgdir"/etc/netdata" is necessary.
Also /etc/netdata owned by root also looks safer in my opinion.