updates to consolidate commands that need to be defined by a new node type

daemon/core/corehandlers.py

@@ -1087,7 +1087,7 @@ class CoreRequestHandler(SocketServer.BaseRequestHandler):
                 # echo back exec message with cmd for spawning interactive terminal
                 if command == "bash":
                     command = "/bin/bash"
-                res = node.client.termcmdstring(command)
+                res = node.termcmdstring(command)
                 tlv_data += coreapi.CoreExecuteTlv.pack(ExecuteTlvs.RESULT.value, res)
                 reply = coreapi.CoreExecMessage.pack(MessageFlags.TTY.value, tlv_data)
                 return reply,
@@ -1097,9 +1097,9 @@ class CoreRequestHandler(SocketServer.BaseRequestHandler):
                 if message.flags & MessageFlags.STRING.value or message.flags & MessageFlags.TEXT.value:
                     # shlex.split() handles quotes within the string
                     if message.flags & MessageFlags.LOCAL.value:
-                        status, res = utils.cmdresult(shlex.split(command))
+                        status, res = utils.cmd_output(command)
                     else:
-                        status, res = node.client.cmdresult(shlex.split(command))
+                        status, res = node.cmd_output(command)
                     logger.info("done exec cmd=%s with status=%d res=(%d bytes)", command, status, len(res))
                     if message.flags & MessageFlags.TEXT.value:
                         tlv_data += coreapi.CoreExecuteTlv.pack(ExecuteTlvs.RESULT.value, res)
@@ -1110,9 +1110,10 @@ class CoreRequestHandler(SocketServer.BaseRequestHandler):
                 # execute the command with no response
                 else:
                     if message.flags & MessageFlags.LOCAL.value:
+                        # TODO: get this consolidated into utils
                         utils.mutedetach(shlex.split(command))
                     else:
-                        node.client.cmd(shlex.split(command), wait=False)
+                        node.cmd(command, wait=False)
         except KeyError:
             logger.exception("error getting object: %s", node_num)
             # XXX wait and queue this message to try again later

daemon/core/coreobj.py

@@ -412,6 +412,47 @@ class PyCoreNode(PyCoreObj):
 
         return common
 
+    def check_cmd(self, cmd):
+        """
+        Runs shell command on node.
+
+        :param list[str]/str cmd: command to run
+        :return: exist status and combined stdout and stderr
+        :rtype: tuple[int, str]
+        :raises subprocess.CalledProcessError: when a non-zero exit status occurs
+        """
+        raise NotImplementedError
+
+    def cmd(self, cmd, wait=True):
+        """
+        Runs shell command on node, with option to not wait for a result.
+
+        :param list[str]/str cmd: command to run
+        :param bool wait: wait for command to exit, defaults to True
+        :return: exit status for command
+        :rtype: int
+        """
+        raise NotImplementedError
+
+    def cmd_output(self, cmd):
+        """
+        Runs shell command on node and get exit status and output.
+
+        :param list[str]/str cmd: command to run
+        :return: exit status and combined stdout and stderr
+        :rtype: tuple[int, str]
+        """
+        raise NotImplementedError
+
+    def termcmdstring(self, sh):
+        """
+        Create a terminal command string.
+
+        :param str sh: shell to execute command in
+        :return: str
+        """
+        raise NotImplementedError
+
 
 class PyCoreNet(PyCoreObj):
     """

daemon/core/emane/__init__.py

@@ -24,7 +24,7 @@ def emane_version():
     cmd = ("emane", "--version")
 
     try:
-        status, result = utils.cmdresult(cmd)
+        status, result = utils.cmd_output(cmd)
     except (OSError, subprocess.CalledProcessError):
         logger.exception("error checking emane version")
         status = -1

daemon/core/emane/emanemanager.py

@@ -103,7 +103,6 @@ class EmaneManager(ConfigurableManager):
             emane_models = utils.load_classes(custom_models_path, EmaneModel)
             self.load_models(emane_models)
 
-
     def logversion(self):
         """
         Log the installed EMANE version.
@@ -912,16 +911,23 @@ class EmaneManager(ConfigurableManager):
 
             # multicast route is needed for OTA data
             cmd = [constants.IP_BIN, "route", "add", otagroup, "dev", otadev]
-            node.client.cmd(cmd, wait=True)
+            try:
+                node.check_cmd(cmd)
+            except subprocess.CalledProcessError:
+                logger.exception("error adding route for OTA data")
+
             # multicast route is also needed for event data if on control network
             if eventservicenetidx >= 0 and eventgroup != otagroup:
                 cmd = [constants.IP_BIN, "route", "add", eventgroup, "dev", eventdev]
-                node.client.cmd(cmd, wait=True)
+                try:
+                    node.check_cmd(cmd)
+                except subprocess.CalledProcessError:
+                    logger.exception("error adding route for event data")
 
             try:
                 cmd = emanecmd + ["-f", os.path.join(path, "emane%d.log" % n), os.path.join(path, "platform%d.xml" % n)]
                 logger.info("Emane.startdaemons2() running %s" % str(cmd))
-                status, output = node.client.cmdresult(cmd)
+                status, output = node.check_cmd(cmd)
                 logger.info("Emane.startdaemons2() return code %d" % status)
                 logger.info("Emane.startdaemons2() output: %s" % output)
             except subprocess.CalledProcessError:
@@ -934,8 +940,8 @@ class EmaneManager(ConfigurableManager):
         try:
             emanecmd += ["-f", os.path.join(path, "emane.log")]
             cmd = emanecmd + [os.path.join(path, "platform.xml")]
-            logger.info("Emane.startdaemons2() running %s" % str(cmd))
+            logger.info("Emane.startdaemons2() running %s" % cmd)
-            subprocess.check_call(cmd, cwd=path)
+            utils.check_cmd(cmd, cwd=path)
         except subprocess.CalledProcessError:
             logger.exception("error starting emane")
 
@@ -954,7 +960,7 @@ class EmaneManager(ConfigurableManager):
                     stop_emane_on_host = True
                     continue
                 if node.up:
