template lookup: fix regression when templating hostvars #64070
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This fixes a regression that was caused by switching from copy() to deepcopy() when 'saving' variables before templating. Since HostVars did not implement the __deepcopy__() method, deepcopy returned incorrect results when host vars were present in the variables. Fixes ansible#63940
ansibot
added
affects_2.10
bcoca
reviewed
Oct 29, 2019
| # however we have to implement the method so we can deepcopy | ||
| # variables' dicts that contain HostVars. | ||
| return self | ||
|
|
There was a problem hiding this comment.
probably need same in hostvarsvars
There was a problem hiding this comment.
I don't think it is needed because deepcopy() on HostVars returns self and so deepcopy() does not continue further down the structure to be applied on HostVarsVars.
ansibot
added
needs_revision
samdoran
removed
the
needs_triage
sivel
approved these changes
Nov 5, 2019
ansibot
added
core_review
Closed
tremble
reviewed
Nov 5, 2019
test/integration/targets/lookups/template_deepcopy/playbook.yml
Outdated
Show resolved
Hide resolved
tremble
reviewed
Nov 5, 2019
| - name: Test that the hostvar was templated correctly | ||
| assert: | ||
| that: | ||
| - templated_foo == "foo\n" |
There was a problem hiding this comment.
Suggested change
| - templated_foo == "foo\n" | |
| - templated_foo == "{{ host_var }}\n" |
There was a problem hiding this comment.
What would be the gain? In tests I don't really care about duplicate code. I prefer to use the actual values for comparisons. That way it does not depend on the templating engine (like {{hostvars['h1'].host_var}} might equal to {{ host_var }} but if it is templated to an incorrect value we won't find out because the comparison would still be true, however it's unlikely).
There was a problem hiding this comment.
Given it's the actual templating engine that's being tested I guess hard coded values makes sense. I would have switched to the variable just because folks looking to write their first tests often look for simple examples and assume the simple example is the way it should always be done.
ansibot
added
needs_revision
mkrizek
added a commit
to mkrizek/ansible
that referenced
this pull request
Nov 6, 2019
This fixes a regression that was caused by switching from copy() to deepcopy() when 'saving' variables before templating. Since HostVars did not implement the __deepcopy__() method, deepcopy returned incorrect results when host vars were present in the variables. Fixes ansible#63940 (cherry picked from commit cd8ce16)
This was referenced Nov 6, 2019
mattclay
pushed a commit
that referenced
this pull request
Nov 12, 2019
This fixes a regression that was caused by switching from copy() to deepcopy() when 'saving' variables before templating. Since HostVars did not implement the __deepcopy__() method, deepcopy returned incorrect results when host vars were present in the variables. Fixes #63940 (cherry picked from commit cd8ce16)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SUMMARY
This fixes a regression that was caused by switching from copy() to
deepcopy() when 'saving' variables before templating. Since HostVars
did not implement the
__deepcopy__()method, deepcopy returned incorrectresults when host vars were present in the variables.
Fixes #63940
ISSUE TYPE
COMPONENT NAME
template lookup