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

Gather required facts if relevant keys in ansible_facts are missing #1102

Closed
wants to merge 1 commit into from

Conversation

spike77453
Copy link

This adds an additional tasks to gather relevant facts if the corresponding keys (i.e. distribution, distribution_version, distribution_major_version, os_family) are not already present in the ansible_facts dict.

Since the documentation does not mention that fact gathering is required before using certain roles, this prevents errors such as #962

One could also add when: ansible_facts_fqdn is not defined ....
However, this is not always required (e.g. when using the role ipaclient only tasks/install.yml uses ansible_facts.fqdn) and ansible_facts.fqdn will always be preset after using ansible.builtin.setup with gather_subset: min.

Copy link
Member

@rjeffman rjeffman left a comment

Choose a reason for hiding this comment

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

Currently, all ansible-freeipa roles require both become: true and gather_facts: true. I like the idea of not requiring gather_facts: true.

Can you improve the commit message to show what problem it fixes, and how? An adaption of your PR text would be good.

And, if fqdn is also needed by the role, it should be added to the when clause of the role, as the idea here is to allow any role to work with gather_facts: false.

@rjeffman
Copy link
Member

Closing this in favor of #1205 with a more strict data retrieval.

Thank you for raising the issue.

@rjeffman rjeffman closed this Feb 12, 2024
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.

2 participants