Skip to content

Commit

Permalink
Remove recurse_list from pillar_source_merging_strategy and add pilla…
Browse files Browse the repository at this point in the history
…r_merge_list (bool) instead
  • Loading branch information
Sean Jenkins committed Dec 29, 2015
1 parent 51dd6d9 commit 41353b7
Show file tree
Hide file tree
Showing 10 changed files with 57 additions and 109 deletions.
3 changes: 3 additions & 0 deletions conf/master
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,9 @@
# on the "renderer" setting and is the default value.
#pillar_source_merging_strategy: smart

# Recursively merge lists by aggregating them instead of replacing them.
#pillar_merge_lists: True


##### Syndic settings #####
##########################################
Expand Down
44 changes: 15 additions & 29 deletions doc/ref/configuration/master.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2255,35 +2255,6 @@ strategy between different sources. It accepts 4 values:
element2: True
baz: quux
* recurse_list:

it will merge recursively mapping of data similar to ``recurse`` but merge
lists by aggregating them instead of replacing them.

.. code-block:: yaml
foo: 43
bar:
- 1
- 2
.. code-block:: yaml
bar:
- 3
baz: quux
will be merged as:

.. code-block:: yaml
foo: 42
bar:
- 1
- 2
- 3
baz: quux
* aggregate:

instructs aggregation of elements between sources that use the #!yamlex renderer.
Expand Down Expand Up @@ -2353,6 +2324,21 @@ strategy between different sources. It accepts 4 values:

Guesses the best strategy based on the "renderer" setting.

``pillar_merge_lists``
----------------------------------

.. versionadded:: 2015.8.0

Default: ``True``

Recursively merge lists by aggregating them instead of replacing them.

.. code-block:: yaml
pillar_merge_lists: True
.. conf_master:: pillar_source_merging_strategy


Syndic Server Settings
======================
Expand Down
4 changes: 3 additions & 1 deletion salt/auth/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,12 +211,14 @@ def auth_data(self):
Gather and create the authorization data sets
'''
auth_data = self.opts['external_auth']
merge_lists = self.opts['pillar_merge_lists']

if 'django' in auth_data and '^model' in auth_data['django']:
auth_from_django = salt.auth.django.retrieve_auth_entries()
auth_data = salt.utils.dictupdate.merge(auth_data,
auth_from_django,
strategy='list')
strategy='list',
merge_lists=merge_lists)

#for auth_back in self.opts.get('external_auth_sources', []):
# fstr = '{0}.perms'.format(auth_back)
Expand Down
4 changes: 4 additions & 0 deletions salt/config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,9 @@
# encountering duplicate values
'pillar_source_merging_strategy': str,

# Recursively merge lists by aggregating them instead of replacing them.
'pillar_merge_lists': bool,

# How to merge multiple top files from multiple salt environments
# (saltenvs); can be 'merge' or 'same'
'top_file_merging_strategy': str,
Expand Down Expand Up @@ -1067,6 +1070,7 @@
'pillar_opts': False,
'pillar_safe_render_error': True,
'pillar_source_merging_strategy': 'smart',
'pillar_merge_lists': True,
'ping_on_rotate': False,
'peer': {},
'preserve_minion_cache': False,
Expand Down
9 changes: 6 additions & 3 deletions salt/modules/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import logging

# Import salt libs
import salt.config
import salt.utils
try:
# Gated for salt-ssh (salt.utils.cloud imports msgpack)
Expand Down Expand Up @@ -357,10 +358,12 @@ def get(key, default='', delimiter=':', merge=None):
'to \'recurse\'.'.format(merge))
merge = 'recurse'

merge_lists = salt.config.master_config('/etc/salt/master').get('pillar_merge_lists')

data = copy.copy(__pillar__.get('master', {}))
data = salt.utils.dictupdate.merge(data, __pillar__, strategy=merge)
data = salt.utils.dictupdate.merge(data, __grains__, strategy=merge)
data = salt.utils.dictupdate.merge(data, __opts__, strategy=merge)
data = salt.utils.dictupdate.merge(data, __pillar__, strategy=merge, merge_lists=merge_lists)
data = salt.utils.dictupdate.merge(data, __grains__, strategy=merge, merge_lists=merge_lists)
data = salt.utils.dictupdate.merge(data, __opts__, strategy=merge, merge_lists=merge_lists)
ret = salt.utils.traverse_dict_and_list(data,
key,
'_|-',
Expand Down
12 changes: 8 additions & 4 deletions salt/pillar/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,8 @@ def render_pstate(self, sls, saltenv, mods, defaults=None):
state,
nstate,
self.merge_strategy,
self.opts.get('renderer', 'yaml'))
self.opts.get('renderer', 'yaml'),
self.opts.get('pillar_merge_lists', 'True'))

if err:
errors += err
Expand Down Expand Up @@ -579,7 +580,8 @@ def render_pillar(self, matches):
pillar,
pstate,
self.merge_strategy,
self.opts.get('renderer', 'yaml'))
self.opts.get('renderer', 'yaml'),
self.opts.get('pillar_merge_lists', 'True'))

return pillar, errors

Expand Down Expand Up @@ -667,7 +669,8 @@ def ext_pillar(self, pillar, pillar_dirs):
pillar,
ext,
self.merge_strategy,
self.opts.get('renderer', 'yaml'))
self.opts.get('renderer', 'yaml'),
self.opts.get('pillar_merge_lists', 'True'))
ext = None
return pillar

Expand All @@ -684,7 +687,8 @@ def compile_pillar(self, ext=True, pillar_dirs=None):
pillar = merge(pillar,
self.opts['pillar'],
self.merge_strategy,
self.opts.get('renderer', 'yaml'))
self.opts.get('renderer', 'yaml'),
self.opts.get('pillar_merge_lists', 'True'))
else:
matches = self.top_matches(top)
pillar, errors = self.render_pillar(matches)
Expand Down
7 changes: 6 additions & 1 deletion salt/pillar/git_pillar.py
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,10 @@ def ext_pillar(minion_id, repo, pillar_dirs):
'pillar_source_merging_strategy',
'smart'
)
merge_lists = __opts__.get(
'pillar_merge_lists',
'True'
)
for pillar_dir, env in six.iteritems(pillar.pillar_dirs):
log.debug(
'git_pillar is processing pillar SLS from {0} for pillar '
Expand All @@ -274,7 +278,8 @@ def ext_pillar(minion_id, repo, pillar_dirs):
ret = salt.utils.dictupdate.merge(
ret,
local_pillar.compile_pillar(ext=False),
strategy=merge_strategy
strategy=merge_strategy,
merge_lists=merge_lists
)
return ret

Expand Down
16 changes: 7 additions & 9 deletions salt/utils/dictupdate.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
log = logging.getLogger(__name__)


def update(dest, upd, recursive_update=True, merge_lists=False):
def update(dest, upd, recursive_update=True, merge_lists=True):
'''
Recursive version of the default dict.update
Expand Down Expand Up @@ -76,7 +76,7 @@ def merge_list(obj_a, obj_b):
return ret


