Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend systemctl is-enabled check to handle "enabled-runtime" too. #77754

Merged
merged 2 commits into from Apr 10, 2024

Conversation

lbt
Copy link
Contributor

@lbt lbt commented May 7, 2022

SUMMARY

In general the existence of a runtime-enabled unit should not prevent a persistent enable being set.

Specifically this handles the case where there is an entry in fstab
for a mountpoint (which is retained to allow manual mount/umount to
take place) and yet a systemd mount unit needs to be deployed to
handle other unit options. There will be a generator-created unit file
which shows the unit as enabled-runtime and the persistent enable of
the mount unit will fail.

Additionally improve the comments and modify the code to use rsplit()
and the "in" notation since "systemctl is-enabled" is documented to
return specific values in the cases of interest.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

ansible.builtin.systemd

ADDITIONAL INFORMATION

@ansibot ansibot added affects_2.14 bug This issue/PR relates to a bug. core_review In order to be merged, this PR must follow the core review workflow. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. new_contributor This PR is the first contribution by a new community member. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels May 7, 2022
Copy link
Member

@bcoca bcoca left a comment

Choose a reason for hiding this comment

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

code lgtm, just need changelog and tests

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels May 9, 2022
@s-hertel s-hertel removed the needs_triage Needs a first human triage before being processed. label May 10, 2022
@ansibot ansibot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels May 18, 2022
@ansibot ansibot removed the new_contributor This PR is the first contribution by a new community member. label Jun 19, 2022
@s-hertel s-hertel added the P3 Priority 3 - Approved, No Time Limitation label Oct 5, 2022
lbt and others added 2 commits April 9, 2024 23:03
In general the existence of a runtime-enabled unit should not prevent
a persistent enable being set.

Specifically this handles the case where there is an entry in fstab
for a mountpoint (which is retained to allow manual mount/umount to
take place) and yet a systemd mount unit needs to be deployed to
handle other unit options. There will be a generator-created unit file
which shows the unit as enabled-runtime and the persistent enable of
the mount unit will fail.

Additionally improve the comments and modify the code to use rsplit()
and the "in" notation since "systemctl is-enabled" is documented to
return specific values in the cases of interest.

Signed-off-by: David Greaves <david@dgreaves.com>
* added changelog
@Akasurde Akasurde force-pushed the extend-systemd-enabled-checking branch from 088bb46 to 10fe45c Compare April 10, 2024 06:10
@ansibot ansibot added stale_review Updates were made after the last review and the last review is more than 7 days old. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Apr 10, 2024
@Akasurde Akasurde requested a review from bcoca April 10, 2024 13:59
@bcoca bcoca merged commit 3076478 into ansible:devel Apr 10, 2024
66 checks passed
@lbt lbt deleted the extend-systemd-enabled-checking branch April 10, 2024 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects_2.14 bug This issue/PR relates to a bug. module This issue/PR relates to a module. P3 Priority 3 - Approved, No Time Limitation stale_review Updates were made after the last review and the last review is more than 7 days old. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants