Merge branch '2017.7' into fix-cmd-run-env-corrupt

salt/grains/metadata.py

@@ -48,9 +48,11 @@ def _search(prefix="latest/"):
     Recursively look up all grains in the metadata server
     '''
     ret = {}
-    linedata = http.query(os.path.join(HOST, prefix))
+    linedata = http.query(os.path.join(HOST, prefix), headers=True)
     if 'body' not in linedata:
         return ret
+    if linedata['headers'].get('Content-Type', 'text/plain') == 'application/octet-stream':
+        return linedata['body']
     for line in linedata['body'].split('\n'):
         if line.endswith('/'):
             ret[line[:-1]] = _search(prefix=os.path.join(prefix, line))

salt/minion.py

@@ -2275,6 +2275,7 @@ class Minion(MinionBase):
                     self.opts,
                     self.functions,
                     self.returners,
+                    utils=self.utils,
                     cleanup=[master_event(type='alive')])
 
             try:

salt/modules/service.py

@@ -45,7 +45,8 @@ def __virtual__():
         'Void',
         'Mint',
         'Raspbian',
-        'XenServer'
+        'XenServer',
+        'Cumulus'
     ))
     if __grains__.get('os', '') in disable:
         return (False, 'Your OS is on the disabled list')

salt/modules/win_dism.py

@@ -426,16 +426,27 @@ def add_package(package,
     Install a package using DISM
 
     Args:
-        package (str): The package to install. Can be a .cab file, a .msu file,
-            or a folder
-        ignore_check (Optional[bool]): Skip installation of the package if the
-            applicability checks fail
-        prevent_pending (Optional[bool]): Skip the installation of the package
-            if there are pending online actions
-        image (Optional[str]): The path to the root directory of an offline
-            Windows image. If `None` is passed, the running operating system is
-            targeted. Default is None.
-        restart (Optional[bool]): Reboot the machine if required by the install
+        package (str):
+            The package to install. Can be a .cab file, a .msu file, or a folder
+
+            .. note::
+                An `.msu` package is supported only when the target image is
+                offline, either mounted or applied.
+
+        ignore_check (Optional[bool]):
+            Skip installation of the package if the applicability checks fail
+
+        prevent_pending (Optional[bool]):
+            Skip the installation of the package if there are pending online
+            actions
+
+        image (Optional[str]):
+            The path to the root directory of an offline Windows image. If
+            ``None`` is passed, the running operating system is targeted.
+            Default is None.
+
+        restart (Optional[bool]):
+            Reboot the machine if required by the install
 
     Returns:
         dict: A dictionary containing the results of the command

salt/modules/yumpkg.py

@@ -2663,20 +2663,18 @@ def mod_repo(repo, basedir=None, **kwargs):
     filerepos[repo].update(repo_opts)
     content = header
     for stanza in six.iterkeys(filerepos):
-        comments = ''
-        if 'comments' in six.iterkeys(filerepos[stanza]):
-            comments = salt.utils.pkg.rpm.combine_comments(
-                    filerepos[stanza]['comments'])
-            del filerepos[stanza]['comments']
-        content += '\n[{0}]'.format(stanza)
+        comments = salt.utils.pkg.rpm.combine_comments(
+            filerepos[stanza].pop('comments', [])
+        )
+        content += '[{0}]\n'.format(stanza)
         for line in six.iterkeys(filerepos[stanza]):
-            content += '\n{0}={1}'.format(
+            content += '{0}={1}\n'.format(
                 line,
                 filerepos[stanza][line]
                     if not isinstance(filerepos[stanza][line], bool)
                     else _bool_to_str(filerepos[stanza][line])
             )
-        content += '\n{0}\n'.format(comments)
+        content += comments + '\n'
 
     with salt.utils.fopen(repofile, 'w') as fileout:
         fileout.write(content)
@@ -2704,14 +2702,29 @@ def _parse_repo_file(filename):
         section_dict.pop('__name__', None)
         config[section] = section_dict
 
-    # Try to extract leading comments
+    # Try to extract header comments, as well as comments for each repo. Read
+    # from the beginning of the file and assume any leading comments are
+    # header comments. Continue to read each section header and then find the
+    # comments for each repo.
     headers = ''
-    with salt.utils.fopen(filename, 'r') as rawfile:
-        for line in rawfile:
-            if line.strip().startswith('#'):
-                headers += '{0}\n'.format(line.strip())
-            else:
-                break
+    section = None
+    with salt.utils.fopen(filename, 'r') as repofile:
+        for line in repofile:
+            line = line.strip()
+            if line.startswith('#'):
+                if section is None:
+                    headers += line + '\n'
+                else:
+                    try:
+                        comments = config[section].setdefault('comments', [])
+                        comments.append(line[1:].lstrip())
+                    except KeyError:
+                        log.debug(
+                            'Found comment in %s which does not appear to '
+                            'belong to any repo section: %s', filename, line
+                        )
+            elif line.startswith('[') and line.endswith(']'):
+                section = line[1:-1]
 
     return (headers, config)
 

salt/state.py

@@ -2207,7 +2207,8 @@ class State(object):
                 if r_state == 'prereq' and not run_dict[tag]['result'] is None:
                     fun_stats.add('pre')
                 else:
-                    fun_stats.add('met')
+                    if run_dict[tag].get('__state_ran__', True):
+                        fun_stats.add('met')
 
         if 'unmet' in fun_stats:
             status = 'unmet'
@@ -2462,6 +2463,7 @@ class State(object):
                             'duration': duration,
                             'start_time': start_time,
                             'comment': 'State was not run because onfail req did not change',
+                            '__state_ran__': False,
                             '__run_num__': self.__run_num,
                             '__sls__': low['__sls__']}
             self.__run_num += 1
@@ -2472,6 +2474,7 @@ class State(object):
                             'duration': duration,
                             'start_time': start_time,
                             'comment': 'State was not run because none of the onchanges reqs changed',
+                            '__state_ran__': False,
                             '__run_num__': self.__run_num,
                             '__sls__': low['__sls__']}
             self.__run_num += 1

salt/utils/pkg/rpm.py

@@ -11,7 +11,6 @@ import subprocess
 
 # Import 3rd-party libs
 from salt.ext import six
-from salt.ext.six.moves import range  # pylint: disable=redefined-builtin
 
 log = logging.getLogger(__name__)
 
@@ -112,10 +111,10 @@ def combine_comments(comments):
     '''
     if not isinstance(comments, list):
         comments = [comments]