def merge_recurse(obj_a, obj_b, merge_lists=False):
def merge_recurse(obj_a, obj_b, merge_lists=True):
copied = copy.deepcopy(obj_a)
return update(copied, obj_b, merge_lists=merge_lists)

Expand All @@ -85,14 +85,14 @@ def merge_aggregate(obj_a, obj_b):
return _yamlex_merge_recursive(obj_a, obj_b, level=1)


def merge_overwrite(obj_a, obj_b):
def merge_overwrite(obj_a, obj_b, merge_lists=True):
for obj in obj_b:
if obj in obj_a:
obj_a[obj] = obj_b[obj]
return merge_recurse(obj_a, obj_b)
return merge_recurse(obj_a, obj_b, merge_lists=merge_lists)


def merge(obj_a, obj_b, strategy='smart', renderer='yaml'):
def merge(obj_a, obj_b, strategy='smart', renderer='yaml', merge_lists=True):
if strategy == 'smart':
if renderer == 'yamlex' or renderer.startswith('yamlex_'):
strategy = 'aggregate'
Expand All @@ -102,14 +102,12 @@ def merge(obj_a, obj_b, strategy='smart', renderer='yaml'):
if strategy == 'list':
merged = merge_list(obj_a, obj_b)
elif strategy == 'recurse':
merged = merge_recurse(obj_a, obj_b)
elif strategy == 'recurse_list':
merged = merge_recurse(obj_a, obj_b, merge_lists=True)
merged = merge_recurse(obj_a, obj_b, merge_lists)
elif strategy == 'aggregate':
#: level = 1 merge at least root data
merged = merge_aggregate(obj_a, obj_b)
elif strategy == 'overwrite':
merged = merge_overwrite(obj_a, obj_b)
merged = merge_overwrite(obj_a, obj_b, merge_lists)
else:
log.warning('Unknown merging strategy \'{0}\', '
'fallback to recurse'.format(strategy))
Expand Down
59 changes: 0 additions & 59 deletions tests/unit/pillar_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,38 +130,6 @@ def test_topfile_order(self, Matcher, get_file_client):
pillar = salt.pillar.Pillar(opts, grains, 'mocked-minion', 'base')
self.assertEqual(pillar.compile_pillar()['ssh'], 'foo')

