Merge pull request #1881 from jwhonce/bug/1867

Fixes #1867

3 files changed, 32 insertions, 23 deletions

diff --git a/contrib/python/podman/podman/libs/containers.py b/contrib/python/podman/podman/libs/containers.py
index 21a94557a..7adecea8f 100644
--- a/contrib/python/podman/podman/libs/containers.py
+++ b/contrib/python/podman/podman/libs/containers.py
@@ -108,19 +108,16 @@ class Container(AttachMixin, StartMixin, collections.UserDict):
             results = podman.ExportContainer(self._id, target)
         return results['tarfile']
 
-    def commit(self,
-               image_name,
-               *args,
-               changes=[],
-               message='',
-               pause=True,
-               **kwargs):  # pylint: disable=unused-argument
+    def commit(self, image_name, **kwargs):
         """Create image from container.
 
-        All changes overwrite existing values.
-          See inspect() to obtain current settings.
+        Keyword arguments:
+            author -- change image's author
+            message -- change image's message, docker format only.
+            pause -- pause container during commit
+            change -- Additional properties to change
 
-        Changes:
+        Change examples:
             CMD=/usr/bin/zsh
             ENTRYPOINT=/bin/sh date
             ENV=TEST=test_containers.TestContainers.test_commit
@@ -129,21 +126,23 @@ class Container(AttachMixin, StartMixin, collections.UserDict):
             USER=bozo:circus
             VOLUME=/data
             WORKDIR=/data/application
+
+        All changes overwrite existing values.
+          See inspect() to obtain current settings.
         """
-        # TODO: Clean up *args, **kwargs after Commit() is complete
-        try:
-            author = kwargs.get('author', getpass.getuser())
-        except Exception:  # pylint: disable=broad-except
-            author = ''
+        author = kwargs.get('author', None) or getpass.getuser()
+        change = kwargs.get('change', None) or []
+        message = kwargs.get('message', None) or ''
+        pause = kwargs.get('pause', None) or True
 
-        for c in changes:
+        for c in change:
             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,
+            results = podman.Commit(self._id, image_name, change, author,
                                     message, pause)
         return results['image']
 
@@ -194,7 +193,7 @@ class Container(AttachMixin, StartMixin, collections.UserDict):
             podman.UnpauseContainer(self._id)
             return self._refresh(podman)
 
-    def update_container(self, *args, **kwargs):          \
+    def update_container(self, *args, **kwargs):  \
             # pylint: disable=unused-argument
         """TODO: Update container..., return id on success."""
         with self._client() as podman:
diff --git a/contrib/python/podman/test/test_containers.py b/contrib/python/podman/test/test_containers.py
index 3de1e54bc..a7a6ac304 100644
--- a/contrib/python/podman/test/test_containers.py
+++ b/contrib/python/podman/test/test_containers.py
@@ -152,7 +152,7 @@ class TestContainers(PodmanTestCase):
         changes.append('WORKDIR=/data/application')
 
         id = self.alpine_ctnr.commit(
-            'alpine3', author='Bozo the clown', changes=changes, pause=True)
+            'alpine3', author='Bozo the clown', change=changes, pause=True)
         img = self.pclient.images.get(id)
         self.assertIsNotNone(img)
 
diff --git a/contrib/python/pypodman/pypodman/lib/actions/commit_action.py b/contrib/python/pypodman/pypodman/lib/actions/commit_action.py
index 21665ad0b..21924e938 100644
--- a/contrib/python/pypodman/pypodman/lib/actions/commit_action.py
+++ b/contrib/python/pypodman/pypodman/lib/actions/commit_action.py
@@ -30,7 +30,8 @@ class Commit(AbstractActionBase):
             choices=('oci', 'docker'),
             default='oci',
             type=str.lower,
-            help='Set the format of the image manifest and metadata',
+            help='Set the format of the image manifest and metadata.'
+            ' (Ignored.)',
         )
         parser.add_argument(
             '--iidfile',
@@ -40,7 +41,8 @@ class Commit(AbstractActionBase):
         parser.add_argument(
             '--message',
             '-m',
-            help='Set commit message for committed image',
+            help='Set commit message for committed image'
+            ' (Only on docker images.)',
         )
         parser.add_argument(
             '--pause',
@@ -80,8 +82,16 @@ class Commit(AbstractActionBase):
                     flush=True)
                 return 1
             else:
-                ident = ctnr.commit(self.opts['image'][0], **self.opts)
-                print(ident)
+                ident = ctnr.commit(
+                    self.opts['image'][0],
+                    change=self.opts.get('change', None),
+                    message=self.opts.get('message', None),
+                    pause=self.opts['pause'],
+                    author=self.opts.get('author', None),
+                )
+
+                if not self.opts['quiet']:
+                    print(ident)
         except podman.ErrorOccurred as e:
             sys.stdout.flush()
             print(