Implement podman.containers.commit()

- Add API support
- Update tests
- Make changes from reviews

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

Closes: #798
Approved by: mheon

4 files changed, 87 insertions, 17 deletions

diff --git a/contrib/python/podman/libs/containers.py b/contrib/python/podman/libs/containers.py
index cbbef23b1..0655010f1 100644
--- a/contrib/python/podman/libs/containers.py
+++ b/contrib/python/podman/libs/containers.py
@@ -1,6 +1,7 @@
 """Models for manipulating containers and storage."""
 import collections
 import functools
+import getpass
 import json
 import signal
 
@@ -93,6 +94,45 @@ class Container(collections.UserDict):
             results = podman.ExportContainer(self.id, target)
         return results['tarfile']
 
+    def commit(self,
+               image_name,
+               *args,
+               changes=[],
+               message='',
+               pause=True,
+               **kwargs):
+        """Create image from container.
+
+        All changes overwrite existing values.
+          See inspect() to obtain current settings.
+
+        Changes:
+            CMD=/usr/bin/zsh
+            ENTRYPOINT=/bin/sh date
+            ENV=TEST=test_containers.TestContainers.test_commit
+            EXPOSE=8888/tcp
+            LABEL=unittest=test_commit
+            USER=bozo:circus
+            VOLUME=/data
+            WORKDIR=/data/application
+        """
+        # TODO: Clean up *args, **kwargs after Commit() is complete
+        try:
+            author = kwargs.get('author', getpass.getuser())
+        except Exception:
+            author = ''
+
+        for c in changes:
+            if c.startswith('LABEL=') and c.count('=') < 2:
+                raise ValueError(
+                    'LABEL should have the format: LABEL=label=value, not {}'.
+                    format(c))
+
+        with self._client() as podman:
+            results = podman.Commit(self.id, image_name, changes, author,
+                                    message, pause)
+        return results['image']
+
     def start(self):
         """Start 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 1d40984a7..d6fd7946d 100644
--- a/contrib/python/podman/libs/images.py
+++ b/contrib/python/podman/libs/images.py
@@ -95,13 +95,6 @@ class Images(object):
             results = podman.CreateImage()
         return results['image']
 
-    def create_from(self, *args, **kwargs):
-        """Create image from container."""
-        # TODO: Should this be on container?
-        with self._client() as podman:
-            results = podman.CreateFromContainer()
-        return results['image']
-
     def build(self, *args, **kwargs):
         """Build container from image.
 
@@ -135,3 +128,7 @@ class Images(object):
             results = podman.SearchImage(id)
         for img in results['images']:
             yield img
+
+    def get(self, id):
+        """Get Image from id."""
+        return next((i for i in self.list() if i.id == id), None)
diff --git a/contrib/python/test/test_containers.py b/contrib/python/test/test_containers.py
index 344d40d0d..1b2603037 100644
--- a/contrib/python/test/test_containers.py
+++ b/contrib/python/test/test_containers.py
@@ -109,13 +109,13 @@ class TestContainers(PodmanTestCase):
         self.alpine_ctnr.refresh()
         self.assertFalse(self.alpine_ctnr.running)
 
-    @unittest.skip('Datetime values from inspect missing offset')
     def test_inspect(self):
         actual = self.alpine_ctnr.inspect()
         self.assertEqual(actual.id, self.alpine_ctnr.id)
-        self.assertEqual(
-            datetime_parse(actual.created),
-            datetime_parse(self.alpine_ctnr.createdat))
+        # TODO: Datetime values from inspect missing offset in CI instance
+        # self.assertEqual(
+        #     datetime_parse(actual.created),
+        #     datetime_parse(self.alpine_ctnr.createdat))
 
     def test_export(self):
         target = os.path.join(self.tmpdir, 'alpine_export_ctnr.tar')
@@ -125,6 +125,44 @@ class TestContainers(PodmanTestCase):
         self.assertTrue(os.path.isfile(target))
         self.assertGreater(os.path.getsize(target), 0)
 
+    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'
+        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)
+        img = self.pclient.images.get(id)
+        self.assertIsNotNone(img)
+
+        details = img.inspect()
+        self.assertEqual(details.author, 'Bozo the clown')
+        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.assertDictEqual({'unittest': 'test_commit'}, details.labels)
+        self.assertEqual('bozo:circus', details.containerconfig['user'])
+        self.assertEqual({'/data': {}}, details.containerconfig['volumes'])
+        self.assertEqual('/data/application',
+                         details.containerconfig['workingdir'])
+
     def test_remove(self):
         before = self.loadCache()
 
diff --git a/contrib/python/test/test_images.py b/contrib/python/test/test_images.py
index c34fc791d..02ca5b236 100644
--- a/contrib/python/test/test_images.py
+++ b/contrib/python/test/test_images.py
@@ -49,11 +49,6 @@ class TestImages(PodmanTestCase):
         with self.assertRaisesNotImplemented():
             self.pclient.images.create()
 
-    @unittest.skip('Code implemented')
-    def test_create_from(self):
-        with self.assertRaisesNotImplemented():
-            self.pclient.images.create_from()
-
     def test_export(self):
         path = os.path.join(self.tmpdir, 'alpine_export.tar')
         target = 'oci-archive:{}:latest'.format(path)
@@ -125,7 +120,7 @@ class TestImages(PodmanTestCase):
         self.assertIn(new_img, actual)
 
         after = self.loadCache()
-        self.assertEqual(len(before), len(after))
+        self.assertGreaterEqual(len(before), len(after))
 
         TestImages.setUpClass()
         self.loadCache()