search

redhat-cop / automation-good-practices

Recommended practices for all elements of automation using Ansible, starting with collections and roles, continuing with playbooks, inventories and plug-ins... These good practices are planned to be used by all Red Hat teams interested but can of course be used by others.
268 stars 66 forks source link

Use set_vars.yml for OS/platform specific vars #62

Closed richm closed 2 years ago

richm commented 2 years ago

Use tasks/set_vars.yml to set OS/platform specific vars. Using a separate file allows role integration tests to access the OS/platform specific vars. Handle the case where a user specifies gather_facts: false in the playbook - have the role gather only the facts it requires.

richm commented 2 years ago

See https://github.com/linux-system-roles/storage/pull/262 and https://github.com/linux-system-roles/storage/pull/260 for examples.

bontreger commented 2 years ago

Are these three use cases independent? (if any one of these cases is true, then this is a recommendation) or together (if all 3 are true).

I feel like the note, roles that require facts to be gathered should state so explicitly, or fail if undefined is the real good practice here.

richm commented 2 years ago

Are these three use cases independent? (if any one of these cases is true, then this is a recommendation) or together (if all 3 are true).

The 3 cases are independent.

Yeah - not sure how to do this - for example, if all you want to do is make the role work when gather_facts: false, this may or may not have anything to do with setting os/platform version specific variables. However, the primary way that roles use facts is to handle os/platform version specific conditionals (such as setting variables) (at least in the case of linux-system-roles). Any suggestions?

I feel like the note, roles that require facts to be gathered should state so explicitly, or fail if undefined is the real good practice here.

Brian C. and I discussed this at length - his point was that roles should just fail: msg: "You must use gather_facts: or setup: in order to use this role" when: distribution is not defined - IMO that UX is not as good as the role gathering the facts it requires - and using cache plugins to cache facts to avoid gathering facts as much as possible will help in either case.

richm commented 2 years ago

Can this be merged now? Any other issues?

mophahr commented 2 years ago

Can this be merged now? Any other issues?

I'd suggest we do that in the next CoP meeting @richm EDIT: I don't see any further issues.

mophahr commented 2 years ago

Approved to be merged in the ACoP meeting