Merge pull request #36474 from rallytime/merge-2016.3

[2016.3] Merge forward from 2015.8 to 2016.3

salt/cloud/clouds/ec2.py

@@ -11,13 +11,10 @@
 .. code-block:: yaml
 
     my-ec2-config:
-      # The EC2 API authentication id, set this and/or key to
-      # 'use-instance-role-credentials' to use the instance role credentials
-      # from the meta-data if running on an AWS instance
+      # EC2 API credentials: Access Key ID and Secret Access Key.
+      # Alternatively, to use IAM Instance Role credentials available via
+      # EC2 metadata set both id and key to 'use-instance-role-credentials'
       id: GKTADJGHEIQSXMKKRBJ08H
-      # The EC2 API authentication key, set this and/or id to
-      # 'use-instance-role-credentials' to use the instance role credentials
-      # from the meta-data if running on an AWS instance
       key: askdjghsdfjkghWupUjasdflkdfklgjsdfjajkghs
       # The ssh keyname to use
       keyname: default

salt/states/git.py

@@ -182,14 +182,39 @@ def _failed_submodule_update(ret, exc, comments=None):
     return _fail(ret, msg, comments)
 
 
-def _not_fast_forward(ret, pre, post, branch, local_branch,
-                      local_changes, comments):
+def _not_fast_forward(ret, rev, pre, post, branch, local_branch,
+                      default_branch, local_changes, comments):
+    branch_msg = ''
+    if branch is None:
+        if rev != 'HEAD':
+            if local_branch != rev:
+                branch_msg = (
+                    ' The desired rev ({0}) differs from the name of the '
+                    'local branch ({1}), if the desired rev is a branch name '
+                    'then a forced update could possibly be avoided by '
+                    'setting the \'branch\' argument to \'{0}\' instead.'
+                    .format(rev, local_branch)
+                )
+        else:
+            if default_branch is not None and local_branch != default_branch:
+                branch_msg = (
+                    ' The default remote branch ({0}) differs from the '
+                    'local branch ({1}). This could be caused by changing the '
+                    'default remote branch, or if the local branch was '
+                    'manually changed. Rather than forcing an update, it '
+                    'may be advisable to set the \'branch\' argument to '
+                    '\'{0}\' instead. To ensure that this state follows the '
+                    '\'{0}\' branch instead of the remote HEAD, set the '
+                    '\'rev\' argument to \'{0}\'.'
+                    .format(default_branch, local_branch)
+                )
+
     pre = _short_sha(pre)
     post = _short_sha(post)
     return _fail(
         ret,
         'Repository would be updated {0}{1}, but {2}. Set \'force_reset\' to '
-        'True to force this update{3}.'.format(
+        'True to force this update{3}.{4}'.format(
             'from {0} to {1}'.format(pre, post)
                 if local_changes and pre != post
                 else 'to {0}'.format(post),
@@ -199,7 +224,8 @@ def _not_fast_forward(ret, pre, post, branch, local_branch,
             'this is not a fast-forward merge'
                 if not local_changes
                 else 'there are uncommitted changes',
-            ' and discard these changes' if local_changes else ''
+            ' and discard these changes' if local_changes else '',
+            branch_msg,
         ),
         comments
     )
@@ -614,14 +640,27 @@ def latest(name,
             'Failed to check remote refs: {0}'.format(_strip_exc(exc))
         )
 
+    if 'HEAD' in all_remote_refs:
+        head_rev = all_remote_refs['HEAD']
+        for refname, refsha in six.iteritems(all_remote_refs):
+            if refname.startswith('refs/heads/'):
+                if refsha == head_rev:
+                    default_branch = refname.partition('refs/heads/')[-1]
+                    break
+        else:
+            default_branch = None
+    else:
+        head_ref = None
+        default_branch = None
+
     desired_upstream = False
     if bare:
         remote_rev = None
         remote_rev_type = None
     else:
         if rev == 'HEAD':
-            if 'HEAD' in all_remote_refs:
-                remote_rev = all_remote_refs['HEAD']
+            if head_rev is not None:
+                remote_rev = head_rev
                 # Just go with whatever the upstream currently is
                 desired_upstream = None
                 remote_rev_type = 'sha1'
@@ -935,10 +974,12 @@ def latest(name,
                 if not force_reset:
                     return _not_fast_forward(
                         ret,
+                        rev,
                         base_rev,
                         remote_rev,
                         branch,
                         local_branch,
+                        default_branch,
                         local_changes,
                         comments)
                 merge_action = 'hard-reset'
@@ -1246,10 +1287,12 @@ def latest(name,
                     if fast_forward is False and not force_reset:
                         return _not_fast_forward(
                             ret,
+                            rev,
                             base_rev,
                             remote_rev,
                             branch,
                             local_branch,
+                            default_branch,
                             local_changes,
                             comments)
 

tests/integration/states/git.py

@@ -260,6 +260,80 @@ def _head(cwd):
             for path in (mirror_dir, admin_dir, clone_dir):
                 shutil.rmtree(path, ignore_errors=True)
 
+    def _changed_local_branch_helper(self, rev, hint):
+        '''
+        We're testing two almost identical cases, the only thing that differs
+        is the rev used for the git.latest state.
+        '''
+        name = os.path.join(integration.TMP, 'salt_repo')
+        cwd = os.getcwd()
+        try:
+            # Clone repo
+            ret = self.run_state(
+                'git.latest',
+                name='https://{0}/saltstack/salt-test-repo.git'.format(self.__domain),
+                rev=rev,
+                target=name
+            )
+            self.assertSaltTrueReturn(ret)
+
+            # Check out a new branch in the clone and make a commit, to ensure
+            # that when we re-run the state, it is not a fast-forward change
+            os.chdir(name)
+            with salt.utils.fopen(os.devnull, 'w') as devnull:
+                subprocess.check_call(['git', 'checkout', '-b', 'new_branch'],
+                                      stdout=devnull, stderr=devnull)
+                with salt.utils.fopen('foo', 'w'):
+                    pass
+                subprocess.check_call(['git', 'add', '.'],
+                                      stdout=devnull, stderr=devnull)
+                subprocess.check_call(['git', 'commit', '-m', 'add file'],
+                                      stdout=devnull, stderr=devnull)
+            os.chdir(cwd)
+
+            # Re-run the state, this should fail with a specific hint in the
+            # comment field.
+            ret = self.run_state(
+                'git.latest',
+                name='https://{0}/saltstack/salt-test-repo.git'.format(self.__domain),
+                rev=rev,
+                target=name
+            )
+            self.assertSaltFalseReturn(ret)
+
+            comment = ret[next(iter(ret))]['comment']
+            self.assertTrue(hint in comment)
+        finally:
+            # Make sure that we change back to the original cwd even if there
+            # was a traceback in the test.
+            os.chdir(cwd)
+            shutil.rmtree(name, ignore_errors=True)
+
+    def test_latest_changed_local_branch_rev_head(self):
+        '''
+        Test for presence of hint in failure message when the local branch has
+        been changed and a the rev is set to HEAD
+
+        This test will fail if the default branch for the salt-test-repo is
+        ever changed.
+        '''
+        self._changed_local_branch_helper(
+            'HEAD',
+            'The default remote branch (develop) differs from the local '
+            'branch (new_branch)'
+        )
+
+    def test_latest_changed_local_branch_rev_develop(self):
+        '''
+        Test for presence of hint in failure message when the local branch has
+        been changed and a non-HEAD rev is specified
+        '''
+        self._changed_local_branch_helper(
+            'develop',
+            'The desired rev (develop) differs from the name of the local '
+            'branch (new_branch)'
+        )
+
     def test_present(self):
         '''
         git.present