[ARVADOS] created: d02bf4d817e50c3c0ee9f5e2dd901c512ea30943

Git user git at public.curoverse.com
Thu Feb 2 17:33:40 EST 2017


        at  d02bf4d817e50c3c0ee9f5e2dd901c512ea30943 (commit)


commit d02bf4d817e50c3c0ee9f5e2dd901c512ea30943
Author: Tom Clegg <tom at curoverse.com>
Date:   Thu Feb 2 17:32:31 2017 -0500

    10969: Add docker_image_formats server config, and corresponding check in `arv keep docker`.

diff --git a/sdk/python/arvados/commands/keepdocker.py b/sdk/python/arvados/commands/keepdocker.py
index 3a0b64c..5f7749b 100644
--- a/sdk/python/arvados/commands/keepdocker.py
+++ b/sdk/python/arvados/commands/keepdocker.py
@@ -36,6 +36,9 @@ keepdocker_parser.add_argument(
 keepdocker_parser.add_argument(
     '-f', '--force', action='store_true', default=False,
     help="Re-upload the image even if it already exists on the server")
+keepdocker_parser.add_argument(
+    '--force-image-format', action='store_true', default=False,
+    help="Proceed even if the image format is not supported by the server")
 
 _group = keepdocker_parser.add_mutually_exclusive_group()
 _group.add_argument(
@@ -81,6 +84,35 @@ def check_docker(proc, description):
         raise DockerError("docker {} returned status code {}".
                           format(description, proc.returncode))
 
+def docker_image_format(image_hash):
+    """Return the registry format ('v1' or 'v2') of the given image."""
+    cmd = popen_docker(['inspect', '--format={{.Id}}', image_hash],
+                        stdout=subprocess.PIPE)
+    try:
+        image_id = next(cmd.stdout).strip()
+        if image_id.startswith('sha256:'):
+            return 'v2'
+        elif ':' not in image_id:
+            return 'v1'
+        else:
+            return 'unknown'
+    finally:
+        check_docker(cmd, "inspect")
+
+def docker_image_compatible(api, image_hash):
+    supported = api._rootDesc.get('dockerImageFormats', [])
+    if not supported:
+        print >>sys.stderr, "arv-keepdocker: warning: server does not specify supported image formats (see docker_image_formats in server config). Continuing."
+        return True
+
+    fmt = docker_image_format(image_hash)
+    if fmt in supported:
+        return True
+    else:
+        print >>sys.stderr, "arv-keepdocker: image format is {!r} " \
+            "but server supports only {!r}".format(fmt, supported)
+        return False
+
 def docker_images():
     # Yield a DockerImage tuple for each installed image.
     list_proc = popen_docker(['images', '--no-trunc'], stdout=subprocess.PIPE)
@@ -313,6 +345,14 @@ def main(arguments=None, stdout=sys.stdout):
         print >>sys.stderr, "arv-keepdocker:", error.message
         sys.exit(1)
 
+    if not docker_image_compatible(api, image_hash):
+        if args.force_image_format:
+            print >>sys.stderr, "arv-keepdocker: forcing incompatible image"
+        else:
+            print >>sys.stderr, "arv-keepdocker: refusing to store " \
+                "incompatible format (use --force-image-format to override)"
+            sys.exit(1)
+
     image_repo_tag = '{}:{}'.format(args.image, args.tag) if not image_hash.startswith(args.image.lower()) else None
 
     if args.name is None:
diff --git a/sdk/python/tests/test_arv_keepdocker.py b/sdk/python/tests/test_arv_keepdocker.py
index bb94db5..151edf3 100644
--- a/sdk/python/tests/test_arv_keepdocker.py
+++ b/sdk/python/tests/test_arv_keepdocker.py
@@ -1,8 +1,12 @@
 #!/usr/bin/env python
 # -*- coding: utf-8 -*-
 
+import arvados
+import hashlib
 import io
+import mock
 import os
+import subprocess
 import sys
 import tempfile
 import unittest
@@ -11,6 +15,10 @@ import arvados.commands.keepdocker as arv_keepdocker
 import arvados_testutil as tutil
 
 
+class StopTest(Exception):
+    pass
+
+
 class ArvKeepdockerTestCase(unittest.TestCase):
     def run_arv_keepdocker(self, args):
         sys.argv = ['arv-keepdocker'] + args
@@ -28,3 +36,75 @@ class ArvKeepdockerTestCase(unittest.TestCase):
                 self.run_arv_keepdocker(['--version'])
         self.assertEqual(out.getvalue(), '')
         self.assertRegexpMatches(err.getvalue(), "[0-9]+\.[0-9]+\.[0-9]+")
+
+    @mock.patch('arvados.commands.keepdocker.find_image_hashes',
+                return_value=['abc123'])
+    @mock.patch('arvados.commands.keepdocker.find_one_image_hash',
+                return_value='abc123')
+    def test_image_format_compatibility(self, _1, _2):
+        old_id = hashlib.sha256('old').hexdigest()
+        new_id = 'sha256:'+hashlib.sha256('new').hexdigest()
+        for supported, img_id, expect_ok in [
+                (['v1'], old_id, True),
+                (['v1'], new_id, False),
+                (None, old_id, True),
+                ([], old_id, True),
+                ([], new_id, True),
+                (['v1', 'v2'], new_id, True),
+                (['v1'], new_id, False),
+                (['v2'], new_id, True)]:
+
+            fakeDD = arvados.api('v1')._rootDesc
+            if supported is None:
+                del fakeDD['dockerImageFormats']
+            else:
+                fakeDD['dockerImageFormats'] = supported
+
+            err = io.BytesIO()
+            out = io.BytesIO()
+
+            with tutil.redirected_streams(stdout=out, stderr=err), \
+                 mock.patch('arvados.api') as api, \
+                 mock.patch('arvados.commands.keepdocker.popen_docker',
+                            return_value=subprocess.Popen(
+                                ['echo', img_id],
+                                stdout=subprocess.PIPE)), \
+                 mock.patch('arvados.commands.keepdocker.prep_image_file',
+                            side_effect=StopTest), \
+                 self.assertRaises(StopTest if expect_ok else SystemExit):
+
+                api()._rootDesc = fakeDD
+                self.run_arv_keepdocker(['--force', 'testimage'])
+
+            self.assertEqual(out.getvalue(), '')
+            if expect_ok:
+                self.assertNotRegexpMatches(
+                    err.getvalue(), "refusing to store",
+                    msg=repr((supported, img_id)))
+            else:
+                self.assertRegexpMatches(
+                    err.getvalue(), "refusing to store",
+                    msg=repr((supported, img_id)))
+            if not supported:
+                self.assertRegexpMatches(
+                    err.getvalue(),
+                    "server does not specify supported image formats",
+                    msg=repr((supported, img_id)))
+
+        fakeDD = arvados.api('v1')._rootDesc
+        fakeDD['dockerImageFormats'] = ['v1']
+        err = io.BytesIO()
+        out = io.BytesIO()
+        with tutil.redirected_streams(stdout=out, stderr=err), \
+             mock.patch('arvados.api') as api, \
+             mock.patch('arvados.commands.keepdocker.popen_docker',
+                        return_value=subprocess.Popen(
+                            ['echo', new_id],
+                            stdout=subprocess.PIPE)), \
+             mock.patch('arvados.commands.keepdocker.prep_image_file',
+                        side_effect=StopTest), \
+             self.assertRaises(StopTest):
+            api()._rootDesc = fakeDD
+            self.run_arv_keepdocker(
+                ['--force', '--force-image-format', 'testimage'])
+        self.assertRegexpMatches(err.getvalue(), "forcing incompatible image")
diff --git a/services/api/app/controllers/arvados/v1/schema_controller.rb b/services/api/app/controllers/arvados/v1/schema_controller.rb
index e2537a5..443c650 100644
--- a/services/api/app/controllers/arvados/v1/schema_controller.rb
+++ b/services/api/app/controllers/arvados/v1/schema_controller.rb
@@ -37,6 +37,7 @@ class Arvados::V1::SchemaController < ApplicationController
         defaultTrashLifetime: Rails.application.config.default_trash_lifetime,
         blobSignatureTtl: Rails.application.config.blob_signature_ttl,
         maxRequestSize: Rails.application.config.max_request_size,
+        dockerImageFormats: Rails.application.config.docker_image_formats,
         parameters: {
           alt: {
             type: "string",
diff --git a/services/api/config/application.default.yml b/services/api/config/application.default.yml
index bb1355d..077c237 100644
--- a/services/api/config/application.default.yml
+++ b/services/api/config/application.default.yml
@@ -282,6 +282,17 @@ common:
   # Docker image to be used when none found in runtime_constraints of a job
   default_docker_image_for_jobs: false
 
+  # List of supported Docker Registry image formats that compute nodes
+  # are able to use. `arv keep docker` will error out if a user tries
+  # to store an image with an unsupported format. Use an empty array
+  # to skip the compatibility check (and display a warning message to
+  # that effect).
+  #
+  # Example for sites running docker < 1.10: ["v1"]
+  # Example for sites running docker >= 1.10: ["v2"]
+  # Example for disabling check: []
+  docker_image_formats: ["v2"]
+
   # :none or :slurm_immediate
   crunch_job_wrapper: :none
 

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list