Implement python podman create and start

- Added alias 'container()' to image model for CreateContainer()
- Fixed return in containers_create.go to wrap error in varlink
  exception
- Added a wait time to container.kill(), number of seconds to wait
  for the container to change state
- Refactored cached_property() to use system libraries
- Refactored tests to speed up performance

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

Closes: #821
Approved by: rhatdan

6 files changed, 188 insertions, 175 deletions

diff --git a/contrib/python/podman/libs/__init__.py b/contrib/python/podman/libs/__init__.py
index 7285921d7..9db5dacf5 100644
--- a/contrib/python/podman/libs/__init__.py
+++ b/contrib/python/podman/libs/__init__.py
@@ -1,8 +1,8 @@
 """Support files for podman API implementation."""
 import datetime
-import threading
-from dateutil.parser import parse as dateutil_parse
+import functools
 
+from dateutil.parser import parse as dateutil_parse
 
 __all__ = [
     'cached_property',
@@ -11,37 +11,28 @@ __all__ = [
 ]
 
 
-class cached_property(object):
-    """cached_property() - computed once per instance, cached as attribute.
+def cached_property(fn):
+    """Cache return to save future expense."""
+    return property(functools.lru_cache(maxsize=8)(fn))
 
-    Maybe this will make a future version of python.
-    """
 
-    def __init__(self, func):
-        """Construct context manager."""
-        self.func = func
-        self.__doc__ = func.__doc__
-        self.lock = threading.RLock()
-
-    def __get__(self, instance, cls=None):
-        """Retrieve previous value, or call func()."""
-        if instance is None:
-            return self
-
-        attrname = self.func.__name__
-        try:
-            cache = instance.__dict__
-        except AttributeError:  # objects with __slots__ have no __dict__
-            msg = ("No '__dict__' attribute on {}"
-                   " instance to cache {} property.").format(
-                       repr(type(instance).__name__), repr(attrname))
-            raise TypeError(msg) from None
-
-        with self.lock:
-            # check if another thread filled cache while we awaited lock
-            if attrname not in cache:
-                cache[attrname] = self.func(instance)
-        return cache[attrname]
+# Cannot subclass collections.UserDict, breaks varlink
+class Config(dict):
+    """Silently ignore None values, only take key once."""
+
+    def __init__(self, **kwargs):
+        """Construct dictionary."""
+        super(Config, self).__init__(kwargs)
+
+    def __setitem__(self, key, value):
+        """Store unique, not None values."""
+        if value is None:
+            return
+
+        if super().__contains__(key):
+            return
+
+        super().__setitem__(key, value)
 
 
 def datetime_parse(string):
diff --git a/contrib/python/podman/libs/containers.py b/contrib/python/podman/libs/containers.py
index 04ea76180..96ec6be37 100644
--- a/contrib/python/podman/libs/containers.py
+++ b/contrib/python/podman/libs/containers.py
@@ -4,6 +4,7 @@ import functools
 import getpass
 import json
 import signal
+import time
 
 
 class Container(collections.UserDict):
@@ -16,28 +17,34 @@ class Container(collections.UserDict):
         self._client = client
         self._id = id
 
-        self._refresh(data)
+        with client() as podman:
+            self._refresh(podman)
+
         assert self._id == self.data['id'],\
             'Requested container id({}) does not match store id({})'.format(
                 self._id, self.id
             )
 
     def __getitem__(self, key):
-        """Get items from parent dict + apply aliases."""
-        if key == 'running':
-            key = 'containerrunning'
+        """Get items from parent dict."""
         return super().__getitem__(key)
 
-    def _refresh(self, data):
-        super().update(data)
-        for k, v in data.items():
+    def _refresh(self, podman):
+        ctnr = podman.GetContainer(self._id)
+        super().update(ctnr['container'])
+
+        for k, v in self.data.items():
             setattr(self, k, v)
-        setattr(self, 'running', data['containerrunning'])
+        if 'containerrunning' in self.data:
+            setattr(self, 'running', self.data['containerrunning'])
+            self.data['running'] = self.data['containerrunning']
+
+        return self
 
     def refresh(self):
         """Refresh status fields for this container."""
-        ctnr = Containers(self._client).get(self.id)
-        self._refresh(ctnr)
+        with self._client() as podman:
+            return self._refresh(podman)
 
     def attach(self, detach_key=None, no_stdin=False, sig_proxy=True):
         """Attach to running container."""
@@ -49,8 +56,7 @@ class Container(collections.UserDict):
         """Show processes running in container."""
         with self._client() as podman:
             results = podman.ListContainerProcesses(self.id)
-        for p in results['container']:
-            yield p
+        yield from results['container']
 
     def changes(self):
         """Retrieve container changes."""
@@ -58,14 +64,24 @@ class Container(collections.UserDict):
             results = podman.ListContainerChanges(self.id)
         return results['container']
 
-    def kill(self, signal=signal.SIGTERM):
-        """Send signal to container, return id if successful.
+    def kill(self, signal=signal.SIGTERM, wait=25):
+        """Send signal to container.
 
         default signal is signal.SIGTERM.
+        wait n of seconds, 0 waits forever.
         """
         with self._client() as podman:
-            results = podman.KillContainer(self.id, signal)
-        return results['container']
+            podman.KillContainer(self.id, signal)
+            timeout = time.time() + wait
+            while True:
+                self._refresh(podman)
+                if self.status != 'running':
+                    return self
+
+                if wait and timeout < time.time():
+                    raise TimeoutError()
+
+                time.sleep(0.5)
 
     def _lower_hook(self):
         """Convert all keys to lowercase."""
@@ -80,10 +96,8 @@ class Container(collections.UserDict):
         """Retrieve details about containers."""
         with self._client() as podman:
             results = podman.InspectContainer(self.id)
-            obj = json.loads(
-                results['container'], object_hook=self._lower_hook())
-            return collections.namedtuple('ContainerInspect',
-                                          obj.keys())(**obj)
+        obj = json.loads(results['container'], object_hook=self._lower_hook())
+        return collections.namedtuple('ContainerInspect', obj.keys())(**obj)
 
     def export(self, target):
         """Export container from store to tarball.
@@ -134,16 +148,16 @@ class Container(collections.UserDict):
         return results['image']
 
     def start(self):
-        """Start container, return id on success."""
+        """Start container, return container on success."""
         with self._client() as podman:
-            results = podman.StartContainer(self.id)
-        return results['container']
+            podman.StartContainer(self.id)
+            return self._refresh(podman)
 
     def stop(self, timeout=25):
         """Stop container, return id on success."""
         with self._client() as podman:
-            results = podman.StopContainer(self.id, timeout)
-        return results['container']
+            podman.StopContainer(self.id, timeout)
+            return self._refresh(podman)
 
     def remove(self, force=False):
         """Remove container, return id on success.
@@ -157,8 +171,8 @@ class Container(collections.UserDict):
     def restart(self, timeout=25):
         """Restart container with timeout, return id on success."""
         with self._client() as podman:
-            results = podman.RestartContainer(self.id, timeout)
-        return results['container']
+            podman.RestartContainer(self.id, timeout)
+            return self._refresh(podman)
 
     def rename(self, target):
         """Rename container, return id on success."""
@@ -177,21 +191,20 @@ class Container(collections.UserDict):
     def pause(self):
         """Pause container, return id on success."""
         with self._client() as podman:
-            results = podman.PauseContainer(self.id)
-        return results['container']
+            podman.PauseContainer(self.id)
+            return self._refresh(podman)
 
     def unpause(self):
         """Unpause container, return id on success."""
         with self._client() as podman:
-            results = podman.UnpauseContainer(self.id)
-        return results['container']
+            podman.UnpauseContainer(self.id)
+            return self._refresh(podman)
 
     def update_container(self, *args, **kwargs):
         """TODO: Update container..., return id on success."""
         with self._client() as podman:
-            results = podman.UpdateContainer()
-        self.refresh()
-        return results['container']
+            podman.UpdateContainer()
+            return self._refresh(podman)
 
     def wait(self):
         """Wait for container to finish, return 'returncode'."""
@@ -210,8 +223,7 @@ class Container(collections.UserDict):
         """Retrieve container logs."""
         with self._client() as podman:
             results = podman.GetContainerLogs(self.id)
-        for line in results:
-            yield line
+        yield from results
 
 
 class Containers(object):
@@ -234,15 +246,6 @@ class Containers(object):
             results = podman.DeleteStoppedContainers()
         return results['containers']
 
-    def create(self, *args, **kwargs):
-        """Create container layer over the specified image.
-
-        See podman-create.1.md for kwargs details.
-        """
-        with self._client() as podman:
-            results = podman.CreateContainer()
-        return results['id']
-
     def get(self, id):
         """Retrieve container details from store."""
         with self._client() as podman:
diff --git a/contrib/python/podman/libs/images.py b/contrib/python/podman/libs/images.py
index d6fd7946d..d617a766b 100644
--- a/contrib/python/podman/libs/images.py
+++ b/contrib/python/podman/libs/images.py
@@ -3,6 +3,9 @@ import collections
 import functools
 import json
 
+from . import Config
+from .containers import Container
+
 
 class Image(collections.UserDict):
     """Model for an Image."""
@@ -25,8 +28,42 @@ class Image(collections.UserDict):
         """Get items from parent dict."""
         return super().__getitem__(key)
 
+    def _split_token(self, values=None, sep='='):
+        mapped = {}
+        if values:
+            for var in values:
+                k, v = var.split(sep, 1)
+                mapped[k] = v
+        return mapped
+
+    def create(self, *args, **kwargs):
+        """Create container from image.
+
+        Pulls defaults from image.inspect()
+        """
+        # Inialize config from parameters
+        with self._client() as podman:
+            details = self.inspect()
+
+        # TODO: remove network settings once defaults implemented on service side
+        config = Config(image_id=self.id, **kwargs)
+        config['command'] = details.containerconfig['cmd']
+        config['env'] = self._split_token(details.containerconfig['env'])
+        config['image'] = details.repotags[0]
+        config['labels'] = self._split_token(details.labels)
+        config['net_mode'] = 'bridge'
+        config['network'] = 'bridge'
+        config['work_dir'] = '/tmp'
+
+        with self._client() as podman:
+            id = podman.CreateContainer(config)['container']
+            cntr = podman.GetContainer(id)
+        return Container(self._client, id, cntr['container'])
+
+    container = create
+
     def export(self, dest, compressed=False):
-        """Write image to dest, return True on success."""
+        """Write image to dest, return id on success."""
         with self._client() as podman:
             results = podman.ExportImage(self.id, dest, compressed)
         return results['image']
@@ -50,8 +87,8 @@ class Image(collections.UserDict):
         """Retrieve details about image."""
         with self._client() as podman:
             results = podman.InspectImage(self.id)
-            obj = json.loads(results['image'], object_hook=self._lower_hook())
-            return collections.namedtuple('ImageInspect', obj.keys())(**obj)
+        obj = json.loads(results['image'], object_hook=self._lower_hook())
+        return collections.namedtuple('ImageInspect', obj.keys())(**obj)
 
     def push(self, target, tlsverify=False):
         """Copy image to target, return id on success."""
@@ -89,12 +126,6 @@ class Images(object):
         for img in results['images']:
             yield Image(self._client, img['id'], img)
 
-    def create(self, *args, **kwargs):
-        """Create image from configuration."""
-        with self._client() as podman:
-            results = podman.CreateImage()
-        return results['image']
-
     def build(self, *args, **kwargs):
         """Build container from image.
 
@@ -125,9 +156,9 @@ class Images(object):
     def search(self, id, limit=25):
         """Search registries for id."""
         with self._client() as podman:
-            results = podman.SearchImage(id)
+            results = podman.SearchImage(id, limit)
         for img in results['images']:
-            yield img
+            yield collections.namedtuple('ImageSearch', img.keys())(**img)
 
     def get(self, id):
         """Get Image from id."""
diff --git a/contrib/python/test/test_containers.py b/contrib/python/test/test_containers.py
index ff87f3a58..5c7c9934b 100644
--- a/contrib/python/test/test_containers.py
+++ b/contrib/python/test/test_containers.py
@@ -1,4 +1,5 @@
 import os
+import signal
 import time
 import unittest
 from test.podman_testcase import PodmanTestCase
@@ -21,42 +22,39 @@ class TestContainers(PodmanTestCase):
         self.host = os.environ['PODMAN_HOST']
 
         self.pclient = podman.Client(self.host)
-        self.ctns = self.loadCache()
-        # TODO: Change to start() when Implemented
-        self.alpine_ctnr.restart()
+        self.loadCache()
 
     def tearDown(self):
         pass
 
     def loadCache(self):
-        with podman.Client(self.host) as pclient:
-            self.ctns = list(pclient.containers.list())
+        self.containers = list(self.pclient.containers.list())
 
         self.alpine_ctnr = next(
-            iter([c for c in self.ctns if 'alpine' in c['image']] or []), None)
-        return self.ctns
+            iter([c for c in self.containers if 'alpine' in c['image']] or []),
+            None)
+
+        if self.alpine_ctnr and self.alpine_ctnr.status != 'running':
+            self.alpine_ctnr.start()
 
     def test_list(self):
