[libvirt] [PATCH sandbox 00/24] Add support for docker v2 registry

I was on the verge of doing a release of libvirt-sandbox to include the docker/virt-builder support when I discovered it didn't work with many new images since docker is only exposing them over its v2 registry API. Hence this series to add support for the v2 API. It also includes a tonne of fixes for bugs encountered during debugging, and adds a new 'list' command for virt-sandbox-image so you can actally see what you have downloaded :-) Daniel P. Berrange (24): Remove transfer annotations on scalar parameters Improve error message for unsupported URIs init: search $PATH for user specified binary image: rename python source impl modules image: remove execute permission from cli.py image: add support for --debug arg image: don't assume ValueError has a multi-line message image: check for errors running mkfs / virt-sandbox virt-builder: check exit status of commands run virt-builder: disable setup of network for virt-builder docker: check exit status of qemu-img command docker: rename destdir to templatedir docker: Remove duplicated call to download_template docker: add missing hashlib import statement docker: fix download progress target value docker: remove unused variables & code in download method docker: introduce a DockerImage class docker: fix setting of Accept header docker: refactor authentication method handling docker: introduce a DockerRegistry class docker: refactor download method exception handling docker: implement support for oauth docker: add support for v2 registry server image: add 'list' command for viewing local templates libvirt-sandbox/image/cli.py | 76 ++- libvirt-sandbox/image/sources/DockerSource.py | 407 ------------ libvirt-sandbox/image/sources/Makefile.am | 6 +- .../image/sources/{Source.py => base.py} | 14 +- libvirt-sandbox/image/sources/docker.py | 683 +++++++++++++++++++++ .../{VirtBuilderSource.py => virtbuilder.py} | 34 +- libvirt-sandbox/image/template.py | 32 +- .../libvirt-sandbox-config-interactive.c | 4 +- .../libvirt-sandbox-config-network-route.c | 2 +- libvirt-sandbox/libvirt-sandbox-config.c | 16 +- .../libvirt-sandbox-context-interactive.c | 6 +- libvirt-sandbox/libvirt-sandbox-init-common.c | 2 +- 12 files changed, 818 insertions(+), 464 deletions(-) mode change 100755 => 100644 libvirt-sandbox/image/cli.py delete mode 100644 libvirt-sandbox/image/sources/DockerSource.py rename libvirt-sandbox/image/sources/{Source.py => base.py} (93%) create mode 100644 libvirt-sandbox/image/sources/docker.py rename libvirt-sandbox/image/sources/{VirtBuilderSource.py => virtbuilder.py} (76%) -- 2.7.4

