Refactor attach()/start() after podman changes

* Update examples
* Update/Clean up unittests
* Add Mixins for container attach()/start()

Signed-off-by: Jhon Honce <jhonce@redhat.com>

Closes: #1080
Approved by: baude

4 files changed, 117 insertions, 80 deletions

diff --git a/contrib/python/podman/libs/_containers_attach.py b/contrib/python/podman/libs/_containers_attach.py
index bd73542b9..df12fa998 100644
--- a/contrib/python/podman/libs/_containers_attach.py
+++ b/contrib/python/podman/libs/_containers_attach.py
@@ -1,16 +1,11 @@
 """Exported method Container.attach()."""
 
+import collections
 import fcntl
-import os
-import select
-import signal
-import socket
+import logging
 import struct
 import sys
 import termios
-import tty
-
-CONMON_BUFSZ = 8192
 
 
 class Mixin:
@@ -20,10 +15,8 @@ class Mixin:
         """Attach to container's PID1 stdin and stdout.
 
         stderr is ignored.
+        PseudoTTY work is done in start().
         """
-        if not self.containerrunning:
-            raise Exception('you can only attach to running containers')
-
         if stdin is None:
             stdin = sys.stdin.fileno()
 
@@ -41,73 +34,42 @@ class Mixin:
             )
 
         # This is the control socket where resizing events are sent to conmon
-        ctl_socket = attach['sockets']['control_socket']
+        # attach['sockets']['control_socket']
+        self.pseudo_tty = collections.namedtuple(
+            'PseudoTTY',
+            ['stdin', 'stdout', 'io_socket', 'control_socket', 'eot'])(
+                stdin,
+                stdout,
+                attach['sockets']['io_socket'],
+                attach['sockets']['control_socket'],
+                eot,
+            )
 
-        def resize_handler(signum, frame):
-            """Send the new window size to conmon.
+    @property
+    def resize_handler(self):
+        """Send the new window size to conmon."""
 
-            The method arguments are not used.
-            """
-            packed = fcntl.ioctl(stdout, termios.TIOCGWINSZ,
+        def wrapped(signum, frame):
+            packed = fcntl.ioctl(self.pseudo_tty.stdout, termios.TIOCGWINSZ,
                                  struct.pack('HHHH', 0, 0, 0, 0))
             rows, cols, _, _ = struct.unpack('HHHH', packed)
+            logging.debug('Resize window({}x{}) using {}'.format(
+                rows, cols, self.pseudo_tty.control_socket))
+
             # TODO: Need some kind of timeout in case pipe is blocked
-            with open(ctl_socket, 'w') as skt:
+            with open(self.pseudo_tty.control_socket, 'w') as skt:
                 # send conmon window resize message
                 skt.write('1 {} {}\n'.format(rows, cols))
 
-        def log_handler(signum, frame):
-            """Send command to reopen log to conmon.
+        return wrapped
+
+    @property
+    def log_handler(self):
+        """Send command to reopen log to conmon."""
 
-            The method arguments are not used.
-            """
-            with open(ctl_socket, 'w') as skt:
+        def wrapped(signum, frame):
+            with open(self.pseudo_tty.control_socket, 'w') as skt:
                 # send conmon reopen log message
                 skt.write('2\n')
 
-        try:
-            # save off the old settings for terminal
-            original_attr = termios.tcgetattr(stdout)
-            tty.setraw(stdin)
-
-            # initialize containers window size
-            resize_handler(None, sys._getframe(0))
-
-            # catch any resizing events and send the resize info
-            # to the control fifo "socket"
-            signal.signal(signal.SIGWINCH, resize_handler)
-
-        except termios.error:
-            original_attr = None
-
-        try:
-            # TODO: socket.SOCK_SEQPACKET may not be supported in Windows
-            with socket.socket(socket.AF_UNIX, socket.SOCK_SEQPACKET) as skt:
-                # Prepare socket for communicating with conmon/container
-                skt.connect(io_socket)
-                skt.sendall(b'\n')
-
-                sources = [skt, stdin]
-                while sources:
-                    readable, _, _ = select.select(sources, [], [])
-                    if skt in readable:
-                        data = skt.recv(CONMON_BUFSZ)
-                        if not data:
-                            sources.remove(skt)
-
-                        # Remove source marker when writing
-                        os.write(stdout, data[1:])
-
-                    if stdin in readable:
-                        data = os.read(stdin, CONMON_BUFSZ)
-                        if not data:
-                            sources.remove(stdin)
-
-                        skt.sendall(data)
-
-                        if eot in data:
-                            sources.clear()
-        finally:
-            if original_attr:
-                termios.tcsetattr(stdout, termios.TCSADRAIN, original_attr)
-                signal.signal(signal.SIGWINCH, signal.SIG_DFL)
+        return wrapped
diff --git a/contrib/python/podman/libs/_containers_start.py b/contrib/python/podman/libs/_containers_start.py
new file mode 100644
index 000000000..ad9f32eab
--- /dev/null
+++ b/contrib/python/podman/libs/_containers_start.py
@@ -0,0 +1,82 @@
+"""Exported method Container.start()."""
+import logging
+import os
+import select
+import signal
+import socket
+import sys
+import termios
+import tty
+
+CONMON_BUFSZ = 8192
+
+
+class Mixin:
+    """Publish start() for inclusion in Container class."""
+
+    def start(self):
+        """Start container, return container on success.
+
+        Will block if container has been detached.
+        """
+        with self._client() as podman:
+            results = podman.StartContainer(self.id)
+            logging.debug('Started Container "{}"'.format(
+                results['container']))
+
+            if not hasattr(self, 'pseudo_tty') or self.pseudo_tty is None:
+                return self._refresh(podman)
+
+            logging.debug('Setting up PseudoTTY for Container "{}"'.format(
+                results['container']))
+
+            try:
+                # save off the old settings for terminal
+                tcoldattr = termios.tcgetattr(self.pseudo_tty.stdin)
+                tty.setraw(self.pseudo_tty.stdin)
+
+                # initialize container's window size
+                self.resize_handler(None, sys._getframe(0))
+
+                # catch any resizing events and send the resize info
+                # to the control fifo "socket"
+                signal.signal(signal.SIGWINCH, self.resize_handler)
+
+            except termios.error:
+                tcoldattr = None
+
+            try:
+                # TODO: Is socket.SOCK_SEQPACKET supported in Windows?
+                with socket.socket(socket.AF_UNIX,
+                                   socket.SOCK_SEQPACKET) as skt:
+                    # Prepare socket for use with conmon/container
+                    skt.connect(self.pseudo_tty.io_socket)
+
+                    sources = [skt, self.pseudo_tty.stdin]
+                    while sources:
+                        logging.debug('Waiting on sources: {}'.format(sources))
+                        readable, _, _ = select.select(sources, [], [])
+
+                        if skt in readable:
+                            data = skt.recv(CONMON_BUFSZ)
+                            if data:
+                                # Remove source marker when writing
+                                os.write(self.pseudo_tty.stdout, data[1:])
+                            else:
+                                sources.remove(skt)
+
+                        if self.pseudo_tty.stdin in readable:
+                            data = os.read(self.pseudo_tty.stdin, CONMON_BUFSZ)
+                            if data:
+                                skt.sendall(data)
+
+                                if self.pseudo_tty.eot in data:
+                                    sources.clear()
+                            else:
+                                sources.remove(self.pseudo_tty.stdin)
+            finally:
+                if tcoldattr:
+                    termios.tcsetattr(self.pseudo_tty.stdin, termios.TCSADRAIN,
+                                      tcoldattr)
+                    signal.signal(signal.SIGWINCH, signal.SIG_DFL)
+            return self._refresh(podman)
diff --git a/contrib/python/podman/libs/containers.py b/contrib/python/podman/libs/containers.py
index a350a128a..6dc2c141e 100644
--- a/contrib/python/podman/libs/containers.py
+++ b/contrib/python/podman/libs/containers.py
@@ -7,9 +7,10 @@ import signal
 import time
 
 from ._containers_attach import Mixin as AttachMixin
+from ._containers_start import Mixin as StartMixin
 
 
-class Container(collections.UserDict, AttachMixin):
+class Container(AttachMixin, StartMixin, collections.UserDict):
     """Model for a container."""
 
     def __init__(self, client, id, data):
@@ -143,12 +144,6 @@ class Container(collections.UserDict, AttachMixin):
                                     message, pause)
         return results['image']
 
-    def start(self):
-        """Start container, return container on success."""
-        with self._client() as podman:
-            podman.StartContainer(self.id)
-            return self._refresh(podman)
-
     def stop(self, timeout=25):
         """Stop container, return id on success."""
         with self._client() as podman:
diff --git a/contrib/python/podman/libs/images.py b/contrib/python/podman/libs/images.py
index 3beadec1d..334ff873c 100644
--- a/contrib/python/podman/libs/images.py
+++ b/contrib/python/podman/libs/images.py
@@ -3,6 +3,7 @@ import collections
 import copy
 import functools
 import json
+import logging
 
 from . import Config
 from .containers import Container
@@ -37,11 +38,8 @@ class Image(collections.UserDict):
 
         Pulls defaults from image.inspect()
         """
-        with self._client() as podman:
-            details = self.inspect()
+        details = self.inspect()
 
-        # TODO: remove network settings once defaults implemented in service
-        # Inialize config from parameters, then add image information
         config = Config(image_id=self.id, **kwargs)
         config['command'] = details.containerconfig['cmd']
         config['env'] = self._split_token(details.containerconfig['env'])
@@ -49,8 +47,8 @@ class Image(collections.UserDict):
         config['labels'] = copy.deepcopy(details.labels)
         config['net_mode'] = 'bridge'
         config['network'] = 'bridge'
-        config['work_dir'] = '/tmp'
 
+        logging.debug('Image {}: create config: {}'.format(self.id, config))
         with self._client() as podman:
             id = podman.CreateContainer(config)['container']
             cntr = podman.GetContainer(id)