-    for idx in range(len(comments)):
-        if not isinstance(comments[idx], six.string_types):
-            comments[idx] = str(comments[idx])
-        comments[idx] = comments[idx].strip()
-        if not comments[idx].startswith('#'):
-            comments[idx] = '#' + comments[idx]
-    return '\n'.join(comments)
+    ret = []
+    for comment in comments:
+        if not isinstance(comment, six.string_types):
+            comment = str(comment)
+        # Normalize for any spaces (or lack thereof) after the #
+        ret.append('# {0}\n'.format(comment.lstrip('#').lstrip()))
+    return ''.join(ret)

salt/utils/schedule.py

@@ -382,7 +382,7 @@ class Schedule(object):
     '''
     instance = None
 
-    def __new__(cls, opts, functions, returners=None, intervals=None, cleanup=None, proxy=None):
+    def __new__(cls, opts, functions, returners=None, intervals=None, cleanup=None, proxy=None, utils=None):
         '''
         Only create one instance of Schedule
         '''
@@ -392,20 +392,21 @@ class Schedule(object):
             # it in a WeakValueDictionary-- which will remove the item if no one
             # references it-- this forces a reference while we return to the caller
             cls.instance = object.__new__(cls)
-            cls.instance.__singleton_init__(opts, functions, returners, intervals, cleanup, proxy)
+            cls.instance.__singleton_init__(opts, functions, returners, intervals, cleanup, proxy, utils)
         else:
             log.debug('Re-using Schedule')
         return cls.instance
 
     # has to remain empty for singletons, since __init__ will *always* be called
-    def __init__(self, opts, functions, returners=None, intervals=None, cleanup=None, proxy=None):
+    def __init__(self, opts, functions, returners=None, intervals=None, cleanup=None, proxy=None, utils=None):
         pass
 
     # an init for the singleton instance to call
-    def __singleton_init__(self, opts, functions, returners=None, intervals=None, cleanup=None, proxy=None):
+    def __singleton_init__(self, opts, functions, returners=None, intervals=None, cleanup=None, proxy=None, utils=None):
         self.opts = opts
         self.proxy = proxy
         self.functions = functions
+        self.utils = utils
         if isinstance(intervals, dict):
             self.intervals = intervals
         else:
@@ -751,10 +752,11 @@ class Schedule(object):
             # This also needed for ZeroMQ transport to reset all functions
             # context data that could keep paretns connections. ZeroMQ will
             # hang on polling parents connections from the child process.
+            utils = self.utils or salt.loader.utils(self.opts)
             if self.opts['__role'] == 'master':
-                self.functions = salt.loader.runner(self.opts)
+                self.functions = salt.loader.runner(self.opts, utils=utils)
             else:
-                self.functions = salt.loader.minion_mods(self.opts, proxy=self.proxy)
+                self.functions = salt.loader.minion_mods(self.opts, proxy=self.proxy, utils=utils)
             self.returners = salt.loader.returners(self.opts, self.functions, proxy=self.proxy)
         ret = {'id': self.opts.get('id', 'master'),
                'fun': func,
@@ -1393,6 +1395,8 @@ class Schedule(object):
                 self.functions = {}
                 returners = self.returners
                 self.returners = {}
+                utils = self.utils
+                self.utils = {}
             try:
                 if multiprocessing_enabled:
                     thread_cls = SignalHandlingMultiprocessingProcess
@@ -1418,6 +1422,7 @@ class Schedule(object):
                 # Restore our function references.
                 self.functions = functions
                 self.returners = returners
+                self.utils = utils
 
 
 def clean_proc_dir(opts):

tests/integration/cloud/providers/test_dimensiondata.py

@@ -18,10 +18,6 @@ from tests.support.helpers import expensiveTest
 from salt.config import cloud_providers_config
 from salt.ext.six.moves import range  # pylint: disable=import-error,redefined-builtin
 
-# Create the cloud instance name to be used throughout the tests
-INSTANCE_NAME = _random_name('CLOUD-TEST-')
-PROVIDER_NAME = 'dimensiondata'
-
 
 def _random_name(size=6):
     '''