-                    node.client.cmd(cmd, wait=False)
+                    node.cmd(cmd, wait=False)
                     # TODO: RJ45 node
         else:
             stop_emane_on_host = True
@@ -1158,10 +1164,10 @@ class EmaneManager(ConfigurableManager):
 
         try:
             if emane.VERSION < emane.EMANE092:
-                status = subprocess.call(cmd)
+                status = subprocess.check_call(cmd)
             else:
-                status = node.client.cmd(cmd, wait=True)
+                status, _ = node.check_cmd(cmd)
-        except IOError:
+        except subprocess.CalledProcessError:
             logger.exception("error checking if emane is running")
 
         return status == 0

daemon/core/misc/utils.py

@@ -5,6 +5,7 @@ Miscellaneous utility functions, wrappers around some subprocess procedures.
 import importlib
 import inspect
 import os
+import shlex
 import subprocess
 import sys
 
@@ -111,6 +112,20 @@ def maketuplefromstr(s, value_type):
     return tuple(value_type(i) for i in values)
 
 
+def split_cmd(cmd):
+    """
+    Convenience method for splitting potential string commands into a shell-like syntax list.
+
+    :param list/str cmd: command list or string
+    :return: shell-like syntax list
+    :rtype: list
+    """
+    # split shell string to shell array for convenience
+    if type(cmd) == str:
+        cmd = shlex.split(cmd)
+    return cmd
+
+
 def mutecall(*args, **kwargs):
     """
     Run a muted call command.
@@ -129,11 +144,12 @@ def check_alloutput(cmd, **kwargs):
     """
     Convenience wrapper to run subprocess.check_output and include stderr as well.
 
-    :param list[str] cmd: command arguments to run
+    :param list[str]/str cmd: command arguments to run
     :param dict kwargs: option for running subprocess.check_output, beyond setting stderr to stdout
     :return: combined stdout and stderr
     :raises subprocess.CalledProcessError: when a non-zero exit status is encountered
     """
+    cmd = split_cmd(cmd)
     kwargs["stderr"] = subprocess.STDOUT
     return subprocess.check_output(cmd, **kwargs)
 
@@ -218,20 +234,43 @@ def mutedetach(*args, **kwargs):
     return subprocess.Popen(*args, **kwargs).pid
 
 
-def cmdresult(args):
+def cmd_output(cmd):
     """
     Execute a command on the host and return a tuple containing the exit status and result string. stderr output
     is folded into the stdout result string.
 
-    :param list args: command arguments
+    :param list[str]/str cmd: command arguments
     :return: command status and stdout
     :rtype: tuple[int, str]
     """
-    cmdid = subprocess.Popen(args, stdin=open(os.devnull, "r"), stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+    # split shell string to shell array for convenience
-    # err will always be None
+    cmd = split_cmd(cmd)
-    result, err = cmdid.communicate()
+    p = subprocess.Popen(cmd, stdin=open(os.devnull, "r"), stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
-    status = cmdid.wait()
+    stdout, _ = p.communicate()
-    return status, result
+    status = p.wait()
+    return status, stdout
+
+
+def check_cmd(cmd, **kwargs):
+    """
+    Execute a command on the host and return a tuple containing the exit status and result string. stderr output
+    is folded into the stdout result string.
+
+    :param list[str]/str cmd: command arguments
+    :param dict kwargs: keyword arguments to pass to subprocess.Popen
+    :return: command status and stdout
+    :rtype: tuple[int, str]
+    :raises subprocess.CalledProcessError: when there is a non-zero exit status
+    """
+    kwargs["stdout"] = subprocess.PIPE
+    kwargs["stderr"] = subprocess.STDOUT
+    cmd = split_cmd(cmd)
+    p = subprocess.Popen(cmd, **kwargs)
+    stdout, _ = p.communicate()
+    status = p.wait()
+    if status:
+        raise subprocess.CalledProcessError(status, cmd, stdout)
+    return status, stdout
 
 
 def hexdump(s, bytes_per_word=2, words_per_line=8):

daemon/core/netns/nodes.py

@@ -103,7 +103,7 @@ class CtrlNet(LxBrNet):
         :return: True if an old bridge was detected, False otherwise
         :rtype: bool
         """
-        retstat, retstr = utils.cmdresult([constants.BRCTL_BIN, "show"])
+        retstat, retstr = utils.cmd_output([constants.BRCTL_BIN, "show"])
         if retstat != 0:
             logger.error("Unable to retrieve list of installed bridges")
         lines = retstr.split("\n")

daemon/core/netns/openvswitch.py

@@ -406,7 +406,7 @@ class OvsCtrlNet(OvsNet):
         Check if there are old control net bridges and delete them
         """
 
-        status, output = utils.cmdresult([constants.OVS_BIN, "list-br"])
+        status, output = utils.cmd_output([constants.OVS_BIN, "list-br"])
         output = output.strip()
         if output:
             for line in output.split("\n"):

daemon/core/netns/vif.py

@@ -59,10 +59,16 @@ class VEth(PyCoreNetIf):
         """
         if not self.up:
             return
+
         if self.node:
-            self.node.client.cmd([constants.IP_BIN, "-6", "addr", "flush", "dev", self.name])
+            try:
+                self.node.check_cmd([constants.IP_BIN, "-6", "addr", "flush", "dev", self.name])
+            except subprocess.CalledProcessError:
+                logger.exception("error shutting down interface")
+
         if self.localname:
             utils.mutedetach([constants.IP_BIN, "link", "delete", self.localname])
+
         self.up = False
 
 
@@ -98,10 +104,10 @@ class TunTap(PyCoreNetIf):
         """
         # TODO: more sophisticated TAP creation here
         #   Debian does not support -p (tap) option, RedHat does.
-        # For now, this is disabled to allow the TAP to be created by another
+        #   For now, this is disabled to allow the TAP to be created by another
-        # system (e.g. EMANE"s emanetransportd)
+        #   system (e.g. EMANE"s emanetransportd)
-        # check_call(["tunctl", "-t", self.name])
+        #   check_call(["tunctl", "-t", self.name])
-        # self.install()
+        #   self.install()
         self.up = True
 
     def shutdown(self):
@@ -112,9 +118,12 @@ class TunTap(PyCoreNetIf):
         """
         if not self.up:
             return
-        self.node.client.cmd([constants.IP_BIN, "-6", "addr", "flush", "dev", self.name])
+
-        # if self.name:
+        try:
-        #    mutedetach(["tunctl", "-d", self.localname])
+            self.node.check_cmd([constants.IP_BIN, "-6", "addr", "flush", "dev", self.name])
+        except subprocess.CalledProcessError:
+            logger.exception("error shutting down tunnel tap")
+
         self.up = False
 
     def waitfor(self, func, attempts=10, maxretrydelay=0.25):
@@ -169,7 +178,7 @@ class TunTap(PyCoreNetIf):
 
         def nodedevexists():
             cmd = [constants.IP_BIN, "link", "show", self.name]
-            return self.node.client.cmd(cmd)
+            return self.node.cmd(cmd)
 
         count = 0
         while True:
@@ -199,15 +208,11 @@ class TunTap(PyCoreNetIf):
         netns = str(self.node.pid)
 
         try:
-            subprocess.check_call([constants.IP_BIN, "link", "set", self.localname, "netns", netns])
+            utils.check_cmd([constants.IP_BIN, "link", "set", self.localname, "netns", netns])
+            self.node.check_cmd([constants.IP_BIN, "link", "set", self.localname, "name", self.name])
+            self.node.check_cmd([constants.IP_BIN, "link", "set", self.name, "up"])
         except subprocess.CalledProcessError:
-            msg = "error installing TAP interface %s, command:" % self.localname
+            logger.exception("error installing TAP interface")
-            msg += "ip link set %s netns %s" % (self.localname, netns)
-            logger.exception(msg)
-            return
-
-        self.node.client.cmd([constants.IP_BIN, "link", "set", self.localname, "name", self.name])
-        self.node.client.cmd([constants.IP_BIN, "link", "set", self.name, "up"])
 
     def setaddrs(self):
         """
@@ -217,7 +222,10 @@ class TunTap(PyCoreNetIf):
         """
         self.waitfordevicenode()
         for addr in self.addrlist:
-            self.node.client.cmd([constants.IP_BIN, "addr", "add", str(addr), "dev", self.name])
+            try:
+                self.node.check_cmd([constants.IP_BIN, "addr", "add", str(addr), "dev", self.name])
+            except subprocess.CalledProcessError:
+                logger.exception("failure setting interface address")
 
 
 class GreTap(PyCoreNetIf):

daemon/core/netns/vnode.py

@@ -114,11 +114,11 @@ class SimpleLxcNode(PyCoreNode):
         try:
             # bring up the loopback interface
             logger.info("bringing up loopback interface")
-            self.client.check_alloutput([constants.IP_BIN, "link", "set", "lo", "up"])
+            self.check_cmd([constants.IP_BIN, "link", "set", "lo", "up"])
 
             # set hostname for node
             logger.info("setting hostname: %s" % self.name)
-            self.client.check_alloutput(["hostname", self.name])
+            self.check_cmd(["hostname", self.name])
         except subprocess.CalledProcessError:
             logger.exception("error setting up loopback and hostname: %s")
 
@@ -171,6 +171,47 @@ class SimpleLxcNode(PyCoreNode):
         """
         pass
 
+    def cmd(self, cmd, wait=True):
+        """
+        Runs shell command on node, with option to not wait for a result.
+
+        :param list[str]/str cmd: command to run
+        :param bool wait: wait for command to exit, defaults to True
+        :return: exit status for command
+        :rtype: int
+        """
+        return self.client.cmd(cmd, wait)
+
+    def cmd_output(self, cmd):
+        """
+        Runs shell command on node and get exit status and output.
+
+        :param list[str]/str cmd: command to run
+        :return: exit status and combined stdout and stderr
+        :rtype: tuple[int, str]
+        """
+        return self.client.cmd_output(cmd)
+
+    def check_cmd(self, cmd):
+        """
+        Runs shell command on node.
+
+        :param list[str]/str cmd: command to run
+        :return: exist status and combined stdout and stderr
+        :rtype: tuple[int, str]
+        :raises subprocess.CalledProcessError: when a non-zero exit status occurs
+        """
+        return self.client.check_cmd(cmd)
+
+    def termcmdstring(self, sh="/bin/sh"):
+        """
+        Create a terminal command string.
+
+        :param str sh: shell to execute command in
+        :return: str
+        """
+        return self.client.termcmdstring(sh)
+
     # TODO: should change how this exception is just swallowed up
     def mount(self, source, target):
         """
@@ -200,7 +241,7 @@ class SimpleLxcNode(PyCoreNode):
         """
         logger.info("unmounting: %s", target)
         try:
-            self.client.check_alloutput([constants.UMOUNT_BIN, "-n", "-l", target])
+            self.check_cmd([constants.UMOUNT_BIN, "-n", "-l", target])
         except subprocess.CalledProcessError:
             logger.exception("error during unmount")
 
@@ -247,8 +288,8 @@ class SimpleLxcNode(PyCoreNode):
 
             if self.up:
                 try:
-                    utils.check_alloutput([constants.IP_BIN, "link", "set", veth.name, "netns", str(self.pid)])
+                    utils.check_cmd([constants.IP_BIN, "link", "set", veth.name, "netns", str(self.pid)])
-                    self.client.check_alloutput([constants.IP_BIN, "link", "set", veth.name, "name", ifname])
+                    self.check_cmd([constants.IP_BIN, "link", "set", veth.name, "name", ifname])
                 except subprocess.CalledProcessError:
                     logger.exception("failure setting eth name")
 
@@ -258,7 +299,7 @@ class SimpleLxcNode(PyCoreNode):
                 # TODO: potentially find better way to query interface ID
                 # retrieve interface information
                 try:
-                    output = self.client.check_alloutput(["ip", "link", "show", veth.name])
+                    _, output = self.check_cmd(["ip", "link", "show", veth.name])
                     logger.info("interface command output: %s", output)
                     output = output.split("\n")
                     veth.flow_id = int(output[0].strip().split(":")[0]) + 1
@@ -320,7 +361,7 @@ class SimpleLxcNode(PyCoreNode):
         if self.up:
             cmd = [constants.IP_BIN, "link", "set", "dev", self.ifname(ifindex), "address", str(addr)]
             try:
-                self.client.check_alloutput(cmd)
+                self.check_cmd(cmd)
             except subprocess.CalledProcessError:
                 logger.exception("error setting MAC address %s: %s", addr)
 
@@ -337,10 +378,10 @@ class SimpleLxcNode(PyCoreNode):
                 # check if addr is ipv6
                 if ":" in str(addr):
                     cmd = [constants.IP_BIN, "addr", "add", str(addr), "dev", self.ifname(ifindex)]
-                    self.client.check_alloutput(cmd)
+                    self.check_cmd(cmd)
                 else:
                     cmd = [constants.IP_BIN, "addr", "add", str(addr), "broadcast", "+", "dev", self.ifname(ifindex)]
-                    self.client.check_alloutput(cmd)
+                    self.check_cmd(cmd)
             except subprocess.CalledProcessError:
                 logger.exception("failure adding interface address")
 
@@ -361,7 +402,7 @@ class SimpleLxcNode(PyCoreNode):
 
         if self.up:
             try:
-                self.client.check_alloutput([constants.IP_BIN, "addr", "del", str(addr), "dev", self.ifname(ifindex)])
+                self.check_cmd([constants.IP_BIN, "addr", "del", str(addr), "dev", self.ifname(ifindex)])
             except subprocess.CalledProcessError:
                 logger.exception("failure deleting address")
 
@@ -394,7 +435,7 @@ class SimpleLxcNode(PyCoreNode):
         """
         if self.up:
             try:
-                self.client.check_alloutput([constants.IP_BIN, "link", "set", self.ifname(ifindex), "up"])
+                self.check_cmd([constants.IP_BIN, "link", "set", self.ifname(ifindex), "up"])
             except subprocess.CalledProcessError:
                 logger.exception("failure bringing interface up")
 
@@ -454,15 +495,15 @@ class SimpleLxcNode(PyCoreNode):
         tmplen = 8
         tmp1 = "tmp." + "".join([random.choice(string.ascii_lowercase) for _ in xrange(tmplen)])
         tmp2 = "tmp." + "".join([random.choice(string.ascii_lowercase) for _ in xrange(tmplen)])
-        utils.check_alloutput([constants.IP_BIN, "link", "add", "name", tmp1, "type", "veth", "peer", "name", tmp2])
+        utils.check_cmd([constants.IP_BIN, "link", "add", "name", tmp1, "type", "veth", "peer", "name", tmp2])
 
-        utils.check_alloutput([constants.IP_BIN, "link", "set", tmp1, "netns", str(self.pid)])
+        utils.check_cmd([constants.IP_BIN, "link", "set", tmp1, "netns", str(self.pid)])
-        self.client.check_alloutput([constants.IP_BIN, "link", "set", tmp1, "name", ifname])
+        self.check_cmd([constants.IP_BIN, "link", "set", tmp1, "name", ifname])
         interface = PyCoreNetIf(node=self, name=ifname, mtu=_DEFAULT_MTU)
         self.addnetif(interface, self.newifindex())
 
-        utils.check_alloutput([constants.IP_BIN, "link", "set", tmp2, "netns", str(othernode.pid)])
+        utils.check_cmd([constants.IP_BIN, "link", "set", tmp2, "netns", str(othernode.pid)])
-        othernode.client.check_alloutput([constants.IP_BIN, "link", "set", tmp2, "name", otherifname])
+        othernode.check_cmd([constants.IP_BIN, "link", "set", tmp2, "name", otherifname])
         other_interface = PyCoreNetIf(node=othernode, name=otherifname, mtu=_DEFAULT_MTU)
         othernode.addnetif(other_interface, othernode.newifindex())
 

daemon/core/netns/vnodeclient.py

@@ -6,13 +6,13 @@ by invoking the vcmd shell command.
 """
 
 import os
-import shlex
 import subprocess
 
 import vcmd
 
 from core import constants
 from core import logger
+from core.misc import utils
 
 VCMD = os.path.join(constants.CORE_BIN_DIR, "vcmd")
 
@@ -61,89 +61,86 @@ class VnodeClient(object):
         """
         self.cmdchnl.close()
 
-    def cmd(self, args, wait=True):
+    def cmd(self, cmd, wait=True):
         """
         Execute a command on a node and return the status (return code).
 
-        :param list args: command arguments
+        :param list[str]/str cmd: command arguments
         :param bool wait: wait for command to end or not
         :return: command status
         :rtype: int
         """
         self._verify_connection()
+        cmd = utils.split_cmd(cmd)
 
         # run command, return process when not waiting
-        p = self.cmdchnl.qcmd(args)
+        p = self.cmdchnl.qcmd(cmd)
         if not wait:
-            return p
+            return 0
 
         # wait for and return exit status
         status = p.wait()
         if status:
-            logger.warn("cmd exited with status %s: %s", status, args)
+            logger.warn("cmd exited with status %s: %s", status, cmd)
+
         return status
 
-    def cmdresult(self, cmd):
+    def cmd_output(self, cmd):
         """
         Execute a command on a node and return a tuple containing the
         exit status and result string. stderr output
         is folded into the stdout result string.
 
-        :param list cmd: command arguments
+        :param list[str]/str cmd: command to run
         :return: command status and combined stdout and stderr output
         :rtype: tuple[int, str]
         """
-        self._verify_connection()
-
-        # split shell string to shell array for convenience
-        if type(cmd) == str:
-            cmd = shlex.split(cmd)
-
         p, stdin, stdout, stderr = self.popen(cmd)
         stdin.close()
         output = stdout.read() + stderr.read()
         stdout.close()
         stderr.close()
         status = p.wait()
-
         return status, output
 
-    def check_alloutput(self, cmd):
+    def check_cmd(self, cmd):
         """
-        Run command and return output, raises exception when non-zero exit status is encountered.
+        Run command and return exit status and combined stdout and stderr.
 
-        :param cmd:
+        :param list[str]/str cmd: command to run
-        :return: combined stdout and stderr combined
+        :return: exit status and combined stdout and stderr
-        :rtype: str
+        :rtype: tuple[int, str]
         :raises subprocess.CalledProcessError: when there is a non-zero exit status
         """
-        status, output = self.cmdresult(cmd)
+        status, output = self.cmd_output(cmd)
         if status:
             raise subprocess.CalledProcessError(status, cmd, output)
-        return output
+        return status, output
 
-    def popen(self, args):
+    def popen(self, cmd):
         """
         Execute a popen command against the node.
 
-        :param list args: command arguments
+        :param list[str]/str cmd: command arguments
         :return: popen object, stdin, stdout, and stderr
         :rtype: tuple
         """
         self._verify_connection()
-        return self.cmdchnl.popen(args)
+        cmd = utils.split_cmd(cmd)
+        return self.cmdchnl.popen(cmd)
 
-    def icmd(self, args):
+    def icmd(self, cmd):
         """
         Execute an icmd against a node.
 
-        :param list args: command arguments
+        :param list[str]/str cmd: command arguments
         :return: command result
         :rtype: int
         """
-        return os.spawnlp(os.P_WAIT, VCMD, VCMD, "-c", self.ctrlchnlname, "--", *args)
+        cmd = utils.split_cmd(cmd)
+        return os.spawnlp(os.P_WAIT, VCMD, VCMD, "-c", self.ctrlchnlname, "--", *cmd)
 
-    def redircmd(self, infd, outfd, errfd, args, wait=True):
+    def redircmd(self, infd, outfd, errfd, cmd, wait=True):
         """
         Execute a command on a node with standard input, output, and
         error redirected according to the given file descriptors.
@@ -151,7 +148,7 @@ class VnodeClient(object):
         :param infd: stdin file descriptor
         :param outfd: stdout file descriptor
         :param errfd: stderr file descriptor
-        :param list args: command arguments
+        :param list[str]/str cmd: command arguments
         :param bool wait: wait flag
         :return: command status
         :rtype: int
@@ -159,14 +156,15 @@ class VnodeClient(object):
         self._verify_connection()
 
         # run command, return process when not waiting
-        p = self.cmdchnl.redircmd(infd, outfd, errfd, args)
+        cmd = utils.split_cmd(cmd)
+        p = self.cmdchnl.redircmd(infd, outfd, errfd, cmd)
         if not wait:
             return p
 
         # wait for and return exit status
         status = p.wait()
         if status:
-            logger.warn("cmd exited with status %s: %s", status, args)
+            logger.warn("cmd exited with status %s: %s", status, cmd)
         return status
 
     def term(self, sh="/bin/sh"):
@@ -193,16 +191,16 @@ class VnodeClient(object):
         """
         return "%s -c %s -- %s" % (VCMD, self.ctrlchnlname, sh)
 
-    def shcmd(self, cmdstr, sh="/bin/sh"):
+    def shcmd(self, cmd, sh="/bin/sh"):
         """
         Execute a shell command.
 
-        :param str cmdstr: command string
+        :param str cmd: command string
         :param str sh: shell to run command in
         :return: command result
         :rtype: int
         """
-        return self.cmd([sh, "-c", cmdstr])
+        return self.cmd([sh, "-c", cmd])
 
     def shcmd_result(self, cmd, sh="/bin/sh"):
         """
@@ -213,7 +211,7 @@ class VnodeClient(object):
         :return: exist status and combined output
         :rtype: tuple[int, str]
         """
-        return self.cmdresult([sh, "-c", cmd])
+        return self.cmd_output([sh, "-c", cmd])
 
     def getaddr(self, ifname, rescan=False):
         """

daemon/core/phys/pnodes.py

@@ -65,29 +65,46 @@ class PhysicalNode(PyCoreNode):
         run a command on the physical node
         """
         os.chdir(self.nodedir)
+        status = -1
+
         try:
             if wait:
                 # os.spawnlp(os.P_WAIT, args)
-                subprocess.call(args)
+                status = subprocess.call(args)
             else:
                 # os.spawnlp(os.P_NOWAIT, args)
                 subprocess.Popen(args)
+                status = 0
         except subprocess.CalledProcessError:
-            logger.exception("cmd exited with status: %s", str(args))
+            logger.exception("cmd exited with status: %s", args)
 
-    def cmdresult(self, args):
+        return status
+
+    def cmd_output(self, args):
         """
         run a command on the physical node and get the result
         """
         os.chdir(self.nodedir)
         # in Python 2.7 we can use subprocess.check_output() here
-        tmp = subprocess.Popen(args, stdin=open(os.devnull, 'r'),
+        p = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
-                               stdout=subprocess.PIPE,
-                               stderr=subprocess.STDOUT)
         # err will always be None
-        result, err = tmp.communicate()
+        stdout, err = p.communicate()
-        status = tmp.wait()
+        status = p.wait()
-        return status, result
+        return status, stdout
+
+    def check_cmd(self, cmd):
+        """
+        Runs shell command on node.
+
+        :param list[str]/str cmd: command to run
+        :return: exist status and combined stdout and stderr
+        :rtype: tuple[int, str]
+        :raises subprocess.CalledProcessError: when a non-zero exit status occurs
+        """
+        status, output = self.cmd_output(cmd)
+        if status:
+            raise subprocess.CalledProcessError(status, cmd, output)
+        return status, output
 
     def shcmd(self, cmdstr, sh="/bin/sh"):
         return self.cmd([sh, "-c", cmdstr])
@@ -99,10 +116,10 @@ class PhysicalNode(PyCoreNode):
         self._netif[ifindex].sethwaddr(addr)
         ifname = self.ifname(ifindex)
         if self.up:
-            (status, result) = self.cmdresult(
+            try:
-                [constants.IP_BIN, "link", "set", "dev", ifname, "address", str(addr)])
+                self.check_cmd([constants.IP_BIN, "link", "set", "dev", ifname, "address", str(addr)])
-            if status:
+            except subprocess.CalledProcessError:
-                logger.error("error setting MAC address %s", str(addr))
+                logger.exception("error setting MAC address %s", addr)
 
     def addaddr(self, ifindex, addr):
         """

daemon/core/service.py

@@ -6,8 +6,7 @@ The CoreServices class handles configuration messages for sending
 a list of available services to the GUI and for configuring individual
 services.
 """
-
+import subprocess
-import shlex
 import time
 from itertools import repeat
 
@@ -307,8 +306,8 @@ class CoreServices(ConfigurableManager):
 
         for cmd in service.getstartup(node, services):
             try:
-                # NOTE: this wait=False can be problematic!
+                # TODO: this wait=False can be problematic!
-                node.client.cmd(shlex.split(cmd), wait=False)
+                node.cmd(cmd, wait=False)
             except:
                 logger.exception("error starting command %s", cmd)
 
@@ -353,8 +352,8 @@ class CoreServices(ConfigurableManager):
 
         for cmd in service._startup:
             try:
-                # NOTE: this wait=False can be problematic!
+                # TODO: this wait=False can be problematic!
-                node.client.cmd(shlex.split(cmd), wait=False)
+                node.cmd(cmd, wait=False)
             except:
                 logger.exception("error starting command %s", cmd)
 
@@ -413,11 +412,9 @@ class CoreServices(ConfigurableManager):
             for cmd in validate_cmds:
                 logger.info("validating service %s using: %s", service._name, cmd)
                 try:
-                    status, result = node.client.cmdresult(shlex.split(cmd))
+                    status, _ = node.check_cmd(cmd)
-                    if status != 0:
+                except subprocess.CalledProcessError:
-                        raise ValueError("non-zero exit status")
+                    logger.exception("validate command failed")
-                except:
-                    logger.exception("validate command failed: %s", cmd)
                     status = -1
 
         return status
@@ -449,11 +446,12 @@ class CoreServices(ConfigurableManager):
         else:
             for cmd in service._shutdown:
                 try:
-                    tmp = node.client.cmd(shlex.split(cmd), wait=True)
+                    status, _ = node.check_cmd(cmd)
-                    status += "%s" % tmp
+                    status = str(status)
-                except:
+                except subprocess.CalledProcessError:
                     logger.exception("error running stop command %s", cmd)
-                    status += "-1"
+                    # TODO: determine if its ok to just return the bad exit status
+                    status = "-1"
         return status
 
     def configure_request(self, config_data):
@@ -761,13 +759,10 @@ class CoreServices(ConfigurableManager):
                 if len(cmds) > 0:
                     for cmd in cmds:
                         try:
-                            # node.cmd(shlex.split(cmd),  wait = False)
+                            node.check_cmd(cmd)
-                            status = node.client.cmd(shlex.split(cmd), wait=True)
+                        except subprocess.CalledProcessError:
-                            if status != 0:
-                                fail += "Start %s(%s)," % (s._name, cmd)
-                        except:
                             logger.exception("error starting command %s", cmd)
-                            fail += "Start %s," % s._name
+                            fail += "Start %s(%s)," % (s._name, cmd)
             if event_type == EventTypes.PAUSE.value:
                 status = self.validatenodeservice(node, s, services)
                 if status != 0:

daemon/core/services/utility.py

@@ -418,7 +418,7 @@ class HttpService(UtilService):
         Detect the apache2 version using the 'a2query' command.
         """
         try:
-            status, result = utils.cmdresult(['a2query', '-v'])
+            status, result = utils.cmd_output(['a2query', '-v'])
         except subprocess.CalledProcessError:
             status = -1
 

daemon/core/session.py

@@ -530,7 +530,7 @@ class Session(object):
             try:
                 utils.readfileintodict(environment_user_file, env)
             except IOError:
-                logger.exception("error reading user core environment settings file: %s", environment_user_file)
+                logger.warn("error reading user core environment settings file: %s", environment_user_file)
 
         return env
 
@@ -609,7 +609,7 @@ class Session(object):
 
         :param int object_id: object id to retrieve
         :return: object for the given id
-        :rtype: core.netns.vnode.LxcNode
+        :rtype: core.coreobj.PyCoreNode
         """
         if object_id not in self.objects:
             raise KeyError("unknown object id %s" % object_id)
@@ -1238,13 +1238,14 @@ class Session(object):
             name = ""
         logger.info("scheduled event %s at time %s data=%s", name, event_time + current_time, data)
 
+    # TODO: if data is None, this blows up, but this ties into how event functions are ran, need to clean that up
     def run_event(self, node_id=None, name=None, data=None):
         """
         Run a scheduled event, executing commands in the data string.
 
         :param int node_id: node id to run event
         :param str name: event name
-        :param data: event data
+        :param str data: event data
         :return: nothing
         """
         now = self.runtime()
@@ -1252,12 +1253,13 @@ class Session(object):
             name = ""
 
         logger.info("running event %s at time %s cmd=%s" % (name, now, data))
-        commands = shlex.split(data)
         if not node_id:
+            # TODO: look to consolidate shlex to utils
+            commands = shlex.split(data)
             utils.mutedetach(commands)
         else:
             node = self.get_object(node_id)
-            node.client.cmd(commands, wait=False)
+            node.cmd(data, wait=False)
 
     def send_objects(self):
         """

daemon/examples/netns/distributed.py

@@ -82,7 +82,7 @@ def main():
     for i in xrange(1, num_local + 1):
         node = session.add_object(cls=nodes.CoreNode, name="n%d" % i, objid=i)
         node.newnetif(switch, ["%s/%s" % (prefix.addr(i), prefix.prefixlen)])
-        node.client.cmd([constants.SYSCTL_BIN, "net.ipv4.icmp_echo_ignore_broadcasts=0"])
+        node.cmd([constants.SYSCTL_BIN, "net.ipv4.icmp_echo_ignore_broadcasts=0"])
         node.setposition(x=150 * i, y=150)
         n.append(node)
 

daemon/examples/netns/emane80211.py

@@ -83,7 +83,7 @@ def main():
     for i in xrange(1, options.numnodes + 1):
         tmp = session.add_object(cls=nodes.CoreNode, name="n%d" % i, objid=i)
         tmp.newnetif(wlan, ["%s/%s" % (prefix.addr(i), prefix.prefixlen)])
-        tmp.client.cmd([constants.SYSCTL_BIN, "net.ipv4.icmp_echo_ignore_broadcasts=0"])
+        tmp.cmd([constants.SYSCTL_BIN, "net.ipv4.icmp_echo_ignore_broadcasts=0"])
         tmp.setposition(x=150 * i, y=150)
         session.services.addservicestonode(tmp, "", services_str)
         n.append(tmp)

daemon/examples/netns/howmanynodes.py

@@ -159,7 +159,7 @@ def main():
         try:
             n = session.add_object(cls=nodes.LxcNode, name="n%d" % i)
             n.newnetif(switch, ["%s/%s" % (prefix.addr(i), prefix.prefixlen)])
-            n.client.cmd([constants.SYSCTL_BIN, "net.ipv4.icmp_echo_ignore_broadcasts=0"])
+            n.cmd([constants.SYSCTL_BIN, "net.ipv4.icmp_echo_ignore_broadcasts=0"])
             if options.services is not None:
                 session.services.addservicestonode(n, "", options.services)
                 n.boot()

daemon/examples/netns/iperf-performance-chain.py

@@ -72,9 +72,9 @@ def main():
         prefix = ipaddress.Ipv4Prefix("10.83.%d.0/24" % i)
         right = session.add_object(cls=nodes.PtpNet)
         tmp.newnetif(right, ["%s/%s" % (prefix.addr(1), prefix.prefixlen)])
-        tmp.client.cmd([constants.SYSCTL_BIN, "net.ipv4.icmp_echo_ignore_broadcasts=0"])
+        tmp.cmd([constants.SYSCTL_BIN, "net.ipv4.icmp_echo_ignore_broadcasts=0"])
-        tmp.client.cmd([constants.SYSCTL_BIN, "net.ipv4.conf.all.forwarding=1"])
+        tmp.cmd([constants.SYSCTL_BIN, "net.ipv4.conf.all.forwarding=1"])
-        tmp.client.cmd([constants.SYSCTL_BIN, "net.ipv4.conf.default.rp_filter=0"])
+        tmp.cmd([constants.SYSCTL_BIN, "net.ipv4.conf.default.rp_filter=0"])
         tmp.setposition(x=100 * i, y=150)
         n.append(tmp)
         left = right

daemon/examples/netns/ospfmanetmdrtest.py

@@ -268,7 +268,7 @@ class ManetExperiment(object):
             self.nodes[i].boot()
         # run the boot.sh script on all nodes to start Quagga
         for i in xrange(numnodes):
-            self.nodes[i].client.cmd(["./%s" % self.nodes[i].bootsh])
+            self.nodes[i].cmd(["./%s" % self.nodes[i].bootsh])
 
     def compareroutes(self, node, kr, zr):
         """ Compare two lists of Route objects.
@@ -386,8 +386,7 @@ class Cmd:
 
     def open(self):
         """ Exceute call to node.popen(). """
