Merge pull request #307 from thomas-mangin/T2226

util: T2226: convert all call to use vyos.util.{popen, cmd, run}

25 files changed, 192 insertions, 230 deletions

diff --git a/src/op_mode/clear_conntrack.py b/src/op_mode/clear_conntrack.py
index 0e52b9086..423694187 100755
--- a/src/op_mode/clear_conntrack.py
+++ b/src/op_mode/clear_conntrack.py
@@ -14,13 +14,13 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-import subprocess
 import sys
 
 from vyos.util import ask_yes_no
+from vyos.util import cmd, DEVNULL
 
 if not ask_yes_no("This will clear all currently tracked and expected connections. Continue?"):
     sys.exit(1)
 else:
-    subprocess.check_call(['/usr/sbin/conntrack -F'], shell=True, stderr=subprocess.DEVNULL)
-    subprocess.check_call(['/usr/sbin/conntrack -F expect'], shell=True, stderr=subprocess.DEVNULL)
+    cmd('/usr/sbin/conntrack -F', stderr=DEVNULL)
+    cmd('/usr/sbin/conntrack -F expect', stderr=DEVNULL)
diff --git a/src/op_mode/connect_disconnect.py b/src/op_mode/connect_disconnect.py
index a22615096..192fd80ec 100755
--- a/src/op_mode/connect_disconnect.py
+++ b/src/op_mode/connect_disconnect.py
@@ -21,6 +21,9 @@ from sys import exit
 from psutil import process_iter
 from time import strftime, localtime, time
 
+from vyos.util import run
+
+
 PPP_LOGFILE = '/var/log/vyatta/ppp_{}.log'
 
 def check_interface(interface):
@@ -56,8 +59,7 @@ def connect(interface):
         tm = strftime("%a %d %b %Y %I:%M:%S %p %Z", localtime(time()))
         with open(PPP_LOGFILE.format(interface), 'a') as f:
             f.write('{}: user {} started PPP daemon for {} by connect command\n'.format(tm, user, interface))
-            cmd = 'umask 0; setsid sh -c "nohup /usr/sbin/pppd call {0} > /tmp/{0}.log 2>&1 &"'.format(interface)
-            os.system(cmd)
+            run('umask 0; setsid sh -c "nohup /usr/sbin/pppd call {0} > /tmp/{0}.log 2>&1 &"'.format(interface))
 
 
 def disconnect(interface):
@@ -75,8 +77,7 @@ def disconnect(interface):
         tm = strftime("%a %d %b %Y %I:%M:%S %p %Z", localtime(time()))
         with open(PPP_LOGFILE.format(interface), 'a') as f:
             f.write('{}: user {} stopped PPP daemon for {} by disconnect command\n'.format(tm, user, interface))
-            cmd = '/usr/bin/poff "{}"'.format(interface)
-            os.system(cmd)
+            run('/usr/bin/poff "{}"'.format(interface))
 
 def main():
     parser = argparse.ArgumentParser()
diff --git a/src/op_mode/dns_forwarding_reset.py b/src/op_mode/dns_forwarding_reset.py
index da4fba3a2..93c2444b9 100755
--- a/src/op_mode/dns_forwarding_reset.py
+++ b/src/op_mode/dns_forwarding_reset.py
@@ -25,6 +25,8 @@ import sys
 import argparse
 
 import vyos.config
+from vyos.util import run
+
 
 parser = argparse.ArgumentParser()
 parser.add_argument("-a", "--all", action="store_true", help="Reset all cache")
@@ -40,10 +42,10 @@ if __name__ == '__main__':
         sys.exit(0)
 
     if args.all:
-        os.system("rec_control wipe-cache \'.$\'")
+        run("rec_control wipe-cache \'.$\'")
         sys.exit(1)
     elif args.domain:
-        os.system("rec_control wipe-cache \'{0}$\'".format(args.domain))
+        run("rec_control wipe-cache \'{0}$\'".format(args.domain))
     else:
         parser.print_help()
         sys.exit(1)
diff --git a/src/op_mode/dns_forwarding_statistics.py b/src/op_mode/dns_forwarding_statistics.py
index f626244a8..c400a72cd 100755
--- a/src/op_mode/dns_forwarding_statistics.py
+++ b/src/op_mode/dns_forwarding_statistics.py
@@ -1,10 +1,10 @@
 #!/usr/bin/env python3
 
-import subprocess
 import jinja2
 import sys
 
 from vyos.config import Config
+from vyos.config import cmd
 
 PDNS_CMD='/usr/bin/rec_control'
 
@@ -26,8 +26,8 @@ if __name__ == '__main__':
 
     data = {}
 
-    data['cache_entries'] = subprocess.check_output([PDNS_CMD, 'get cache-entries']).decode()
-    data['cache_size'] = "{0:.2f}".format( int(subprocess.check_output([PDNS_CMD, 'get cache-bytes']).decode()) / 1024 )
+    data['cache_entries'] = cmd(f'{PDNS_CMD} get cache-entries')
+    data['cache_size'] = "{0:.2f}".format( int(cmd(f'{PDNS_CMD} get cache-bytes')) / 1024 )
 
     tmpl = jinja2.Template(OUT_TMPL_SRC)
     print(tmpl.render(data))
diff --git a/src/op_mode/dynamic_dns.py b/src/op_mode/dynamic_dns.py
index 0d457e247..d991848ad 100755
--- a/src/op_mode/dynamic_dns.py
+++ b/src/op_mode/dynamic_dns.py
@@ -21,6 +21,8 @@ import sys
 import time
 
 from vyos.config import Config
+from vyos.util import run
+
 
 cache_file = r'/var/cache/ddclient/ddclient.cache'
 
@@ -84,9 +86,9 @@ def show_status():
 
 
 def update_ddns():
-    os.system('systemctl stop ddclient')
+    run('systemctl stop ddclient')
     os.remove(cache_file)
-    os.system('systemctl start ddclient')
+    run('systemctl start ddclient')
 
 
 def main():
