Improve ansible-test environment checking between tests. (#46459)

* Add unified diff output to environment validation.

This makes it easier to see where the environment changed.

* Compare Python interpreters by version to pip shebangs.

This helps expose cases where pip executables use a different
Python interpreter than is expected.

* Query `pip.__version__` instead of using `pip --version`.

This is a much faster way to query the pip version. It also more
closely matches how we invoke pip within ansible-test.

* Remove redundant environment scan between tests.

This reuses the environment scan from the end of the previous test
as the basis for comparison during the next test.

(cherry picked from commit 0dc7f38787)

test/runner/lib/executor.py

@@ -14,6 +14,8 @@ import functools
 import pipes
 import sys
 import hashlib
+import difflib
+import filecmp
 
 import lib.pytar
 import lib.thread
@@ -741,6 +743,8 @@ def command_integration_filtered(args, targets, all_targets):
 
     results = {}
 
+    current_environment = None  # type: EnvironmentDescription | None
+
     for target in targets_iter:
         if args.start_at and not found:
             found = target.name == args.start_at
@@ -757,7 +761,8 @@ def command_integration_filtered(args, targets, all_targets):
 
         cloud_environment = get_cloud_environment(args, target)
 
-        original_environment = EnvironmentDescription(args)
+        original_environment = current_environment if current_environment else EnvironmentDescription(args)
+        current_environment = None
 
         display.info('>>> Environment Description\n%s' % original_environment, verbosity=3)
 
@@ -816,9 +821,11 @@ def command_integration_filtered(args, targets, all_targets):
                     display.verbosity = args.verbosity = 6
 
             start_time = time.time()
-            original_environment.validate(target.name, throw=True)
+            current_environment = EnvironmentDescription(args)
             end_time = time.time()
 
+            EnvironmentDescription.check(original_environment, current_environment, target.name, throw=True)
+
             results[target.name]['validation_seconds'] = int(end_time - start_time)
 
             passed.append(target)
@@ -1534,27 +1541,84 @@ class EnvironmentDescription(object):
             self.data = {}
             return
 
+        warnings = []
+
         versions = ['']
         versions += SUPPORTED_PYTHON_VERSIONS
         versions += list(set(v.split('.')[0] for v in SUPPORTED_PYTHON_VERSIONS))
 
         python_paths = dict((v, find_executable('python%s' % v, required=False)) for v in sorted(versions))
-        python_versions = dict((v, self.get_version([python_paths[v], '-V'])) for v in sorted(python_paths) if python_paths[v])
-
         pip_paths = dict((v, find_executable('pip%s' % v, required=False)) for v in sorted(versions))
-        pip_versions = dict((v, self.get_version([pip_paths[v], '--version'])) for v in sorted(pip_paths) if pip_paths[v])
+        program_versions = dict((v, self.get_version([python_paths[v], 'test/runner/versions.py'], warnings)) for v in sorted(python_paths) if python_paths[v])
         pip_interpreters = dict((v, self.get_shebang(pip_paths[v])) for v in sorted(pip_paths) if pip_paths[v])
         known_hosts_hash = self.get_hash(os.path.expanduser('~/.ssh/known_hosts'))
 
+        for version in sorted(versions):
+            self.check_python_pip_association(version, python_paths, pip_paths, pip_interpreters, warnings)
+
+        for warning in warnings:
+            display.warning(warning, unique=True)
+
         self.data = dict(
             python_paths=python_paths,
-            python_versions=python_versions,
             pip_paths=pip_paths,
-            pip_versions=pip_versions,
+            program_versions=program_versions,
             pip_interpreters=pip_interpreters,
             known_hosts_hash=known_hosts_hash,
+            warnings=warnings,
         )
 
+    @staticmethod
+    def check_python_pip_association(version, python_paths, pip_paths, pip_interpreters, warnings):
+        """
+        :type version: str
+        :param python_paths: dict[str, str]
+        :param pip_paths:  dict[str, str]
+        :param pip_interpreters:  dict[str, str]
+        :param warnings: list[str]
+        """
+        python_label = 'Python%s' % (' %s' % version if version else '')
+
+        pip_path = pip_paths.get(version)
+        python_path = python_paths.get(version)
+
+        if not python_path and not pip_path:
+            # neither python or pip is present for this version
+            return
+
+        if not python_path:
+            warnings.append('A %s interpreter was not found, yet a matching pip was found at "%s".' % (python_label, pip_path))
+            return
+
+        if not pip_path:
+            warnings.append('A %s interpreter was found at "%s", yet a matching pip was not found.' % (python_label, python_path))
+            return
+
+        pip_shebang = pip_interpreters.get(version)
+
+        match = re.search(r'#!\s*(?P<command>[^\s]+)', pip_shebang)
+
+        if not match:
+            warnings.append('A %s pip was found at "%s", but it does not have a valid shebang: %s' % (python_label, pip_path, pip_shebang))
+            return
+
+        pip_interpreter = os.path.realpath(match.group('command'))
+        python_interpreter = os.path.realpath(python_path)
+
+        if pip_interpreter == python_interpreter:
+            return
+
+        try:
+            identical = filecmp.cmp(pip_interpreter, python_interpreter)
+        except OSError:
+            identical = False
+
+        if identical:
+            return
+
+        warnings.append('A %s pip was found at "%s", but it uses interpreter "%s" instead of "%s".' % (
+            python_label, pip_path, pip_interpreter, python_interpreter))
+
     def __str__(self):
         """
         :rtype: str
@@ -1569,18 +1633,40 @@ class EnvironmentDescription(object):
         """
         current = EnvironmentDescription(self.args)
 
-        original_json = str(self)
+        return self.check(self, current, target_name, throw)
+
+    @staticmethod
+    def check(original, current, target_name, throw):
+        """
+        :type original: EnvironmentDescription
+        :type current: EnvironmentDescription
+        :type target_name: str
+        :type throw: bool
+        :rtype: bool
+        """
+        original_json = str(original)
         current_json = str(current)
 
         if original_json == current_json:
             return True
 
+        unified_diff = '\n'.join(difflib.unified_diff(
+            a=original_json.splitlines(),
+            b=current_json.splitlines(),
+            fromfile='original.json',
+            tofile='current.json',
+            lineterm='',
+        ))
+
         message = ('Test target "%s" has changed the test environment!\n'
                    'If these changes are necessary, they must be reverted before the test finishes.\n'
                    '>>> Original Environment\n'
                    '%s\n'
                    '>>> Current Environment\n'
-                   '%s' % (target_name, original_json, current_json))
+                   '%s\n'
+                   '>>> Environment Diff\n'
+                   '%s'
+                   % (target_name, original_json, current_json, unified_diff))
 
         if throw:
             raise ApplicationError(message)