-        self.id, self.stdin, self.out, self.err = \
+        self.id, self.stdin, self.out, self.err = self.node.client.popen(self.args)
-            self.node.client.popen(self.args)
 
     def parse(self):
         """ This method is overloaded by child classes and should return some

daemon/examples/netns/switch.py

@@ -57,7 +57,7 @@ def main():
     for i in xrange(1, options.numnodes + 1):
         tmp = session.add_object(cls=nodes.CoreNode, name="n%d" % i, objid=i)
         tmp.newnetif(switch, ["%s/%s" % (prefix.addr(i), prefix.prefixlen)])
-        tmp.client.cmd([constants.SYSCTL_BIN, "net.ipv4.icmp_echo_ignore_broadcasts=0"])
+        tmp.cmd([constants.SYSCTL_BIN, "net.ipv4.icmp_echo_ignore_broadcasts=0"])
         tmp.setposition(x=150 * i, y=150)
         n.append(tmp)
 

daemon/examples/netns/switchtest.py

@@ -36,9 +36,9 @@ def test(numnodes, testsec):
         tmp = session.add_object(cls=nodes.LxcNode, name="n%d" % i)
         tmp.newnetif(net, ["%s/%s" % (prefix.addr(i), prefix.prefixlen)])
         n.append(tmp)
-    n[0].client.cmd(["iperf", "-s", "-D"])
+    n[0].cmd(["iperf", "-s", "-D"])
     n[-1].client.icmd(["iperf", "-t", str(int(testsec)), "-c", str(prefix.addr(1))])
-    n[0].client.cmd(["killall", "-9", "iperf"])
+    n[0].cmd(["killall", "-9", "iperf"])
 
     raw_input("press enter to exit")
     session.shutdown()

daemon/examples/netns/wlanemanetests.py

@@ -166,7 +166,7 @@ class ClientServerCmd(Cmd):
         self.client_open()  # client
         status = self.client_id.wait()
         # stop the server
-        self.node.client.cmdresult(["killall", self.args[0]])
+        self.node.cmd_output(["killall", self.args[0]])
         r = self.parse()
         self.cleanup()
         return r
@@ -207,7 +207,7 @@ class PingCmd(Cmd):
     def run(self):
         if self.verbose:
             self.info("%s initial test ping (max 1 second)..." % self.node.name)
-        (status, result) = self.node.client.cmdresult(["ping", "-q", "-c", "1", "-w", "1", self.addr])
+        (status, result) = self.node.cmd_output(["ping", "-q", "-c", "1", "-w", "1", self.addr])
         if status != 0:
             self.warn("initial ping from %s to %s failed! result:\n%s" %
                       (self.node.name, self.addr, result))
@@ -226,7 +226,7 @@ class PingCmd(Cmd):
             stats = stats_str.split("/")
             avg_latency = float(stats[1])
             mdev = float(stats[3].split(" ")[0])
-        except Exception, e:
+        except:
             self.warn("ping parsing exception: %s" % e)
         return avg_latency, mdev
 
@@ -487,13 +487,13 @@ class Experiment(object):
         if i > 1:
             neigh_left = "%s" % prefix.addr(i - 1)
             cmd = routecmd + [neigh_left, "dev", node.netif(0).name]
-            (status, result) = node.client.cmdresult(cmd)
+            (status, result) = node.cmd_output(cmd)
             if status != 0:
                 self.warn("failed to add interface route: %s" % cmd)
         if i < numnodes:
             neigh_right = "%s" % prefix.addr(i + 1)
             cmd = routecmd + [neigh_right, "dev", node.netif(0).name]
-            (status, result) = node.client.cmdresult(cmd)
+            (status, result) = node.cmd_output(cmd)
             if status != 0:
                 self.warn("failed to add interface route: %s" % cmd)
 
@@ -507,7 +507,7 @@ class Experiment(object):
             else:
                 gw = neigh_right
             cmd = routecmd + [addr, "via", gw]
-            (status, result) = node.client.cmdresult(cmd)
+            (status, result) = node.cmd_output(cmd)
             if status != 0:
                 self.warn("failed to add route: %s" % cmd)
 
@@ -635,7 +635,7 @@ class Experiment(object):
         if self.verbose:
             self.info("%s initial test ping (max 1 second)..." % \
                       self.firstnode.name)
-        (status, result) = self.firstnode.client.cmdresult(["ping", "-q", "-c", "1",
+        (status, result) = self.firstnode.cmd_output(["ping", "-q", "-c", "1",
                                                                  "-w", "1", self.lastaddr])
         if status != 0:
             self.warn("initial ping from %s to %s failed! result:\n%s" % \

daemon/examples/netns/wlantest.py

@@ -37,9 +37,9 @@ def test(numnodes, testsec):
         tmp.newnetif(net, ["%s/%s" % (prefix.addr(i), prefix.prefixlen)])
         n.append(tmp)
     net.link(n[0].netif(0), n[-1].netif(0))
-    n[0].client.cmd(["iperf", "-s", "-D"])
+    n[0].cmd(["iperf", "-s", "-D"])
     n[-1].client.icmd(["iperf", "-t", str(int(testsec)), "-c", str(prefix.addr(1))])
-    n[0].client.cmd(["killall", "-9", "iperf"])
+    n[0].cmd(["killall", "-9", "iperf"])
     session.shutdown()
 
 

daemon/tests/conftest.py

@@ -5,23 +5,18 @@ Unit test fixture module.
 import os
 
 import pytest
-
 from mock.mock import MagicMock
 
 from core import services
-from core.coreserver import CoreServer
-from core.misc import nodemaps
-from core.misc import nodeutils
-from core.netns import nodes
-from core.session import Session
 from core.api.coreapi import CoreConfMessage
 from core.api.coreapi import CoreEventMessage
 from core.api.coreapi import CoreExecMessage
 from core.api.coreapi import CoreLinkMessage
 from core.api.coreapi import CoreNodeMessage
 from core.corehandlers import CoreRequestHandler
-from core.enumerations import ConfigTlvs
+from core.coreserver import CoreServer
 from core.enumerations import CORE_API_PORT
+from core.enumerations import ConfigTlvs
 from core.enumerations import EventTlvs
 from core.enumerations import EventTypes
 from core.enumerations import ExecuteTlvs
@@ -32,6 +27,8 @@ from core.enumerations import NodeTlvs
 from core.enumerations import NodeTypes
 from core.misc import ipaddress
 from core.misc.ipaddress import MacAddress
+from core.netns import nodes
+from core.session import Session
 
 EMANE_SERVICES = "zebra|OSPFv3MDR|IPForward"
 
@@ -186,18 +183,13 @@ class Core(object):
     def ping(self, from_name, to_name):
         from_node = self.nodes[from_name]
         to_ip = str(self.get_ip(to_name))
-        return from_node.client.cmd(["ping", "-c", "3", to_ip])
+        return from_node.cmd(["ping", "-c", "3", to_ip])
 
     def ping_output(self, from_name, to_name):
         from_node = self.nodes[from_name]
         to_ip = str(self.get_ip(to_name))
-        vcmd, stdin, stdout, stderr = from_node.client.popen(["ping", "-i", "0.05", "-c", "3", to_ip])
+        _, output = from_node.check_cmd(["ping", "-i", "0.05", "-c", "3", to_ip])
-        return stdout.read().strip()
+        return output
-
-    def iping(self, from_name, to_name):
-        from_node = self.nodes[from_name]
-        to_ip = str(self.get_ip(to_name))
-        from_node.client.icmd(["ping", "-i", "0.01", "-c", "10", to_ip])
 
     def iperf(self, from_name, to_name):
         from_node = self.nodes[from_name]
@@ -206,8 +198,8 @@ class Core(object):
 
         # run iperf server, run client, kill iperf server
         vcmd, stdin, stdout, stderr = to_node.client.popen(["iperf", "-s", "-u", "-y", "C"])
-        from_node.client.cmd(["iperf", "-u", "-t", "5", "-c", to_ip])
+        from_node.cmd(["iperf", "-u", "-t", "5", "-c", to_ip])
-        to_node.client.cmd(["killall", "-9", "iperf"])
+        to_node.cmd(["killall", "-9", "iperf"])
 
         return stdout.read().strip()
 

daemon/tests/test_core.py

@@ -177,7 +177,7 @@ class TestCore:
         # check various command using vcmd module
         command = ["ls"]
         assert not client.cmd(command)
-        status, output = client.cmdresult(command)
+        status, output = client.cmd_output(command)
         assert not status
         p, stdin, stdout, stderr = client.popen(command)
         assert not p.status()
@@ -187,7 +187,7 @@ class TestCore:
 
         # check various command using command line
         assert not client.cmd(command)
-        status, output = client.cmdresult(command)
+        status, output = client.cmd_output(command)
         assert not status
         p, stdin, stdout, stderr = client.popen(command)
         assert not p.wait()