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

Handle situation where ansible_architecure may not be defined when gathering facts #55466

Open
wants to merge 2 commits into
base: devel
from

Conversation

Projects
None yet
3 participants
@samdoran
Copy link
Member

commented Apr 17, 2019

SUMMARY

Fixes #55400

There is no guarantee that ansible_architecture is defined in the facts passed to get_cpu_facts(). If the key doesn't exist, return an empty string instead of None to avoid AttributeError.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

lib/ansible/module_utils/facts/hardware/linux.py

ADDITIONAL INFORMATION

This is not a problem during normal playbook execution. It only occurs if get_cpu_facts() is called directly or indirectly by some means other than running setup, e.g., gathering facts directly from within a module by calling internal fact gathering APIs.

@bcoca
Copy link
Member

left a comment

you would give incorrect result in this case, you should really complain you didn't get the required data

@@ -240,7 +240,7 @@ def get_cpu_facts(self, collected_facts=None):
# The fields for ARM CPUs do not always include 'vendor_id' or 'model name',
# and sometimes includes both 'processor' and 'Processor'.
# Always use 'processor' count for ARM systems
if collected_facts.get('ansible_architecture').startswith(('armv', 'aarch')):
if collected_facts.get('ansible_architecture', '').startswith(('armv', 'aarch')):

This comment has been minimized.

Copy link
@bcoca

bcoca Apr 17, 2019

Member

you would still need to change 2 lines down

This comment has been minimized.

Copy link
@bcoca

bcoca Apr 17, 2019

Member

also the other files in facts that make the same assumptions

This comment has been minimized.

Copy link
@samdoran

samdoran May 14, 2019

Author Member

The comparison two lines down is safe because it will default to None.

if collected_facts.get('ansible_architecture') != 's390x':

I looked through the facts code and most instances fail safe, meaning they don't do things that would result in an exception like I did on line 243 (None has not startswith() method. Doh!).

@ansibot ansibot added needs_revision and removed core_review labels Apr 17, 2019

@ansibot ansibot added the stale_ci label Apr 26, 2019

@samdoran samdoran force-pushed the samdoran:issue/55400-facts-linux-cpu-architecture branch from d85757c to f95918c May 14, 2019

@ansibot ansibot removed the stale_ci label May 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.