Refactor ipautil.run

The ipautil.run function now returns an object with returncode and
output are accessible as attributes.

The stdout and stderr of all commands are logged (unless skip_output is given).

The stdout/stderr contents must be explicitly requested with a keyword
argument, otherwise they are None.
This is because in Python 3, the output needs to be decoded, and that can
fail if it's not decodable (human-readable) text.

The raw (bytes) output is always available from the result object,
as is "leniently" decoded output suitable for logging.

All calls are changed to reflect this.

A use of Popen in cainstance is changed to ipautil.run.

Reviewed-By: Jan Cholasta <jcholast@redhat.com>

4 files changed, 43 insertions, 27 deletions

diff --git a/install/certmonger/dogtag-ipa-ca-renew-agent-submit b/install/certmonger/dogtag-ipa-ca-renew-agent-submit
index 44993b038..5a6b7fa22 100755
--- a/install/certmonger/dogtag-ipa-ca-renew-agent-submit
+++ b/install/certmonger/dogtag-ipa-ca-renew-agent-submit
@@ -156,15 +156,23 @@ def request_cert():
     args = [path] + sys.argv[1:]
     if os.environ.get('CERTMONGER_CA_PROFILE') == 'caCACert':
         args += ['-N', '-O', 'bypassCAnotafter=true']
-    stdout, stderr, rc = ipautil.run(args, raiseonerr=False, env=os.environ)
-    sys.stderr.write(stderr)
+    result = ipautil.run(args, raiseonerr=False, env=os.environ,
+                        capture_output=True)
+    if six.PY2:
+        sys.stderr.write(result.raw_error_output)
+    else:
+        # Write bytes directly
+        sys.stderr.buffer.write(result.raw_error_output)  #pylint: disable=no-member
     sys.stderr.flush()
 
-    syslog.syslog(syslog.LOG_NOTICE, "dogtag-ipa-renew-agent returned %d" % rc)
+    syslog.syslog(syslog.LOG_NOTICE,
+                  "dogtag-ipa-renew-agent returned %d" % result.returncode)
 
+    stdout = result.output
     if stdout.endswith('\n'):
         stdout = stdout[:-1]
 
+    rc = result.returncode
     if rc == WAIT_WITH_DELAY:
         delay, sep, cookie = stdout.partition('\n')
         return (rc, delay, cookie)
diff --git a/install/certmonger/ipa-server-guard b/install/certmonger/ipa-server-guard
index 7ce3e43fc..2c5409718 100755
--- a/install/certmonger/ipa-server-guard
+++ b/install/certmonger/ipa-server-guard
@@ -30,6 +30,8 @@ import sys
 import syslog
 import traceback
 
+import six
+
 from ipapython import ipautil
 from ipaserver.install import certs
 
@@ -39,14 +41,18 @@ def main():
         raise RuntimeError("Not enough arguments")
 
     with certs.renewal_lock:
-        stdout, stderr, rc = ipautil.run(sys.argv[1:], raiseonerr=False,
-                                         env=os.environ)
-        sys.stdout.write(stdout)
+        result = ipautil.run(sys.argv[1:], raiseonerr=False, env=os.environ)
+        if six.PY2:
+            sys.stdout.write(result.raw_output)
+            sys.stderr.write(result.raw_error_output)
+        else:
+            # Write bytes directly
+            sys.stdout.buffer.write(result.raw_output)  #pylint: disable=no-member
+            sys.stderr.buffer.write(result.raw_error_output)  #pylint: disable=no-member
         sys.stdout.flush()
-        sys.stderr.write(stderr)
         sys.stderr.flush()
 
-    return rc
+    return result.returncode
 
 
 try:
diff --git a/install/oddjob/com.redhat.idm.trust-fetch-domains b/install/oddjob/com.redhat.idm.trust-fetch-domains
index 019545b93..ea82e086e 100755
--- a/install/oddjob/com.redhat.idm.trust-fetch-domains
+++ b/install/oddjob/com.redhat.idm.trust-fetch-domains
@@ -26,9 +26,8 @@ def retrieve_keytab(api, ccache_name, oneway_keytab_name, oneway_principal):
     if os.path.isfile(oneway_keytab_name):
         os.unlink(oneway_keytab_name)
 
