From a4c0cdedb90897104002c3a746782a8d55b7ab23 Mon Sep 17 00:00:00 2001 From: Jhon Honce Date: Wed, 31 Oct 2018 17:29:29 -0700 Subject: Add ChangeAction to parse sub-options from --change * Covers both commit and import commands * Cleaned up export command * Removed unneeded calls to super().__init__() Fixes #1702 Signed-off-by: Jhon Honce --- contrib/python/podman/podman/libs/images.py | 2 +- contrib/python/pypodman/pypodman/lib/__init__.py | 5 ++- .../pypodman/pypodman/lib/actions/commit_action.py | 32 ++++---------- .../pypodman/pypodman/lib/actions/export_action.py | 22 ++++------ .../pypodman/pypodman/lib/actions/import_action.py | 41 ++++++++++------- .../python/pypodman/pypodman/lib/parser_actions.py | 51 +++++++++++++++++++++- 6 files changed, 96 insertions(+), 57 deletions(-) diff --git a/contrib/python/podman/podman/libs/images.py b/contrib/python/podman/podman/libs/images.py index 982546cd2..9453fb416 100644 --- a/contrib/python/podman/podman/libs/images.py +++ b/contrib/python/podman/podman/libs/images.py @@ -137,7 +137,7 @@ class Images(): results = podman.DeleteUnusedImages() return results['images'] - def import_image(self, source, reference, message=None, changes=None): + def import_image(self, source, reference, message='', changes=None): """Read image tarball from source and save in image store.""" with self._client() as podman: results = podman.ImportImage(source, reference, message, changes) diff --git a/contrib/python/pypodman/pypodman/lib/__init__.py b/contrib/python/pypodman/pypodman/lib/__init__.py index 5525ddaef..be1b5f467 100644 --- a/contrib/python/pypodman/pypodman/lib/__init__.py +++ b/contrib/python/pypodman/pypodman/lib/__init__.py @@ -4,14 +4,15 @@ import sys import podman from pypodman.lib.action_base import AbstractActionBase from pypodman.lib.parser_actions import (BooleanAction, BooleanValidate, - PathAction, PositiveIntAction, - UnitAction) + ChangeAction, PathAction, + PositiveIntAction, UnitAction) from pypodman.lib.podman_parser import PodmanArgumentParser from pypodman.lib.report import Report, ReportColumn # Silence pylint overlording... assert BooleanAction assert BooleanValidate +assert ChangeAction assert PathAction assert PositiveIntAction assert UnitAction diff --git a/contrib/python/pypodman/pypodman/lib/actions/commit_action.py b/contrib/python/pypodman/pypodman/lib/actions/commit_action.py index 0da6a2078..21665ad0b 100644 --- a/contrib/python/pypodman/pypodman/lib/actions/commit_action.py +++ b/contrib/python/pypodman/pypodman/lib/actions/commit_action.py @@ -2,7 +2,7 @@ import sys import podman -from pypodman.lib import AbstractActionBase, BooleanAction +from pypodman.lib import AbstractActionBase, BooleanAction, ChangeAction class Commit(AbstractActionBase): @@ -12,7 +12,9 @@ class Commit(AbstractActionBase): def subparser(cls, parent): """Add Commit command to parent parser.""" parser = parent.add_parser( - 'commit', help='create image from container') + 'commit', + help='create image from container', + ) parser.add_argument( '--author', help='Set the author for the committed image', @@ -20,11 +22,7 @@ class Commit(AbstractActionBase): parser.add_argument( '--change', '-c', - choices=('CMD', 'ENTRYPOINT', 'ENV', 'EXPOSE', 'LABEL', 'ONBUILD', - 'STOPSIGNAL', 'USER', 'VOLUME', 'WORKDIR'), - action='append', - type=str.upper, - help='Apply the following possible changes to the created image', + action=ChangeAction, ) parser.add_argument( '--format', @@ -69,27 +67,11 @@ class Commit(AbstractActionBase): ) parser.set_defaults(class_=cls, method='commit') - def __init__(self, args): - """Construct Commit class.""" - if not args.container: - raise ValueError('You must supply one container id' - ' or name to be used as source.') - if not args.image: - raise ValueError('You must supply one image id' - ' or name to be created.') - super().__init__(args) - - # used only on client - del self.opts['image'] - del self.opts['container'] - def commit(self): """Create image from container.""" try: try: ctnr = self.client.containers.get(self._args.container[0]) - ident = ctnr.commit(**self.opts) - print(ident) except podman.ContainerNotFound as e: sys.stdout.flush() print( @@ -97,6 +79,9 @@ class Commit(AbstractActionBase): file=sys.stderr, flush=True) return 1 + else: + ident = ctnr.commit(self.opts['image'][0], **self.opts) + print(ident) except podman.ErrorOccurred as e: sys.stdout.flush() print( @@ -104,3 +89,4 @@ class Commit(AbstractActionBase): file=sys.stderr, flush=True) return 1 + return 0 diff --git a/contrib/python/pypodman/pypodman/lib/actions/export_action.py b/contrib/python/pypodman/pypodman/lib/actions/export_action.py index f62cd3535..7ef178c4c 100644 --- a/contrib/python/pypodman/pypodman/lib/actions/export_action.py +++ b/contrib/python/pypodman/pypodman/lib/actions/export_action.py @@ -12,13 +12,16 @@ class Export(AbstractActionBase): def subparser(cls, parent): """Add Export command to parent parser.""" parser = parent.add_parser( - 'export', help='export container to tarball') + 'export', + help='export container to tarball', + ) parser.add_argument( '--output', '-o', metavar='PATH', nargs=1, - help='Write to a file', + required=True, + help='Write to this file on host', ) parser.add_argument( 'container', @@ -27,23 +30,11 @@ class Export(AbstractActionBase): ) parser.set_defaults(class_=cls, method='export') - def __init__(self, args): - """Construct Export class.""" - if not args.container: - raise ValueError('You must supply one container id' - ' or name to be used as source.') - - if not args.output: - raise ValueError('You must supply one filename' - ' to be created as tarball using --output.') - super().__init__(args) - def export(self): """Create tarball from container filesystem.""" try: try: ctnr = self.client.containers.get(self._args.container[0]) - ctnr.export(self._args.output[0]) except podman.ContainerNotFound as e: sys.stdout.flush() print( @@ -51,6 +42,8 @@ class Export(AbstractActionBase): file=sys.stderr, flush=True) return 1 + else: + ctnr.export(self._args.output[0]) except podman.ErrorOccurred as e: sys.stdout.flush() print( @@ -58,3 +51,4 @@ class Export(AbstractActionBase): file=sys.stderr, flush=True) return 1 + return 0 diff --git a/contrib/python/pypodman/pypodman/lib/actions/import_action.py b/contrib/python/pypodman/pypodman/lib/actions/import_action.py index 49b8a5a57..43448144a 100644 --- a/contrib/python/pypodman/pypodman/lib/actions/import_action.py +++ b/contrib/python/pypodman/pypodman/lib/actions/import_action.py @@ -2,7 +2,7 @@ import sys import podman -from pypodman.lib import AbstractActionBase +from pypodman.lib import AbstractActionBase, ChangeAction class Import(AbstractActionBase): @@ -12,18 +12,19 @@ class Import(AbstractActionBase): def subparser(cls, parent): """Add Import command to parent parser.""" parser = parent.add_parser( - 'import', help='import tarball as image filesystem') + 'import', + help='import tarball as image filesystem', + ) parser.add_argument( '--change', '-c', - action='append', - choices=('CMD', 'ENTRYPOINT', 'ENV', 'EXPOSE', 'LABEL', - 'STOPSIGNAL', 'USER', 'VOLUME', 'WORKDIR'), - type=str.upper, - help='Apply the following possible instructions', + action=ChangeAction, ) parser.add_argument( - '--message', '-m', help='Set commit message for imported image.') + '--message', + '-m', + help='Set commit message for imported image.', + ) parser.add_argument( 'source', metavar='PATH', @@ -38,18 +39,25 @@ class Import(AbstractActionBase): ) parser.set_defaults(class_=cls, method='import_') - def __init__(self, args): - """Construct Import class.""" - super().__init__(args) - def import_(self): """Import tarball as image filesystem.""" + # ImportImage() validates it's parameters therefore we need to create + # pristine dict() for keywords + options = {} + if 'message' in self.opts: + options['message'] = self.opts['message'] + if 'change' in self.opts and self.opts['change']: + options['changes'] = self.opts['change'] + + reference = self.opts['reference'][0] if 'reference' in self.opts\ + else None + try: ident = self.client.images.import_image( - self.opts.source, - self.opts.reference, - message=self.opts.message, - changes=self.opts.change) + self.opts['source'][0], + reference, + **options, + ) print(ident) except podman.ErrorOccurred as e: sys.stdout.flush() @@ -58,3 +66,4 @@ class Import(AbstractActionBase): file=sys.stderr, flush=True) return 1 + return 0 diff --git a/contrib/python/pypodman/pypodman/lib/parser_actions.py b/contrib/python/pypodman/pypodman/lib/parser_actions.py index 2a5859e47..c10b85495 100644 --- a/contrib/python/pypodman/pypodman/lib/parser_actions.py +++ b/contrib/python/pypodman/pypodman/lib/parser_actions.py @@ -4,9 +4,10 @@ Supplimental argparse.Action converters and validaters. The constructors are very verbose but remain for IDE support. """ import argparse +import copy import os -# API defined by argparse.Action shut up pylint +# API defined by argparse.Action therefore shut up pylint # pragma pylint: disable=redefined-builtin # pragma pylint: disable=too-few-public-methods # pragma pylint: disable=too-many-arguments @@ -63,6 +64,54 @@ class BooleanAction(argparse.Action): setattr(namespace, self.dest, val) +class ChangeAction(argparse.Action): + """Convert and validate change argument.""" + + def __init__(self, + option_strings, + dest, + nargs=None, + const=None, + default=[], + type=None, + choices=None, + required=False, + help=None, + metavar='OPT=VALUE'): + """Create ChangeAction object.""" + help = (help or '') + ('Apply change(s) to the new image.' + ' May be given multiple times.') + + super().__init__( + option_strings=option_strings, + dest=dest, + nargs=nargs, + const=const, + default=default, + type=type, + choices=choices, + required=required, + help=help, + metavar=metavar) + + def __call__(self, parser, namespace, values, option_string=None): + """Convert and Validate input.""" + print(self.dest) + items = getattr(namespace, self.dest, None) or [] + items = copy.copy(items) + + choices = ('CMD', 'ENTRYPOINT', 'ENV', 'EXPOSE', 'LABEL', 'ONBUILD', + 'STOPSIGNAL', 'USER', 'VOLUME', 'WORKDIR') + + opt, val = values.split('=', 1) + if opt not in choices: + parser.error('{} is not a supported "--change" option,' + ' valid options are: {}'.format( + opt, ', '.join(choices))) + items.append(values) + setattr(namespace, self.dest, items) + + class UnitAction(argparse.Action): """Validate number given is positive integer, with optional suffix.""" -- cgit v1.2.3-54-g00ecf