Rename checkKinit to check_kinit and use subprocess.run instead of Popen

diff --git a/python/invirt/remctl.py b/python/invirt/remctl.py
index a835001..d64a10f 100644 (file)
--- a/python/invirt/remctl.py
+++ b/python/invirt/remctl.py
@@ -13,16 +13,17 @@ def kinit(principal=None, keytab=None):
         principal = 'daemon/' + getfqdn()
     if keytab is None:
         keytab = '/etc/invirt/keytab'
-    p = subprocess.Popen(['kinit', "-k", "-t", keytab, principal],
-                         stderr=subprocess.PIPE)
-    e = p.wait()
-    if e:
-        raise CodeError("Error %s in kinit: %s" % (e, p.stderr.read()))
 
-def checkKinit(principal=None, keytab=None):
+    subprocess.run(['kinit', '-k', '-t', keytab, principal],
+                   check_output=True, encoding='utf-8', check=True)
+
+def check_kinit(principal=None, keytab=None):
     """If we lack tickets, kinit."""
-    p = subprocess.Popen(['klist', '-s'])
-    if p.wait():
+
+    try:
+        subprocess.run(['klist', '-s'])
+    except subprocess.CalledProcessError:
+        #TODO: Does this return a specific error code that we can check for?
         kinit(principal, keytab)
 
 def remctl(host, *args, **kws):
@@ -30,16 +31,8 @@ def remctl(host, *args, **kws):
 
     kinits if necessary, and outputs errors to stderr.
     """
-    checkKinit(kws.get('principal'), kws.get('keytab'))
-    p = subprocess.Popen(['remctl', host]
-                         + list(args),
-                         stdout=subprocess.PIPE,
-                         stderr=subprocess.PIPE)
-    stdout, stderr = p.communicate()
-    if kws.get('err'):
-        return stdout, stderr
-    if p.returncode:
-        print('Error', p.returncode, 'on remctl', args, ':', file=sys.stderr)
-        print(stderr, file=sys.stderr)
-        raise CodeError('ERROR on remctl')
-    return stdout
+
+    check_kinit(kwargs.get('principal'), kwargs.get('keytab'))
+
+    return subprocess.run(['remctl', host] + list(args),
+                          check_output=True, encoding='utf-8', check=True)