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.

Sign up for GitHub

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

Jump to bottom

module/systemd: fix logic determining if a service needs to be enable… #46318

Merged
merged 2 commits into from
Oct 10, 2018
Merged

module/systemd: fix logic determining if a service needs to be enable… #46318

merged 2 commits into from
Oct 10, 2018

Conversation

vladimir-mencl-eresearch
Copy link
Contributor

@vladimir-mencl-eresearch vladimir-mencl-eresearch commented Sep 30, 2018 •
edited

SUMMARY

Backport fix in #46245 into stable-2.7
Fix logic determining whether a service with both systemd and initd files is enabled or disabled.

In situations where systemd thinks service is disabled, but rc.d symlinks mark it as enabled,
this module wrongly assumes the service is enabled.

Fix this logic: disabled means disabled

Only when the output from does NOT include disabled, consider the status of rc.d symlinks.
This essentially replicates the fixes done to the systemd handling in the "service" module in 3c89a21

Fixes #22303

Fixes #44409

Fixes #39116

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

systemd

ANSIBLE VERSION
ansible 2.6.4
  config file = None
  configured module search path = [u'/Users/vlad/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/Cellar/ansible/2.6.4/libexec/lib/python2.7/site-packages/ansible
  executable location = /usr/local/bin/ansible
  python version = 2.7.15 (default, Jun 17 2018, 12:46:58) [GCC 4.2.1 Compatible Apple LLVM 9.1.0 (clang-902.0.39.2)]
ADDITIONAL INFORMATION
Manual setup for the test-case:
  systemctl disable postgresql ; mv /etc/rc5.d/K02postgresql /etc/rc5.d/S02postgresql
(Not sure how exactly it happens naturally - but it does happen on our LXC/LXD containers)

Before this change:
ansible -m service  -a 'name=postgresql enabled=yes'
responds with changed: False
After this change: 
responds with changed: True

@vladimir-mencl-eresearch
module/systemd: fix logic determining if a service needs to be enabled (
9b877d9 
#46245)

* modules/systemd: fix logic: allow scope to default to 'system'

Fix logic introduced in 7ea9094: if 'scope' param is not specified,
it defaults to system, but the value of module.params['scope'] is None,
not 'system' - so allow for that.

* modules/systemd: fix logic: disabled means disabled

Fix logic determining whether a service with both systemd and initd files is enabled or disabled.

In situations where systemd thinks service is disabled, but rc.d symlinks mark it as enabled,
this module wrongly assumes the service is enabled.

Fix this logic: disabled means disabled

Only when the output from does NOT include disabled, consider the status of rc.d symlinks.

This essentially replicates the fixes done to the systemd handling in the "service" module in 3c89a21

Fixes #22303

Fixes #44409

(cherry picked from commit ef131c7)
@ansibot ansibot added affects_2.7 This issue/PR affects Ansible v2.7 backport This PR does not target the devel branch. 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. small_patch support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Sep 30, 2018
@bcoca bcoca removed the needs_triage Needs a first human triage before being processed. label Oct 1, 2018
@vladimir-mencl-eresearch vladimir-mencl-eresearch mentioned this pull request Oct 2, 2018
Merged
@abadger
Copy link
Contributor

abadger commented Oct 9, 2018

@vladimir-mencl-eresearch Could you add a changelog fragment so that this can be merged? You can look at other changelog fragments to know what they should look like: https://github.com/ansible/ansible/tree/stable-2.7/changelogs/fragments

@webknjaz webknjaz requested review from bcoca and removed request for bcoca October 9, 2018 18:17
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Oct 9, 2018
@vladimir-mencl-eresearch
Copy link
Contributor Author

Thanks @abadger , just added that (and also did the same for the 2.6 backport PR #46317)

Please let me know if there's anything else that needs to be done to get this merged.

Cheers,
Vlad

@ansibot ansibot added support:community This issue/PR relates to code supported by the Ansible community. and removed small_patch 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 Oct 10, 2018
@abadger abadger merged commit 83194a0 into ansible:stable-2.7 Oct 10, 2018
@abadger
Copy link
Contributor

abadger commented Oct 10, 2018

Merged for the 2.7.1 release

@vladimir-mencl-eresearch vladimir-mencl-eresearch deleted the backport/2.7/46245 branch October 28, 2018 23:06
@ansible ansible locked and limited conversation to collaborators Jul 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
affects_2.7 This issue/PR affects Ansible v2.7 backport This PR does not target the devel branch. 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. support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@vladimir-mencl-eresearch @abadger @bcoca @ansibot