-        actual = self.loadCache()
-        self.assertGreaterEqual(len(actual), 2)
+        self.assertGreaterEqual(len(self.containers), 2)
         self.assertIsNotNone(self.alpine_ctnr)
         self.assertIn('alpine', self.alpine_ctnr.image)
 
     def test_delete_stopped(self):
-        before = self.loadCache()
-        self.assertEqual(self.alpine_ctnr.id, self.alpine_ctnr.stop())
+        before = len(self.containers)
+
+        self.alpine_ctnr.stop()
+        target = self.alpine_ctnr.id
         actual = self.pclient.containers.delete_stopped()
-        self.assertIn(self.alpine_ctnr.id, actual)
-        after = self.loadCache()
+        self.assertIn(target, actual)
 
-        self.assertLess(len(after), len(before))
-        TestContainers.setUpClass()
         self.loadCache()
+        after = len(self.containers)
 
-    @unittest.skip('TODO: Reinstate with create PR')
-    def test_create(self):
-        with self.assertRaisesNotImplemented():
-            self.pclient.containers.create()
+        self.assertLess(after, before)
+        TestContainers.setUpClass()
 
     def test_get(self):
         actual = self.pclient.containers.get(self.alpine_ctnr.id)
@@ -75,19 +73,16 @@ class TestContainers(PodmanTestCase):
         self.assertGreaterEqual(len(actual), 2)
 
     def test_start_stop_wait(self):
-        self.assertEqual(self.alpine_ctnr.id, self.alpine_ctnr.stop())
-        self.alpine_ctnr.refresh()
-        self.assertFalse(self.alpine_ctnr['running'])
+        ctnr = self.alpine_ctnr.stop()
+        self.assertFalse(ctnr['running'])
 
-        self.assertEqual(self.alpine_ctnr.id, self.alpine_ctnr.restart())
-        self.alpine_ctnr.refresh()
-        self.assertTrue(self.alpine_ctnr.running)
+        ctnr.start()
+        self.assertTrue(ctnr.running)
 
-        self.assertEqual(self.alpine_ctnr.id, self.alpine_ctnr.stop())
-        self.alpine_ctnr.refresh()
-        self.assertFalse(self.alpine_ctnr['containerrunning'])
+        ctnr.stop()
+        self.assertFalse(ctnr['containerrunning'])
 
-        actual = self.alpine_ctnr.wait()
+        actual = ctnr.wait()
         self.assertGreaterEqual(actual, 0)
 
     def test_changes(self):
@@ -104,19 +99,16 @@ class TestContainers(PodmanTestCase):
 
     def test_kill(self):
         self.assertTrue(self.alpine_ctnr.running)
-        self.assertEqual(self.alpine_ctnr.id, self.alpine_ctnr.kill(9))
-        time.sleep(2)
-
-        self.alpine_ctnr.refresh()
-        self.assertFalse(self.alpine_ctnr.running)
+        ctnr = self.alpine_ctnr.kill(signal.SIGKILL)
+        self.assertFalse(ctnr.running)
 
     def test_inspect(self):
         actual = self.alpine_ctnr.inspect()
         self.assertEqual(actual.id, self.alpine_ctnr.id)
         # TODO: Datetime values from inspect missing offset in CI instance
         # self.assertEqual(
-        #     datetime_parse(actual.created),
-        #     datetime_parse(self.alpine_ctnr.createdat))
+        # datetime_parse(actual.created),
+        # datetime_parse(self.alpine_ctnr.createdat))
 
     def test_export(self):
         target = os.path.join(self.tmpdir, 'alpine_export_ctnr.tar')
@@ -129,21 +121,21 @@ class TestContainers(PodmanTestCase):
     def test_commit(self):
         # TODO: Test for STOPSIGNAL when supported by OCI
         # TODO: Test for message when supported by OCI
-        # TODO: Test for EXPOSE when issue#795 fixed
-        #       'EXPOSE=8888/tcp, 8888/udp'
+        details = self.pclient.images.get(
+            self.alpine_ctnr.inspect().image).inspect()
+        changes = ['ENV=' + i for i in details.containerconfig['env']]
+        changes.append('CMD=/usr/bin/zsh')
+        changes.append('ENTRYPOINT=/bin/sh date')
+        changes.append('ENV=TEST=test_containers.TestContainers.test_commit')
+        changes.append('EXPOSE=80')
+        changes.append('EXPOSE=8888')
+        changes.append('LABEL=unittest=test_commit')
+        changes.append('USER=bozo:circus')
+        changes.append('VOLUME=/data')
+        changes.append('WORKDIR=/data/application')
+
         id = self.alpine_ctnr.commit(
-            'alpine3',
-            author='Bozo the clown',
-            changes=[
-                'CMD=/usr/bin/zsh',
-                'ENTRYPOINT=/bin/sh date',
-                'ENV=TEST=test_containers.TestContainers.test_commit',
-                'LABEL=unittest=test_commit',
-                'USER=bozo:circus',
-                'VOLUME=/data',
-                'WORKDIR=/data/application',
-            ],
-            pause=True)
+            'alpine3', author='Bozo the clown', changes=changes, pause=True)
         img = self.pclient.images.get(id)
         self.assertIsNotNone(img)
 
