[libvirt] [PATCH 13/16] Add InteractiveContainer support. First use case will be OpenShift.

[Daniel P. Berrange]

On Tue, Apr 02, 2013 at 06:11:29PM -0400, Dan Walsh wrote:
> Differentiating on which kind of container to create based off of the
> 
> --command == InteractiveContainer
> --unitfile == ServiceContainer
> 
> Resorted create args to be shown aphabetically except for the --command and --unitfile which I want to come at the end.
> ---
>  bin/virt-sandbox-service | 139 +++++++++++++++++++++++++++++++++--------------
>  1 file changed, 99 insertions(+), 40 deletions(-)
> 
> diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service
> index f4d0eff..b559cf5 100755
> --- a/bin/virt-sandbox-service
> +++ b/bin/virt-sandbox-service
> @@ -413,6 +413,45 @@ class Container:
>          mount = LibvirtSandbox.ConfigMountRam.new(dest, size);
>          self.config.add_mount(mount)
>  
> +class InteractiveContainer(Container):
> +    def __init__(self, name=None, uri = "lxc:///", path = Container.DEFAULT_PATH, config=None, create=False):
> +        Container.__init__(self, name, uri, path, config, create)
> +
> +        if create:
> +            self.config = LibvirtSandbox.ConfigInteractive.new(name)
> +
> +    def _gen_filesystems(self):
> +        Container._gen_filesystems(self)
> +        self.add_bind_mount(self.dest, self.path)
> +
> +    def _create(self):
> +        #
> +        # Create an InteractiveContainer
> +        #
> +        Container.create(self)
> +        self._gen_filesystems()
> +
> +        if self.image:
> +            self._create_image()
> +            self._umount()
> +            sys.stdout.write(_("Created sandbox container image %s\n") % self.image)
> +        else:
> +            sys.stdout.write(_("Created sandbox container dir %s\n") % self.dest)
> +        self.save_config()
> +
> +    def create(self):
> +        try:
> +            self._create()
> +        except Exception, e:
> +            try:
> +                self._delete()
> +            except Exception, e2:
> +                pass
> +            raise e
> +
> +    def set_command(self, command):
> +        self.config.set_command(command)
> +
>  class ServiceContainer(Container):
>      IGNORE_DIRS        = [ "/var/run/", "/etc/logrotate.d/", "/etc/pam.d" ]
>      DEFAULT_DIRS       = [ "/etc", "/var" ]
> @@ -836,19 +875,26 @@ MB = int(1000000)
>  
>  def delete(args):
>      config = read_config(args.name)
> -    container = ServiceContainer(uri=args.uri, config = config)
> +    if isinstance(config, gi.repository.LibvirtSandbox.ConfigInteractive):
> +        container = InteractiveContainer(uri=args.uri, config = config)
> +    else:
> +        container = ServiceContainer(uri=args.uri, config = config)
>      container.delete()
>  
>  def create(args):
> -    container = ServiceContainer(name = args.name, uri=args.uri, create = True)
> -    container.set_copy(args.copy)
> +    if args.command:
> +        container = InteractiveContainer(name = args.name, uri=args.uri, create = True)
> +        container.set_command(args.command.split())

This is bad because it does not take account of shell quoting rules so
if you have a command

   /usr/bin/foo "some bar"

You'll create

  ['/usr/bin/foo', 'some', 'bar']

> @@ -1022,28 +1075,34 @@ def gen_create_args(subparser):
>      parser = subparser.add_parser("create",
>                                    help=_("create a sandbox container"))
>  
> +    parser.add_argument("-n", "--network", dest="network",
> +                        action=SetNet, default=[],
> +                        help=_("Specify the network configuration"))

> -    parser.add_argument("-N", "--network", dest="network",
> -                        action=SetNet,
> -                        help=_("Specify the network configuration"))

You've changed  '-N' into '-n' - please don't - the use of '-N' was intentionale
to match  'virt-sandbox' command line arg names.

>  
> -    image = parser.add_argument_group("Create On Disk Image File")
> -
> -    image.add_argument("-i", "--imagesize", dest="imagesize", default = None,
> -                       action=SizeAction,
> -                       help=_("create image of this many megabytes."))
> -    parser.add_argument("-C", "--copy", default=False,
> -                        action="store_true",
> -                        help=_("copy content from /etc and /var directories that will be mounted within the sandbox"))
> +    ctype = parser.add_argument_group(_("Type of sandbox container to create"))
> +    group = ctype.add_mutually_exclusive_group(required=True)
> +    group.add_argument("-c", "--command",
> +                        dest="command", default=None,
> +                        help=_("Command to run within the sandbox container"))

IMHO it is better to make this work like virt-sandbox, where you can pass the
command + its args as args on the end of the command line eg instead of

    virt-sandbox-service create -c "/usr/bin/foo bar wizz"

We should allow

    virt-sandbox-service create -c /usr/bin/foo bar wizz

    virt-sandbox-service create -c -- /usr/bin/foo bar wizz

so we don't have todo parsing of the -c arg - we just get the string list
straight from the sys.argv


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|