Skip to content

Implement ExecReload for older SystemD versions - #146

Open
Sharpie wants to merge 1 commit into
OpenVoxProject:mainfrom
Sharpie:old-systemd-compat
Open

Implement ExecReload for older SystemD versions#146
Sharpie wants to merge 1 commit into
OpenVoxProject:mainfrom
Sharpie:old-systemd-compat

Conversation

@Sharpie

@Sharpie Sharpie commented Sep 5, 2026

Copy link
Copy Markdown
Member

Pull Request (PR) description

Support for Type=notify-reload was implemented in SystemD v253. Several still-supported operatings systems, Amazon 2023, EL 8 & 9, SLES 15, Ubuntu 22.04, are using older SystemD versions.

This commit restores cli/reload.erb and sets it up as the default ExecReload along with Type=notify. For operating systems that have modern SystemD versions, fpm.rb deletes the ExecReload line and swaps in Type=notify-reload.

This Pull Request (PR) fixes the following issues

@Sharpie Sharpie added the bug Something isn't working label Sep 5, 2026
Comment thread resources/puppetlabs/lein-ezbake/template/global/ext/fpm.rb Outdated

# Type=notify-reload was introduced in systemd v253. Anything older has to stay
# on Type=notify plus an explicit ExecReload.
def notify_reload?(options)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Suggestion, but seems legit: fold this into the existing per-platform branches instead of adding a second table

fpm.rb already has one place per platform where facts like java, java_bin, and systemd_el get decided: the rpm branch keyed on operating_system/os_version (L267-302) and the deb branch keyed on options.dist (L396-407). Both fail loudly on anything they don't recognise. notify_reload? introduces a third dispatch with a differently shaped key, and it has already drifted from the other two:

  • The deb path never populates os_version. controller.sh only passes --dist for debs (and --os-version is Integer-typed, so 22.04 couldn't parse anyway), so the key is always [:debian, ''] and the [:ubuntu, '22.04'] arm is unreachable. Ubuntu 22.04 (systemd 249) therefore falls through to else true and gets Type=notify-reload with ExecReload stripped.
  • ubuntu20.04 (systemd 245) is still accepted at L398 and still in the default cows, but isn't listed here, so it also falls through to true.
  • The .to_s exists only so '8' | '9' can match an Integer.

Because it's a deny-list with else true, every future platform added to the rpm or deb branch has to be mirrored here or it silently gets the unsafe value. Type=notify plus a blocking ExecReload works on every systemd version, so the safe default is false.

Proposed shape: set options.notify_reload alongside java_bin in the branches that already exist, default it to false, and drop the helper.

# defaults block
options.notify_reload = false  # Type=notify-reload needs systemd >= 253

# rpm branch
if options.operating_system == :fedora
  options.notify_reload = true
  ...
elsif options.operating_system == :el || options.operating_system == :redhatfips
  if options.os_version == 8 || options.operating_system == :redhatfips
    ...                         # systemd 239 / FIPS: stays false
  elsif options.os_version >= 10
    options.notify_reload = true
    ...
  elsif options.os_version == 9
    ...                         # systemd 252: stays false
  end
# amazon 2023 (systemd 252) and sles 15 (systemd 249): leave false

# deb branch
case options.dist
when 'ubuntu20.04'                # systemd 245
  ...
when 'ubuntu22.04'                # systemd 249
  options.java = 'openjdk-25-jre-headless'
  options.java_bin = '/usr/lib/jvm/java-25-openjdk-amd64/bin/java'
when 'debian13', 'debian14', 'ubuntu24.04', 'ubuntu25.04', 'ubuntu25.10', 'ubuntu26.04', 'ubuntu26.10'
  options.notify_reload = true
  ...
end

# patch_files
if real_path.end_with?('.service') && options.notify_reload

patch_files runs after all of this, so the flag is set by the time it's consulted. This gives each platform exactly one description, fixes the Ubuntu 22.04 and 20.04 cases, and means an unlisted platform gets the conservative behaviour rather than the broken one.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a look at folding this in to the existing case structures where we set java_bin and java and it would require re-flowing and duplicating a few branches of those cases.

I think it is better to have this bit of logic as a stand-alone block as it is more compact and will eventually go away once a few OS releases have passed.

The concerns about Ubuntu 20.04 and older Debian are moot as we don't package Server and DB for those OSes. else true with no fail clause is a fine default as everything eventually runs a new enough SystemD.

Ubuntu 22.04 being missed because options.dist needs to be used instead of options.os_version was a good catch --- I've fixed that.

Comment thread resources/puppetlabs/lein-ezbake/template/global/ext/cli/reload.erb Outdated
Comment thread resources/puppetlabs/lein-ezbake/template/global/ext/cli/reload.erb Outdated
Comment thread resources/puppetlabs/lein-ezbake/template/global/ext/cli/reload.erb Outdated
@Sharpie
Sharpie force-pushed the old-systemd-compat branch 2 times, most recently from 0338b06 to 1937c1c Compare September 5, 2026 16:29
Comment thread resources/puppetlabs/lein-ezbake/template/global/ext/fpm.rb
Support for `Type=notify-reload` was implemented in SystemD v253.
Several still-supported operatings systems, Amazon 2023, EL 8 & 9,
SLES 15, Ubuntu 22.04, are using older SystemD versions.

This commit restores `cli/reload.erb` and sets it up as the default
`ExecReload` along with `Type=notify`. For operating systems that
have modern SystemD versions, `fpm.rb` deletes the `ExecReload` line
and swaps in `Type=notify-reload`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Charlie Sharpsteen <charlie@overlookinfratech.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants