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

Fix 2 issues in sysvinit module #42786

Merged
merged 4 commits into from
Jul 17, 2018
Merged

Fix 2 issues in sysvinit module #42786

merged 4 commits into from
Jul 17, 2018

Conversation

Lambdara
Copy link
Contributor

@Lambdara Lambdara commented Jul 14, 2018 •
edited

SUMMARY

The sysvinit module had 2 problems that I was running into. Firstly, even without setting any desired state for enabled it would still always report changed because it was comparing the actual state to this None variable (having not been set).

Secondly the command that was executed to bring a service to a started state was just wrong, and I rewrote it to make it work for me. I hope I don't break it for others as it is quite different, but at the same time the current result made absolutely no sense to me.

Fixes #42620

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

sysvinit

ANSIBLE VERSION
ansible 2.7.0.dev0 (devel 5864871fc1) last updated 2018/07/14 17:59:02 (GMT +000)
  config file = None
  configured module search path = [u'/root/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /ansible/lib/ansible
  executable location = /usr/local/bin/ansible
  python version = 2.7.15rc1 (default, Apr 15 2018, 21:51:34) [GCC 7.3.0]
ADDITIONAL INFORMATION

I think everything is explained by me in issue #42620

@ansibot ansibot added affects_2.7 This issue/PR affects Ansible v2.7 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 Jul 14, 2018
@Akasurde Akasurde mentioned this pull request Jul 16, 2018
Closed
@Akasurde Akasurde removed the needs_triage Needs a first human triage before being processed. label Jul 16, 2018
@bcoca bcoca merged commit f26272a into ansible:devel Jul 17, 2018
@Lambdara Lambdara mentioned this pull request Jul 18, 2018
Closed
@pilou- pilou- mentioned this pull request Aug 4, 2018
Merged
pilou- added a commit to pilou-/ansible that referenced this pull request Aug 5, 2018
@pilou-
service tests: add test for ansible#42786
26fda3c
gundalow pushed a commit that referenced this pull request Aug 8, 2018
@pilou- @gundalow
improve 'service' integration tests (#43655)
c5f37f6 
* service tests: check that daemon is really running

(spoiler: it isn't)

* service tests: add PIDFile directive in systemd unit

* service tests: check 'changed' too

* service tests: fix indentation & use changed test

* service tests: #16694 has been fixed a long time ago

* service tests: refactor

- always execute cleaning tasks
- move tests tasks in a dedicated file

* service tests: add test for #42786

* service tests: display value of ansible_service_mgr

* service tests: allow to run tests twice in a row

stop & disable ansible test service

* service tests: 'pattern' value must be a substring

'pattern' parameter is poorly named

* service tests: check ansible_test service status

* service tests: test daemon must handle SIGHUP

because 'initctl reload' sends SIGHUP, otherwise test daemon stops when
receiving the signal

* service tests: remove upstart override file too

and check that files were removed using raw module and stat command
pilou- pushed a commit to pilou-/ansible that referenced this pull request Aug 8, 2018
@Lambdara @pilou-
Fix 2 issues in sysvinit module (ansible#42786)
ca50d04 
* Do not compare result to unset parameter in sysvinit module

* Fix misformed command in sysvinit module

* Small None-comparison style fix in sysvinit module

(cherry picked from commit f26272a)
@pilou- pilou- mentioned this pull request Aug 8, 2018
Merged
@pilou- pilou- mentioned this pull request Aug 8, 2018
Merged
mattclay pushed a commit that referenced this pull request Aug 8, 2018
@pilou- @mattclay
[stable-2.6] Fix 2 issues in sysvinit module (backport/2.6/42786) (#4…
516ed4d 
…3831)

* Fix 2 issues in sysvinit module (#42786)

* Do not compare result to unset parameter in sysvinit module

* Fix misformed command in sysvinit module

* Small None-comparison style fix in sysvinit module

(cherry picked from commit f26272a)

* Update changelog
pilou- added a commit to pilou-/ansible that referenced this pull request Aug 8, 2018
@pilou-
improve 'service' integration tests (ansible#43655)
7d5cb41 
* service tests: check that daemon is really running

(spoiler: it isn't)

* service tests: add PIDFile directive in systemd unit

* service tests: check 'changed' too

* service tests: fix indentation & use changed test

* service tests: ansible#16694 has been fixed a long time ago

* service tests: refactor

- always execute cleaning tasks
- move tests tasks in a dedicated file

* service tests: add test for ansible#42786

* service tests: display value of ansible_service_mgr

* service tests: allow to run tests twice in a row

stop & disable ansible test service

* service tests: 'pattern' value must be a substring

'pattern' parameter is poorly named

* service tests: check ansible_test service status

* service tests: test daemon must handle SIGHUP

because 'initctl reload' sends SIGHUP, otherwise test daemon stops when
receiving the signal

* service tests: remove upstart override file too

and check that files were removed using raw module and stat command

(cherry picked from commit c5f37f6)
mattclay pushed a commit that referenced this pull request Aug 9, 2018
@pilou- @mattclay
improve 'service' integration tests (#43655)
3918255 
* service tests: check that daemon is really running

(spoiler: it isn't)

* service tests: add PIDFile directive in systemd unit

* service tests: check 'changed' too

* service tests: fix indentation & use changed test

* service tests: #16694 has been fixed a long time ago

* service tests: refactor

- always execute cleaning tasks
- move tests tasks in a dedicated file

* service tests: add test for #42786

* service tests: display value of ansible_service_mgr

* service tests: allow to run tests twice in a row

stop & disable ansible test service

* service tests: 'pattern' value must be a substring

'pattern' parameter is poorly named

* service tests: check ansible_test service status

* service tests: test daemon must handle SIGHUP

because 'initctl reload' sends SIGHUP, otherwise test daemon stops when
receiving the signal

* service tests: remove upstart override file too

and check that files were removed using raw module and stat command

(cherry picked from commit c5f37f6)
@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

@achinthagunasekara achinthagunasekara achinthagunasekara approved these changes

Assignees
No one assigned
Labels
affects_2.7 This issue/PR affects Ansible v2.7 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. 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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Service module doesn't start service, always reports changed, using service program
5 participants
@Lambdara @achinthagunasekara @Akasurde @bcoca @ansibot