@@ -32,6 +28,10 @@ def _random_name(size=6):
         for x in range(size)
     )
 
+# Create the cloud instance name to be used throughout the tests
+INSTANCE_NAME = _random_name()
+PROVIDER_NAME = 'dimensiondata'
+
 
 class DimensionDataTest(ShellCase):
     '''

tests/integration/cloud/providers/test_profitbricks.py

@@ -78,7 +78,7 @@ class ProfitBricksTest(ShellCase):
         username = config[profile_str][DRIVER_NAME]['username']
         password = config[profile_str][DRIVER_NAME]['password']
         datacenter_id = config[profile_str][DRIVER_NAME]['datacenter_id']
-        if username == '' or password == '' or datacenter_id == '':
+        if username in ('' or 'foo') or password in ('' or 'bar') or datacenter_id == '':
             self.skipTest(
                 'A username, password, and an datacenter must be provided to '
                 'run these tests. Check '

tests/integration/files/file/base/requisites/onfail_multiple_required.sls

@@ -0,0 +1,25 @@
+a:
+  cmd.run:
+    - name: exit 1
+
+b:
+  cmd.run:
+    - name: echo b
+    - onfail:
+      - cmd: a
+
+c:
+  cmd.run:
+    - name: echo c
+    - onfail:
+      - cmd: a
+    - require:
+      - cmd: b
+
+d:
+  cmd.run:
+    - name: echo d
+    - onfail:
+      - cmd: a
+    - require:
+      - cmd: c

tests/integration/files/file/base/requisites/onfail_multiple_required_no_run.sls

@@ -0,0 +1,25 @@
+a:
+  cmd.run:
+    - name: exit 0
+
+b:
+  cmd.run:
+    - name: echo b
+    - onfail:
+      - cmd: a
+
+c:
+  cmd.run:
+    - name: echo c
+    - onfail:
+      - cmd: a
+    - require:
+      - cmd: b
+
+d:
+  cmd.run:
+    - name: echo d
+    - onfail:
+      - cmd: a
+    - require:
+      - cmd: c

tests/integration/modules/test_state.py

@@ -1095,6 +1095,56 @@ class StateModuleTest(ModuleCase, SaltReturnAssertsMixin):
         test_data = state_run['cmd_|-test_non_failing_state_|-echo "Should not run"_|-run']
         self.assertIn('duration', test_data)
 
+    def test_multiple_onfail_requisite_with_required(self):
+        '''
+        test to ensure multiple states are run
+        when specified as onfails for a single state.
+        This is a test for the issue:
+        https://github.com/saltstack/salt/issues/46552
+        '''
+
+        state_run = self.run_function('state.sls', mods='requisites.onfail_multiple_required')
+
+        retcode = state_run['cmd_|-b_|-echo b_|-run']['changes']['retcode']
+        self.assertEqual(retcode, 0)
+
+        retcode = state_run['cmd_|-c_|-echo c_|-run']['changes']['retcode']
+        self.assertEqual(retcode, 0)
+
+        retcode = state_run['cmd_|-d_|-echo d_|-run']['changes']['retcode']
+        self.assertEqual(retcode, 0)
+
+        stdout = state_run['cmd_|-b_|-echo b_|-run']['changes']['stdout']
+        self.assertEqual(stdout, 'b')
+
+        stdout = state_run['cmd_|-c_|-echo c_|-run']['changes']['stdout']
+        self.assertEqual(stdout, 'c')
+
+        stdout = state_run['cmd_|-d_|-echo d_|-run']['changes']['stdout']
+        self.assertEqual(stdout, 'd')
+
+    def test_multiple_onfail_requisite_with_required_no_run(self):
+        '''
+        test to ensure multiple states are not run
+        when specified as onfails for a single state
+        which fails.
+        This is a test for the issue:
+        https://github.com/saltstack/salt/issues/46552
+        '''
+
+        state_run = self.run_function('state.sls', mods='requisites.onfail_multiple_required_no_run')
+
+        expected = 'State was not run because onfail req did not change'
+
+        stdout = state_run['cmd_|-b_|-echo b_|-run']['comment']
+        self.assertEqual(stdout, expected)
+
+        stdout = state_run['cmd_|-c_|-echo c_|-run']['comment']
+        self.assertEqual(stdout, expected)
+
+        stdout = state_run['cmd_|-d_|-echo d_|-run']['comment']
+        self.assertEqual(stdout, expected)
+
     # listen tests
 
     def test_listen_requisite(self):

tests/integration/states/test_pkgrepo.py

@@ -22,16 +22,16 @@ import salt.utils
 import salt.ext.six as six
 
 
+@destructiveTest
+@skipIf(salt.utils.is_windows(), 'minion is windows')
 class PkgrepoTest(ModuleCase, SaltReturnAssertsMixin):
     '''
     pkgrepo state tests
     '''
-    @destructiveTest
-    @skipIf(salt.utils.is_windows(), 'minion is windows')
     @requires_system_grains
     def test_pkgrepo_01_managed(self, grains):
         '''