diff --git a/src/op_mode/flow_accounting_op.py b/src/op_mode/flow_accounting_op.py
index a39eaf871..7f3ad7476 100755
--- a/src/op_mode/flow_accounting_op.py
+++ b/src/op_mode/flow_accounting_op.py
@@ -19,10 +19,11 @@ import sys
 import argparse
 import re
 import ipaddress
-import subprocess
 import os.path
 from tabulate import tabulate
 
+from vyos.util import cmd, run
+
 # some default values
 uacctd_pidfile = '/var/run/uacctd.pid'
 uacctd_pipefile = '/tmp/uacctd.pipe'
@@ -69,26 +70,16 @@ def _is_host(host):
 
 # check if flow-accounting running
 def _uacctd_running():
-    command = "/usr/bin/sudo /bin/systemctl status uacctd > /dev/null"
-    return_code = subprocess.call(command, shell=True)
-    if not return_code == 0:
-        return False
-
-    # return True if all checks were passed
-    return True
+    command = '/usr/bin/sudo /bin/systemctl status uacctd > /dev/null'
+    return run(command) == 0
 
 # get list of interfaces
 def _get_ifaces_dict():
     # run command to get ifaces list
-    command = "/bin/ip link show"
-    process = subprocess.Popen(command.split(' '), stdout=subprocess.PIPE, universal_newlines=True)
-    stdout, stderr = process.communicate()
-    if not process.returncode == 0:
-        print("Failed to get interfaces list: command \"{}\" returned exit code: {}".format(command, process.returncode))
-        sys.exit(1)
+    out = cmd('/bin/ip link show', universal_newlines=True)
 
     # read output
-    ifaces_out = stdout.splitlines()
+    ifaces_out = out.splitlines()
 
     # make a dictionary with interfaces and indexes
     ifaces_dict = {}
@@ -103,15 +94,12 @@ def _get_ifaces_dict():
 # get list of flows
 def _get_flows_list():
     # run command to get flows list
-    command = "/usr/bin/pmacct -s -O json -T flows -p {}".format(uacctd_pipefile)
-    process = subprocess.Popen(command.split(' '), stdout=subprocess.PIPE, universal_newlines=True)
-    stdout, stderr = process.communicate()
-    if not process.returncode == 0:
-        print("Failed to get flows list: command \"{}\" returned exit code: {}\nError: {}".format(command, process.returncode, stderr))
-        sys.exit(1)
+    out = cmd(f'/usr/bin/pmacct -s -O json -T flows -p {uacctd_pipefile}',
+                universal_newlines=True,
+                message='Failed to get flows list')
 
     # read output
-    flows_out = stdout.splitlines()
+    flows_out = out.splitlines()
 
     # make a list with flows
     flows_list = []
@@ -208,21 +196,15 @@ if not _uacctd_running():
 # restart pmacct daemon
 if cmd_args.action == 'restart':
     # run command to restart flow-accounting
-    command = '/usr/bin/sudo /bin/systemctl restart uacctd'
-    return_code = subprocess.call(command.split(' '))
-    if not return_code == 0:
-        print("Failed to restart flow-accounting: command \"{}\" returned exit code: {}".format(command, return_code))
-        sys.exit(1)
+    cmd('/usr/bin/sudo /bin/systemctl restart uacctd',
+        message='Failed to restart flow-accounting')
 
 # clear in-memory collected flows
 if cmd_args.action == 'clear':
     _check_imt()
     # run command to clear flows
-    command = "/usr/bin/pmacct -e -p {}".format(uacctd_pipefile)
-    return_code = subprocess.call(command.split(' '))
-    if not return_code == 0:
-        print("Failed to clear flows: command \"{}\" returned exit code: {}".format(command, return_code))
-        sys.exit(1)
+    cmd(f'/usr/bin/pmacct -e -p {uacctd_pipefile}',
+        message='Failed to clear flows')
 
 # show table with flows
 if cmd_args.action == 'show':
diff --git a/src/op_mode/format_disk.py b/src/op_mode/format_disk.py
index 5a3b250ee..9d3797f17 100755
--- a/src/op_mode/format_disk.py
+++ b/src/op_mode/format_disk.py
@@ -17,13 +17,12 @@
 import argparse
 import os
 import re
-import subprocess
 import sys
 from datetime import datetime
 from time import sleep
 
 from vyos.util import is_admin, ask_yes_no
-
+from vyos.util import run, cmd, DEVNULL
 
 def list_disks():
     disks = set()
@@ -37,10 +36,7 @@ def list_disks():
 
 def is_busy(disk: str):
     """Check if given disk device is busy by re-reading it's partition table"""
-
-    cmd = 'sudo blockdev --rereadpt /dev/{}'.format(disk)
-    status = subprocess.call([cmd], shell=True, stderr=subprocess.DEVNULL)
-    return status != 0
+    return run(f'sudo blockdev --rereadpt /dev/{disk}', stderr=DEVNULL) != 0
 
 
 def backup_partitions(disk: str):
@@ -49,8 +45,7 @@ def backup_partitions(disk: str):
     device_path = '/dev/' + disk
     backup_ts = datetime.now().strftime('%Y-%m-%d-%H:%M')
     backup_file = '/var/tmp/backup_{}.{}'.format(disk, backup_ts)
-    cmd = 'sudo /sbin/sfdisk -d {} > {}'.format(device_path, backup_file)
-    subprocess.check_call([cmd], shell=True)
+    cmd(f'sudo /sbin/sfdisk -d {device_path} > {backup_file}')
 
 
 def list_partitions(disk: str):
@@ -68,13 +63,11 @@ def list_partitions(disk: str):
 
 
 def delete_partition(disk: str, partition_idx: int):
-    cmd = 'sudo /sbin/parted /dev/{} rm {}'.format(disk, partition_idx)
-    subprocess.check_call([cmd], shell=True)
+    cmd(f'sudo /sbin/parted /dev/{disk} rm {partition_idx}')
 
 
 def format_disk_like(target: str, proto: str):
-    cmd = 'sudo /sbin/sfdisk -d /dev/{} | sudo /sbin/sfdisk --force /dev/{}'.format(proto, target)
-    subprocess.check_call([cmd], shell=True)
+    cmd(f'sudo /sbin/sfdisk -d /dev/{proto} | sudo /sbin/sfdisk --force /dev/{target}')
 
 
 if __name__ == '__main__':
diff --git a/src/op_mode/generate_ssh_server_key.py b/src/op_mode/generate_ssh_server_key.py
index f205919b8..f65d383c0 100755
--- a/src/op_mode/generate_ssh_server_key.py
+++ b/src/op_mode/generate_ssh_server_key.py
@@ -14,14 +14,14 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-import subprocess
 import sys
 
 from vyos.util import ask_yes_no
+from vyos.util import cmd
 
 if not ask_yes_no('Do you really want to remove the existing SSH host keys?'):
     sys.exit(0)
-else:
-    subprocess.check_call(['sudo rm -v /etc/ssh/ssh_host_*'], shell=True)
-    subprocess.check_call(['sudo dpkg-reconfigure openssh-server'], shell=True)
-    subprocess.check_call(['sudo systemctl restart ssh'], shell=True)
+
+cmd('sudo rm -v /etc/ssh/ssh_host_*')
+cmd('sudo dpkg-reconfigure openssh-server')
+cmd('sudo systemctl restart ssh')
diff --git a/src/op_mode/lldp_op.py b/src/op_mode/lldp_op.py
index 4d8fdbc99..c8a5543b6 100755
--- a/src/op_mode/lldp_op.py
+++ b/src/op_mode/lldp_op.py
@@ -20,9 +20,10 @@ import jinja2
 
 from xml.dom import minidom
 from sys import exit
-from subprocess import Popen, PIPE, STDOUT
 from tabulate import tabulate
 
+from vyos.util import popen
+
 parser = argparse.ArgumentParser()
 parser.add_argument("-a", "--all", action="store_true", help="Show LLDP neighbors on all interfaces")
 parser.add_argument("-i", "--interface", action="store", help="Show LLDP neighbors on specific interface")
@@ -40,9 +41,8 @@ Device ID                 Local     Proto  Cap   Platform             Port ID
 
 def _get_neighbors():
     command = '/usr/sbin/lldpcli -f xml show neighbors'
-    p = Popen(command, stdout=PIPE, stderr=STDOUT, shell=True)
-    tmp = p.communicate()[0].strip()
-    return tmp.decode()
+    out,_ = popen(command)
+    return out
 
 def extract_neighbor(neighbor):
     """
diff --git a/src/op_mode/powerctrl.py b/src/op_mode/powerctrl.py
index 54fc12be3..772bb8198 100755
--- a/src/op_mode/powerctrl.py
+++ b/src/op_mode/powerctrl.py
@@ -17,12 +17,11 @@
 import os
 import sys
 import argparse
-import subprocess
 import re
 
 from datetime import datetime, timedelta, time as type_time, date as type_date
-from subprocess import check_output, CalledProcessError, STDOUT
 from vyos.util import ask_yes_no
+from vyos.util import cmd, run
 
 systemd_sched_file = "/run/systemd/shutdown/scheduled"
 
@@ -45,23 +44,20 @@ def parse_date(s):
   return None
 
 def get_shutdown_status():
-  try:
-    if os.path.exists(systemd_sched_file):
-      # Get scheduled from systemd file
-      with open(systemd_sched_file, 'r') as f:
-        data = f.read().rstrip('\n')
-        r_data = {}
-        for line in data.splitlines():
-          tmp_split = line.split("=")
-          if tmp_split[0] == "USEC":
-            # Convert USEC to  human readable format
-            r_data['DATETIME'] = datetime.utcfromtimestamp(int(tmp_split[1])/1000000).strftime('%Y-%m-%d %H:%M:%S')
-          else:
-            r_data[tmp_split[0]] = tmp_split[1]
-        return r_data
-    return None
-  except CalledProcessError:
-    return None
+  if os.path.exists(systemd_sched_file):
+    # Get scheduled from systemd file
+    with open(systemd_sched_file, 'r') as f:
+      data = f.read().rstrip('\n')
+      r_data = {}
+      for line in data.splitlines():
+        tmp_split = line.split("=")
+        if tmp_split[0] == "USEC":
+          # Convert USEC to  human readable format
+          r_data['DATETIME'] = datetime.utcfromtimestamp(int(tmp_split[1])/1000000).strftime('%Y-%m-%d %H:%M:%S')
+        else:
+          r_data[tmp_split[0]] = tmp_split[1]
+      return r_data
+  return None
 
 def check_shutdown():
   output = get_shutdown_status()
@@ -76,13 +72,13 @@ def check_shutdown():
 def cancel_shutdown():
   output = get_shutdown_status()
   if output and 'MODE' in output:
+    timenow = datetime.now().strftime('%Y-%m-%d %H:%M:%S')
     try:
-      timenow = datetime.now().strftime('%Y-%m-%d %H:%M:%S')
-      cmd = check_output(["/sbin/shutdown","-c","--no-wall"])
-      message = "Scheduled %s has been cancelled %s" % (output['MODE'], timenow)
-      os.system("wall %s" % message)
-    except CalledProcessError as e:
+      cmd('/sbin/shutdown -c --no-wall')
+    except OSError as e:
       sys.exit("Could not cancel a reboot or poweroff: %s" % e)
+    message = "Scheduled %s has been cancelled %s" % (output['MODE'], timenow)
+    run(f'wall {message}')
   else:
     print("Reboot or poweroff is not scheduled")
 
@@ -99,14 +95,14 @@ def execute_shutdown(time, reboot = True, ask=True):
     chk_vyatta_based_reboots()
     ###
 
-    cmd = check_output(["/sbin/shutdown",action,"now"],stderr=STDOUT)
-    print(cmd.decode().split(",",1)[0])
+    out = cmd(f'/sbin/shutdown {action} now')
+    print(out.split(",",1)[0])
     return
   elif len(time) == 1:
     # Assume the argument is just time
     ts = parse_time(time[0])
     if ts:
-      cmd = check_output(["/sbin/shutdown", action, time[0]], stderr=STDOUT)
+      cmd(f'/sbin/shutdown {action} {time[0]}')
     else:
       sys.exit("Invalid time \"{0}\". The valid format is HH:MM".format(time[0]))
   elif len(time) == 2:
@@ -117,7 +113,7 @@ def execute_shutdown(time, reboot = True, ask=True):
       t = datetime.combine(ds, ts)
       td = t - datetime.now()
       t2 = 1 + int(td.total_seconds())//60 # Get total minutes
-      cmd = check_output(["/sbin/shutdown", action, str(t2)], stderr=STDOUT)
+      cmd('/sbin/shutdown {action} {t2}')
     else:
       if not ts:
         sys.exit("Invalid time \"{0}\". The valid format is HH:MM".format(time[0]))
@@ -136,7 +132,7 @@ def chk_vyatta_based_reboots():
   if os.path.exists(f):
     jid = open(f).read().strip()
     if jid != 0:
-      subprocess.call(['sudo', 'atrm', jid])
+      run(f'sudo atrm {jid}')
     os.remove(f)
 
 def main():
diff --git a/src/op_mode/reset_openvpn.py b/src/op_mode/reset_openvpn.py
index 4c29fbbba..618cad5ea 100755
--- a/src/op_mode/reset_openvpn.py
+++ b/src/op_mode/reset_openvpn.py
@@ -17,10 +17,9 @@
 import sys
 import os
 
-from subprocess import Popen, PIPE
 from time import sleep
 from netifaces import interfaces
-from vyos.util import process_running
+from vyos.util import process_running, cmd
 
 def get_config_name(intf):
     cfg_file = r'/opt/vyatta/etc/openvpn/openvpn-{}.conf'.format(intf)
@@ -30,9 +29,6 @@ def get_pid_file(intf):
     pid_file = r'/var/run/openvpn/{}.pid'.format(intf)
     return pid_file
 
-def subprocess_cmd(command):
-    p = Popen(command, stdout=PIPE, shell=True)
-    p.communicate()
 
 if __name__ == '__main__':
     if (len(sys.argv) < 1):
@@ -43,12 +39,12 @@ if __name__ == '__main__':
     if os.path.isfile(get_config_name(interface)):
         pidfile = '/var/run/openvpn/{}.pid'.format(interface)
         if process_running(pidfile):
-            cmd = 'start-stop-daemon'
-            cmd += ' --stop'
-            cmd += ' --oknodo'
-            cmd += ' --quiet'
-            cmd += ' --pidfile ' + pidfile
-            subprocess_cmd(cmd)
+            command = 'start-stop-daemon'
+            command += ' --stop'
+            command += ' --oknodo'
+            command += ' --quiet'
+            command += ' --pidfile ' + pidfile
+            cmd(command)
 
         # When stopping OpenVPN we need to wait for the 'old' interface to
         # vanish from the Kernel, if it is not gone, OpenVPN will report:
@@ -57,18 +53,18 @@ if __name__ == '__main__':
             sleep(0.250) # 250ms
 
         # re-start OpenVPN process
-        cmd = 'start-stop-daemon'
-        cmd += ' --start'
-        cmd += ' --oknodo'
-        cmd += ' --quiet'
-        cmd += ' --pidfile ' + get_pid_file(interface)
-        cmd += ' --exec /usr/sbin/openvpn'
+        command = 'start-stop-daemon'
+        command += ' --start'
+        command += ' --oknodo'
+        command += ' --quiet'
+        command += ' --pidfile ' + get_pid_file(interface)
+        command += ' --exec /usr/sbin/openvpn'
         # now pass arguments to openvpn binary
-        cmd += ' --'
-        cmd += ' --daemon openvpn-' + interface
-        cmd += ' --config ' + get_config_name(interface)
+        command += ' --'
+        command += ' --daemon openvpn-' + interface
+        command += ' --config ' + get_config_name(interface)
 
-        subprocess_cmd(cmd)
+        cmd(command)
     else:
         print("OpenVPN interface {} does not exist!".format(interface))
         sys.exit(1)
diff --git a/src/op_mode/reset_vpn.py b/src/op_mode/reset_vpn.py
index 52677b58d..b47212f88 100755
--- a/src/op_mode/reset_vpn.py
+++ b/src/op_mode/reset_vpn.py
@@ -16,52 +16,53 @@
 
 # import os
 import sys
-import subprocess
 import argparse
 #import re
 
-pptp_cmd = ["/usr/bin/accel-cmd", "-p 2003"]
-l2tp_cmd = ["/usr/bin/accel-cmd", "-p 2004"]
+from vyos.util import run, DEVNULL
+
+pptp_base = '/usr/bin/accel-cmd -p 2003 terminate {} {}'
+l2tp_base = '/usr/bin/accel-cmd -p 2004 terminate {} {}'
 
 def terminate_sessions(username='', interface='', protocol=''):
     if username:
         if username == "all_users":
             if protocol == "pptp":
-                pptp_cmd.append("terminate all")
-                subprocess.call(pptp_cmd, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
+                pptp_cmd = pptp_base.format('all','')
+                run(pptp_cmd, stdout=DEVNULL, stderr=DEVNULL)
                 return
             elif protocol == "l2tp":
-                l2tp_cmd.append("terminate all")
-                subprocess.call(l2tp_cmd, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
+                l2tp_cmd = l2tp_base.format('all', '')
+                run(l2tp_cmd, stdout=DEVNULL, stderr=DEVNULL)
                 return
             else:
-                pptp_cmd.append("terminate all")
-                subprocess.call(pptp_cmd, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
-                l2tp_cmd.append("terminate all")
-                subprocess.call(l2tp_cmd, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
+                pptp_cmd = pptp_base.format('all', '')
+                run(pptp_cmd, stdout=DEVNULL, stderr=DEVNULL)
+                l2tp_cmd = l2tp_base.format('all', '')
+                run(l2tp_cmd, stdout=DEVNULL, stderr=DEVNULL)
                 return
 
         if protocol == "pptp":
-            pptp_cmd.append("terminate username {0}".format(username))
-            subprocess.call(pptp_cmd, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
+            pptp_cmd = pptp_base.format('username', username)
+            run(pptp_cmd, stdout=DEVNULL, stderr=DEVNULL)
             return
         elif protocol == "l2tp":
-            l2tp_cmd.append("terminate username {0}".format(username))
-            subprocess.call(l2tp_cmd, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
+            l2tp_cmd = l2tp_base.format('username', username)
+            run(l2tp_cmd, stdout=DEVNULL, stderr=DEVNULL)
             return
         else:
-            pptp_cmd.append("terminate username {0}".format(username))
-            subprocess.call(pptp_cmd, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
+            pptp_cmd = pptp_base.format('username', username)
+            run(pptp_cmd, stdout=DEVNULL, stderr=DEVNULL)
             l2tp_cmd.append("terminate username {0}".format(username))
-            subprocess.call(l2tp_cmd, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
+            run(l2tp_cmd, stdout=DEVNULL, stderr=DEVNULL)
             return
 
     # rewrite `terminate by interface` if pptp will have pptp%d interface naming
     if interface:
-        pptp_cmd.append("terminate if {0}".format(interface))
-        subprocess.call(pptp_cmd, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
-        l2tp_cmd.append("terminate if {0}".format(interface))
-        subprocess.call(l2tp_cmd, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
+        pptp_cmd = pptp_base.format('if', interface)
+        run(pptp_cmd, stdout=DEVNULL, stderr=DEVNULL)
+        l2tp_cmd = l2tp_base.format('if', interface)
+        run(l2tp_cmd, stdout=DEVNULL, stderr=DEVNULL)
        
 
 def main():
diff --git a/src/op_mode/restart_dhcp_relay.py b/src/op_mode/restart_dhcp_relay.py
index ab02d1eb3..057b4dcd8 100755
--- a/src/op_mode/restart_dhcp_relay.py
+++ b/src/op_mode/restart_dhcp_relay.py
@@ -23,6 +23,8 @@ import argparse
 import os
 
 import vyos.config
+from vyos.util import run
+
 
 parser = argparse.ArgumentParser()
 parser.add_argument("--ipv4", action="store_true", help="Restart IPv4 DHCP relay")
@@ -37,7 +39,7 @@ if __name__ == '__main__':
         if not c.exists_effective('service dhcp-relay'):
             print("DHCP relay service not configured")
         else:
-            os.system('sudo systemctl restart isc-dhcp-relay.service')
+            run('sudo systemctl restart isc-dhcp-relay.service')
 
         sys.exit(0)
     elif args.ipv6:
@@ -45,7 +47,7 @@ if __name__ == '__main__':
         if not c.exists_effective('service dhcpv6-relay'):
             print("DHCPv6 relay service not configured")
         else:
-            os.system('sudo systemctl restart isc-dhcpv6-relay.service')
+            run('sudo systemctl restart isc-dhcpv6-relay.service')
 
         sys.exit(0)
     else:
diff --git a/src/op_mode/restart_frr.py b/src/op_mode/restart_frr.py
index da6407e23..6304e72db 100755
--- a/src/op_mode/restart_frr.py
+++ b/src/op_mode/restart_frr.py
@@ -17,12 +17,13 @@
 
 import sys
 import argparse
-import subprocess
 import logging
 from logging.handlers import SysLogHandler
 from pathlib import Path
 import psutil
 
+from vyos.util import run
+
 # some default values
 watchfrr = '/usr/lib/frr/watchfrr.sh'
 vtysh = '/usr/bin/vtysh'
@@ -86,7 +87,7 @@ def _write_config():
     Path(frrconfig_tmp).mkdir(parents=False, exist_ok=True)
     # save frr.conf to it
     command = "{} -n -w --config_dir {} 2> /dev/null".format(vtysh, frrconfig_tmp)
-    return_code = subprocess.call(command, shell=True)
+    return_code = run(command)
     if not return_code == 0:
         logger.error("Failed to save active config: \"{}\" returned exit code: {}".format(command, return_code))
         return False
@@ -108,7 +109,7 @@ def _cleanup():
 # check if daemon is running
 def _daemon_check(daemon):
     command = "{} print_status {}".format(watchfrr, daemon)
-    return_code = subprocess.call(command, shell=True)
+    return_code = run(command)
     if not return_code == 0:
         logger.error("Daemon \"{}\" is not running".format(daemon))
         return False
@@ -119,7 +120,7 @@ def _daemon_check(daemon):
 # restart daemon
 def _daemon_restart(daemon):
     command = "{} restart {}".format(watchfrr, daemon)
-    return_code = subprocess.call(command, shell=True)
+    return_code = run(command)
     if not return_code == 0:
         logger.error("Failed to restart daemon \"{}\"".format(daemon))
         return False
@@ -135,7 +136,7 @@ def _reload_config(daemon):
     else:
         command = "{} -n -b --config_dir {} 2> /dev/null".format(vtysh, frrconfig_tmp)
 
-    return_code = subprocess.call(command, shell=True)
+    return_code = run(command)
     if not return_code == 0:
         logger.error("Failed to reinstall configuration")
         return False
diff --git a/src/op_mode/show_acceleration.py b/src/op_mode/show_acceleration.py
index 3ba0e85dd..05d3d8906 100755
--- a/src/op_mode/show_acceleration.py
+++ b/src/op_mode/show_acceleration.py
@@ -19,14 +19,15 @@ import sys
 import os
 import re
 import argparse
-import subprocess
+
 from vyos.config import Config
+from vyos.util import popen, run
+
 
 def detect_qat_dev():
-    ret = subprocess.Popen(['sudo', 'lspci',  '-nn'], stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
-    (output, err) = ret.communicate()
+    output, err = popen('sudo lspci -nn', decode='utf-8')
     if not err:
-        data = re.findall('(8086:19e2)|(8086:37c8)|(8086:0435)|(8086:6f54)', output.decode("utf-8"))
+        data = re.findall('(8086:19e2)|(8086:37c8)|(8086:0435)|(8086:6f54)', output)
         #If QAT devices found
         if data:
             return
@@ -42,15 +43,12 @@ def show_qat_status():
         sys.exit(1)
 
     # Show QAT service
-    os.system('sudo /etc/init.d/vyos-qat-utilities status')
+    run('sudo /etc/init.d/vyos-qat-utilities status')
 
 # Return QAT devices
 def get_qat_devices():
-    ret = subprocess.Popen(['sudo', '/etc/init.d/vyos-qat-utilities',  'status'], stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
-    (output, err) = ret.communicate()
+    data_st, err = popen('sudo /etc/init.d/vyos-qat-utilities status', decode='utf-8')
     if not err:
-        #print(output)
-        data_st = output.decode("utf-8")
         elm_lst = re.findall('qat_dev\d', data_st)
         print('\n'.join(elm_lst))
 
@@ -58,11 +56,10 @@ def get_qat_devices():
 def get_qat_proc_path(qat_dev):
     q_type = ""
     q_bsf  = ""
-    ret = subprocess.Popen(['sudo', '/etc/init.d/vyos-qat-utilities',  'status'], stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
-    (output, err) = ret.communicate()
+    output, err = popen('sudo /etc/init.d/vyos-qat-utilities status', decode='utf-8')
     if not err:
         # Parse QAT service output
-        data_st = output.decode("utf-8").split("\n")
+        data_st = output.split("\n")
         for elm_str in range(len(data_st)):
             if re.search(qat_dev, data_st[elm_str]):
                 elm_list = data_st[elm_str].split(", ")
@@ -97,22 +94,22 @@ args = parser.parse_args()
 if args.hw:
     detect_qat_dev()
     # Show availible Intel QAT devices
-    os.system('sudo lspci -nn | egrep -e \'8086:37c8|8086:19e2|8086:0435|8086:6f54\'')
+    run('sudo lspci -nn | egrep -e \'8086:37c8|8086:19e2|8086:0435|8086:6f54\'')
 elif args.flow and args.dev:
     check_qat_if_conf()
-    os.system('sudo cat '+get_qat_proc_path(args.dev)+"fw_counters")
+    run('sudo cat '+get_qat_proc_path(args.dev)+"fw_counters")
 elif args.interrupts:
     check_qat_if_conf()
     # Delete _dev from args.dev
-    os.system('sudo cat /proc/interrupts | grep qat')
+    run('sudo cat /proc/interrupts | grep qat')
 elif args.status:
     check_qat_if_conf()
     show_qat_status()
 elif args.conf and args.dev:
     check_qat_if_conf()
-    os.system('sudo cat '+get_qat_proc_path(args.dev)+"dev_cfg")
+    run('sudo cat '+get_qat_proc_path(args.dev)+"dev_cfg")
 elif args.dev_list:
     get_qat_devices()
 else:
     parser.print_help()
-    sys.exit(1)
\ No newline at end of file
+    sys.exit(1)
diff --git a/src/op_mode/show_dhcp.py b/src/op_mode/show_dhcp.py
index f801ba753..4e3e08263 100755
--- a/src/op_mode/show_dhcp.py
+++ b/src/op_mode/show_dhcp.py
@@ -24,9 +24,12 @@ import collections
 import os
 from datetime import datetime
 
-from vyos.config import Config
 from isc_dhcp_leases import Lease, IscDhcpLeases
 
+from vyos.config import Config
+from vyos.util import run
+
+
 lease_file = "/config/dhcpd.leases"
 pool_key = "shared-networkname"
 
@@ -190,7 +193,7 @@ if __name__ == '__main__':
         sys.exit(0)
 
     # if dhcp server is down, inactive leases may still be shown as active, so warn the user.
-    if os.system('systemctl -q is-active isc-dhcpv4-server.service') != 0:
+    if run('systemctl -q is-active isc-dhcpv4-server.service') != 0:
         print("WARNING: DHCP server is configured but not started. Data may be stale.")
 
     if args.leases:
diff --git a/src/op_mode/show_dhcpv6.py b/src/op_mode/show_dhcpv6.py
index ae63af39b..4ef4849ff 100755
--- a/src/op_mode/show_dhcpv6.py
+++ b/src/op_mode/show_dhcpv6.py
@@ -24,9 +24,11 @@ import collections
 import os
 from datetime import datetime
 
-from vyos.config import Config
 from isc_dhcp_leases import Lease, IscDhcpLeases
 
+from vyos.config import Config
+from vyos.util import run
+
 lease_file = "/config/dhcpdv6.leases"
 pool_key = "shared-networkname"
 
@@ -177,7 +179,7 @@ if __name__ == '__main__':
         sys.exit(0)
 
     # if dhcp server is down, inactive leases may still be shown as active, so warn the user.
-    if os.system('systemctl -q is-active isc-dhcpv6-server.service') != 0:
+    if run('systemctl -q is-active isc-dhcpv6-server.service') != 0:
         print("WARNING: DHCPv6 server is configured but not started. Data may be stale.")
 
     if args.leases:
diff --git a/src/op_mode/show_vpn_ra.py b/src/op_mode/show_vpn_ra.py
index cf6119c2f..2323193b1 100755
--- a/src/op_mode/show_vpn_ra.py
+++ b/src/op_mode/show_vpn_ra.py
@@ -17,8 +17,8 @@
 import os
 import sys
 import re
-import subprocess
-# from subprocess import Popen, PIPE
+
+from vyos.util import popen
 
 # chech connection to pptp and l2tp daemon
 def get_sessions():
@@ -31,18 +31,16 @@ def get_sessions():
     len_def_header = 170
     
     # Check pptp
-    ret = subprocess.Popen(pptp_cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
-    (output, err) = ret.communicate()
-    if not err and len(output.decode("utf-8")) > len_def_header and not re.search(err_pattern, output.decode("utf-8")):
-        print(output.decode("utf-8"))
+    output, err = popen(pptp_cmd, decode='utf-8')
+    if not err and len(output) > len_def_header and not re.search(err_pattern, output):
+        print(output)
     else:
         absent_pptp = True
 
     # Check l2tp
-    ret = subprocess.Popen(l2tp_cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
-    (output, err) = ret.communicate()
-    if not err and len(output.decode("utf-8")) > len_def_header and not re.search(err_pattern, output.decode("utf-8")):
-        print(output.decode("utf-8"))
+    output, err = popen(l2tp_cmd, decode='utf-8')
+    if not err and len(output) > len_def_header and not re.search(err_pattern, output):
+        print(output)
     else:
         absent_l2tp = True
 
diff --git a/src/op_mode/show_vrf.py b/src/op_mode/show_vrf.py
index 66c33e607..b6bb73d01 100755
--- a/src/op_mode/show_vrf.py
+++ b/src/op_mode/show_vrf.py
@@ -16,10 +16,10 @@
 
 import argparse
 import jinja2
-
-from subprocess import check_output
 from json import loads
 
+from vyos.util import cmd
+
 vrf_out_tmpl = """
 VRF name          state     mac address        flags                     interfaces
 --------          -----     -----------        -----                     ----------
@@ -31,12 +31,12 @@ VRF name          state     mac address        flags                     interfa
 
 def list_vrfs():
     command = 'ip -j -br link show type vrf'
-    answer = loads(check_output(command.split()).decode())
+    answer = loads(cmd(command))
     return [_ for _ in answer if _]
 
 def list_vrf_members(vrf):
     command = f'ip -j -br link show master {vrf}'
-    answer = loads(check_output(command.split()).decode())
+    answer = loads(cmd(command))
     return [_ for _ in answer if _]
 
 parser = argparse.ArgumentParser()
diff --git a/src/op_mode/show_wireless.py b/src/op_mode/show_wireless.py
index aff882559..b5ee3aee1 100755
--- a/src/op_mode/show_wireless.py
+++ b/src/op_mode/show_wireless.py
@@ -19,19 +19,15 @@ import re
 
 from sys import exit
 from copy import deepcopy
-from subprocess import Popen, PIPE, STDOUT
 
 from vyos.config import Config
+from vyos.util import popen
 
 parser = argparse.ArgumentParser()
 parser.add_argument("-s", "--scan", help="Scan for Wireless APs on given interface, e.g. 'wlan0'")
 parser.add_argument("-b", "--brief", action="store_true", help="Show wireless configuration")
 parser.add_argument("-c", "--stations", help="Show wireless clients connected on interface, e.g. 'wlan0'")
 