-    (stdout, stderr, retcode) = ipautil.run(getkeytab_args,
-                                            env={'KRB5CCNAME': ccache_name, 'LANG': 'C'},
-                                            raiseonerr=False)
+    ipautil.run(getkeytab_args, env={'KRB5CCNAME': ccache_name, 'LANG': 'C'},
+                raiseonerr=False)
     # Make sure SSSD is able to read the keytab
     try:
         sssd = pwd.getpwnam('sssd')
diff --git a/install/tools/ipa-replica-conncheck b/install/tools/ipa-replica-conncheck
index 10e3437bd..fadc61314 100755
--- a/install/tools/ipa-replica-conncheck
+++ b/install/tools/ipa-replica-conncheck
@@ -82,7 +82,8 @@ class SshExec(object):
         elif 'KRB5CCNAME' in os.environ:
             env['KRB5CCNAME'] = os.environ['KRB5CCNAME']
 
-        return ipautil.run(cmd, env=env, raiseonerr=False)
+        return ipautil.run(cmd, env=env, raiseonerr=False,
+                           capture_output=True, capture_error=True)
 
 
 class CheckedPort(object):
@@ -432,21 +433,21 @@ def main():
                         sys.exit("Principal password required")
 
 
-                stderr=''
-                (stdout, stderr, returncode) = ipautil.run([paths.KINIT, principal],
+                result = ipautil.run([paths.KINIT, principal],
                      env={'KRB5_CONFIG':KRB5_CONFIG, 'KRB5CCNAME':CCACHE_FILE},
-                    stdin=password, raiseonerr=False)
-                if returncode != 0:
-                    raise RuntimeError("Cannot acquire Kerberos ticket: %s" % stderr)
+                    stdin=password, raiseonerr=False, capture_error=True)
+                if result.returncode != 0:
+                    raise RuntimeError("Cannot acquire Kerberos ticket: %s" %
+                                        result.error_output)
 
                 # Verify kinit was actually successful
-                stderr=''
-                (stdout, stderr, returncode) = ipautil.run([paths.BIN_KVNO,
+                result = ipautil.run([paths.BIN_KVNO,
                      'host/%s' % options.master],
                     env={'KRB5_CONFIG':KRB5_CONFIG, 'KRB5CCNAME':CCACHE_FILE},
-                    raiseonerr=False)
-                if returncode != 0:
-                    raise RuntimeError("Could not get ticket for master server: %s" % stderr)
+                    raiseonerr=False, capture_error=True)
+                if result.returncode != 0:
+                    raise RuntimeError("Could not get ticket for master server: %s" %
+                                        result.error_output)
 
             try:
                 print_info("Check RPC connection to remote master")
@@ -519,18 +520,20 @@ def main():
                 ssh = SshExec(user, options.master)
 
                 print_info("Check SSH connection to remote master")
-                stdout, stderr, returncode = ssh('echo OK', verbose=True)
-                if returncode != 0:
+                result = ssh('echo OK', verbose=True)
+                if result.returncode != 0:
                     print('Could not SSH into remote host. Error output:')
-                    for line in stderr.splitlines():
+                    for line in result.error_output.splitlines():
                         print('    %s' % line)
                     raise RuntimeError('Could not SSH to remote host.')
 
                 print_info("Execute check on remote master")
-                stdout, stderr, returncode = ssh(
+                result = ssh(
                     "/usr/sbin/ipa-replica-conncheck " +
                         " ".join(remote_check_opts))
-                print_info(stdout)
+                returncode = result.returncode
+                stderr = result.error_output
+                print_info(result.output)
             if returncode != 0:
                 raise RuntimeError("Remote master check failed with following error message(s):\n%s" % stderr)
         else: