CLOS-4518: Reenable systemd timers left disabled by cron-to-timer migrations

[prilr]

Problem

After a CL8-CL9 upgrade, logrotate.timer is left disabled despite a vendor preset of enable, so logrotate never runs and logs grow unbounded.
Reported by a customer: /var/log/messages and modsec_audit.log reached substantial sizes.

Opening investigation suspected .rpmnew/.rpmsave cron residue, but that was not relevant - the cron file is simply gone from the EL9 package, and the timer that replaced it was never enabled.

Root cause

Three facts compound:

1. EL8 logrotate ships /etc/cron.daily/logrotate and no timer. EL9 drops the cron file and ships logrotate.timer, vendor preset enable.
2. During an in-place upgrade logrotate is an RPM upgrade, so its %systemd_post scriptlet — guarded by [ $1 -eq 1 ] — does not apply the
   timer's preset. Presets are only applied on a fresh install.
3. leapp's systemd state transition would be the safety net, but common/libraries/systemd.py scans with _SYSTEMCTL_CMD_OPTIONS = ['--type=service', ...], and get_system_service_preset_files() emits presets only for units ending in .service. Timers, sockets and paths are structurally invisible to the whole mechanism.

Approach

The initial approach idea used a hardcoded list of affected timers.
However, it would (obviously) miss components that we didn't write into it, so was deemed unsuitable to handling various customer machine configurations. This implementation uses a rule instead of an enumeration:

Enable a timer only if it is absent on the source system, disabled on the target, and its target vendor preset is enable.

A timer that did not exist on the source cannot have been disabled by the administrator, so applying the target preset is assumed safe - it reproduces what a fresh install would have done.

This is the same reasoning the service-state transition already applies to units new on the target.

Timers that did exist on the source keep whatever state the normal transition gave them, so a deliberate systemctl disable always survives the upgrade. If the source inventory message is missing, the actor does nothing and logs a warning, rather than guessing.

Test run

Full CL8-CL9 upgrade on a no-panel VM with mdadm installed:

Timer	Source (CL8)	After upgrade
logrotate.timer	absent	enabled + active, next run 00:00	OK
raid-check.timer	absent	enabled + active, next run Sun 01:00	OK
mdcheck_start / mdcheck_continue / mdmonitor-oneshot	present, disabled	still disabled	OK state respected
mlocate-updatedb.timer	present, enabled	still enabled	OK, same

The third row is of interest: a naive "enable every disabled preset-enabled timer" implementation would have wrongly flipped those three.

The upgrade itself was clean - CL9.8 final state, 3000+ packages, no errors.

[azheregelya]

Codex review — Request changes

I found two blocking regressions and one major correctness gap.

1. Blocking: target-only handling defeats the libvirt exclusion

_filter_ignored_services() implements the CL8+ libvirt exclusion by removing virtqemud.service and related names from services_source. The changed target filter then retains those source-absent units and reclassifies them as new. With the CL9 preset enable virtqemud.service, a disabled target is queued in to_enable, reversing the exclusion added to prevent invalid monolithic/modular libvirt combinations.

Please carry an explicit ignored-name set into target filtering (or otherwise exclude these target units) and add a full-process regression test.

Relevant code · Original exclusion commit

2. Blocking: preset ordering differs from systemd

get_system_unit_presets() relies on _get_system_preset_files(), which sorts absolute paths, placing all /etc files before all /usr files. systemd instead resolves same-name directory overrides and then sorts retained files by basename. For /usr/.../10-vendor.preset: disable foo.timer plus /etc/.../99-local.preset: enable foo.timer, systemd selects disable; this helper returns enable, and the caller starts the timer. /run/systemd/system-preset is also omitted.

Please match the ordering and override rules from systemd.preset(5), or delegate effective-preset evaluation to systemd, with mixed-directory tests.

Preset discovery · New helper

3. Major: template-timer preset instances are lost

_parse_preset_entry() hardcodes .service when expanding template instances, so enable backup@.timer daily becomes backup@daily.service; the new .timer filter removes it. The actor also selects candidates only from list-unit-files, which may not contain an unenabled preset instance.

Please preserve the template's actual suffix, include preset-declared instances in candidate selection, and add an end-to-end timer-template test.

Parser and helper

Validation

• 72 focused tests passed.
• Changed-file Flake8, isort, non-ASCII lint, and git diff --check passed.
• Security scan found no vulnerabilities.
• The passing tests do not cover the scenarios above.

— Codex review

[prilr]

Finding 1 is a real regression in this PR. Fixed, along with 3. 2 left as is, with reasoning below.

1. Libvirt exclusion defeated - fixed

Reproduced before fixing:

services_source after _filter_ignored_services: {}
kept by _filter_irrelevant_services: ['virtqemud.service']
desired_states: {'virtqemud.service': 'enabled'}
to_enable: ['virtqemud.service']

Fix stops conflating ignored with absent: _filter_ignored_services() returns the excluded names, _filter_irrelevant_services() drops those target units too, and the now-false comment is corrected.

Added the full-process regression test.

3. Template-instance unit type - fixed

Confirmed: _parse_preset_entry() hardcoded .service, so enable template@.timer daily weekly produced template@daily.service / template@weekly.service and the type filter dropped them.

Now derived from the matched template file via splitext. Added a .timer template fixture and a parametrized case; also corrected the disable * and _parse_preset_files expectations that the new fixture affects.

Note that no EL8-EL9 preset declares template instances for a non-service unit - on a stock CL9 the only @ entries are getty@.service and getty@tty1.service, and there are no template-with-instance entries at all.

Did not take the second half of that finding (include preset-declared instances in candidate selection). Template instances have no unit file, so they are inherently outside a list-unit-files-based scan - covering them means enumerating preset-declared instances, which is a larger change with no current driver.

The model docstring already states instances are out of scope.

2. Preset ordering - real, but deliberately outside of this PR

The mechanism is correct, including /run/systemd/system-preset being omitted. _get_system_preset_files() sorts absolute paths, so every /etc file precedes every /usr file, whereas systemd.preset(5) sorts by basename across directories. Your example holds.

The reason it was left outside of scope: this PR doesn't introduce it, and it isn't specific to timers.

256:    preset_files = _get_system_preset_files()      # get_system_unit_presets      (new)
257:    presets = _parse_preset_files(...)
276:    preset_files = _get_system_preset_files()      # get_system_service_preset_files (existing)
277:    presets = _parse_preset_files(...)

Both helpers share the same discovery and parsing - deliberately, so override and first-match-wins semantics stay identical. Every existing service enable/disable decision already runs on this ordering. Correcting it only inside the new helper would leave services and timers disagreeing about the same preset files, which is worse than the current consistent behavior.

I'll be filing a separate PR for this one.