The (transfer) annotation is only valid on parameters which are non-scalar in nature. Latest gobject will warn about this mistake. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-sandbox/libvirt-sandbox-config-interactive.c | 4 ++-- libvirt-sandbox/libvirt-sandbox-config-network-route.c | 2 +- libvirt-sandbox/libvirt-sandbox-config.c | 16 ++++++++-------- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/libvirt-sandbox/libvirt-sandbox-config-interactive.c b/libvirt-sandbox/libvirt-sandbox-config-interactive.c index 27b4c16..88bc015 100644 --- a/libvirt-sandbox/libvirt-sandbox-config-interactive.c +++ b/libvirt-sandbox/libvirt-sandbox-config-interactive.c @@ -246,7 +246,7 @@ GVirSandboxConfigInteractive *gvir_sandbox_config_interactive_new(const gchar *n /** * gvir_sandbox_config_interactive_set_tty: * @config: (transfer none): the sandbox config - * @tty: (transfer none): true if the container should have a tty + * @tty: true if the container should have a tty * * Set whether the container console should have a interactive tty. */ @@ -263,7 +263,7 @@ void gvir_sandbox_config_interactive_set_tty(GVirSandboxConfigInteractive *confi * * Retrieves the sandbox tty flag * - * Returns: (transfer none): the tty flag + * Returns: the tty flag */ gboolean gvir_sandbox_config_interactive_get_tty(GVirSandboxConfigInteractive *config) { diff --git a/libvirt-sandbox/libvirt-sandbox-config-network-route.c b/libvirt-sandbox/libvirt-sandbox-config-network-route.c index 1664d18..c6bf6f3 100644 --- a/libvirt-sandbox/libvirt-sandbox-config-network-route.c +++ b/libvirt-sandbox/libvirt-sandbox-config-network-route.c @@ -234,7 +234,7 @@ void gvir_sandbox_config_network_route_set_prefix(GVirSandboxConfigNetworkRoute * * Retrieves the prefix length of the route * - * Returns: (transfer none): the prefix length + * Returns: the prefix length */ guint gvir_sandbox_config_network_route_get_prefix(GVirSandboxConfigNetworkRoute *config) { diff --git a/libvirt-sandbox/libvirt-sandbox-config.c b/libvirt-sandbox/libvirt-sandbox-config.c index 0ade178..8709736 100644 --- a/libvirt-sandbox/libvirt-sandbox-config.c +++ b/libvirt-sandbox/libvirt-sandbox-config.c @@ -699,7 +699,7 @@ const gchar *gvir_sandbox_config_get_kmodpath(GVirSandboxConfig *config) /** * gvir_sandbox_config_set_shell: * @config: (transfer none): the sandbox config - * @shell: (transfer none): true if the container should have a shell + * @shell: true if the container should have a shell * * Set whether the container console should have a interactive shell. */ @@ -716,7 +716,7 @@ void gvir_sandbox_config_set_shell(GVirSandboxConfig *config, gboolean shell) * * Retrieves the sandbox shell flag * - * Returns: (transfer none): the shell flag + * Returns: the shell flag */ gboolean gvir_sandbox_config_get_shell(GVirSandboxConfig *config) { @@ -728,7 +728,7 @@ gboolean gvir_sandbox_config_get_shell(GVirSandboxConfig *config) /** * gvir_sandbox_config_set_userid: * @config: (transfer none): the sandbox config - * @uid: (transfer none): the sandbox user ID + * @uid: the sandbox user ID * * Set the user ID to invoke the sandbox application under. * Defaults to the user ID of the current program. @@ -746,7 +746,7 @@ void gvir_sandbox_config_set_userid(GVirSandboxConfig *config, guint uid) * * Get the user ID to invoke the sandbox application under. * - * Returns: (transfer none): the user ID + * Returns: the user ID */ guint gvir_sandbox_config_get_userid(GVirSandboxConfig *config) { @@ -758,7 +758,7 @@ guint gvir_sandbox_config_get_userid(GVirSandboxConfig *config) /** * gvir_sandbox_config_set_groupid: * @config: (transfer none): the sandbox config - * @gid: (transfer none): the sandbox group ID + * @gid: the sandbox group ID * * Set the group ID to invoke the sandbox application under. * Defaults to the group ID of the current program. @@ -776,7 +776,7 @@ void gvir_sandbox_config_set_groupid(GVirSandboxConfig *config, guint gid) * * Get the group ID to invoke the sandbox application under. * - * Returns: (transfer none): the group ID + * Returns: the group ID */ guint gvir_sandbox_config_get_groupid(GVirSandboxConfig *config) { @@ -1849,7 +1849,7 @@ const gchar *gvir_sandbox_config_get_security_label(GVirSandboxConfig *config) /** * gvir_sandbox_config_set_security_dynamic: * @config: (transfer none): the sandbox config - * @dynamic: (transfer none): the security mode + * @dynamic: the security mode * * Set the SELinux security dynamic for the sandbox. The default * dynamic is "svirt_sandbox_t" @@ -1866,7 +1866,7 @@ void gvir_sandbox_config_set_security_dynamic(GVirSandboxConfig *config, gboolea * * Retrieve the sandbox security mode * - * Returns: (transfer none): the security mode + * Returns: the security mode */ gboolean gvir_sandbox_config_get_security_dynamic(GVirSandboxConfig *config) { -- 2.7.4

Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-sandbox/libvirt-sandbox-context-interactive.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libvirt-sandbox/libvirt-sandbox-context-interactive.c b/libvirt-sandbox/libvirt-sandbox-context-interactive.c index 3ab63ec..5ecaf09 100644 --- a/libvirt-sandbox/libvirt-sandbox-context-interactive.c +++ b/libvirt-sandbox/libvirt-sandbox-context-interactive.c @@ -229,13 +229,15 @@ static gboolean gvir_sandbox_context_interactive_start(GVirSandboxContext *ctxt, if (!g_str_equal(uri, "lxc:///") && !g_str_equal(uri, "qemu:///system")) { g_set_error(error, GVIR_SANDBOX_CONTEXT_INTERACTIVE_ERROR, 0, - _("Only 'lxc:///' or 'qemu:///system' URIs supported when running as root")); + _("URI '%s' unsupported as root, try 'lxc:///' or 'qemu:///system'"), + uri); goto cleanup; } } else { if (!g_str_equal(uri, "qemu:///session")) { g_set_error(error, GVIR_SANDBOX_CONTEXT_INTERACTIVE_ERROR, 0, - _("Only 'qemu:///session' URIs supported when running as non-root")); + _("URI '%s' unsupported as non-root, try 'qemu:///session'"), + uri); goto cleanup; } } -- 2.7.4

When launching a sandbox, don't assume the binary is a fully qualified path, let the kernel search $PATH for it. This is required by many docker images as their init program has no qualified path. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-sandbox/libvirt-sandbox-init-common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libvirt-sandbox/libvirt-sandbox-init-common.c b/libvirt-sandbox/libvirt-sandbox-init-common.c index af7e457..7ea63cf 100644 --- a/libvirt-sandbox/libvirt-sandbox-init-common.c +++ b/libvirt-sandbox/libvirt-sandbox-init-common.c @@ -516,7 +516,7 @@ static gboolean run_command(GVirSandboxConfig *config, abort(); } - execv(appargv[0], appargv); + execvp(appargv[0], appargv); fprintf(stderr, "Cannot execute '%s': %s\n", appargv[0], strerror(errno)); exit(EXIT_FAILURE); } -- 2.7.4

It is more normal to have python source files in all lowercase, so rename the existing modules. --- libvirt-sandbox/image/sources/Makefile.am | 6 +++--- libvirt-sandbox/image/sources/{Source.py => base.py} | 0 libvirt-sandbox/image/sources/{DockerSource.py => docker.py} | 4 ++-- .../image/sources/{VirtBuilderSource.py => virtbuilder.py} | 6 ++++-- libvirt-sandbox/image/template.py | 4 ++-- 5 files changed, 11 insertions(+), 9 deletions(-) rename libvirt-sandbox/image/sources/{Source.py => base.py} (100%) rename libvirt-sandbox/image/sources/{DockerSource.py => docker.py} (99%) rename libvirt-sandbox/image/sources/{VirtBuilderSource.py => virtbuilder.py} (98%) diff --git a/libvirt-sandbox/image/sources/Makefile.am b/libvirt-sandbox/image/sources/Makefile.am index 52e9a7e..817baa0 100644 --- a/libvirt-sandbox/image/sources/Makefile.am +++ b/libvirt-sandbox/image/sources/Makefile.am @@ -2,9 +2,9 @@ pythonimagedir = $(pythondir)/libvirt_sandbox/image/sources pythonimage_DATA = \ __init__.py \ - Source.py \ - DockerSource.py \ - VirtBuilderSource.py \ + base.py \ + docker.py \ + virtbuilder.py \ $(NULL) EXTRA_DIST = $(pythonimage_DATA) diff --git a/libvirt-sandbox/image/sources/Source.py b/libvirt-sandbox/image/sources/base.py similarity index 100% rename from libvirt-sandbox/image/sources/Source.py rename to libvirt-sandbox/image/sources/base.py diff --git a/libvirt-sandbox/image/sources/DockerSource.py b/libvirt-sandbox/image/sources/docker.py similarity index 99% rename from libvirt-sandbox/image/sources/DockerSource.py rename to libvirt-sandbox/image/sources/docker.py index fb21bda..275a082 100644 --- a/libvirt-sandbox/image/sources/DockerSource.py +++ b/libvirt-sandbox/image/sources/docker.py @@ -21,7 +21,6 @@ # Author: Eren Yagdiran <erenyagdiran@gmail.com> # -from Source import Source import urllib2 import sys import json @@ -31,6 +30,7 @@ import subprocess import shutil import urlparse +from . import base class DockerConfParser(): @@ -48,7 +48,7 @@ class DockerConfParser(): else: return [] -class DockerSource(Source): +class DockerSource(base.Source): def _check_cert_validate(self): major = sys.version_info.major diff --git a/libvirt-sandbox/image/sources/VirtBuilderSource.py b/libvirt-sandbox/image/sources/virtbuilder.py similarity index 98% rename from libvirt-sandbox/image/sources/VirtBuilderSource.py rename to libvirt-sandbox/image/sources/virtbuilder.py index 1c7ea49..6dfa6df 100644 --- a/libvirt-sandbox/image/sources/VirtBuilderSource.py +++ b/libvirt-sandbox/image/sources/virtbuilder.py @@ -19,12 +19,14 @@ # Author: Cedric Bosdonnat <cbosdonnat@suse.com> # -from Source import Source import os import os.path import subprocess -class VirtBuilderSource(Source): +from . import base + + +class VirtBuilderSource(base.Source): def _get_template_name(self, template): # We shouldn't have '/' in the names, but let's make sure diff --git a/libvirt-sandbox/image/template.py b/libvirt-sandbox/image/template.py index 58904a2..751cd4b 100644 --- a/libvirt-sandbox/image/template.py +++ b/libvirt-sandbox/image/template.py @@ -64,11 +64,11 @@ class Template(object): try: p = re.compile("\W") + sourcemod = "".join(p.split(self.source)) sourcename = "".join([i.capitalize() for i in p.split(self.source)]) mod = importlib.import_module( - "libvirt_sandbox.image.sources." + - sourcename + "Source") + "libvirt_sandbox.image.sources." + sourcemod) classname = sourcename + "Source" classimpl = getattr(mod, classname) return classimpl() -- 2.7.4

Individual python module files do not need to have execute permissions. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-sandbox/image/cli.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100755 => 100644 libvirt-sandbox/image/cli.py diff --git a/libvirt-sandbox/image/cli.py b/libvirt-sandbox/image/cli.py old mode 100755 new mode 100644 -- 2.7.4

Add a --debug arg which disables catching of exceptions when running virt-sandbox-image, to make it easier to diagnose unexpected crashes Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-sandbox/image/cli.py | 54 +++++++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/libvirt-sandbox/image/cli.py b/libvirt-sandbox/image/cli.py index 9b9c00b..a2e0bf1 100644 --- a/libvirt-sandbox/image/cli.py +++ b/libvirt-sandbox/image/cli.py @@ -143,6 +143,11 @@ def requires_name(parser): parser.add_argument("-n","--name", help=_("Name of the running sandbox")) +def requires_debug(parser): + parser.add_argument("-d","--debug", + default=False, action="store_true", + help=_("Run in debug mode")) + def check_connect(connectstr): supportedDrivers = ['lxc:///','qemu:///session','qemu:///system'] if not connectstr in supportedDrivers: @@ -183,6 +188,7 @@ Example supported URI formats: def gen_delete_args(subparser): parser = gen_command_parser(subparser, "delete", _("Delete template data")) + requires_debug(parser) requires_template(parser) requires_template_dir(parser) parser.set_defaults(func=delete) @@ -190,6 +196,7 @@ def gen_delete_args(subparser): def gen_create_args(subparser): parser = gen_command_parser(subparser, "create", _("Create image from template data")) + requires_debug(parser) requires_template(parser) requires_connect(parser) requires_template_dir(parser) @@ -198,6 +205,7 @@ def gen_create_args(subparser): def gen_run_args(subparser): parser = gen_command_parser(subparser, "run", _("Run an already built image")) + requires_debug(parser) requires_name(parser) requires_template(parser) requires_connect(parser) @@ -221,26 +229,30 @@ def main(): gen_create_args(subparser) gen_run_args(subparser) - try: - args = parser.parse_args() + args = parser.parse_args() + if args.debug: args.func(args) sys.exit(0) - except KeyboardInterrupt, e: - sys.exit(0) - except ValueError, e: - for line in e: - for l in line: - sys.stderr.write("%s: %s\n" % (sys.argv[0], l)) - sys.stderr.flush() - sys.exit(1) - except IOError, e: - sys.stderr.write("%s: %s: %s\n" % (sys.argv[0], e.filename, e.reason)) - sys.stderr.flush() - sys.exit(1) - except OSError, e: - sys.stderr.write("%s: %s\n" % (sys.argv[0], e)) - sys.stderr.flush() - sys.exit(1) - except Exception, e: - print e.message - sys.exit(1) + else: + try: + args.func(args) + sys.exit(0) + except KeyboardInterrupt, e: + sys.exit(0) + except ValueError, e: + for line in e: + for l in line: + sys.stderr.write("%s: %s\n" % (sys.argv[0], l)) + sys.stderr.flush() + sys.exit(1) + except IOError, e: + sys.stderr.write("%s: %s: %s\n" % (sys.argv[0], e.filename, e.reason)) + sys.stderr.flush() + sys.exit(1) + except OSError, e: + sys.stderr.write("%s: %s\n" % (sys.argv[0], e)) + sys.stderr.flush() + sys.exit(1) + except Exception, e: + print e.message + sys.exit(1) -- 2.7.4

Just print out the value error text directly, instead of trying to interpret it as a multi-line message. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-sandbox/image/cli.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/libvirt-sandbox/image/cli.py b/libvirt-sandbox/image/cli.py index a2e0bf1..b0d864f 100644 --- a/libvirt-sandbox/image/cli.py +++ b/libvirt-sandbox/image/cli.py @@ -240,9 +240,7 @@ def main(): except KeyboardInterrupt, e: sys.exit(0) except ValueError, e: - for line in e: - for l in line: - sys.stderr.write("%s: %s\n" % (sys.argv[0], l)) + sys.stderr.write("%s: %s\n" % (sys.argv[0], e)) sys.stderr.flush() sys.exit(1) except IOError, e: -- 2.7.4

When running the sandbox commands, use check_call() instead of call() so that we detect errors running external commands. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-sandbox/image/sources/base.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libvirt-sandbox/image/sources/base.py b/libvirt-sandbox/image/sources/base.py index 8f6ccba..f70551d 100644 --- a/libvirt-sandbox/image/sources/base.py +++ b/libvirt-sandbox/image/sources/base.py @@ -128,7 +128,7 @@ class Source(): '/sbin/mkfs.ext3', '/dev/disk/by-tag/disk_image'] cmd = cmd + params - subprocess.call(cmd) + subprocess.check_call(cmd) def extract_tarball(self, diskfile, format, tarfile, connect): cmd = ['virt-sandbox'] @@ -148,4 +148,4 @@ class Source(): '-C', '/mnt'] cmd = cmd + params - subprocess.call(cmd) + subprocess.check_call(cmd) -- 2.7.4

Currently if any command fails, the virt-builder source just carries on, with predictably bad results. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-sandbox/image/sources/virtbuilder.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libvirt-sandbox/image/sources/virtbuilder.py b/libvirt-sandbox/image/sources/virtbuilder.py index 6dfa6df..28fbffb 100644 --- a/libvirt-sandbox/image/sources/virtbuilder.py +++ b/libvirt-sandbox/image/sources/virtbuilder.py @@ -47,16 +47,16 @@ class VirtBuilderSource(base.Source): imagepath = "%s/%s.qcow2" % (templatedir, templatename) cmd = ["virt-builder", templatename, "-o", imagepath_original, "--format", "qcow2"] - subprocess.call(cmd) + subprocess.check_call(cmd) try: # We need to convert this image into a single partition one. tarfile = "%s/%s.tar" % (templatedir, templatename) cmd = ["virt-tar-out", "-a", imagepath_original, "/", tarfile] - subprocess.call(cmd) + subprocess.check_call(cmd) cmd = ["qemu-img", "create", "-q", "-f", "qcow2", imagepath, "10G"] - subprocess.call(cmd) + subprocess.check_call(cmd) self.format_disk(imagepath, "qcow2", connect) self.extract_tarball(imagepath, "qcow2", tarfile, connect) @@ -81,7 +81,7 @@ class VirtBuilderSource(base.Source): "-f", "qcow2", "-o", "backing_fmt=qcow2,backing_file=%s" % diskfile, tempfile] - subprocess.call(cmd) + subprocess.check_call(cmd) return tempfile def get_env(self,template, templatedir): -- 2.7.4

When getting the pristine image, we don't customize it or run any scripts, so we can safely disable use of the network. This avoids a possible failure scenario that virt-builder cannot report upfront, where is not configured virbr0. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-sandbox/image/sources/virtbuilder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libvirt-sandbox/image/sources/virtbuilder.py b/libvirt-sandbox/image/sources/virtbuilder.py index 28fbffb..6e71b36 100644 --- a/libvirt-sandbox/image/sources/virtbuilder.py +++ b/libvirt-sandbox/image/sources/virtbuilder.py @@ -45,7 +45,7 @@ class VirtBuilderSource(base.Source): templatename = self._get_template_name(template) imagepath_original = "%s/%s-original.qcow2" % (templatedir, templatename) imagepath = "%s/%s.qcow2" % (templatedir, templatename) - cmd = ["virt-builder", templatename, + cmd = ["virt-builder", templatename, "--no-network", "-o", imagepath_original, "--format", "qcow2"] subprocess.check_call(cmd) -- 2.7.4

Use check_call() instead of call() so that we abort if something goes wrong with qemu-img Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-sandbox/image/sources/docker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libvirt-sandbox/image/sources/docker.py b/libvirt-sandbox/image/sources/docker.py index 275a082..fc2faf2 100644 --- a/libvirt-sandbox/image/sources/docker.py +++ b/libvirt-sandbox/image/sources/docker.py @@ -276,7 +276,7 @@ class DockerSource(base.Source): cmd.append(templateImage) if parentImage is None: cmd.append("10G") - subprocess.call(cmd) + subprocess.check_call(cmd) if parentImage is None: self.format_disk(templateImage, "qcow2", connect) -- 2.7.4

Use the more common naming for the variable pointing to the template directory Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-sandbox/image/sources/docker.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libvirt-sandbox/image/sources/docker.py b/libvirt-sandbox/image/sources/docker.py index fc2faf2..57256f3 100644 --- a/libvirt-sandbox/image/sources/docker.py +++ b/libvirt-sandbox/image/sources/docker.py @@ -288,17 +288,17 @@ class DockerSource(base.Source): connect) parentImage = templateImage - def _get_image_list(self, template, destdir): + def _get_image_list(self, template, templatedir): imageparent = {} imagenames = {} - imagedirs = os.listdir(destdir) + imagedirs = os.listdir(templatedir) for imagetagid in imagedirs: - indexfile = destdir + "/" + imagetagid + "/index.json" + indexfile = templatedir + "/" + imagetagid + "/index.json" if os.path.exists(indexfile): with open(indexfile,"r") as f: index = json.load(f) imagenames[index["name"]] = imagetagid - jsonfile = destdir + "/" + imagetagid + "/template.json" + jsonfile = templatedir + "/" + imagetagid + "/template.json" if os.path.exists(jsonfile): with open(jsonfile,"r") as f: data = json.load(f) -- 2.7.4

Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-sandbox/image/sources/docker.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/libvirt-sandbox/image/sources/docker.py b/libvirt-sandbox/image/sources/docker.py index 57256f3..1e5859e 100644 --- a/libvirt-sandbox/image/sources/docker.py +++ b/libvirt-sandbox/image/sources/docker.py @@ -258,8 +258,6 @@ class DockerSource(base.Source): raise def create_template(self, template, templatedir, connect=None): - self.download_template(template, templatedir) - if not self._was_downloaded(template, templatedir): self.download_template(template, templatedir) -- 2.7.4

Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-sandbox/image/sources/docker.py | 1 + 1 file changed, 1 insertion(+) diff --git a/libvirt-sandbox/image/sources/docker.py b/libvirt-sandbox/image/sources/docker.py index 1e5859e..3e0aa9f 100644 --- a/libvirt-sandbox/image/sources/docker.py +++ b/libvirt-sandbox/image/sources/docker.py @@ -29,6 +29,7 @@ import os import subprocess import shutil import urlparse +import hashlib from . import base -- 2.7.4

The download code was mistakenly using the length of the json manifest in place of the length of the image file blob. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-sandbox/image/sources/docker.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/libvirt-sandbox/image/sources/docker.py b/libvirt-sandbox/image/sources/docker.py index 3e0aa9f..3b59051 100644 --- a/libvirt-sandbox/image/sources/docker.py +++ b/libvirt-sandbox/image/sources/docker.py @@ -138,8 +138,6 @@ class DockerSource(base.Source): jsonfile) createdFiles.append(jsonfile) - layersize = int(res.info().getheader("Content-Length")) - datacsum = None if layerid in checksums: datacsum = checksums[layerid] @@ -148,7 +146,7 @@ class DockerSource(base.Source): registryendpoint, "/v1/images/" + layerid + "/layer", headers, - datafile, datacsum, layersize) + datafile, datacsum) createdFiles.append(datafile) index = { @@ -173,10 +171,14 @@ class DockerSource(base.Source): pass def _save_data(self, template, server, path, headers, - dest, checksum=None, datalen=None): + dest, checksum=None): try: res = self._get_url(template, server, path, headers) + datalen = res.info().getheader("Content-Length") + if datalen is not None: + datalen = int(datalen) + csum = None if checksum is not None: csum = hashlib.sha256() -- 2.7.4

Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-sandbox/image/sources/docker.py | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/libvirt-sandbox/image/sources/docker.py b/libvirt-sandbox/image/sources/docker.py index 3b59051..c989662 100644 --- a/libvirt-sandbox/image/sources/docker.py +++ b/libvirt-sandbox/image/sources/docker.py @@ -89,9 +89,6 @@ class DockerSource(base.Source): registryendpoint = res.info().getheader('X-Docker-Endpoints') token = res.info().getheader('X-Docker-Token') - checksums = {} - for layer in data: - pass headers = {} if token is not None: @@ -101,8 +98,6 @@ class DockerSource(base.Source): "/v1/repositories" + template.path + "/tags", headers) - cookie = res.info().getheader('Set-Cookie') - tag = template.params.get("tag", "latest") if not tag in data: raise ValueError(["Tag '%s' does not exist for image '%s'" % (tag, template)]) @@ -138,15 +133,11 @@ class DockerSource(base.Source): jsonfile) createdFiles.append(jsonfile) - datacsum = None - if layerid in checksums: - datacsum = checksums[layerid] - self._save_data(template, registryendpoint, "/v1/images/" + layerid + "/layer", headers, - datafile, datacsum) + datafile) createdFiles.append(datafile) index = { -- 2.7.4

Instead of directly using the Template class in docker code, introduce a DockerImage class which represents the data in a more convenient manner. In particular it will canonicalize the image names eg a bare image name with no "/", like "ubuntu", should be "library/ubuntu" The index.json file now stores the repo name, image name and tag so we resolve the correct matching image where there are multiple versions of the same image stored locally. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-sandbox/image/sources/docker.py | 110 +++++++++++++++++++++++--------- 1 file changed, 81 insertions(+), 29 deletions(-) diff --git a/libvirt-sandbox/image/sources/docker.py b/libvirt-sandbox/image/sources/docker.py index c989662..01cf498 100644 --- a/libvirt-sandbox/image/sources/docker.py +++ b/libvirt-sandbox/image/sources/docker.py @@ -49,6 +49,39 @@ class DockerConfParser(): else: return [] +class DockerImage(): + + def __init__(self, repo, name, tag=None): + + self.repo = repo + self.name = name + self.tag = tag + + if self.tag is None: + self.tag = "latest" + + if self.repo is None: + self.repo = "library" + + def __repr__(self): + return "%s/%s,tag=%s" % (self.repo, self.name, self.tag) + + @classmethod + def from_template(cls, template): + bits = template.path[1:].split("/") + if len(bits) == 1: + return cls(repo=None, + name=bits[0], + tag=template.params.get("tag")) + elif len(bits) == 2: + return cls(repo=bits[0], + name=bits[1], + tag=template.params.get("tag")) + else: + raise Exception("Expected image name, or repo & image name for path, not '%s'", + template.path) + + class DockerSource(base.Source): def _check_cert_validate(self): @@ -60,9 +93,9 @@ class DockerSource(base.Source): if (major == 2 and sys.hexversion < py2_7_9_hexversion) or (major == 3 and sys.hexversion < py3_4_3_hexversion): sys.stderr.write(SSL_WARNING) - def _was_downloaded(self, template, templatedir): + def _was_downloaded(self, image, templatedir): try: - self._get_image_list(template, templatedir) + self._get_image_list(image, templatedir) return True except Exception: return False @@ -70,19 +103,22 @@ class DockerSource(base.Source): def has_template(self, template, templatedir): try: - configfile, diskfile = self._get_template_data(template, templatedir) + image = DockerImage.from_template(template) + configfile, diskfile = self._get_template_data(image, templatedir) return os.path.exists(diskfile) except Exception: return False - def download_template(self, template, templatedir): + def download_template(self, image, template, templatedir): self._check_cert_validate() try: (data, res) = self._get_json(template, None, - "/v1/repositories" + template.path + "/images", + "/v1/repositories/%s/%s/images" % ( + image.repo, image.name, + ), {"X-Docker-Token": "true"}) except urllib2.HTTPError, e: raise ValueError(["Image '%s' does not exist" % template]) @@ -95,13 +131,15 @@ class DockerSource(base.Source): headers["Authorization"] = "Token " + token (data, res) = self._get_json(template, registryendpoint, - "/v1/repositories" + template.path + "/tags", + "/v1/repositories/%s/%s/tags" %( + image.repo, image.name + ), headers) - tag = template.params.get("tag", "latest") - if not tag in data: - raise ValueError(["Tag '%s' does not exist for image '%s'" % (tag, template)]) - imagetagid = data[tag] + if image.tag not in data: + raise ValueError(["Tag '%s' does not exist for image '%s'" % + (image.tag, template)]) + imagetagid = data[image.tag] (data, res) = self._get_json(template, registryendpoint, @@ -141,7 +179,9 @@ class DockerSource(base.Source): createdFiles.append(datafile) index = { - "name": template.path, + "repo": image.repo, + "name": image.name, + "tag": image.tag, } indexfile = templatedir + "/" + imagetagid + "/index.json" @@ -252,10 +292,12 @@ class DockerSource(base.Source): raise def create_template(self, template, templatedir, connect=None): - if not self._was_downloaded(template, templatedir): - self.download_template(template, templatedir) + image = DockerImage.from_template(template) - imagelist = self._get_image_list(template, templatedir) + if not self._was_downloaded(image, templatedir): + self.download_template(image, template, templatedir) + + imagelist = self._get_image_list(image, templatedir) imagelist.reverse() parentImage = None @@ -280,7 +322,7 @@ class DockerSource(base.Source): connect) parentImage = templateImage - def _get_image_list(self, template, templatedir): + def _get_image_list(self, image, templatedir): imageparent = {} imagenames = {} imagedirs = os.listdir(templatedir) @@ -289,7 +331,10 @@ class DockerSource(base.Source): if os.path.exists(indexfile): with open(indexfile,"r") as f: index = json.load(f) - imagenames[index["name"]] = imagetagid + thisimage = DockerImage(index.get("repo"), + index["name"], + index.get("tag")) + imagenames[str(thisimage)] = imagetagid jsonfile = templatedir + "/" + imagetagid + "/template.json" if os.path.exists(jsonfile): with open(jsonfile,"r") as f: @@ -297,9 +342,9 @@ class DockerSource(base.Source): parent = data.get("parent",None) if parent: imageparent[imagetagid] = parent - if not template.path in imagenames: - raise ValueError(["Image %s does not exist locally" % template.path]) - imagetagid = imagenames[template.path] + if str(image) not in imagenames: + raise ValueError(["Image %s does not exist locally" % image]) + imagetagid = imagenames[str(image)] imagelist = [] while imagetagid != None: imagelist.append(imagetagid) @@ -308,6 +353,8 @@ class DockerSource(base.Source): return imagelist def delete_template(self, template, templatedir): + image = DockerImage.from_template(template) + imageusage = {} imageparent = {} imagenames = {} @@ -317,7 +364,10 @@ class DockerSource(base.Source): if os.path.exists(indexfile): with open(indexfile,"r") as f: index = json.load(f) - imagenames[index["name"]] = imagetagid + thisimage = DockerImage(index.get("repo"), + index["name"], + index.get("tag")) + imagenames[str(thisimage)] = imagetagid jsonfile = templatedir + "/" + imagetagid + "/template.json" if os.path.exists(jsonfile): with open(jsonfile,"r") as f: @@ -331,10 +381,9 @@ class DockerSource(base.Source): imageparent[imagetagid] = parent - if not template.path in imagenames: - raise ValueError(["Image %s does not exist locally" % template.path]) - - imagetagid = imagenames[template.path] + if str(image) not in imagenames: + raise ValueError(["Image %s does not exist locally" % image]) + imagetagid = imagenames[str(image)] while imagetagid != None: debug("Remove %s\n" % imagetagid) parent = imageparent.get(imagetagid,None) @@ -357,15 +406,16 @@ class DockerSource(base.Source): parent = None imagetagid = parent - def _get_template_data(self, template, templatedir): - imageList = self._get_image_list(template, templatedir) + def _get_template_data(self, image, templatedir): + imageList = self._get_image_list(image, templatedir) toplayer = imageList[0] diskfile = templatedir + "/" + toplayer + "/template.qcow2" configfile = templatedir + "/" + toplayer + "/template.json" return configfile, diskfile def get_disk(self, template, templatedir, imagedir, sandboxname): - configfile, diskfile = self._get_template_data(template, templatedir) + image = DockerImage.from_template(template) + configfile, diskfile = self._get_template_data(image, templatedir) tempfile = imagedir + "/" + sandboxname + ".qcow2" if not os.path.exists(imagedir): os.makedirs(imagedir) @@ -377,7 +427,8 @@ class DockerSource(base.Source): return tempfile def get_command(self, template, templatedir, userargs): - configfile, diskfile = self._get_template_data(template, templatedir) + image = DockerImage.from_template(template) + configfile, diskfile = self._get_template_data(image, templatedir) configParser = DockerConfParser(configfile) cmd = configParser.getCommand() entrypoint = configParser.getEntrypoint() @@ -391,7 +442,8 @@ class DockerSource(base.Source): return entrypoint + cmd def get_env(self, template, templatedir): - configfile, diskfile = self._get_template_data(template, templatedir) + image = DockerImage.from_template(template) + configfile, diskfile = self._get_template_data(image, templatedir) configParser = DockerConfParser(configfile) return configParser.getEnvs() -- 2.7.4

The code for adding the Accept header was doing so based on whether 'if json' but 'json' resolved to a module import name, not a local boolean. So the header was always added even for requests not expected to be json. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-sandbox/image/sources/docker.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/libvirt-sandbox/image/sources/docker.py b/libvirt-sandbox/image/sources/docker.py index 01cf498..658d90a 100644 --- a/libvirt-sandbox/image/sources/docker.py +++ b/libvirt-sandbox/image/sources/docker.py @@ -266,8 +266,6 @@ class DockerSource(base.Source): debug("Fetching %s..." % url) req = urllib2.Request(url=url) - if json: - req.add_header("Accept", "application/json") for h in headers.keys(): req.add_header(h, headers[h]) @@ -283,6 +281,11 @@ class DockerSource(base.Source): def _get_json(self, template, server, path, headers): try: + if headers is None: + headers = {} + else: + headers = copy.copy(headers) + headers["Accept"] = "application/json") res = self._get_url(template, server, path, headers) data = json.loads(res.read()) debug("OK\n") -- 2.7.4