-def _cmd(command):
-    p = Popen(command, stdout=PIPE, stderr=STDOUT, shell=True)
-    tmp = p.communicate()[0].strip()
-    return tmp.decode()
 
 def show_brief():
     config = Config()
@@ -57,7 +53,8 @@ def show_brief():
     return interfaces
 
 def ssid_scan(intf):
-    tmp = _cmd('/sbin/iw dev {} scan ap-force'.format(intf))
+    # XXX: This ignores errors
+    tmp, _ = popen(f'/sbin/iw dev {intf} scan ap-force')
     networks = []
     data = {
         'ssid': '',
@@ -89,7 +86,8 @@ def ssid_scan(intf):
     return networks
 
 def show_clients(intf):
-    tmp = _cmd('/sbin/iw dev {} station dump'.format(intf))
+    # XXX: This ignores errors
+    tmp, _ = popen(f'/sbin/iw dev {intf} station dump')
     clients = []
     data = {
         'mac': '',
diff --git a/src/op_mode/snmp.py b/src/op_mode/snmp.py
index e08441f0e..b09eab97f 100755
--- a/src/op_mode/snmp.py
+++ b/src/op_mode/snmp.py
@@ -24,6 +24,7 @@ import sys
 import argparse
 
 from vyos.config import Config
+from vyos.util import run
 
 config_file_daemon = r'/etc/snmp/snmpd.conf'
 
@@ -53,7 +54,7 @@ def show_all():
 
 def show_community(c, h):
     print('Status of SNMP community {0} on {1}'.format(c, h), flush=True)
-    os.system('/usr/bin/snmpstatus -t1 -v1 -c {0} {1}'.format(c, h))
+    run('/usr/bin/snmpstatus -t1 -v1 -c {0} {1}'.format(c, h))
 
 if __name__ == '__main__':
     args = parser.parse_args()
diff --git a/src/op_mode/snmp_ifmib.py b/src/op_mode/snmp_ifmib.py
index 3a0e0d4b2..2479936bd 100755
--- a/src/op_mode/snmp_ifmib.py
+++ b/src/op_mode/snmp_ifmib.py
@@ -22,36 +22,24 @@
 import sys
 import argparse
 import netifaces
-import subprocess
 
 from vyos.config import Config
+from vyos.util import popen
 
 parser = argparse.ArgumentParser(description='Retrieve SNMP interfaces information')
 parser.add_argument('--ifindex', action='store', nargs='?', const='all', help='Show interface index')
 parser.add_argument('--ifalias', action='store', nargs='?', const='all', help='Show interface aliase')
 parser.add_argument('--ifdescr', action='store', nargs='?', const='all', help='Show interface description')
 
-def show_ifindex(i):
-    proc = subprocess.Popen(['/bin/ip', 'link', 'show', i], stdout=subprocess.PIPE)
-    (out, err) = proc.communicate()
-    # convert output to string
-    string = out.decode("utf-8")
-
-    index = 'ifIndex = ' + string.split(':')[0]
+def show_ifindex(intf):
+    out, err = popen(f'/bin/ip link show {intf}', decode='utf-8')
+    index = 'ifIndex = ' + out.split(':')[0]
     return index.replace('\n', '')
 
-def show_ifalias(i):
-    proc = subprocess.Popen(['/bin/ip', 'link', 'show', i], stdout=subprocess.PIPE)
-    (out, err) = proc.communicate()
-    # convert output to string
-    string = out.decode("utf-8")
-
-    if 'alias' in string:
-        alias = 'ifAlias = ' + string.split('alias')[1].lstrip()
-    else:
-        alias = 'ifAlias = ' + i
-
-    return alias.replace('\n', '')
+def show_ifalias(intf):
+    out, err = popen(f'/bin/ip link show {intf}', decode='utf-8')
+    alias = out.split('alias')[1].lstrip() if 'alias' in out else intf
+    return 'ifAlias = ' + alias.replace('\n', '')
 
 def show_ifdescr(i):
     ven_id = ''
@@ -74,14 +62,13 @@ def show_ifdescr(i):
         return ret
 
     device = str(ven_id) + ':' + str(dev_id)
-    proc = subprocess.Popen(['/usr/bin/lspci', '-mm', '-d', device], stdout=subprocess.PIPE)
-    (out, err) = proc.communicate()
+    out, err = popen(f'/usr/bin/lspci -mm -d {device}', decode='utf-8')
 
     vendor = ""
     device = ""
 
     # convert output to string
-    string = out.decode("utf-8").split('"')
+    string = out.split('"')
     if len(string) > 3:
       vendor = string[3]
 
diff --git a/src/op_mode/system_integrity.py b/src/op_mode/system_integrity.py
index 886d94f16..c0e3d1095 100755
--- a/src/op_mode/system_integrity.py
+++ b/src/op_mode/system_integrity.py
@@ -18,18 +18,19 @@
 
 import sys
 import os
-import subprocess
 import re
 import itertools
 from datetime import datetime, timedelta
 
+from vyos.util import cmd
+
 verf = r'/usr/libexec/vyos/op_mode/version.py'
 
 def get_sys_build_version():
   if not os.path.exists(verf):
     return None
 
-  a = subprocess.check_output(['/usr/libexec/vyos/op_mode/version.py']).decode()
+  a = cmd('/usr/libexec/vyos/op_mode/version.py')
   if re.search('^Built on:.+',a, re.M) == None:
     return None
 
diff --git a/src/op_mode/version.py b/src/op_mode/version.py
index 5aff0f767..34eca44b1 100755
--- a/src/op_mode/version.py
+++ b/src/op_mode/version.py
@@ -22,7 +22,6 @@
 
 import os
 import sys
-import subprocess
 import argparse
 import json
 
@@ -31,6 +30,8 @@ import pystache
 import vyos.version
 import vyos.limericks
 
+from vyos.util import cmd, run
+
 
 parser = argparse.ArgumentParser()
 parser.add_argument("-a", "--all", action="store_true", help="Include individual package versions")
@@ -73,15 +74,15 @@ if __name__ == '__main__':
     version_data = vyos.version.get_version_data()
 
     # Get system architecture (well, kernel architecture rather)
-    version_data['system_arch'] = subprocess.check_output('uname -m', shell=True).decode().strip()
+    version_data['system_arch'] = cmd('uname -m')
 
 
     # Get hypervisor name, if any
     system_type = "bare metal"
     try:
-        hypervisor = subprocess.check_output('hvinfo 2>/dev/null', shell=True).decode().strip()
+        hypervisor = cmd('hvinfo 2>/dev/null')
         system_type = "{0} guest".format(hypervisor)
-    except subprocess.CalledProcessError:
+    except OSError:
         # hvinfo returns 1 if it cannot detect any hypervisor
         pass
     version_data['system_type'] = system_type
@@ -93,9 +94,9 @@ if __name__ == '__main__':
     # while on livecd it's just "filesystem.squashfs", that's how we tell a livecd boot
     # from an installed image
     boot_via = "installed image"
-    if subprocess.call(""" grep -e '^overlay.*/filesystem.squashfs' /proc/mounts >/dev/null""", shell=True) == 0:
+    if run(""" grep -e '^overlay.*/filesystem.squashfs' /proc/mounts >/dev/null""") == 0:
         boot_via = "livecd"
-    elif subprocess.call(""" grep '^overlay /' /proc/mounts >/dev/null """, shell=True) != 0:
+    elif run(""" grep '^overlay /' /proc/mounts >/dev/null """) != 0:
         boot_via = "legacy non-image installation"
     version_data['boot_via'] = boot_via
     
@@ -118,7 +119,7 @@ if __name__ == '__main__':
 
         if args.all:
            print("Package versions:")
-           os.system("dpkg -l")
+           run("dpkg -l")
 
         if args.funny:
             print(vyos.limericks.get_random())
diff --git a/src/op_mode/wireguard.py b/src/op_mode/wireguard.py
index c684f8a47..d940d79eb 100755
--- a/src/op_mode/wireguard.py
+++ b/src/op_mode/wireguard.py
@@ -20,7 +20,6 @@ import argparse
 import os
 import sys
 import shutil
-import subprocess
 import syslog as sl
 import re
 
@@ -28,6 +27,7 @@ from vyos.ifconfig import WireGuardIf
 
 from vyos import ConfigError
 from vyos.config import Config
+from vyos.util import run
 
 dir = r'/config/auth/wireguard'
 psk = dir + '/preshared.key'
@@ -36,16 +36,14 @@ def check_kmod():
     """ check if kmod is loaded, if not load it """
     if not os.path.exists('/sys/module/wireguard'):
         sl.syslog(sl.LOG_NOTICE, "loading wirguard kmod")
-        if os.system('sudo modprobe wireguard') != 0:
+        if run('sudo modprobe wireguard') != 0:
             sl.syslog(sl.LOG_ERR, "modprobe wireguard failed")
             raise ConfigError("modprobe wireguard failed")
 
 def generate_keypair(pk, pub):
     """ generates a keypair which is stored in /config/auth/wireguard """
     old_umask = os.umask(0o027)
-    ret = subprocess.call(
-        ['wg genkey | tee ' + pk + '|wg pubkey > ' + pub], shell=True)
-    if ret != 0:
+    if run(f'wg genkey | tee {pk} | wg pubkey > {pub}') != 0:
         raise ConfigError("wireguard key-pair generation failed")
     else:
         sl.syslog(
@@ -69,9 +67,9 @@ def genkey(location):
     else:
         """ if keypair is bing executed from a running iso """
         if not os.path.exists(location):
-            subprocess.call(['sudo mkdir -p ' + location], shell=True)
-            subprocess.call(['sudo chgrp vyattacfg ' + location], shell=True)
-            subprocess.call(['sudo chmod 750 ' + location], shell=True)
+            run(f'sudo mkdir -p {location}')
+            run(f'sudo chgrp vyattacfg {location}')
+            run(f'sudo chmod 750 {location}')
         generate_keypair(pk, pub)
     os.umask(old_umask)
 
@@ -90,7 +88,7 @@ def genpsk():
         it's stored only in the cli config
     """
 
-    subprocess.call(['wg genpsk'], shell=True)
+    run('wg genpsk')
 
 def list_key_dirs():
     """ lists all dirs under /config/auth/wireguard """