Merge pull request #2251 from jakedt/fixaci

Fix port mapping for ACI conversion from newer Docker manifests.

README.md

@@ -179,7 +179,7 @@ TEST=true python -m test.test_api_usage -f SuiteName
 
 ```
 # To run all tests
-TEST=true PYTHONPATH="." py.test --verbose test/
+TEST=true PYTHONPATH="." py.test --verbose
 
 # To run a specific test module
 TEST=true PYTHONPATH="." py.test --verbose test/registry_tests.py

image/appc/__init__.py

@@ -6,6 +6,7 @@ from uuid import uuid4
 
 from app import app
 from util.registry.streamlayerformat import StreamLayerMerger
+from util.dict_wrappers import JSONPathDict
 from image.common import TarImageFormatter
 
 
@@ -17,8 +18,8 @@ class AppCImageFormatter(TarImageFormatter):
   Image formatter which produces an tarball according to the AppC specification.
   """
 
-  def stream_generator(self, namespace, repository, tag, repo_image,
+  def stream_generator(self, repo_image, tag, synthetic_image_id, get_image_iterator,
-                       synthetic_image_id, get_image_iterator, get_layer_iterator):
+                       get_layer_iterator):
     image_mtime = 0
     created = next(get_image_iterator()).v1_metadata.created
     if created is not None:
@@ -29,7 +30,11 @@ class AppCImageFormatter(TarImageFormatter):
     #   rootfs - The root file system
 
     # Yield the manifest.
-    manifest = self._build_manifest(namespace, repository, tag, repo_image, synthetic_image_id)
+    manifest = json.dumps(DockerV1ToACIManifestTranslator.build_manifest(
+      tag,
+      repo_image,
+      synthetic_image_id
+    ))
     yield self.tar_file('manifest', manifest, mtime=image_mtime)
 
     # Yield the merged layer dtaa.
@@ -39,6 +44,8 @@ class AppCImageFormatter(TarImageFormatter):
     for entry in layer_merger.get_generator():
       yield entry
 
+
+class DockerV1ToACIManifestTranslator(object):
   @staticmethod
   def _build_isolators(docker_config):
     """
@@ -94,20 +101,6 @@ class AppCImageFormatter(TarImageFormatter):
 
     return isolators
 
-  @staticmethod
-  def _get_docker_config_value(docker_config, key, default_value):
-    # Try the key itself.
-    result = docker_config.get(key)
-    if result is not None:
-      return result or default_value
-
-    # The the lowercase version of the key.
-    result = docker_config.get(key.lower())
-    if result is not None:
-      return result or default_value
-
-    return default_value
-
   @staticmethod
   def _build_ports(docker_config):
     """
@@ -120,7 +113,13 @@ class AppCImageFormatter(TarImageFormatter):
     """
     ports = []
 
-    for docker_port in AppCImageFormatter._get_docker_config_value(docker_config, 'Ports', []):
+    exposed_ports = docker_config['ExposedPorts']
+    if exposed_ports is not None:
+      port_list = exposed_ports.keys()
+    else:
+      port_list = docker_config['Ports'] or docker_config['ports'] or []
+
+    for docker_port in port_list:
       protocol = 'tcp'
       port_number = -1
 
@@ -154,9 +153,11 @@ class AppCImageFormatter(TarImageFormatter):
     volumes = []
 
     def get_name(docker_volume_path):
-      return "volume-%s" % AppCImageFormatter._ac_name(docker_volume_path)
+      volume_name = DockerV1ToACIManifestTranslator._ac_name(docker_volume_path)
+      return "volume-%s" % volume_name
 
-    for docker_volume_path in AppCImageFormatter._get_docker_config_value(docker_config, 'Volumes', []):
+    volume_list = docker_config['Volumes'] or docker_config['volumes'] or []
+    for docker_volume_path in volume_list:
       if not docker_volume_path:
         continue
 
@@ -168,19 +169,21 @@ class AppCImageFormatter(TarImageFormatter):
     return volumes
 
   @staticmethod
-  def _build_manifest(namespace, repository, tag, repo_image, synthetic_image_id):
+  def build_manifest(repo_image, tag, synthetic_image_id):
     """ Builds an ACI manifest of an existing repository image. """
-    docker_layer_data = repo_image.compat_metadata
+    docker_layer_data = JSONPathDict(repo_image.compat_metadata)
-    config = docker_layer_data.get('config', {})
+    config = docker_layer_data['config'] or {}
 
+    namespace = repo_image.repository.namespace_name
+    repo_name = repo_image.repository.name
     source_url = "%s://%s/%s/%s:%s" % (app.config['PREFERRED_URL_SCHEME'],
                                        app.config['SERVER_HOSTNAME'],
-                                       namespace, repository, tag)
+                                       namespace, repo_name, tag)
 
     # ACI requires that the execution command be absolutely referenced. Therefore, if we find
     # a relative command, we give it as an argument to /bin/sh to resolve and execute for us.
-    entrypoint = config.get('Entrypoint', []) or []
+    entrypoint = config['Entrypoint'] or []
-    exec_path = entrypoint + (config.get('Cmd', []) or [])
+    exec_path = entrypoint + (config['Cmd'] or [])
     if exec_path and not exec_path[0].startswith('/'):
       exec_path = ['/bin/sh', '-c', '""%s""' % ' '.join(exec_path)]
 
@@ -201,7 +204,7 @@ class AppCImageFormatter(TarImageFormatter):
     manifest = {
       "acKind": "ImageManifest",
       "acVersion": "0.6.1",
-      "name": '%s/%s/%s' % (hostname.lower(), namespace.lower(), repository.lower()),
+      "name": '%s/%s/%s' % (hostname.lower(), namespace.lower(), repo_name.lower()),
       "labels": [
         {
           "name": "version",
@@ -224,9 +227,9 @@ class AppCImageFormatter(TarImageFormatter):
         "eventHandlers": [],
         "workingDirectory": config.get('WorkingDir', '') or '/',
         "environment": [{"name": key, "value": value} for (key, value) in env_vars],
-        "isolators": AppCImageFormatter._build_isolators(config),
+        "isolators": DockerV1ToACIManifestTranslator._build_isolators(config),
-        "mountPoints": AppCImageFormatter._build_volumes(config),
+        "mountPoints": DockerV1ToACIManifestTranslator._build_volumes(config),
-        "ports": AppCImageFormatter._build_ports(config),
+        "ports": DockerV1ToACIManifestTranslator._build_ports(config),
         "annotations": [
           {"name": "created", "value": docker_layer_data.get('created', '')},
           {"name": "homepage", "value": source_url},
@@ -235,4 +238,4 @@ class AppCImageFormatter(TarImageFormatter):
       },
     }
 
-    return json.dumps(manifest)
+    return manifest

image/appc/test/appc_test.py

@@ -0,0 +1,98 @@
+import pytest
+
+from image.appc import DockerV1ToACIManifestTranslator
+from data.interfaces.verbs import RepositoryReference, ImageWithBlob
+
+
+EXAMPLE_MANIFEST_OBJ = {
+  "architecture": "amd64",
+  "config": {
+    "Hostname": "1d811a9194c4",
+    "Domainname": "",
+    "User": "",
+    "AttachStdin": False,
+    "AttachStdout": False,
+    "AttachStderr": False,
+    "ExposedPorts": {
+      "2379/tcp": {},
+      "2380/tcp": {}
+    },
+    "Tty": False,
+    "OpenStdin": False,
+    "StdinOnce": False,
+    "Env": [
+      "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
+    ],
+    "Cmd": [
+      "/usr/local/bin/etcd"
+    ],
+    "ArgsEscaped": True,
+    "Image": "sha256:4c86d1f362d42420c137846fae31667ee85ce6f2cab406cdff26a8ff8a2c31c4",
+    "Volumes": None,
+    "WorkingDir": "",
+    "Entrypoint": None,
+    "OnBuild": [],
+    "Labels": {}
+  },
+  "container": "5a3565ce9b808a0eb0bcbc966dad624f76ad308ad24e11525b5da1201a1df135",
+  "container_config": {
+    "Hostname": "1d811a9194c4",
+    "Domainname": "",
+    "User": "",
+    "AttachStdin": False,
+    "AttachStdout": False,
+    "AttachStderr": False,
+    "ExposedPorts": {
+      "2379/tcp": {},
+      "2380/tcp": {}
+    },
+    "Tty": False,
+    "OpenStdin": False,
+    "StdinOnce": False,
+    "Env": [
+      "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
+    ],
+    "Cmd": [
+      "/bin/sh",
+      "-c",
+      "#(nop) CMD [\"/usr/local/bin/etcd\"]"
+    ],
+    "ArgsEscaped": True,
+    "Image": "sha256:4c86d1f362d42420c137846fae31667ee85ce6f2cab406cdff26a8ff8a2c31c4",
+    "Volumes": None,
+    "WorkingDir": "",
+    "Entrypoint": None,
+    "OnBuild": [],
+    "Labels": {}
+  },
+  "created": "2016-11-11T19:03:55.137387628Z",
+  "docker_version": "1.11.1",
+  "id": "3314a3781a526fe728e2e96cfcfb3cc0de901b5c102e6204e8b0155c8f7d5fd2",
+  "os": "linux",
+  "parent": "625342ec4d0f3d7a96fd3bb1ef0b4b0b6bc65ebb3d252fd33af0691f7984440e",
+  "throwaway": True
+}
+
+
+@pytest.fixture
+def repo_image():
+  repo_ref = RepositoryReference(1, 'simple', 'devtable')
+  return ImageWithBlob(1, None, EXAMPLE_MANIFEST_OBJ, repo_ref, 1, None)
+
+
+def test_port_conversion(repo_image):
+  output = DockerV1ToACIManifestTranslator.build_manifest(repo_image, 'v3.0.15', 'abcdef')
+  ports = output['app']['ports']
+  ports.sort()
+  assert {'name':'port-2379', 'port':2379, 'protocol':'tcp'} == ports[0]
+  assert {'name':'port-2380', 'port':2380, 'protocol':'tcp'} == ports[1]
+
+
+def test_legacy_port_conversion(repo_image):
+  del repo_image.compat_metadata['config']['ExposedPorts']
+  repo_image.compat_metadata['config']['ports'] = ['8080', '8081']
+  output = DockerV1ToACIManifestTranslator.build_manifest(repo_image, 'v3.0.15', 'abcdef')
+  ports = output['app']['ports']
+  ports.sort()
+  assert {'name':'port-8080', 'port':8080, 'protocol':'tcp'} == ports[0]
+  assert {'name':'port-8081', 'port':8081, 'protocol':'tcp'} == ports[1]

image/common.py

@@ -7,18 +7,17 @@ class TarImageFormatter(object):
   Base class for classes which produce a tar containing image and layer data.
   """
 
-  def build_stream(self, namespace, repository, tag, repo_image, synthetic_image_id,
+  def build_stream(self, repo_image, tag, synthetic_image_id, get_image_iterator,
-                   get_image_iterator, get_layer_iterator):
+                   get_layer_iterator):
     """
     Builds and streams a synthetic .tar.gz that represents the formatted tar created by this class's
     implementation.
     """
-    return GzipWrap(self.stream_generator(namespace, repository, tag, repo_image,
+    return GzipWrap(self.stream_generator(repo_image, tag, synthetic_image_id, get_image_iterator,
-                                          synthetic_image_id, get_image_iterator,
                                           get_layer_iterator))
 
-  def stream_generator(self, namespace, repository, tag, repo_image, synthetic_image_id,
+  def stream_generator(self, repo_image, tag, synthetic_image_id, get_image_iterator,
-                       get_image_iterator, get_layer_iterator):
+                       get_layer_iterator):
     raise NotImplementedError
 
   def tar_file(self, name, contents, mtime=None):

image/docker/squashed.py

@@ -28,8 +28,8 @@ class SquashedDockerImageFormatter(TarImageFormatter):
   # daemon dies when trying to load the entire tar into memory.
   SIZE_MULTIPLIER = 1.2
 
-  def stream_generator(self, namespace, repository, tag, repo_image, synthetic_image_id,
+  def stream_generator(self, repo_image, tag, synthetic_image_id, get_image_iterator,
-                       get_image_iterator, get_layer_iterator):
+                       get_layer_iterator):
     image_mtime = 0
     created = next(get_image_iterator()).v1_metadata.created
     if created is not None:
@@ -50,6 +50,8 @@ class SquashedDockerImageFormatter(TarImageFormatter):
 
     hostname = app.config['SERVER_HOSTNAME']
     repositories = {}
+    namespace = repo_image.repository.namespace_name
+    repository = repo_image.repository.name
     repositories[hostname + '/' + namespace + '/' + repository] = synthetic_layer_info
 
     yield self.tar_file('repositories', json.dumps(repositories), mtime=image_mtime)
@@ -74,7 +76,8 @@ class SquashedDockerImageFormatter(TarImageFormatter):
         estimated_file_size += image.blob.uncompressed_size
       else:
         image_json = image.compat_metadata
-        estimated_file_size += image_json.get('Size', 0) * SquashedDockerImageFormatter.SIZE_MULTIPLIER
+        estimated_file_size += (image_json.get('Size', 0) *
+                                SquashedDockerImageFormatter.SIZE_MULTIPLIER)
 
     # Make sure the estimated file size is an integer number of bytes.
     estimated_file_size = int(math.ceil(estimated_file_size))

tox.ini

@@ -14,4 +14,4 @@ setenv =
     registry: FILE="registry_tests.py"
     unittest: FILE=""
 commands =
-         py.test --verbose test/{env:FILE} -vv {posargs}
+         py.test --verbose {env:FILE} -vv {posargs}

util/dict_wrappers.py

@@ -74,3 +74,7 @@ class JSONPathDict(object):
       return JSONPathDict(match)
 
     return match
+
+  def keys(self):
+    return self._object.keys()
+