Skip to content

support gather_facts: false; support setup-snapshot.yml#79

Merged
richm merged 1 commit intolinux-system-roles:masterfrom
richm:no-gather-facts-setup-snapshot
Apr 14, 2022
Merged

support gather_facts: false; support setup-snapshot.yml#79
richm merged 1 commit intolinux-system-roles:masterfrom
richm:no-gather-facts-setup-snapshot

Conversation

@richm
Copy link
Copy Markdown
Collaborator

@richm richm commented Apr 13, 2022

Some users use gather_facts: false in their playbooks. This changes
the role to work in that case, by gathering only the facts it requires
to run.
CI testing can be sped up by creating a snapshot image pre-installed
with packages. tests/setup-snapshot.yml can be used by a CI system
to do this.

Some users use `gather_facts: false` in their playbooks.  This changes
the role to work in that case, by gathering only the facts it requires
to run.
CI testing can be sped up by creating a snapshot image pre-installed
with packages.  tests/setup-snapshot.yml can be used by a CI system
to do this.
@richm richm requested a review from nhosoi April 13, 2022 17:03
Comment thread tasks/set_vars.yml
- "{{ ansible_facts['os_family'] }}.yml"
- "default.yml"
paths:
- "{{ role_path }}/vars"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, why you don't use loop and __vars_file here as you usually do in the other set_vars.yml like this?
https://github.com/linux-system-roles/certificate/blob/master/tasks/set_vars.yml#L8-L21

Copy link
Copy Markdown
Collaborator Author

@richm richm Apr 13, 2022

Choose a reason for hiding this comment

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

With the certificate role, it was safe to convert the first_found method to the "is file" method - every parameter is correctly set or overridden. For other roles, it didn't seem safe, so I didn't convert. I probably should not have converted certificate like this - it is not related to the PR - I thought it would simplify porting the change to other roles - but other roles are not so simple.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

In the case of the kernel_settings role, it is safe to convert the first_found method to the "is file" method - but I think we should not make this change in order to support gather_facts: false and setup-snapshot.yml

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the details, @richm. I am currently working on timesync which shares the same issue. I'm not going to convert it to loop, then. Thanks!

@richm richm merged commit be405a7 into linux-system-roles:master Apr 14, 2022
@richm richm deleted the no-gather-facts-setup-snapshot branch April 14, 2022 15:24
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