@patch('salt.pillar.salt.fileclient.get_file_client', autospec=True)
@patch('salt.pillar.salt.minion.Matcher') # autospec=True disabled due to py3 mock bug
def test_pillar_multiple_matches(self, Matcher, get_file_client):
# Uses the ``recurse_list`` strategy.
opts = {
'renderer': 'yaml',
'state_top': '',
'pillar_roots': [],
'extension_modules': '',
'environment': 'base',
'file_roots': [],
'pillar_source_merging_strategy': 'recurse_list',
}
grains = {
'os': 'Ubuntu',
'os_family': 'Debian',
'oscodename': 'raring',
'osfullname': 'Ubuntu',
'osrelease': '13.04',
'kernel': 'Linux'
}
self._setup_test_topfile_mocks(Matcher, get_file_client, 1, 2)
pillar = salt.pillar.Pillar(opts, grains, 'mocked-minion', 'base')
# Pillars should be merged, but only once per pillar file.
self.assertDictEqual(pillar.compile_pillar()['generic'], {
'key1': ['value1', 'value2', 'value3'],
'key2': {
'sub_key1': [],
'sub_key2': [],
}
})

def _setup_test_topfile_mocks(self, Matcher, get_file_client,
nodegroup_order, glob_order):
# Write a simple topfile and two pillar state files
Expand Down Expand Up @@ -232,33 +200,6 @@ def get_state(sls, env):

client.get_state.side_effect = get_state

@patch('salt.pillar.salt.fileclient.get_file_client', autospec=True)
@patch('salt.pillar.salt.minion.Matcher') # autospec=True disabled due to py3 mock bug
def test_pillar_include(self, Matcher, get_file_client):
# Uses the ``recurse_list`` strategy.
opts = {
'renderer': 'yaml',
'state_top': '',
'pillar_roots': [],
'extension_modules': '',
'environment': 'base',
'file_roots': [],
'pillar_source_merging_strategy': 'recurse_list',
}
grains = {
'os': 'Ubuntu',
'os_family': 'Debian',
'oscodename': 'raring',
'osfullname': 'Ubuntu',
'osrelease': '13.04',
'kernel': 'Linux'
}
self._setup_test_include_mocks(Matcher, get_file_client)
pillar = salt.pillar.Pillar(opts, grains, 'mocked-minion', 'base').compile_pillar()
# Both pillar modules should only be loaded once.
self.assertEqual(pillar['p1'], ['value1_3', 'value1_1', 'value1_2'])
self.assertEqual(pillar['p2'], ['value2_1', 'value2_2'])

def _setup_test_include_mocks(self, Matcher, get_file_client):
self.top_file = top_file = tempfile.NamedTemporaryFile()
top_file.write(b'''
Expand Down
8 changes: 5 additions & 3 deletions tests/unit/utils/dictupdate_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ def test_update(self):
# level 1 value changes (list replacement)
mdict = copy.deepcopy(self.dict1)
mdict['A'] = [1, 2]
res = dictupdate.update(copy.deepcopy(mdict), {'A': [2, 3]})
res = dictupdate.update(copy.deepcopy(mdict), {'A': [2, 3]},
merge_lists=False)
mdict['A'] = [2, 3]
self.assertEqual(res, mdict)

Expand All @@ -50,7 +51,8 @@ def test_update(self):
# level 2 value changes (list replacement)
mdict = copy.deepcopy(self.dict1)
mdict['C']['D'] = ['a', 'b']
res = dictupdate.update(copy.deepcopy(mdict), {'C': {'D': ['c', 'd']}})
res = dictupdate.update(copy.deepcopy(mdict), {'C': {'D': ['c', 'd']}},
merge_lists=False)
mdict['C']['D'] = ['c', 'd']
self.assertEqual(res, mdict)

Expand All @@ -75,7 +77,7 @@ def test_update(self):
mdict = copy.deepcopy(self.dict1)
mdict['C']['F']['G'] = ['a', 'b']
res = dictupdate.update(copy.deepcopy(mdict),
{'C': {'F': {'G': ['c', 'd']}}})
{'C': {'F': {'G': ['c', 'd']}}}, merge_lists=False)
mdict['C']['F']['G'] = ['c', 'd']
self.assertEqual(res, mdict)

Expand Down

0 comments on commit 41353b7

Please sign in to comment.