@@ -1590,17 +1676,19 @@ class EnvironmentDescription(object):
         return False
 
     @staticmethod
-    def get_version(command):
+    def get_version(command, warnings):
         """
         :type command: list[str]
-        :rtype: str
+        :type warnings: list[str]
+        :rtype: list[str]
         """
         try:
             stdout, stderr = raw_command(command, capture=True, cmd_verbosity=2)
-        except SubprocessError:
+        except SubprocessError as ex:
+            warnings.append(u'%s' % ex)
             return None  # all failures are equal, we don't care why it failed, only that it did
 
-        return (stdout or '').strip() + (stderr or '').strip()
+        return [line.strip() for line in ((stdout or '').strip() + (stderr or '').strip()).splitlines()]
 
     @staticmethod
     def get_shebang(path):
@@ -1609,7 +1697,7 @@ class EnvironmentDescription(object):
         :rtype: str
         """
         with open(path) as script_fd:
-            return script_fd.readline()
+            return script_fd.readline().strip()
 
     @staticmethod
     def get_hash(path):

test/runner/versions.py

@@ -0,0 +1,15 @@
+#!/usr/bin/env python
+"""Show python and pip versions."""
+
+import os
+import sys
+
+try:
+    import pip
+except ImportError:
+    pip = None
+
+print(sys.version)
+
+if pip:
+    print('pip %s from %s' % (pip.__version__, os.path.dirname(pip.__file__)))