The docker registry v1 and v2 versions have completely different authentication methods that need handling. The v2 OAuth scheme requires modifying request headers and re-trying requests after getting an auth token. Introduce a pluggable framework for auth can be hooked into the _get_url() method to deal with the request and response objects, as well as errors. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-sandbox/image/sources/docker.py | 143 +++++++++++++++++++++++++------- 1 file changed, 113 insertions(+), 30 deletions(-) diff --git a/libvirt-sandbox/image/sources/docker.py b/libvirt-sandbox/image/sources/docker.py index 658d90a..a54f563 100644 --- a/libvirt-sandbox/image/sources/docker.py +++ b/libvirt-sandbox/image/sources/docker.py @@ -30,6 +30,7 @@ import subprocess import shutil import urlparse import hashlib +from abc import ABCMeta, abstractmethod from . import base @@ -82,8 +83,83 @@ class DockerImage(): template.path) +class DockerAuth(): + + __metaclass__ = ABCMeta + def __init__(self): + pass + + @abstractmethod + def prepare_req(self, req): + pass + + @abstractmethod + def process_res(self, res): + pass + + @abstractmethod + def process_err(self, err): + return False + + +class DockerAuthNop(DockerAuth): + + def prepare_req(self, req): + pass + + def process_res(self, res): + pass + + def process_err(self, err): + return False + + +class DockerAuthBasic(DockerAuth): + + def __init__(self, username, password): + self.username = username + self.password = password + self.token = None + + def prepare_req(self, req): + if self.username is not None: + auth = base64.encodestring( + '%s:%s' % (self.username, self.password)).replace('\n', '') + + req.add_header("Authorization", "Basic %s" % auth) + + req.add_header("X-Docker-Token", "true") + + def process_res(self, res): + self.token = res.info().getheader('X-Docker-Token') + + def process_err(self, err): + return False + + +class DockerAuthToken(DockerAuth): + + def __init__(self, token): + self.token = token + + def prepare_req(self, req): + req.add_header("Authorization", "Token %s" % self.token) + + def process_res(self, res): + pass + + def process_err(self, err): + return False + + class DockerSource(base.Source): + def __init__(self): + self.auth_handler = DockerAuthNop() + + def set_auth_handler(self, auth_handler): + self.auth_handler = auth_handler + def _check_cert_validate(self): major = sys.version_info.major SSL_WARNING = "SSL certificates couldn't be validated by default. You need to have 2.7.9/3.4.3 or higher" @@ -113,28 +189,29 @@ class DockerSource(base.Source): def download_template(self, image, template, templatedir): self._check_cert_validate() + basicauth = DockerAuthBasic(template.username, template.password) + self.set_auth_handler(basicauth) try: (data, res) = self._get_json(template, None, "/v1/repositories/%s/%s/images" % ( image.repo, image.name, - ), - {"X-Docker-Token": "true"}) + )) except urllib2.HTTPError, e: raise ValueError(["Image '%s' does not exist" % template]) registryendpoint = res.info().getheader('X-Docker-Endpoints') - token = res.info().getheader('X-Docker-Token') - headers = {} - if token is not None: - headers["Authorization"] = "Token " + token + if basicauth.token is not None: + self.set_auth_handler(DockerAuthToken(basicauth.token)) + else: + self.set_auth_handler(DockerAuthNop()) + (data, res) = self._get_json(template, registryendpoint, "/v1/repositories/%s/%s/tags" %( image.repo, image.name - ), - headers) + )) if image.tag not in data: raise ValueError(["Tag '%s' does not exist for image '%s'" % @@ -143,8 +220,7 @@ class DockerSource(base.Source): (data, res) = self._get_json(template, registryendpoint, - "/v1/images/" + imagetagid + "/ancestry", - headers) + "/v1/images/" + imagetagid + "/ancestry") if data[0] != imagetagid: raise ValueError(["Expected first layer id '%s' to match image id '%s'", @@ -167,14 +243,12 @@ class DockerSource(base.Source): res = self._save_data(template, registryendpoint, "/v1/images/" + layerid + "/json", - headers, jsonfile) createdFiles.append(jsonfile) self._save_data(template, registryendpoint, "/v1/images/" + layerid + "/layer", - headers, datafile) createdFiles.append(datafile) @@ -201,10 +275,10 @@ class DockerSource(base.Source): except: pass - def _save_data(self, template, server, path, headers, + def _save_data(self, template, server, path, dest, checksum=None): try: - res = self._get_url(template, server, path, headers) + res = self._get_url(template, server, path) datalen = res.info().getheader("Content-Length") if datalen is not None: @@ -247,7 +321,7 @@ class DockerSource(base.Source): debug("FAIL %s\n" % str(e)) raise - def _get_url(self, template, server, path, headers): + def _get_url(self, template, server, path, headers=None): if template.protocol is None: protocol = "https" else: @@ -266,25 +340,34 @@ class DockerSource(base.Source): debug("Fetching %s..." % url) req = urllib2.Request(url=url) - for h in headers.keys(): - req.add_header(h, headers[h]) - - #www Auth header starts - if template.username and template.password: - base64string = base64.encodestring( - '%s:%s' % (template.username, - template.password)).replace('\n', '') - req.add_header("Authorization", "Basic %s" % base64string) - #www Auth header finish + if headers is not None: + for h in headers.keys(): + req.add_header(h, headers[h]) - return urllib2.urlopen(req) + self.auth_handler.prepare_req(req) - def _get_json(self, template, server, path, headers): try: - if headers is None: - headers = {} + res = urllib2.urlopen(req) + self.auth_handler.process_res(res) + return res + except urllib2.HTTPError as e: + if e.code == 401: + retry = self.auth_handler.process_err(e) + if retry: + debug("Re-Fetching %s..." % url) + self.auth_handler.prepare_req(req) + res = urllib2.urlopen(req) + self.auth_handler.process_res(res) + return res + else: + debug("Not re-fetching") + raise else: - headers = copy.copy(headers) + raise + + def _get_json(self, template, server, path): + try: + headers = {} headers["Accept"] = "application/json") res = self._get_url(template, server, path, headers) data = json.loads(res.read()) -- 2.7.4

Introduce a class to handle HTTP requests with a docker registry server, and associated auth credentials. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-sandbox/image/sources/docker.py | 264 ++++++++++++++++---------------- 1 file changed, 136 insertions(+), 128 deletions(-) diff --git a/libvirt-sandbox/image/sources/docker.py b/libvirt-sandbox/image/sources/docker.py index a54f563..b2706b1 100644 --- a/libvirt-sandbox/image/sources/docker.py +++ b/libvirt-sandbox/image/sources/docker.py @@ -31,6 +31,7 @@ import shutil import urlparse import hashlib from abc import ABCMeta, abstractmethod +import copy from . import base @@ -152,14 +153,132 @@ class DockerAuthToken(DockerAuth): return False -class DockerSource(base.Source): +class DockerRegistry(): - def __init__(self): + def __init__(self, uri_base): + + self.uri_base = list(urlparse.urlparse(uri_base)) self.auth_handler = DockerAuthNop() def set_auth_handler(self, auth_handler): self.auth_handler = auth_handler + def set_server(self, server): + self.uri_base[1] = server + + @classmethod + def from_template(cls, template): + protocol = template.protocol + hostname = template.hostname + port = template.port + + if protocol is None: + protocol = "https" + if hostname is None: + hostname = "index.docker.io" + + if port is None: + server = hostname + else: + server = "%s:%s" % (hostname, port) + + url = urlparse.urlunparse((protocol, server, "", None, None, None)) + + return cls(url) + + def get_url(self, path, headers=None): + url_bits = copy.copy(self.uri_base) + url_bits[2] = path + url = urlparse.urlunparse(url_bits) + debug("Fetching %s..." % url) + + req = urllib2.Request(url=url) + + if headers is not None: + for h in headers.keys(): + req.add_header(h, headers[h]) + + self.auth_handler.prepare_req(req) + + try: + res = urllib2.urlopen(req) + self.auth_handler.process_res(res) + return res + except urllib2.HTTPError as e: + if e.code == 401: + retry = self.auth_handler.process_err(e) + if retry: + debug("Re-Fetching %s..." % url) + self.auth_handler.prepare_req(req) + res = urllib2.urlopen(req) + self.auth_handler.process_res(res) + return res + else: + debug("Not re-fetching") + raise + else: + raise + + def save_data(self, path, dest, checksum=None): + try: + res = self.get_url(path) + + datalen = res.info().getheader("Content-Length") + if datalen is not None: + datalen = int(datalen) + + csum = None + if checksum is not None: + csum = hashlib.sha256() + + pattern = [".", "o", "O", "o"] + patternIndex = 0 + donelen = 0 + + with open(dest, "w") as f: + while 1: + buf = res.read(1024*64) + if not buf: + break + if csum is not None: + csum.update(buf) + f.write(buf) + + if datalen is not None: + donelen = donelen + len(buf) + debug("\x1b[s%s (%5d Kb of %5d Kb)\x1b8" % ( + pattern[patternIndex], (donelen/1024), (datalen/1024) + )) + patternIndex = (patternIndex + 1) % 4 + + debug("\x1b[K") + if csum is not None: + csumstr = "sha256:" + csum.hexdigest() + if csumstr != checksum: + debug("FAIL checksum '%s' does not match '%s'" % (csumstr, checksum)) + os.remove(dest) + raise IOError("Checksum '%s' for data does not match '%s'" % (csumstr, checksum)) + debug("OK\n") + return res + except Exception, e: + debug("FAIL %s\n" % str(e)) + raise + + def get_json(self, path): + try: + headers = {} + headers["Accept"] = "application/json" + res = self.get_url(path, headers) + data = json.loads(res.read()) + debug("OK\n") + return (data, res) + except Exception, e: + debug("FAIL %s\n" % str(e)) + raise + + +class DockerSource(base.Source): + def _check_cert_validate(self): major = sys.version_info.major SSL_WARNING = "SSL certificates couldn't be validated by default. You need to have 2.7.9/3.4.3 or higher" @@ -189,38 +308,33 @@ class DockerSource(base.Source): def download_template(self, image, template, templatedir): self._check_cert_validate() + registry = DockerRegistry.from_template(template) basicauth = DockerAuthBasic(template.username, template.password) - self.set_auth_handler(basicauth) + registry.set_auth_handler(basicauth) try: - (data, res) = self._get_json(template, - None, - "/v1/repositories/%s/%s/images" % ( - image.repo, image.name, - )) + (data, res) = registry.get_json("/v1/repositories/%s/%s/images" % ( + image.repo, image.name, + )) except urllib2.HTTPError, e: raise ValueError(["Image '%s' does not exist" % template]) registryendpoint = res.info().getheader('X-Docker-Endpoints') if basicauth.token is not None: - self.set_auth_handler(DockerAuthToken(basicauth.token)) + registry.set_auth_handler(DockerAuthToken(basicauth.token)) else: - self.set_auth_handler(DockerAuthNop()) + registry.set_auth_handler(DockerAuthNop()) - (data, res) = self._get_json(template, - registryendpoint, - "/v1/repositories/%s/%s/tags" %( - image.repo, image.name - )) + (data, res) = registry.get_json("/v1/repositories/%s/%s/tags" %( + image.repo, image.name + )) if image.tag not in data: raise ValueError(["Tag '%s' does not exist for image '%s'" % (image.tag, template)]) imagetagid = data[image.tag] - (data, res) = self._get_json(template, - registryendpoint, - "/v1/images/" + imagetagid + "/ancestry") + (data, res) = registry.get_json("/v1/images/" + imagetagid + "/ancestry") if data[0] != imagetagid: raise ValueError(["Expected first layer id '%s' to match image id '%s'", @@ -240,16 +354,12 @@ class DockerSource(base.Source): datafile = layerdir + "/template.tar.gz" if not os.path.exists(jsonfile) or not os.path.exists(datafile): - res = self._save_data(template, - registryendpoint, - "/v1/images/" + layerid + "/json", - jsonfile) + res = registry.save_data("/v1/images/" + layerid + "/json", + jsonfile) createdFiles.append(jsonfile) - self._save_data(template, - registryendpoint, - "/v1/images/" + layerid + "/layer", - datafile) + registry.save_data("/v1/images/" + layerid + "/layer", + datafile) createdFiles.append(datafile) index = { @@ -275,108 +385,6 @@ class DockerSource(base.Source): except: pass - def _save_data(self, template, server, path, - dest, checksum=None): - try: - res = self._get_url(template, server, path) - - datalen = res.info().getheader("Content-Length") - if datalen is not None: - datalen = int(datalen) - - csum = None - if checksum is not None: - csum = hashlib.sha256() - - pattern = [".", "o", "O", "o"] - patternIndex = 0 - donelen = 0 - - with open(dest, "w") as f: - while 1: - buf = res.read(1024*64) - if not buf: - break - if csum is not None: - csum.update(buf) - f.write(buf) - - if datalen is not None: - donelen = donelen + len(buf) - debug("\x1b[s%s (%5d Kb of %5d Kb)\x1b8" % ( - pattern[patternIndex], (donelen/1024), (datalen/1024) - )) - patternIndex = (patternIndex + 1) % 4 - - debug("\x1b[K") - if csum is not None: - csumstr = "sha256:" + csum.hexdigest() - if csumstr != checksum: - debug("FAIL checksum '%s' does not match '%s'" % (csumstr, checksum)) - os.remove(dest) - raise IOError("Checksum '%s' for data does not match '%s'" % (csumstr, checksum)) - debug("OK\n") - return res - except Exception, e: - debug("FAIL %s\n" % str(e)) - raise - - def _get_url(self, template, server, path, headers=None): - if template.protocol is None: - protocol = "https" - else: - protocol = template.protocol - - if server is None: - if template.hostname is None: - server = "index.docker.io" - else: - if template.port is not None: - server = "%s:%d" % (template.hostname, template.port) - else: - server = template.hostname - - url = urlparse.urlunparse((protocol, server, path, None, None, None)) - debug("Fetching %s..." % url) - - req = urllib2.Request(url=url) - if headers is not None: - for h in headers.keys(): - req.add_header(h, headers[h]) - - self.auth_handler.prepare_req(req) - - try: - res = urllib2.urlopen(req) - self.auth_handler.process_res(res) - return res - except urllib2.HTTPError as e: - if e.code == 401: - retry = self.auth_handler.process_err(e) - if retry: - debug("Re-Fetching %s..." % url) - self.auth_handler.prepare_req(req) - res = urllib2.urlopen(req) - self.auth_handler.process_res(res) - return res - else: - debug("Not re-fetching") - raise - else: - raise - - def _get_json(self, template, server, path): - try: - headers = {} - headers["Accept"] = "application/json") - res = self._get_url(template, server, path, headers) - data = json.loads(res.read()) - debug("OK\n") - return (data, res) - except Exception, e: - debug("FAIL %s\n" % str(e)) - raise - def create_template(self, template, templatedir, connect=None): image = DockerImage.from_template(template) -- 2.7.4

Split off code which rolls back and deletes files when download fails. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-sandbox/image/sources/docker.py | 91 +++++++++++++++++---------------- 1 file changed, 46 insertions(+), 45 deletions(-) diff --git a/libvirt-sandbox/image/sources/docker.py b/libvirt-sandbox/image/sources/docker.py index b2706b1..3cc321b 100644 --- a/libvirt-sandbox/image/sources/docker.py +++ b/libvirt-sandbox/image/sources/docker.py @@ -304,8 +304,26 @@ class DockerSource(base.Source): except Exception: return False - def download_template(self, image, template, templatedir): + try: + createdFiles = [] + createdDirs = [] + + self._download_template_impl(image, template, templatedir, createdFiles, createdDirs) + except Exception as e: + for f in createdFiles: + try: + os.remove(f) + except: + pass + for d in createdDirs: + try: + shutil.rmtree(d) + except: + pass + raise + + def _download_template_impl(self, image, template, templatedir, createdFiles, createdDirs): self._check_cert_validate() registry = DockerRegistry.from_template(template) @@ -340,50 +358,33 @@ class DockerSource(base.Source): raise ValueError(["Expected first layer id '%s' to match image id '%s'", data[0], imagetagid]) - try: - createdFiles = [] - createdDirs = [] - - for layerid in data: - layerdir = templatedir + "/" + layerid - if not os.path.exists(layerdir): - os.makedirs(layerdir) - createdDirs.append(layerdir) - - jsonfile = layerdir + "/template.json" - datafile = layerdir + "/template.tar.gz" - - if not os.path.exists(jsonfile) or not os.path.exists(datafile): - res = registry.save_data("/v1/images/" + layerid + "/json", - jsonfile) - createdFiles.append(jsonfile) - - registry.save_data("/v1/images/" + layerid + "/layer", - datafile) - createdFiles.append(datafile) - - index = { - "repo": image.repo, - "name": image.name, - "tag": image.tag, - } - - indexfile = templatedir + "/" + imagetagid + "/index.json" - print("Index file " + indexfile) - with open(indexfile, "w") as f: - f.write(json.dumps(index)) - except Exception as e: - traceback.print_exc() - for f in createdFiles: - try: - os.remove(f) - except: - pass - for d in createdDirs: - try: - shutil.rmtree(d) - except: - pass + for layerid in data: + layerdir = templatedir + "/" + layerid + if not os.path.exists(layerdir): + os.makedirs(layerdir) + createdDirs.append(layerdir) + + jsonfile = layerdir + "/template.json" + datafile = layerdir + "/template.tar.gz" + + if not os.path.exists(jsonfile) or not os.path.exists(datafile): + res = registry.save_data("/v1/images/" + layerid + "/json", + jsonfile) + createdFiles.append(jsonfile) + + registry.save_data("/v1/images/" + layerid + "/layer", + datafile) + createdFiles.append(datafile) + + index = { + "repo": image.repo, + "name": image.name, + "tag": image.tag, + } + + indexfile = templatedir + "/" + imagetagid + "/index.json" + with open(indexfile, "w") as f: + f.write(json.dumps(index)) def create_template(self, template, templatedir, connect=None): image = DockerImage.from_template(template) -- 2.7.4

Latest docker v2 registry uses OAuth for creating tokens, identified by the "Bearer" method in the 'WWW-Authenticate' header. Add a DockerAuthBearer impl to deal with this. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-sandbox/image/sources/docker.py | 54 +++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/libvirt-sandbox/image/sources/docker.py b/libvirt-sandbox/image/sources/docker.py index 3cc321b..cc8d05b 100644 --- a/libvirt-sandbox/image/sources/docker.py +++ b/libvirt-sandbox/image/sources/docker.py @@ -153,6 +153,60 @@ class DockerAuthToken(DockerAuth): return False +class DockerAuthBearer(DockerAuth): + + def __init__(self): + self.token = None + + def prepare_req(self, req): + if self.token is not None: + debug("Adding token %s" % self.token) + req.add_header("Authorization", "Bearer %s" % self.token) + else: + debug("No token") + + def process_res(self, res): + pass + + def process_err(self, err): + method = err.headers.get("WWW-Authenticate", None) + if method is None: + return False + + if not method.startswith("Bearer "): + return False + + challenge = method[7:] + + bits = challenge.split(",") + attrs = {} + for bit in bits: + subbit = bit.split("=") + attrs[subbit[0]] = subbit[1][1:-1] + + url = attrs["realm"] + del attrs["realm"] + if "error" in attrs: + del attrs["error"] + + params = "&".join([ + "%s=%s" % (attr, attrs[attr]) + for attr in attrs.keys() + ]) + if params != "": + url = url + "?" + params + + debug("Requesting auth at %s\n" % url) + req = urllib2.Request(url=url) + req.add_header("Accept", "application/json") + + res = urllib2.urlopen(req) + data = json.loads(res.read()) + self.token = data["token"] + debug("Saved %s" % self.token) + return True + + class DockerRegistry(): def __init__(self, uri_base): -- 2.7.4

Many images are no longer available to download the via v1 registry server. Implement the v2 registry server protocol, in combination with the v2.1 metadata file format. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-sandbox/image/sources/docker.py | 61 +++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/libvirt-sandbox/image/sources/docker.py b/libvirt-sandbox/image/sources/docker.py index cc8d05b..291a305 100644 --- a/libvirt-sandbox/image/sources/docker.py +++ b/libvirt-sandbox/image/sources/docker.py @@ -217,6 +217,17 @@ class DockerRegistry(): def set_auth_handler(self, auth_handler): self.auth_handler = auth_handler + def supports_v2(self): + try: + (data, res) = self.get_json("/v2/") + ver = res.info().getheader("Docker-Distribution-Api-Version") + except urllib2.HTTPError as e: + ver = e.headers.get("Docker-Distribution-Api-Version", None) + + if ver is None: + return False + return ver.startswith("registry/2") + def set_server(self, server): self.uri_base[1] = server @@ -381,6 +392,13 @@ class DockerSource(base.Source): self._check_cert_validate() registry = DockerRegistry.from_template(template) + registry.set_auth_handler(DockerAuthBearer()) + if registry.supports_v2(): + self._download_template_impl_v2(registry, image, template, templatedir, createdFiles, createdDirs) + else: + self._download_template_impl_v1(registry, image, template, templatedir, createdFiles, createdDirs) + + def _download_template_impl_v1(self, registry, image, template, templatedir, createdFiles, createdDirs): basicauth = DockerAuthBasic(template.username, template.password) registry.set_auth_handler(basicauth) try: @@ -440,6 +458,49 @@ class DockerSource(base.Source): with open(indexfile, "w") as f: f.write(json.dumps(index)) + + def _download_template_impl_v2(self, registry, image, template, templatedir, createdFiles, createdDirs): + (manifest, res) = registry.get_json( "/v2/%s/%s/manifests/%s" % ( + image.repo, image.name, image.tag)) + + layerChecksums = [ + layer["blobSum"] for layer in manifest["fsLayers"] + ] + layers = [ + json.loads(entry["v1Compatibility"]) for entry in manifest["history"] + ] + + for i in range(len(layerChecksums)): + layerChecksum = layerChecksums[i] + config = layers[i] + + layerdir = templatedir + "/" + config["id"] + if not os.path.exists(layerdir): + os.makedirs(layerdir) + createdDirs.append(layerdir) + + jsonfile = layerdir + "/template.json" + datafile = layerdir + "/template.tar.gz" + + with open(jsonfile, "w") as fh: + fh.write(json.dumps(config)) + + registry.save_data("/v2/%s/%s/blobs/%s" % ( + image.repo, image.name, layerChecksum), + datafile, checksum=layerChecksum) + + + index = { + "repo": image.repo, + "name": image.name, + "tag": image.tag, + } + + indexfile = templatedir + "/" + layers[0]["id"] + "/index.json" + with open(indexfile, "w") as f: + f.write(json.dumps(index)) + + def create_template(self, template, templatedir, connect=None): image = DockerImage.from_template(template) -- 2.7.4

Introduce a command able to list locally stored image templates: $ virt-sandbox-image list docker:library/ubuntu?tag=14.04.1 docker:library/ubuntu?tag=14.04.2 virt-builder:/fedora-23 or restrict to a single source type $ virt-sandbox-image list --source docker docker:library/ubuntu?tag=14.04.1 docker:library/ubuntu?tag=14.04.2 Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-sandbox/image/cli.py | 24 ++++++++++++++++++++++ libvirt-sandbox/image/sources/base.py | 10 ++++++++++ libvirt-sandbox/image/sources/docker.py | 22 ++++++++++++++++++++ libvirt-sandbox/image/sources/virtbuilder.py | 18 +++++++++++++++++ libvirt-sandbox/image/template.py | 30 +++++++++++++++++++--------- 5 files changed, 95 insertions(+), 9 deletions(-) diff --git a/libvirt-sandbox/image/cli.py b/libvirt-sandbox/image/cli.py index b0d864f..66854e4 100644 --- a/libvirt-sandbox/image/cli.py +++ b/libvirt-sandbox/image/cli.py @@ -135,6 +135,18 @@ def run(args): os.unlink(diskfile) source.post_run(tmpl, template_dir, name) +def list_cached(args): + tmpls = [] + if args.source is not None: + tmpls.extend(template.Template.get_all(args.source, + "%s/%s" % (args.template_dir, args.source))) + else: + for source in ["docker", "virt-builder"]: + tmpls.extend(template.Template.get_all(source, + "%s/%s" % (args.template_dir, source))) + for tmpl in tmpls: + print tmpl + def requires_template(parser): parser.add_argument("template", help=_("URI of the template")) @@ -221,6 +233,17 @@ def gen_run_args(subparser): parser.set_defaults(func=run) +def gen_list_args(subparser): + parser = gen_command_parser(subparser, "list", + _("List locally cached images")) + requires_debug(parser) + requires_template_dir(parser) + + parser.add_argument("-s","--source", + help=_("Name of the template source")) + + parser.set_defaults(func=list_cached) + def main(): parser = argparse.ArgumentParser(description="Sandbox Container Image Tool") @@ -228,6 +251,7 @@ def main(): gen_delete_args(subparser) gen_create_args(subparser) gen_run_args(subparser) + gen_list_args(subparser) args = parser.parse_args() if args.debug: diff --git a/libvirt-sandbox/image/sources/base.py b/libvirt-sandbox/image/sources/base.py index f70551d..e4e4e41 100644 --- a/libvirt-sandbox/image/sources/base.py +++ b/libvirt-sandbox/image/sources/base.py @@ -34,6 +34,16 @@ class Source(): def __init__(self): pass + @abstractmethod + def list_templates(self, templatedir): + """ + :param templatedir: local directory path in which to store the template + + Get a list of all templates that are locally cached + + :returns: a list of libvirt_sandbox.template.Template objects + """ + pass @abstractmethod def has_template(self, template, templatedir): diff --git a/libvirt-sandbox/image/sources/docker.py b/libvirt-sandbox/image/sources/docker.py index 291a305..dd72db7 100644 --- a/libvirt-sandbox/image/sources/docker.py +++ b/libvirt-sandbox/image/sources/docker.py @@ -32,6 +32,7 @@ import urlparse import hashlib from abc import ABCMeta, abstractmethod import copy +from libvirt_sandbox.image.template import Template from . import base @@ -360,6 +361,27 @@ class DockerSource(base.Source): except Exception: return False + def list_templates(self, templatedir): + indexes = [] + imagedirs = os.listdir(templatedir) + for imagetagid in imagedirs: + indexfile = templatedir + "/" + imagetagid + "/index.json" + if os.path.exists(indexfile): + with open(indexfile,"r") as f: + index = json.load(f) + indexes.append(index) + + return [ + Template(source="docker", + protocol=None, + hostname=None, + port=None, + username=None, + password=None, + path="/%s/%s" % (index.get("repo", "library"), index["name"]), + params={ + "tag": index.get("tag", "latest"), + }) for index in indexes] def has_template(self, template, templatedir): try: diff --git a/libvirt-sandbox/image/sources/virtbuilder.py b/libvirt-sandbox/image/sources/virtbuilder.py index 6e71b36..fefe0dd 100644 --- a/libvirt-sandbox/image/sources/virtbuilder.py +++ b/libvirt-sandbox/image/sources/virtbuilder.py @@ -24,6 +24,7 @@ import os.path import subprocess from . import base +from libvirt_sandbox.image.template import Template class VirtBuilderSource(base.Source): @@ -65,6 +66,23 @@ class VirtBuilderSource(base.Source): os.unlink(imagepath_original) os.unlink(tarfile) + def list_templates(self, templatedir): + files = [] + imagefiles = os.listdir(templatedir) + for filename in imagefiles: + if not filename.endswith(".qcow2"): + continue + files.append(filename[0:-6]) + + return [ + Template(source="virt-builder", + protocol=None, + hostname=None, + port=None, + username=None, + password=None, + path="/%s" % filename, + params={}) for filename in files] def delete_template(self, template, templatedir): os.unlink("%s/%s.qcow2" % (templatedir, self._get_template_name(template))) diff --git a/libvirt-sandbox/image/template.py b/libvirt-sandbox/image/template.py index 751cd4b..79dc33d 100644 --- a/libvirt-sandbox/image/template.py +++ b/libvirt-sandbox/image/template.py @@ -58,22 +58,27 @@ class Template(object): if self.params is None: self.params = {} - def get_source_impl(self): - if self.source == "": - raise Exception("Missing scheme in image URI") - + @classmethod + def _get_source_impl(klass, source): try: p = re.compile("\W") - sourcemod = "".join(p.split(self.source)) - sourcename = "".join([i.capitalize() for i in p.split(self.source)]) + sourcemod = "".join(p.split(source)) + sourcename = "".join([i.capitalize() for i in p.split(source)]) mod = importlib.import_module( "libvirt_sandbox.image.sources." + sourcemod) classname = sourcename + "Source" classimpl = getattr(mod, classname) return classimpl() - except Exception: - raise Exception("Invalid source: '%s'" % self.source) + except Exception as e: + print e + raise Exception("Invalid source: '%s'" % source) + + def get_source_impl(self): + if self.source == "": + raise Exception("Missing scheme in image URI") + + return self._get_source_impl(self.source) def __repr__(self): if self.protocol is not None: @@ -96,7 +101,8 @@ class Template(object): netloc = None query = "&".join([key + "=" + self.params[key] for key in self.params.keys()]) - return urlparse.urlunparse((scheme, netloc, self.path, None, query, None)) + ret = urlparse.urlunparse((scheme, netloc, self.path, None, query, None)) + return ret @classmethod def from_uri(klass, uri): @@ -119,3 +125,9 @@ class Template(object): o.hostname, o.port, o.username, o.password, o.path, query) + + @classmethod + def get_all(klass, source, templatedir): + impl = klass._get_source_impl(source) + + return impl.list_templates(templatedir) -- 2.7.4

On Fri, 2016-07-15 at 14:08 +0100, Daniel P. Berrange wrote:
Introduce a command able to list locally stored image templates:
$ virt-sandbox-image list docker:library/ubuntu?tag=14.04.1 docker:library/ubuntu?tag=14.04.2 virt-builder:/fedora-23
or restrict to a single source type
$ virt-sandbox-image list --source docker docker:library/ubuntu?tag=14.04.1 docker:library/ubuntu?tag=14.04.2
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-sandbox/image/cli.py | 24 ++++++++++++++++++++++ libvirt-sandbox/image/sources/base.py | 10 ++++++++++ libvirt-sandbox/image/sources/docker.py | 22 ++++++++++++++++++++ libvirt-sandbox/image/sources/virtbuilder.py | 18 +++++++++++++++++ libvirt-sandbox/image/template.py | 30 +++++++++++++++++++--------- 5 files changed, 95 insertions(+), 9 deletions(-)
diff --git a/libvirt-sandbox/image/cli.py b/libvirt-sandbox/image/cli.py index b0d864f..66854e4 100644 --- a/libvirt-sandbox/image/cli.py +++ b/libvirt-sandbox/image/cli.py @@ -135,6 +135,18 @@ def run(args): os.unlink(diskfile) source.post_run(tmpl, template_dir, name)
+def list_cached(args): + tmpls = [] + if args.source is not None: + tmpls.extend(template.Template.get_all(args.source, + "%s/%s" % (args.template_dir, args.source))) + else: + for source in ["docker", "virt-builder"]: + tmpls.extend(template.Template.get_all(source, + "%s/%s" % (args.template_dir, source))) + for tmpl in tmpls: + print tmpl + def requires_template(parser): parser.add_argument("template", help=_("URI of the template")) @@ -221,6 +233,17 @@ def gen_run_args(subparser):
parser.set_defaults(func=run)
+def gen_list_args(subparser): + parser = gen_command_parser(subparser, "list", + _("List locally cached images")) + requires_debug(parser) + requires_template_dir(parser) + + parser.add_argument("-s","--source", + help=_("Name of the template source")) + + parser.set_defaults(func=list_cached) + def main(): parser = argparse.ArgumentParser(description="Sandbox Container Image Tool")
@@ -228,6 +251,7 @@ def main(): gen_delete_args(subparser) gen_create_args(subparser) gen_run_args(subparser) + gen_list_args(subparser)
args = parser.parse_args() if args.debug: diff --git a/libvirt-sandbox/image/sources/base.py b/libvirt-sandbox/image/sources/base.py index f70551d..e4e4e41 100644 --- a/libvirt-sandbox/image/sources/base.py +++ b/libvirt-sandbox/image/sources/base.py @@ -34,6 +34,16 @@ class Source(): def __init__(self): pass
+ @abstractmethod + def list_templates(self, templatedir): + """ + :param templatedir: local directory path in which to store the template + + Get a list of all templates that are locally cached + + :returns: a list of libvirt_sandbox.template.Template objects + """ + pass
@abstractmethod def has_template(self, template, templatedir): diff --git a/libvirt-sandbox/image/sources/docker.py b/libvirt-sandbox/image/sources/docker.py index 291a305..dd72db7 100644 --- a/libvirt-sandbox/image/sources/docker.py +++ b/libvirt-sandbox/image/sources/docker.py @@ -32,6 +32,7 @@ import urlparse import hashlib from abc import ABCMeta, abstractmethod import copy +from libvirt_sandbox.image.template import Template
from . import base
@@ -360,6 +361,27 @@ class DockerSource(base.Source): except Exception: return False
+ def list_templates(self, templatedir): + indexes = [] + imagedirs = os.listdir(templatedir) + for imagetagid in imagedirs: + indexfile = templatedir + "/" + imagetagid + "/index.json" + if os.path.exists(indexfile): + with open(indexfile,"r") as f: + index = json.load(f) + indexes.append(index) + + return [ + Template(source="docker", + protocol=None, + hostname=None, + port=None, + username=None, + password=None, + path="/%s/%s" % (index.get("repo", "library"), index["name"]), + params={ + "tag": index.get("tag", "latest"), + }) for index in indexes]
def has_template(self, template, templatedir): try: diff --git a/libvirt-sandbox/image/sources/virtbuilder.py b/libvirt-sandbox/image/sources/virtbuilder.py index 6e71b36..fefe0dd 100644 --- a/libvirt-sandbox/image/sources/virtbuilder.py +++ b/libvirt-sandbox/image/sources/virtbuilder.py @@ -24,6 +24,7 @@ import os.path import subprocess
from . import base +from libvirt_sandbox.image.template import Template
class VirtBuilderSource(base.Source): @@ -65,6 +66,23 @@ class VirtBuilderSource(base.Source): os.unlink(imagepath_original) os.unlink(tarfile)
+ def list_templates(self, templatedir): + files = [] + imagefiles = os.listdir(templatedir) + for filename in imagefiles: + if not filename.endswith(".qcow2"): + continue + files.append(filename[0:-6]) + + return [ + Template(source="virt-builder", + protocol=None, + hostname=None, + port=None, + username=None, + password=None, + path="/%s" % filename, + params={}) for filename in files]
def delete_template(self, template, templatedir): os.unlink("%s/%s.qcow2" % (templatedir, self._get_template_name(template))) diff --git a/libvirt-sandbox/image/template.py b/libvirt-sandbox/image/template.py index 751cd4b..79dc33d 100644 --- a/libvirt-sandbox/image/template.py +++ b/libvirt-sandbox/image/template.py @@ -58,22 +58,27 @@ class Template(object): if self.params is None: self.params = {}
- def get_source_impl(self): - if self.source == "": - raise Exception("Missing scheme in image URI") - + @classmethod + def _get_source_impl(klass, source): try: p = re.compile("\W") - sourcemod = "".join(p.split(self.source)) - sourcename = "".join([i.capitalize() for i in p.split(self.source)]) + sourcemod = "".join(p.split(source)) + sourcename = "".join([i.capitalize() for i in p.split(source)])
mod = importlib.import_module( "libvirt_sandbox.image.sources." + sourcemod) classname = sourcename + "Source" classimpl = getattr(mod, classname) return classimpl() - except Exception: - raise Exception("Invalid source: '%s'" % self.source) + except Exception as e: + print e + raise Exception("Invalid source: '%s'" % source) + + def get_source_impl(self): + if self.source == "": + raise Exception("Missing scheme in image URI") + + return self._get_source_impl(self.source)
def __repr__(self): if self.protocol is not None: @@ -96,7 +101,8 @@ class Template(object): netloc = None
query = "&".join([key + "=" + self.params[key] for key in self.params.keys()]) - return urlparse.urlunparse((scheme, netloc, self.path, None, query, None)) + ret = urlparse.urlunparse((scheme, netloc, self.path, None, query, None)) + return ret
@classmethod def from_uri(klass, uri): @@ -119,3 +125,9 @@ class Template(object): o.hostname, o.port, o.username, o.password, o.path, query) + + @classmethod + def get_all(klass, source, templatedir): + impl = klass._get_source_impl(source) + + return impl.list_templates(templatedir)
The whole series looks good to me. ACK. -- Cedric
participants (2)
-
Cedric Bosdonnat
-
Daniel P. Berrange