debian: remove invoke-rc.d calls from postrm scripts#66352
debian: remove invoke-rc.d calls from postrm scripts#66352SrinivasaBharath merged 1 commit intoceph:mainfrom
Conversation
|
If one would be really "nice" one could check for the system running systemd or something else, but IMO it does not really make sense to run ceph on Debian without systemd. But all files that basically just have the [0]: i.e. look like: #!/bin/sh
set -e
#DEBHELPER# |
|
hey can i help u out mate |
Previously, we called "invoke-rc.d ceph stop" in postrm scripts to support sysvinit-based installations. However, this fails on systemd- based systems, which are now the default on modern Debian distributions. When invoke-rc.d detects systemd as the init system, it delegates to systemctl, converting "invoke-rc.d ceph stop" to "systemctl stop ceph.service". Since Ceph provides ceph.target and template units (ceph-osd@.service, ceph-mon@.service, etc.) rather than a monolithic ceph.service, this command always fails with exit code 5 (LSB EXIT_NOTINSTALLED). This failure prevents the auto-generated cleanup sections added by debhelper from executing properly, which can leave the system in an inconsistent state during package removal. Changes: - Remove the invoke-rc.d call entirely. Systemd will handle service cleanup through its own mechanisms when the package is removed. - Remove redundant "exit 0" statement (the script exits successfully by default, and "set -e" is no longer needed without commands that might fail). - Remove vim modeline comment, as it's unnecessary for a 3-line script. - Eventually remove this 3-line script stanza, as this is exactly what debhelper provides us. Signed-off-by: Kefu Chai <k.chai@proxmox.com>
0a75489 to
4f48af0
Compare
|
changelog:
|
|
@ThomasLamprecht hi Thomas, thanks for the insights and suggestion. incorporated your suggestion. tested at https://shaman.ceph.com/builds/ceph/wip-pr-66352-kefu/0b044bfb0a691f7cd19f34ee1ccf85daf6c2fc01/ i checked by uncompressing the .deb package, and verified that, after removing the |
|
@alimaredia could you help review this change? |
ThomasLamprecht
left a comment
There was a problem hiding this comment.
FWIW, this now looks OK to me as is.
|
@tchaikov I removed my review on this PR. I don't feel I'm able to help out with a review here. |
|
thanks @alimaredia ! @cbodley do you happen to know who can help review debian packaging changes in our project? |
cbodley
left a comment
There was a problem hiding this comment.
given the ack from Thomas, i'm happy to approve as long as this goes through qa. is there any additional testing we should do to validate the behavior against systemd?
i installed the packages built with this change in a container, and uninstalled them without errors. also, there are a lot of proxmox users using the postrm script equivalent to the one generated with this change, so they are practically testing the change. i think it should suffice as a proof that this change is correct. |
|
RADOS Approved: https://tracker.ceph.com/issues/74325#note-6 |
Previously, we called
invoke-rc.d ceph stopin postrm scripts to support sysvinit-based installations. However, this fails on systemd- based systems, which are now the default on modern Debian distributions.When invoke-rc.d detects systemd as the init system, it delegates to systemctl, converting
invoke-rc.d ceph stopto "systemctl stop ceph.service". Since Ceph provides ceph.target and template units (ceph-osd@.service,ceph-mon@.service, etc.) rather than a monolithic ceph.service, this command always fails with exit code 5 (LSBEXIT_NOTINSTALLED).This failure prevents the auto-generated cleanup sections added by debhelper from executing properly, which can leave the system in an inconsistent state during package removal.
Changes:
exit 0statement (the script exits successfully by default, andset -eis no longer needed without commands that might fail).Contribution Guidelines
To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an
xbetween the brackets:[x]. Spaces and capitalization matter when checking off items this way.Checklist
Show available Jenkins commands
jenkins test classic perfJenkins Job | Jenkins Job Definitionjenkins test crimson perfJenkins Job | Jenkins Job Definitionjenkins test signedJenkins Job | Jenkins Job Definitionjenkins test make checkJenkins Job | Jenkins Job Definitionjenkins test make check arm64Jenkins Job | Jenkins Job Definitionjenkins test submodulesJenkins Job | Jenkins Job Definitionjenkins test dashboardJenkins Job | Jenkins Job Definitionjenkins test dashboard cephadmJenkins Job | Jenkins Job Definitionjenkins test apiJenkins Job | Jenkins Job Definitionjenkins test docsReadTheDocs | Github Workflow Definitionjenkins test ceph-volume allJenkins Jobs | Jenkins Jobs Definitionjenkins test windowsJenkins Job | Jenkins Job Definitionjenkins test rook e2eJenkins Job | Jenkins Job DefinitionYou must only issue one Jenkins command per-comment. Jenkins does not understand
comments with more than one command.