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

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