@@ -152,12 +144,14 @@ class TestContainers(PodmanTestCase):
         self.assertListEqual(['/usr/bin/zsh'], details.containerconfig['cmd'])
         self.assertListEqual(['/bin/sh date'],
                              details.containerconfig['entrypoint'])
-        self.assertListEqual(
-            ['TEST=test_containers.TestContainers.test_commit'],
-            details.containerconfig['env'])
-        # self.assertDictEqual({
-        #     '8888/tcp': {}
-        # }, details.containerconfig['exposedports'])
+        self.assertIn('TEST=test_containers.TestContainers.test_commit',
+                      details.containerconfig['env'])
+        self.assertTrue(
+            [e for e in details.containerconfig['env'] if 'PATH=' in e])
+        self.assertDictEqual({
+            '80': {},
+            '8888': {},
+        }, details.containerconfig['exposedports'])
         self.assertDictEqual({'unittest': 'test_commit'}, details.labels)
         self.assertEqual('bozo:circus', details.containerconfig['user'])
         self.assertEqual({'/data': {}}, details.containerconfig['volumes'])
@@ -165,28 +159,27 @@ class TestContainers(PodmanTestCase):
                          details.containerconfig['workingdir'])
 
     def test_remove(self):
-        before = self.loadCache()
+        before = len(self.containers)
 
         with self.assertRaises(podman.ErrorOccurred):
             self.alpine_ctnr.remove()
 
         self.assertEqual(
             self.alpine_ctnr.id, self.alpine_ctnr.remove(force=True))
-        after = self.loadCache()
+        self.loadCache()
+        after = len(self.containers)
 
-        self.assertLess(len(after), len(before))
+        self.assertLess(after, before)
         TestContainers.setUpClass()
-        self.loadCache()
 
     def test_restart(self):
         self.assertTrue(self.alpine_ctnr.running)
         before = self.alpine_ctnr.runningfor
 
-        self.assertEqual(self.alpine_ctnr.id, self.alpine_ctnr.restart())
+        ctnr = self.alpine_ctnr.restart()
+        self.assertTrue(ctnr.running)
 
-        self.alpine_ctnr.refresh()
         after = self.alpine_ctnr.runningfor
-        self.assertTrue(self.alpine_ctnr.running)
 
         # TODO: restore check when restart zeros counter
         # self.assertLess(after, before)
@@ -202,22 +195,22 @@ class TestContainers(PodmanTestCase):
     def test_pause_unpause(self):
         self.assertTrue(self.alpine_ctnr.running)
 
-        self.assertEqual(self.alpine_ctnr.id, self.alpine_ctnr.pause())
-        self.alpine_ctnr.refresh()
-        self.assertFalse(self.alpine_ctnr.running)
+        ctnr = self.alpine_ctnr.pause()
+        self.assertEqual(ctnr.status, 'paused')
 
-        self.assertEqual(self.alpine_ctnr.id, self.alpine_ctnr.unpause())
-        self.alpine_ctnr.refresh()
-        self.assertTrue(self.alpine_ctnr.running)
+        ctnr = self.alpine_ctnr.unpause()
+        self.assertTrue(ctnr.running)
+        self.assertTrue(ctnr.status, 'running')
 
     def test_stats(self):
-        self.alpine_ctnr.restart()
+        self.assertTrue(self.alpine_ctnr.running)
+
         actual = self.alpine_ctnr.stats()
         self.assertEqual(self.alpine_ctnr.id, actual.id)
         self.assertEqual(self.alpine_ctnr.names, actual.name)
 
     def test_logs(self):
-        self.alpine_ctnr.restart()
+        self.assertTrue(self.alpine_ctnr.running)
         actual = list(self.alpine_ctnr.logs())
         self.assertIsNotNone(actual)
 
diff --git a/contrib/python/test/test_images.py b/contrib/python/test/test_images.py
index 02ca5b236..f9eaee139 100644
--- a/contrib/python/test/test_images.py
+++ b/contrib/python/test/test_images.py
@@ -46,8 +46,11 @@ class TestImages(PodmanTestCase):
             self.pclient.images.build()
 
     def test_create(self):
-        with self.assertRaisesNotImplemented():
-            self.pclient.images.create()
+        actual = self.alpine_image.container()
+        self.assertIsNotNone(actual)
+        self.assertEqual(actual.status, 'configured')
+        cntr = actual.start()
+        self.assertIn(cntr.status, ['running', 'exited'])
 
     def test_export(self):
         path = os.path.join(self.tmpdir, 'alpine_export.tar')
@@ -58,9 +61,7 @@ class TestImages(PodmanTestCase):
         self.assertTrue(os.path.isfile(path))
 
     def test_history(self):
-        count = 0
-        for record in self.alpine_image.history():
-            count += 1
+        for count, record in enumerate(self.alpine_image.history()):
             self.assertEqual(record.id, self.alpine_image.id)
         self.assertGreater(count, 0)
 
@@ -89,13 +90,6 @@ class TestImages(PodmanTestCase):
         with self.assertRaises(podman.ErrorOccurred):
             self.alpine_image.remove()
 
-        # TODO: remove this block once force=True works
-        with podman.Client(self.host) as pclient:
-            for ctnr in pclient.containers.list():
-                if 'alpine' in ctnr.image:
-                    ctnr.stop()
-                    ctnr.remove()
-
         actual = self.alpine_image.remove(force=True)
         self.assertEqual(self.alpine_image.id, actual)
         after = self.loadCache()
@@ -136,11 +130,11 @@ class TestImages(PodmanTestCase):
 
     def test_search(self):
         actual = self.pclient.images.search('alpine', 25)
-        names, lengths = itertools.tee(actual)
+        names, length = itertools.tee(actual)
 
         for img in names:
-            self.assertIn('alpine', img['name'])
-        self.assertTrue(0 < len(list(lengths)) <= 25)
+            self.assertIn('alpine', img.name)
+        self.assertTrue(0 < len(list(length)) <= 25)
 
 
 if __name__ == '__main__':
diff --git a/contrib/python/test/test_runner.sh b/contrib/python/test/test_runner.sh
index ba55a7237..1cccbe1f5 100755
--- a/contrib/python/test/test_runner.sh
+++ b/contrib/python/test/test_runner.sh
@@ -40,7 +40,7 @@ fi
 export PATH=../../bin:$PATH
 
 function showlog {
-  [ -s "$1" ] && (echo $1 =====; cat "$1")
+  [ -s "$1" ] && (echo $1 =====; cat "$1"; echo)
 }
 
 # Need a location to store the podman socket
@@ -109,8 +109,9 @@ else
 fi
 
 set +x
-pkill podman
+pkill -9 podman
 pkill -9 conmon
 
+showlog /tmp/test_runner.output
 showlog /tmp/alpine.log
 showlog /tmp/busybox.log