Merge pull request #24 from HPENetworking/new_shell_api_migration

chg: dev: Migrated all nodes shells to new Topology shell API.

lib/topology_docker/nodes/host.py

@@ -23,7 +23,7 @@
 from __future__ import print_function, division
 
 from topology_docker.node import DockerNode
-from topology_docker.shell import DockerShell
+from topology_docker.shell import DockerBashShell
 
 
 class HostNode(DockerNode):
@@ -39,8 +39,8 @@ class HostNode(DockerNode):
     def __init__(self, identifier, image='ubuntu:latest', **kwargs):
 
         super(HostNode, self).__init__(identifier, image=image, **kwargs)
-        self._shells['bash'] = DockerShell(
-            self.container_id, 'sh -c "TERM=dumb bash"', 'root@.*:.*# '
+        self._shells['bash'] = DockerBashShell(
+            self.container_id, 'bash'
         )
 
 

lib/topology_docker/nodes/openswitch.py

@@ -27,8 +27,8 @@
 from json import loads
 
 from topology_docker.node import DockerNode
-from topology_docker.shell import DockerShell
 from topology_docker.utils import ensure_dir
+from topology_docker.shell import DockerShell, DockerBashShell
 
 
 SETUP_SCRIPT = """\
@@ -231,19 +231,19 @@ def __init__(
         self.shared_dir = shared_dir
 
         # Add vtysh (default) and bash shell
+        # FIXME: Create a subclass to handle better the particularities of
+        # vtysh, like prompt setup etc.
         self._shells['vtysh'] = DockerShell(
             self.container_id, 'vtysh', '(^|\n)switch(\([\-a-zA-Z0-9]*\))?#'
         )
-        self._shells['bash'] = DockerShell(
-            self.container_id, 'sh -c "TERM=dumb bash"', 'bash-.*#'
+        self._shells['bash'] = DockerBashShell(
+            self.container_id, 'bash'
         )
-        self._shells['bash_swns'] = DockerShell(
-            self.container_id,
-            'sh -c "TERM=dumb ip netns exec swns bash"',
-            'bash-.*#'
+        self._shells['bash_swns'] = DockerBashShell(
+            self.container_id, 'ip netns exec swns bash'
         )
-        self._shells['vsctl'] = DockerShell(
-            self.container_id, 'sh -c "TERM=dumb bash"', 'bash-.*#',
+        self._shells['vsctl'] = DockerBashShell(
+            self.container_id, 'bash',
             prefix='ovs-vsctl ', timeout=60
         )
 

lib/topology_docker/nodes/p4switch.py

@@ -28,8 +28,8 @@
 from shlex import split as shsplit
 
 from topology_docker.node import DockerNode
-from topology_docker.shell import DockerShell
 from topology_docker.utils import ensure_dir
+from topology_docker.shell import DockerBashShell
 
 
 class P4SwitchNode(DockerNode):
@@ -72,50 +72,56 @@ def __init__(
         self.shared_dir = shared_dir
 
         # Add bash shell
-        self._shells['bash'] = DockerShell(
-            self.container_id, 'sh -c "TERM=dumb bash"', 'root@.*:.*# '
-        )
-
-        # Add SAI shell (https://github.com/p4lang/switchsai)
-        self._shells['sai'] = DockerShell(
-            self.container_id,
-            'sh -c "TERM=dumb bash"',
-            'root@.*:.*# ',
-            '/p4factory/targets/switch/tests/pd_thrift/switch_sai_rpc-remote '
-            '-h localhost:9092 '
-        )
-
-        # Add switchapi shell (https://github.com/p4lang/switchapi)
-        self._shells['api'] = DockerShell(
-            self.container_id,
-            'sh -c "TERM=dumb bash"',
-            'root@.*:.*# ',
-            '/p4factory/targets/switch/tests/pd_thrift/switch_api_rpc-remote '
-            '-h localhost:9091 '
+        self._shells['bash'] = DockerBashShell(
+            self.container_id, 'bash'
         )
 
+        # NOTE: It seems that nobody is using this shells.
+        # They got commented when migrating to the new Shell API as not enough
+        # knowledge was gathered to determine how to migrate them.
+
+        # Add SAI shell
+        # https://github.com/p4lang/switch/tree/master/switchsai
+        # self._shells['sai'] = DockerShell(
+        #     self.container_id,
+        #     'sh -c "TERM=dumb bash"',
+        #     'root@.*:.*# ',
+        #     '/p4factory/targets/switch/tests/pd_thrift/switch_sai_rpc-remote'
+        #     ' -h localhost:9092 '
+        # )
+        #
+        # Add switchapi shell
+        # https://github.com/p4lang/switch/tree/master/switchapi
+        # self._shells['api'] = DockerShell(
+        #     self.container_id,
+        #     'sh -c "TERM=dumb bash"',
+        #     'root@.*:.*# ',
+        #     '/p4factory/targets/switch/tests/pd_thrift/switch_api_rpc-remote'
+        #     ' -h localhost:9091 '
+        # )
+        #
         # Add PD shells
-        self._shells['pd_conn_mgr'] = DockerShell(
-            self.container_id,
-            'sh -c "TERM=dumb bash"',
-            'root@.*:.*# ',
-            '/p4factory/targets/switch/tests/pd_thrift/conn_mgr-remote '
-            '-h localhost:9090 '
-        )
-        self._shells['pd_mc'] = DockerShell(
-            self.container_id,
-            'sh -c "TERM=dumb bash"',
-            'root@.*:.*# ',
-            '/p4factory/targets/switch/tests/pd_thrift/mc-remote '
-            '-h localhost:9090 '
-        )
-        self._shells['pd_p4'] = DockerShell(
-            self.container_id,
-            'sh -c "TERM=dumb bash"',
-            'root@.*:.*# ',
-            '/p4factory/targets/switch/tests/pd_thrift/dc-remote '
-            '-h localhost:9090 '
-        )
+        # self._shells['pd_conn_mgr'] = DockerShell(
+        #     self.container_id,
+        #     'sh -c "TERM=dumb bash"',
+        #     'root@.*:.*# ',
+        #     '/p4factory/targets/switch/tests/pd_thrift/conn_mgr-remote '
+        #     '-h localhost:9090 '
+        # )
+        # self._shells['pd_mc'] = DockerShell(
+        #     self.container_id,
+        #     'sh -c "TERM=dumb bash"',
+        #     'root@.*:.*# ',
+        #     '/p4factory/targets/switch/tests/pd_thrift/mc-remote '
+        #     '-h localhost:9090 '
+        # )
+        # self._shells['pd_p4'] = DockerShell(
+        #     self.container_id,
+        #     'sh -c "TERM=dumb bash"',
+        #     'root@.*:.*# ',
+        #     '/p4factory/targets/switch/tests/pd_thrift/dc-remote '
+        #     '-h localhost:9090 '
+        # )
 
     def notify_post_build(self):
         """

lib/topology_docker/nodes/ryu.py

@@ -31,8 +31,8 @@
 from shlex import split as shsplit
 
 from topology_docker.node import DockerNode
-from topology_docker.shell import DockerShell
 from topology_docker.utils import ensure_dir
+from topology_docker.shell import DockerBashShell
 
 
 class RyuControllerNode(DockerNode):
@@ -84,8 +84,8 @@ def __init__(
             copy(self.app_name, self.shared_dir)
 
         # Add bash shell
-        self._shells['bash'] = DockerShell(
-            self.container_id, 'sh -c "TERM=dumb bash"', 'root@.*:.*# '
+        self._shells['bash'] = DockerBashShell(
+            self.container_id, 'bash'
         )
 
     def notify_post_build(self):

lib/topology_docker/shell.py

@@ -22,63 +22,44 @@
 from __future__ import unicode_literals, absolute_import
 from __future__ import print_function, division
 
-from pexpect import spawn
+from topology.platforms.shell import PExpectShell, PExpectBashShell
 
 
-class DockerShell(object):
+class DockerExecMixin(object):
     """
-    Shell helper class.
+    Docker ``exec`` connection mixin for the Topology shell API.
 
-    This class wrapps a ``docker exec`` call to a given shell command and
-    manages a pexpect spawn object for it.
+    This class implements a ``_get_connect_command()`` method that allows to
+    interact with a shell through a ``docker exec`` interactive command, and
+    extends the constructor to request for container related parameters.
 
-    It implementes the ``__call__`` method that allows to call the objects
-    as it were the contained shell.
+    :param str container: Container unique identifier.
+    :param str command: Command to be executed with the ``docker exec`` that
+     will launch an interactive session.
     """
 
-    def __init__(
-            self, container, shell, prompt,
-            prefix=None, timeout=None, encoding='utf-8'):
+    def __init__(self, container, command, *args, **kwargs):
         self._container = container
-        self._shell = shell
-        self._prompt = prompt
-        self._prefix = prefix
-        self._timeout = timeout or -1
-        self._encoding = encoding
-        self._spawn = None
+        self._command = command
+        super(DockerExecMixin, self).__init__(*args, **kwargs)
 
-    def __call__(self, command):
+    def _get_connect_command(self):
+        return 'docker exec -i -t {} {}'.format(
+            self._container, self._command
+        )
 
-        # Lazy-spawn
-        if self._spawn is None:
-            self._spawn = spawn(
-                'docker exec -i -t {} {}'.format(
-                    self._container, self._shell
-                ),
-                echo=False
-            )
-            # Cut output at first prompt
-            self._spawn.expect(self._prompt, timeout=self._timeout)
 
-        # Prefix command if required
-        if self._prefix is not None:
-            command = self._prefix + command
-
-        self._spawn.sendline(command)
-        self._spawn.expect(self._prompt, timeout=self._timeout)
-
-        # Convert binary representation to unicode using encoding
-        raw = self._spawn.before.decode(self._encoding)
-
-        # Remove leading and trailing whitespaces and normalize newlines
-        lines = raw.strip().replace('\r', '').splitlines()
-        del raw
+class DockerShell(DockerExecMixin, PExpectShell):
+    """
+    Generic ``docker exec`` shell for unspecified interactive session.
+    """
 
-        # Remove echo command if it exists
-        if lines and lines[0].strip() == command.strip():
-            lines.pop(0)
 
-        return '\n'.join(lines)
+class DockerBashShell(DockerExecMixin, PExpectBashShell):
+    """
+    Specialized ``docker exec`` shell that will run and setup a bash
+    interactive session.
+    """
 
 
-__all__ = ['DockerShell']
+__all__ = ['DockerShell', 'DockerBashShell']

requirements.txt

@@ -1,4 +1,3 @@
 six
-pexpect
 docker-py
 topology>=1.5.0