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

cv_configlet : implement diff and set changed to true when appropriate in check mode #121

Conversation

b-abadie
Copy link
Contributor

Fixes #120

@titom73
Copy link
Contributor

titom73 commented Jan 21, 2020

Hi @b-abadie

Could you please comment with what this PR changes in inputs and outputs ?

Regards,

@b-abadie
Copy link
Contributor Author

Sure.
This PR does not change what input cv_configlet expects.

It does change what cv_configlet outputs : if one or more configlets have been modified, the modules outputs an additional key, diff, that will be interpreted and displayed by Ansible. This diff is of type prepared (see here)
This text outputs is actually a concatenation of all configlets' diffs, as we can update more than one in a single cv_configlet execution.

Note that my patch leverage the existing call to compare(), i.e. the diff was generated but not used verbatim.

Additionally, the PR changes the behavior of cv_configlet : when in check mode (dry run), the module now returns changed as true if any configlet would have been added, deleted or modified. If I recall correctly (this change was sitting in a branch of mine), the diff is not displayed by Ansible when the task has changed set to false (i.e. green in ansible-playbook).
But even without the diff, it seems to me that this behavior is preferred, as I stated in #120 .

@titom73 titom73 self-requested a review January 23, 2020 16:18
@titom73 titom73 added module: cv_configlet Issue related to cv_configlet module status: To be merged Issue solved in a volatile branch type: enhancement New feature or request labels Jan 23, 2020
@titom73 titom73 added this to the v1.1 milestone Jan 23, 2020
@titom73 titom73 closed this Jan 28, 2020
@titom73 titom73 added status: testing PR under testing and removed status: To be merged Issue solved in a volatile branch labels Mar 16, 2020
@titom73 titom73 mentioned this pull request Apr 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: cv_configlet Issue related to cv_configlet module status: testing PR under testing type: enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants