Add "none" as a pillar merging strategy #36435
Conversation
…ings when it is selected
…nv was passed retry
|
@yhekma Thanks very much! This code does look fairly safe although the test suite doesn't seem to like it. Would you mind investigating? |
|
@cachedout sure, I'll have a look tomorrow |
|
@yashlyft Much appreciated. Thank you! |
|
@cachedout is there a way to retrigger the tests? Apart from pushing a new commit I mean? |
|
Go Go Jenkins! |
|
@yhekma Done. The phrase above does the trick. :] |
|
@cachedout I cannot figure out why the tests are failing. It looks like it doesn't have anything to do with my code changes. Can you confirm or am I overlooking something? |
|
Go Go Jenkins! |
|
@yashlyft I think that the remaining test failures are unrelated to your changes. This is likely ready for a full review and then hopefully a merge. |
|
@cachedout Ok, great, let me know when and if I need to do anything. Ps. |
|
@yhekma Guh, sorry about that. Github's autocomplete likes to let me embarrass myself from time to time. ;] We'll get this in ASAP. I'll take another look at it after the tests finish running. |
| @@ -7571,9 +7571,14 @@ New in version 2014.7.0. | |||
| Default: \fBsmart\fP | |||
| .sp | |||
| The pillar_source_merging_strategy option allows you to configure merging | |||
| strategy between different sources. It accepts 4 values: | |||
| strategy between different sources. It accepts 5 values: | |||
There was a problem hiding this comment.
For future reference, you don't need to edit the man pages. They are automatically rebuilt on each release.
| @@ -2791,6 +2791,9 @@ Default: ``smart`` | |||
| The pillar_source_merging_strategy option allows you to configure merging | |||
| strategy between different sources. It accepts 4 values: | |||
|
|
|||
| * ``none``: | |||
| it will not do any merging at all and only parse the pillar data from the passed environment and 'base' if no environment was specified. | |||
There was a problem hiding this comment.
Could you please capitalize the first word in this sentence?
| if self.opts.get('pillar_source_merging_strategy', None) == "none": | ||
| if self.saltenv and saltenv != self.saltenv: | ||
| continue | ||
| if not self.saltenv and not saltenv == 'base': |
There was a problem hiding this comment.
Typically we'd like to see these conditionals combined but here I think it enhances readability. Good call. 👍
|
@cachedout since I believe this "fixes" #29421 which is flagged as a bug and as high severity, can I expect this to be backported to let's say 2016.3? Right now we (eBay) have created our own patch for our own package, but would like to get back to upstream:) |
|
Typically it is our policy to only include new features in major releases but given the nature of the issue in #29421 I think it's fair to consider this a bug fix and backport it. I'll mark it as such. |
cachedout
added
the
bugfix-bckport
|
@cachedout great, thanks! |
rallytime
added
ZZZ[Done]-back-ported-bf
* Added "none" option for pillar_source_merging_strategy * added none to merging strategy in dictupdate so we no longer log warnings when it is selected * Updated documentation * fix for when pillar_source_merging_strategy is not set * Fix looking up pillar data when merging is set to "none" and no saltenv was passed * Fix looking up pillar data when merging is set to "none" and no saltenv was passed retry * Let's not break when no merging strategy is passed * Capitalisation * Update on doc
* upstream/develop: (41 commits) Fix typo in postgres_user.py Python 3 module reload compatability Fix rst table syntax in logging docs section Fix lineterminator on kapacitor diffs Wait for kill in ProcessManager should be greater in main process than in subprocess. Don't set the `daemon` flag for LoggingQueue process. archive.extracted: Use `user`/`group`, not `archive_user` Mercurial Module: Pass the identity_path portion as own arg Fixup the rabbitmq_user state test failure (saltstack#36541) Back-port saltstack#36435 to 2016.3 (saltstack#36532) Be explicit about the salt.utils.templates import (saltstack#36535) Wrap the entire GrainsAppendTestCase class with destructiveTest (saltstack#36537) states.archive: use archive.cmd_unzip when possible modules.archive.unzip: log a warning about perms Per issue 36451, fixed bug where gpg.trust_key calls get_key for the current user, not the input user Cache the renderer in the pillar Adding openstack heat module execution and state Updated tests to reflect changes in salt.utils.vmware.wait_for_pid Added tests for get_root_folder Added get_root_folder method in salt.utils.vmware ...
What does this PR do?
Add "none" option to the pillar merging strategy to prevent merging of pillar data altogether, which is referenced in #29421
What issues does this PR fix or reference?
#29421
New Behavior
When pillar_source_merging_strategy is set to "none", no pillar merging is done and only the pillar data of the environment that is called is used
Tests written?
No
Please review Salt's Contributing Guide for best practices.