#1078329 sysstat service enablement state mismatches debconf and /etc/default/sysstat setting

Package:
sysstat
Source:
sysstat
Description:
system performance tools for Linux
Submitter:
Robie Basak
Date:
2024-08-09 13:54:04 UTC
Severity:
normal
Tags:
#1078329#5
Date:
2024-08-09 13:22:29 UTC
From:
To:
When systemd presets are applied, the various systemd services shipped
by sysstat become enabled, even if the debconf sysstat/enable setting
and /etc/default/sysstat asks for them to be disabled.

This happens, for example, if the user prepares a "golden image" and
then boots it, resulting in /etc/machine-id being recreated and systemd
running the equivalent of `systemctl preset-all`. Then the sysstat
services will be enabled even though /etc/default/sysstat says
ENABLED="no".

Further, the manage_systemd_services() implementation in
sysstat.postinst calls systemctl directly instead of using
deb-systemd-invoke, thus bypassing user choices via policy-rc.d.

To fix both of these issues at once, I suggest that you instead use
ExecCondition in the systemd services to implement the behaviour
specified through the debconf sysstat/enable setting and the ENABLED
variable in /etc/default/sysstat.

This way, dh_installsystemd is used as normal, so the various complex
interactions between maintainer scripts and user choices, with respect
to enable, disable, presets, policy-rc.d, etc will all work correctly as
expected.

Please see the attached patch, which is my current work in progress.
I've been careful to use precisely the same logic as used in the
existing maintainer scripts to interpret /etc/default/sysstat, to ensure
that edge-case behaviour does not change.

In Ubuntu, we started to install sysstat by default in 24.04, which is
why this issue came up. On every fresh installation of Ubuntu 24.04,
/etc/default/sysstat is mismatching the real state of the services. I
intend to fix this issue in Ubuntu shortly using something like the
attached patch, together with upgrade path handling. In order to avoid
unnecessary divergence, I'd appreciate discussion if you'd like to
approach the fix for this bug in Debian differently.

Thanks,

Robie

diff --git a/debian/patches/series b/debian/patches/series
index afceded..507edfe 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -11,3 +11,4 @@
 14-simtest-run-all.patch
 15-sa2-bash.patch
 16-empty-dirs.patch
+service-conditional-on-setting
diff --git a/debian/patches/service-conditional-on-setting b/debian/patches/service-conditional-on-setting
new file mode 100644
index 0000000..a2bc5a0
--- /dev/null
+++ b/debian/patches/service-conditional-on-setting
@@ -0,0 +1,27 @@
+--- a/cron/sysstat-collect.service.in
++++ b/cron/sysstat-collect.service.in
+@@ -13,5 +13,6 @@
+ [Service]
+ Type=oneshot
+ User=@CRON_OWNER@
++ExecCondition=sh -c 'ENABLED=false; [ -r /etc/default/sysstat ] && . /etc/default/sysstat; test "$${ENABLED}" = "true"'
+ ExecStart=@SA_LIB_DIR@/sa1 1 1
+
+--- a/cron/sysstat-summary.service.in
++++ b/cron/sysstat-summary.service.in
+@@ -12,4 +12,5 @@
+ [Service]
+ Type=oneshot
+ User=@CRON_OWNER@
++ExecCondition=sh -c 'ENABLED=false; [ -r /etc/default/sysstat ] && . /etc/default/sysstat; test "$${ENABLED}" = "true"'
+ ExecStart=@SA_LIB_DIR@/sa2 -A
+--- a/sysstat.service.in
++++ b/sysstat.service.in
+@@ -13,6 +13,7 @@
+ Type=oneshot
+ RemainAfterExit=yes
+ User=@CRON_OWNER@
++ExecCondition=sh -c 'ENABLED=false; [ -r /etc/default/sysstat ] && . /etc/default/sysstat; test "$${ENABLED}" = "true"'
+ ExecStart=@SA_LIB_DIR@/sa1 --boot
+
+ [Install]
diff --git a/debian/sysstat.postinst b/debian/sysstat.postinst
index 870d678..6ae366e 100644
--- a/debian/sysstat.postinst
+++ b/debian/sysstat.postinst
@@ -44,42 +44,6 @@ EOF
     rm -f "$def_file"
 }

#1078329#10
Date:
2024-08-09 13:44:53 UTC
From:
To:
Also the new sysstat-rotate.service probably needs this treatment, too.