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

Resolve an issue with the systemd/sysvinit ansible conflict #65

Merged

Conversation

andrewhowdencom
Copy link
Contributor

Currently, there appears to be an issue with Ansible in which, if a
service is already enabled with sysvinit, the service will not be
enabled with Ansible. This appears to be the condition with this
repository; likely, the upstream package is bringing in the sysvinit
script (as it's not being deployed by ansible) and enabling it.

This commit implements a workaround as documented
ansible/ansible-modules-core#3764 (comment).
While, confusingly, the package is "using" the sysvinit style service
tool for enabling the service, systemd implements a compatibility layer
that allows enabling it via this shim. This correctly enables the
service.

See ansible/ansible#22303
See geerlingguy/ansible-role-php@84ab337

Currently, there appears to be an issue with Ansible in which, if a
service is already enabled with sysvinit, the service will not be
enabled with Ansible. This appears to be the condition with this
repository; likely, the upstream package is bringing in the sysvinit
script (as it's not being deployed by ansible) and enabling it.

This commit implements a workaround as documented
ansible/ansible-modules-core#3764 (comment).
While, confusingly, the package is "using" the sysvinit style service
tool for enabling the service, systemd implements a compatibility layer
that allows enabling it via this shim. This correctly enables the
service.

See ansible/ansible#22303
See geerlingguy/ansible-role-php@84ab337
tobiasdam added a commit to usableprivacy/upribox that referenced this pull request Jul 11, 2017
tobiasdam added a commit to usableprivacy/upribox that referenced this pull request Jul 14, 2017
 - systemd module (and service module, which uses systemd module per deault) has a problem with sysvinit services
 - see: geerlingguy/ansible-role-varnish#65
tasks/main.yml Outdated
with_items: "{{ varnish_enabled_services | default([]) }}"
when: >
varnish_enabled_services and
(ansible_os_family == 'Debian' and ansible_distribution_release == "xenial")
Copy link
Owner

Choose a reason for hiding this comment

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

This line needs a newline.

tasks/main.yml Outdated
use: "service"
with_items: "{{ varnish_enabled_services | default([]) }}"
when: >
varnish_enabled_services and
Copy link
Owner

Choose a reason for hiding this comment

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

For compound when conditions, you can use the style:

  when:
    - varnish_enabled_services
    - ansible_os_family == 'Debian' and ansible_distribution_release == "xenial"

I just discovered this recently, and am trying to get all my roles to use that style for complex conditionals; can you update these to use that style too?

Recently, Ansible introduced the capability to sum "when" conditions as
a yaml list (something like a json array). When formatted thusly:

  when:
    - foo == "bar"
    - baz == "herp"

All conditionals are logically "and"ed together.

See http://docs.ansible.com/ansible/playbooks_conditionals.html#the-when-statement

Further, a line break was added to the bottom of this file.
name: "{{ item }}"
state: started
enabled: yes
use: "service"
Copy link
Owner

Choose a reason for hiding this comment

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

Another question here—isn't this task the exact same as the task above? I don't see why we'd need to have two tasks that do the exact same thing, but one for Xenial, and another for all other versions...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ehhrm good catch. The initial one shouldn't use service -- instead, it should just use the Ansible default init system, in case the shim for sysvinit disappears at some point. I'll fix that up.

Previous work implemented a workaround for an ansible issue in which, in
some cases, Ansible would not detect correctly whether a service is
enabled. However, the author inadvertently committed a change to both
tasks.

This commit removes the change to the original task such that it will
use the ansible service auto-dectection, and not sysvinit
@andrewhowdencom
Copy link
Contributor Author

fixed. Not tested -- I have no way of testing taht change anyways; it's non-xenial specific. I gather CI should pick it up?

@geerlingguy geerlingguy merged commit e3d772e into geerlingguy:master Aug 1, 2017
@geerlingguy
Copy link
Owner

Yep, CI tests look good. Thanks!

stefanodallapalma added a commit to stefanodallapalma/radon-repository-miner-testing that referenced this pull request Jun 1, 2021
stefanodallapalma added a commit to stefanodallapalma/radon-repository-miner-testing that referenced this pull request Jun 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants