Merge pull request #40588 from rallytime/merge-2016.11

[2016.11] Merge forward from 2016.3 to 2016.11

salt/cloud/clouds/vultrpy.py

@@ -331,7 +331,8 @@ def create(vm_):
         '''
         data = show_instance(vm_['name'], call='action')
         pprint.pprint(data)
-        if str(data.get('main_ip', '0')) == '0':
+        main_ip = str(data.get('main_ip', '0'))
+        if main_ip.startswith('0'):
             time.sleep(3)
             return False
         return data['main_ip']

salt/modules/dockerng.py

@@ -572,7 +572,11 @@ def _get_docker_py_versioninfo():
 def _get_client(**kwargs):
     client_kwargs = {}
     if 'client_timeout' in kwargs:
-        client_kwargs['timeout'] = kwargs.pop('client_timeout')
+        client_timeout = kwargs.pop('client_timeout')
+        if client_timeout is not None:
+            # Passing a kwarg of timeout=None causes problems in docker-py, so
+            # only set this kwarg if the value is not None.
+            client_kwargs['timeout'] = client_timeout
     for key, val in (('base_url', 'docker.url'),
                      ('version', 'docker.version')):
         param = __salt__['config.get'](val, NOTSET)
@@ -630,6 +634,8 @@ def _docker_client(wrapped):
         '''
         if 'docker.client' not in __context__:
             __context__['docker.client'] = _get_client(**kwargs)
+        # Don't pass through client_timeout to the wrapped function
+        kwargs.pop('client_timeout', None)
         return wrapped(*args, **salt.utils.clean_kwargs(**kwargs))
     return wrapper
 

salt/modules/file.py

@@ -4631,7 +4631,7 @@ def manage_file(name,
         if not sfn:
             return _error(
                 ret, 'Source file \'{0}\' not found'.format(source))
-        htype = source_sum.get('hash_type', __opts__.get('hash_type', 'md5'))
+        htype = source_sum.get('hash_type', __opts__['hash_type'])
         # Recalculate source sum now that file has been cached
         source_sum = {
             'hash_type': htype,
@@ -4654,12 +4654,12 @@ def manage_file(name,
 
         # Only test the checksums on files with managed contents
         if source and not (not follow_symlinks and os.path.islink(real_name)):
-            name_sum = get_hash(real_name, source_sum.get('hash_type', __opts__.get('hash_type', 'md5')))
+            name_sum = get_hash(real_name, source_sum.get('hash_type', __opts__['hash_type']))
         else:
             name_sum = None
 
         # Check if file needs to be replaced
-        if source and (name_sum is None or source_sum.get('hsum', __opts__.get('hash_type', 'md5')) != name_sum):
+        if source and (name_sum is None or source_sum.get('hsum', __opts__['hash_type']) != name_sum):
             if not sfn:
                 sfn = __salt__['cp.cache_file'](source, saltenv)
             if not sfn:

salt/modules/pillar.py

@@ -18,7 +18,7 @@ import salt.ext.six as six
 import salt.pillar
 import salt.utils
 from salt.defaults import DEFAULT_TARGET_DELIM
-from salt.exceptions import CommandExecutionError, SaltInvocationError
+from salt.exceptions import CommandExecutionError
 
 __proxyenabled__ = ['*']
 
@@ -51,11 +51,16 @@ def get(key,
 
         pkg:apache
 
-    merge
-        Specify whether or not the retrieved values should be recursively
-        merged into the passed default.
+    merge : False
+        If ``True``, the retrieved values will be merged into the passed
+        default. When the default and the retrieved value are both
+        dictionaries, the dictionaries will be recursively merged.
 
         .. versionadded:: 2014.7.0
+        .. versionchanged:: 2016.3.7,2016.11.4,Nitrogen
+            If the default and the retrieved value are not of the same type,
+            then merging will be skipped and the retrieved value will be
+            returned. Earlier releases raised an error in these cases.
 
     delimiter
         Specify an alternate delimiter to use when traversing a nested dict.
@@ -94,32 +99,53 @@ def get(key,
     pillar_dict = __pillar__ if saltenv is None else items(saltenv=saltenv)
 
     if merge:
-        if default is None:
-            log.debug('pillar.get: default is None, skipping merge')
-        else:
-            if not isinstance(default, dict):
-                raise SaltInvocationError(
-                    'default must be a dictionary or None when merge=True'
-                )
+        if isinstance(default, dict):
             ret = salt.utils.traverse_dict_and_list(
                 pillar_dict,
                 key,
                 {},
                 delimiter)
-            if isinstance(ret, collections.Mapping) and \
-                    isinstance(default, collections.Mapping):
+            if isinstance(ret, collections.Mapping):
                 default = copy.deepcopy(default)
                 return salt.utils.dictupdate.update(
                     default,
                     ret,
                     merge_lists=opt_merge_lists)
+            else:
+                log.error(
+                    'pillar.get: Default (%s) is a dict, but the returned '
+                    'pillar value (%s) is of type \'%s\'. Merge will be '
+                    'skipped.', default, ret, type(ret).__name__
+                )
+        elif isinstance(default, list):
+            ret = salt.utils.traverse_dict_and_list(
+                pillar_dict,
+                key,
+                [],
+                delimiter)
+            if isinstance(ret, list):
+                default = copy.deepcopy(default)
+                default.extend([x for x in ret if x not in default])
+                return default
+            else:
+                log.error(
+                    'pillar.get: Default (%s) is a list, but the returned '
+                    'pillar value (%s) is of type \'%s\'. Merge will be '
+                    'skipped.', default, ret, type(ret).__name__
+                )
+        else:
+            log.error(
+                'pillar.get: Default (%s) is of type \'%s\', must be a dict '
+                'or list to merge. Merge will be skipped.',
+                default, type(default).__name__
+            )
 
     ret = salt.utils.traverse_dict_and_list(pillar_dict,
                                             key,
                                             default,
                                             delimiter)
     if ret is KeyError:
-        raise KeyError("Pillar key not found: {0}".format(key))
+        raise KeyError('Pillar key not found: {0}'.format(key))
 
     return ret
 

tests/integration/cloud/providers/vultr.py

@@ -158,10 +158,12 @@ class VultrTest(integration.ShellCase):
         '''
         # check if instance with salt installed returned
         try:
+            create_vm = self.run_cloud('-p vultr-test {0}'.format(INSTANCE_NAME), timeout=500)
             self.assertIn(
                 INSTANCE_NAME,
-                [i.strip() for i in self.run_cloud('-p vultr-test {0}'.format(INSTANCE_NAME), timeout=500)]
+                [i.strip() for i in create_vm]
             )
+            self.assertNotIn('Failed to start', str(create_vm))
         except AssertionError:
             self.run_cloud('-d {0} --assume-yes'.format(INSTANCE_NAME), timeout=500)
             raise

tests/unit/modules/pillar_test.py

@@ -58,16 +58,57 @@ class PillarModuleTestCase(TestCase):
         else:
             self.assertEqual(pillarmod.ls(), ['a', 'b'])
 
+    @skipIf(NO_MOCK, NO_MOCK_REASON)
     def test_pillar_get_default_merge(self):
+        defaults = {'int': 1,
+                    'string': 'foo',
+                    'list': ['foo'],
+                    'dict': {'foo': 'bar', 'subkey': {'foo': 'bar'}}}
+
         pillarmod.__opts__ = {}
-        pillarmod.__pillar__ = {'key': 'value'}
-        default = {'default': 'plop'}
+        pillarmod.__pillar__ = {'int': 2,
+                                'string': 'bar',
+                                'list': ['bar', 'baz'],
+                                'dict': {'baz': 'qux', 'subkey': {'baz': 'qux'}}}
 
-        res = pillarmod.get(key='key', default=default)
-        self.assertEqual("value", res)
+        # Test that we raise a KeyError when pillar_raise_on_missing is True
+        with patch.dict(pillarmod.__opts__, {'pillar_raise_on_missing': True}):
+            self.assertRaises(KeyError, pillarmod.get, 'missing')
+        # Test that we return an empty string when it is not
+        self.assertEqual(pillarmod.get('missing'), '')
 
-        res = pillarmod.get(key='missing pillar', default=default)
-        self.assertEqual({'default': 'plop'}, res)
+        # Test with no default passed (it should be KeyError) and merge=True.
+        # The merge should be skipped and the value returned from __pillar__
+        # should be returned.
+        for item in pillarmod.__pillar__:
+            self.assertEqual(
+                pillarmod.get(item, merge=True),
+                pillarmod.__pillar__[item]
+            )
+
+        # Test merging when the type of the default value is not the same as
+        # what was returned. Merging should be skipped and the value returned
+        # from __pillar__ should be returned.
+        for default_type in defaults:
+            for data_type in ('dict', 'list'):
+                if default_type == data_type:
+                    continue
+                self.assertEqual(
+                    pillarmod.get(item, default=defaults[default_type], merge=True),
+                    pillarmod.__pillar__[item]
+                )
+
+        # Test recursive dict merging
+        self.assertEqual(
+            pillarmod.get('dict', default=defaults['dict'], merge=True),
+            {'foo': 'bar', 'baz': 'qux', 'subkey': {'foo': 'bar', 'baz': 'qux'}}
+        )
+
+        # Test list merging
+        self.assertEqual(
+            pillarmod.get('list', default=defaults['list'], merge=True),
+            ['foo', 'bar', 'baz']
+        )
 
     def test_pillar_get_default_merge_regression_38558(self):
         """Test for pillar.get(key=..., default=..., merge=True)