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

[Daniel P. Berrange]

On Wed, Apr 03, 2013 at 10:34:33AM -0400, Daniel J Walsh wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 04/03/2013 08:47 AM, Daniel P. Berrange wrote:
> > 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
> > 
> 
> Shouldn't the syntax be
> 
> virt-sandbox-service create foo1 -- /usr/bin/foo bar wizz
> 
> And
> 
> virt-sandbox-service create -u httpd.service httpd1

Opps, yes, I missed out the sandbox name in my examples.


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 :|