-        This is a destructive test as it adds a repository.
+        Test adding a repo
         '''
         os_grain = self.run_function('grains.item', ['os'])['os']
         os_release_info = tuple(self.run_function('grains.item', ['osrelease_info'])['osrelease_info'])
@@ -56,12 +56,9 @@ class PkgrepoTest(ModuleCase, SaltReturnAssertsMixin):
         for state_id, state_result in six.iteritems(ret):
             self.assertSaltTrueReturn(dict([(state_id, state_result)]))
 
-    @destructiveTest
-    @skipIf(salt.utils.is_windows(), 'minion is windows')
     def test_pkgrepo_02_absent(self):
         '''
-        This is a destructive test as it removes the repository added in the
-        above test.
+        Test removing the repo from the above test
         '''
         os_grain = self.run_function('grains.item', ['os'])['os']
         os_release_info = tuple(self.run_function('grains.item', ['osrelease_info'])['osrelease_info'])
@@ -78,3 +75,56 @@ class PkgrepoTest(ModuleCase, SaltReturnAssertsMixin):
         self.assertReturnNonEmptySaltType(ret)
         for state_id, state_result in six.iteritems(ret):
             self.assertSaltTrueReturn(dict([(state_id, state_result)]))
+
+    @requires_system_grains
+    def test_pkgrepo_03_with_comments(self, grains):
+        '''
+        Test adding a repo with comments
+        '''
+        os_family = grains['os_family'].lower()
+
+        if os_family in ('redhat', 'suse'):
+            kwargs = {
+                'name': 'examplerepo',
+                'baseurl': 'http://example.com/repo',
+                'enabled': False,
+                'comments': ['This is a comment']
+            }
+        elif os_family in ('debian',):
+            self.skipTest('Debian/Ubuntu test case needed')
+        else:
+            self.skipTest("No test case for os_family '{0}'".format(os_family))
+
+        try:
+            # Run the state to add the repo
+            ret = self.run_state('pkgrepo.managed', **kwargs)
+            self.assertSaltTrueReturn(ret)
+
+            # Run again with modified comments
+            kwargs['comments'].append('This is another comment')
+            ret = self.run_state('pkgrepo.managed', **kwargs)
+            self.assertSaltTrueReturn(ret)
+            ret = ret[next(iter(ret))]
+            self.assertEqual(
+                ret['changes'],
+                {
+                    'comments': {
+                        'old': ['This is a comment'],
+                        'new': ['This is a comment',
+                                'This is another comment']
+                    }
+                }
+            )
+
+            # Run a third time, no changes should be made
+            ret = self.run_state('pkgrepo.managed', **kwargs)
+            self.assertSaltTrueReturn(ret)
+            ret = ret[next(iter(ret))]
+            self.assertFalse(ret['changes'])
+            self.assertEqual(
+                ret['comment'],
+                "Package repo '{0}' already configured".format(kwargs['name'])
+            )
+        finally:
+            # Clean up
+            self.run_state('pkgrepo.absent', name=kwargs['name'])

tests/runtests.py

@@ -694,6 +694,7 @@ class SaltTestsuiteParser(SaltCoverageTestingParser):
                         continue
                     results = self.run_suite('', name, suffix='test_*.py', load_from_name=True)
                     status.append(results)
+                return status
             for suite in TEST_SUITES:
                 if suite != 'unit' and getattr(self.options, suite):
                     status.append(self.run_integration_suite(**TEST_SUITES[suite]))