ansible_service_mgr fact for sysvinit incorrect in ansible 2.4.0 #30753

toke · 2017-09-22T13:14:36Z

ISSUE TYPE

Bug Report

COMPONENT NAME

Ansible fact / variable ansible_service_mgr

ANSIBLE VERSION

ansible 2.4.0.0

CONFIGURATION

does not matter

OS / ENVIRONMENT

Target Debian Linux 7.11 (possible other Systems with sysvinit)

SUMMARY

Prior to ansible 2.4.0.0 ansible_service_mgr contained the value sysvinit, after the update to 2.4.0.0 ansible falls back to service. I would call it "alternative fact" right now, at least some roles fail due to this behaviour.

STEPS TO REPRODUCE

Outputting the fact ansible_service_mgr for a host running sysvinit (for example Debian 7.11 with sysvinit)

ansible -i host.example.com, -m setup  host.example.com | grep ansible_service_mgr

EXPECTED RESULTS

"ansible_service_mgr": "sysvinit",

ACTUAL RESULTS

"ansible_service_mgr": "service",

The text was updated successfully, but these errors were encountered:

toke · 2017-09-22T14:35:42Z

Looking into https://github.com/ansible/ansible/blob/stable-2.4/lib/ansible/module_utils/facts/system/service_mgr.py I see several checks for comparison I'll did these by hand:

toke@desktop~$ ansible -i machine.example.com, -m setup machine.example.com |grep ansible_system
        "ansible_system": "Linux", 
        "ansible_system_vendor": "oVirt",
ssh toke@machine
toke@machine~$ ps -p 1 -o comm|tail -n 1
init
toke@machine:~$ cat /proc/1/comm
init
toke@machine:~$ [ -d /etc/init.d/ ] && echo "yes" 
yes

this allows for proper dependencies on facts already collected fixes ansible#30753 fixes ansible#30623

alikins · 2017-09-25T21:18:48Z

hmm, I was pretty sure this was related to ordering of collected facts (what #30777 addresses)
but now I wonder if service_mgr is just using the wrong fact names (without the 'ansible_' prefix)
(See devel...alikins:test_svc_mgr for a potential change
to fix)

* Fix fact failures cause by ordering of collectors Some fact collectors need info collected by other facts. (for ex, service_mgr needs to know 'ansible_system'). This info is passed to the Collector.collect method via the 'collected_facts' info. But, the order the fact collectors were running in is not a set order, so collectors like service_mgr could run before the PlatformFactCollect ('ansible_system', etc), so the 'ansible_system' fact would not exist yet. Depending on the collector and the deps, this can result in incorrect behavior and wrong or missing facts. To make the ordering of the collectors more consistent and predictable, the code that builds that list is now driven by the order of collectors in default_collectors.py, and the rest of the code tries to preserve it. * Flip the loops when building collector names iterate over the ordered default_collectors list selecting them for the final list in order instead of driving it from the unordered collector_names set. This lets the list returned by select_collector_classes to stay in the same order as default_collectors.collectors For collectors that have implicit deps on other fact collectors, the default collectors can be ordered to include those early. * default_collectors.py now uses a handful of sub lists of collectors that can be ordered in default_collectors.collectors. fixes #30753 fixes #30623 (cherry picked from commit 95abc1d)

alikins · 2017-09-28T14:59:52Z

fix was also cherry-picked to stable-2.4 in 4025b47
(from 95abc1d from #30777)

* Fix fact failures cause by ordering of collectors Some fact collectors need info collected by other facts. (for ex, service_mgr needs to know 'ansible_system'). This info is passed to the Collector.collect method via the 'collected_facts' info. But, the order the fact collectors were running in is not a set order, so collectors like service_mgr could run before the PlatformFactCollect ('ansible_system', etc), so the 'ansible_system' fact would not exist yet. Depending on the collector and the deps, this can result in incorrect behavior and wrong or missing facts. To make the ordering of the collectors more consistent and predictable, the code that builds that list is now driven by the order of collectors in default_collectors.py, and the rest of the code tries to preserve it. * Flip the loops when building collector names iterate over the ordered default_collectors list selecting them for the final list in order instead of driving it from the unordered collector_names set. This lets the list returned by select_collector_classes to stay in the same order as default_collectors.collectors For collectors that have implicit deps on other fact collectors, the default collectors can be ordered to include those early. * default_collectors.py now uses a handful of sub lists of collectors that can be ordered in default_collectors.collectors. fixes ansible#30753 fixes ansible#30623

…rkaround for ansible/ansible#30753

toke · 2017-10-17T12:20:49Z

Not sure if the fixes should work by now but testing against devel or v2.4.1.0-0.3.rc1 I still get the value "service" for the fact "ansible_service_mgr".

@alikins Should this issue reopened?

toke · 2017-10-17T13:14:47Z

I've put together a small vagrant test: https://gist.github.com/toke/d5de03c70c9c7f1a2ec6acd7a9e3753a

toke · 2017-10-24T16:36:23Z

@alikins Thanks for your effort.
This fixes the issue. Let's hope it will make it in 2.4.1.

The platform/distro/etc facts were being passed in correctly, but service_mgr.py was looking up the wrong names ('system' vs 'ansible_system') resulting in service_mgr falling back to default 'service' result. Fixes #30753, #31095

The platform/distro/etc facts were being passed in correctly, but service_mgr.py was looking up the wrong names ('system' vs 'ansible_system') resulting in service_mgr falling back to default 'service' result. Fixes #30753, #31095 (cherry picked from commit 6203e89)

aabdnn · 2017-10-27T08:29:57Z

I've just tested 2.4.1, and it's still reporting "service" instead of "upstart" for CentOS 6

abadger · 2017-10-27T08:43:38Z

From #32086:

Cherry-picked for the 2.4.2beta1 release. I'll cut that tarball later

this week

Add deps/requires for fact collectors Fact collectors can now set a required_facts class attribute that will be a set of the names of fact collectors they require to be run first. ie, if a collector needs to know the ansible_distribution, it should set it's required_facts to include 'distribution' required_facts = set(['distribution']) If a collector requires another collector, it gets added to the selected collector names. We then topological sort the ordering of the collectors so that deps work out (ie, 'distribution' will run before 'service_mgr') required_facts were added to the collectors for: - network (requires 'distribution', 'platform') - hardware (requires 'platform') - service_mgr (requires 'distribution', 'platform') Fix name references for facts (need 'ansible_' prefix) is service_mgr Fixes #30753

…ed as systemd based system that is fail, caused by: ansible/ansible#30753

Add deps/requires for fact collectors Fact collectors can now set a required_facts class attribute that will be a set of the names of fact collectors they require to be run first. ie, if a collector needs to know the ansible_distribution, it should set it's required_facts to include 'distribution' required_facts = set(['distribution']) If a collector requires another collector, it gets added to the selected collector names. We then topological sort the ordering of the collectors so that deps work out (ie, 'distribution' will run before 'service_mgr') required_facts were added to the collectors for: - network (requires 'distribution', 'platform') - hardware (requires 'platform') - service_mgr (requires 'distribution', 'platform') Fix name references for facts (need 'ansible_' prefix) is service_mgr Fixes ansible#30753

ansibot added affects_2.4 This issue/PR affects Ansible v2.4 bug_report needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Sep 22, 2017

toke changed the title ~~Ansible 2.4.0 breaks ansible_service_mgr fact~~ Ansible 2.4.0 breaks ansible_service_mgr fact (sysvinit) Sep 22, 2017

bcoca added a commit to bcoca/ansible that referenced this issue Sep 22, 2017

order collecters by priority

87ede41

this allows for proper dependencies on facts already collected fixes ansible#30753 fixes ansible#30623

bcoca mentioned this issue Sep 22, 2017

order collecters by priority #30758

Closed

bcoca removed the needs_triage Needs a first human triage before being processed. label Sep 22, 2017

alikins mentioned this issue Sep 22, 2017

Fact collector ordering #30777

Merged

alikins closed this as completed in 95abc1d Sep 28, 2017

alikins changed the title ~~Ansible 2.4.0 breaks ansible_service_mgr fact (sysvinit)~~ ansible_service_mgr fact for sysvinit incorrect in ansible 2.4.0 Sep 29, 2017

alikins mentioned this issue Oct 5, 2017

Fact collector ordering deps #31362

Merged

interatom added a commit to wcm-io-devops/ansible-aem-cms that referenced this issue Oct 9, 2017

Also setup init script when ansible_service_mgr reports 'service'. Wo…

c3c9391

…rkaround for ansible/ansible#30753

toke mentioned this issue Oct 24, 2017

ansible_service_mgr fact for sysvinit incorrect in ansible 2.4.0+ and devel #32068

Closed

alikins reopened this Oct 24, 2017

alikins mentioned this issue Oct 24, 2017

Fix service_mgr fact collection #32086

Merged

alikins closed this as completed in #32086 Oct 24, 2017

toke mentioned this issue Oct 28, 2017

When using filter in setup module returned variables may be wrong #32286

Closed

pyup-bot mentioned this issue Jan 1, 2018

Scheduled monthly dependency update for January napalm255/ansible-vmware_inventory#11

Closed

KAMI911 added a commit to KAMI911/ansible-role-wildfly that referenced this issue Jan 29, 2018

Bump Ansible version from 2.4.0.0 to 2.4.2.0\n\n* Centos 6 got detect…

add70f8

…ed as systemd based system that is fail, caused by: ansible/ansible#30753

KAMI911 added a commit to KAMI911/ansible-role-wildfly that referenced this issue Jan 30, 2018

Try to workaround systemd problem, caused by: ansible/ansible#30753

4a9ce69

KAMI911 added a commit to KAMI911/ansible-role-wildfly that referenced this issue Jan 30, 2018

Try to workaround systemd problem, caused by: ansible/ansible#30753

523ae35

ansibot added bug This issue/PR relates to a bug. and removed bug_report labels Mar 7, 2018

raviops100 mentioned this issue Nov 26, 2018

i unable to execute ansible roles , i already inststalled role and when try to run playbook i am facing the following error QT-DevOps/DevOpsIssues#276

Closed

ansible locked and limited conversation to collaborators Apr 26, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ansible_service_mgr fact for sysvinit incorrect in ansible 2.4.0 #30753

ansible_service_mgr fact for sysvinit incorrect in ansible 2.4.0 #30753

toke commented Sep 22, 2017

toke commented Sep 22, 2017 •

edited

Loading

alikins commented Sep 25, 2017

alikins commented Sep 28, 2017

toke commented Oct 17, 2017 •

edited

Loading

toke commented Oct 17, 2017

toke commented Oct 24, 2017

aabdnn commented Oct 27, 2017

abadger commented Oct 27, 2017 via email

ansible_service_mgr fact for sysvinit incorrect in ansible 2.4.0 #30753

ansible_service_mgr fact for sysvinit incorrect in ansible 2.4.0 #30753

Comments

toke commented Sep 22, 2017

ISSUE TYPE

COMPONENT NAME

ANSIBLE VERSION

CONFIGURATION

OS / ENVIRONMENT

SUMMARY

STEPS TO REPRODUCE

EXPECTED RESULTS

ACTUAL RESULTS

toke commented Sep 22, 2017 • edited Loading

alikins commented Sep 25, 2017

alikins commented Sep 28, 2017

toke commented Oct 17, 2017 • edited Loading

toke commented Oct 17, 2017

toke commented Oct 24, 2017

aabdnn commented Oct 27, 2017

abadger commented Oct 27, 2017 via email

toke commented Sep 22, 2017 •

edited

Loading

toke commented Oct 17, 2017 •

edited

Loading