You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by qi...@apache.org on 2019/04/15 10:34:59 UTC

[mesos] branch 1.7.x updated (150644e -> 62ac762)

This is an automated email from the ASF dual-hosted git repository.

qianzhang pushed a change to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git.


    from 150644e  Added MESOS-8467 to the 1.7.3 CHANGELOG.
     new 9abc873  Windows: Update curl version to 7.61.0.
     new ee22ae2  Windows: Parse version 2 schema 2 Docker image manifest.
     new 53979cc  Windows: Fetch blobs with V2S2 Docker image manifest.
     new 5699a39  Windows: Enable DockerFetcher in Windows agent.
     new b4b0337  Windows: Enabled `DockerFetcherPluginTest` suite.
     new 8893615  Windows: Add comment about V2S2 Docker image Manifest.
     new fbb1cef  Added 'prettyjws' option to docker manifest V2 Schema1 accept header.
     new e1ce3d0  Refactored the UCR docker store to construct 'Image' proto at pullers.
     new 14857c3  Added protobuf for docker v2 schema2 config_digest in 'Image'.
     new adc699d  Supported docker manifest v2 schema2.
     new b9e25b5  Added a TODO for additional URLs support.
     new e750b7a  Fixed docker fetcher plugin unit test for v2s2 change.
     new 2272228  Added gcr registry test.
     new 2a4b2d5  Added a unit test for Mesos containerizer image force pulling.
     new b3c1adf  Fixed use-after-free bug in Docker provisioner store.
     new f8b64a4  Fixed the URI fetcher image fetch test failure on windows.
     new 62ac762  Added MESOS-9159 and MESOS-9675 to the 1.7.3 CHANGELOG.

The 17 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 3rdparty/CMakeLists.txt                            |   4 +-
 3rdparty/cmake/Versions.cmake                      |   4 +-
 CHANGELOG                                          |   2 +
 include/mesos/docker/spec.hpp                      |  18 ++
 .../mesos/{fetcher/fetcher.hpp => docker/v2_2.hpp} |   8 +-
 .../mesos/docker/v2_2.proto                        |  35 +-
 src/CMakeLists.txt                                 |  16 +-
 src/Makefile.am                                    |  11 +-
 src/docker/spec.cpp                                |  66 ++++
 src/slave/containerizer/mesos/containerizer.cpp    |   2 +
 .../mesos/provisioner/docker/image_tar_puller.cpp  |  22 +-
 .../mesos/provisioner/docker/image_tar_puller.hpp  |   2 +-
 .../mesos/provisioner/docker/message.proto         |   8 +-
 .../mesos/provisioner/docker/metadata_manager.cpp  |  28 +-
 .../mesos/provisioner/docker/metadata_manager.hpp  |  12 +-
 .../mesos/provisioner/docker/puller.hpp            |   4 +-
 .../mesos/provisioner/docker/registry_puller.cpp   | 294 ++++++++++++++---
 .../mesos/provisioner/docker/registry_puller.hpp   |   2 +-
 .../mesos/provisioner/docker/store.cpp             |  66 +++-
 src/tests/CMakeLists.txt                           |   2 +-
 src/tests/containerizer/docker_spec_tests.cpp      | 121 +++++++
 .../containerizer/provisioner_docker_tests.cpp     | 154 ++++++++-
 src/tests/environment.cpp                          |   5 +-
 src/tests/health_check_tests.cpp                   |  10 +-
 src/tests/uri_fetcher_tests.cpp                    |  85 +++--
 src/uri/fetcher.cpp                                |   3 -
 src/uri/fetcher.hpp                                |   5 -
 src/uri/fetchers/docker.cpp                        | 354 +++++++++++++++++----
 src/uri/fetchers/docker.hpp                        |   4 +
 29 files changed, 1084 insertions(+), 263 deletions(-)
 copy include/mesos/{fetcher/fetcher.hpp => docker/v2_2.hpp} (85%)
 copy src/slave/containerizer/mesos/provisioner/docker/message.proto => include/mesos/docker/v2_2.proto (57%)


[mesos] 11/17: Added a TODO for additional URLs support.

Posted by qi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

qianzhang pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit b9e25b5410e171890d7076037b5c9204389592a2
Author: Gilbert Song <so...@gmail.com>
AuthorDate: Sat Mar 23 21:14:15 2019 -0700

    Added a TODO for additional URLs support.
    
    Review: https://reviews.apache.org/r/70289
---
 src/uri/fetchers/docker.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/uri/fetchers/docker.cpp b/src/uri/fetchers/docker.cpp
index e11e54f..631db2d 100644
--- a/src/uri/fetchers/docker.cpp
+++ b/src/uri/fetchers/docker.cpp
@@ -968,6 +968,8 @@ Future<Nothing> DockerFetcherPluginProcess::urlFetchBlob(
     return Failure("Schema 2 manifest does not exist");
   }
 
+  // TODO(gilbert): Support v2s2 additional urls for non-windows platforms.
+  // We should avoid parsing the manifest for each layer.
   Try<spec::v2_2::ImageManifest> manifest = spec::v2_2::parse(_manifest.get());
   if (manifest.isError()) {
     return Failure(


[mesos] 07/17: Added 'prettyjws' option to docker manifest V2 Schema1 accept header.

Posted by qi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

qianzhang pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit fbb1cef9e7da85498098dc630b0c73ea1ab2b11c
Author: Gilbert Song <so...@gmail.com>
AuthorDate: Sat Mar 23 20:54:08 2019 -0700

    Added 'prettyjws' option to docker manifest V2 Schema1 accept header.
    
    Review: https://reviews.apache.org/r/70287
---
 src/uri/fetchers/docker.cpp | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/uri/fetchers/docker.cpp b/src/uri/fetchers/docker.cpp
index a87d7f0..ffb5194 100644
--- a/src/uri/fetchers/docker.cpp
+++ b/src/uri/fetchers/docker.cpp
@@ -687,7 +687,10 @@ Future<Nothing> DockerFetcherPluginProcess::fetch(
   // Note: The 'Accept' header is required for Amazon ECR. See:
   // https://forums.aws.amazon.com/message.jspa?messageID=780440
   http::Headers manifestHeaders = {
-    {"Accept", "application/vnd.docker.distribution.manifest.v1+json"}
+    {"Accept",
+     "application/vnd.docker.distribution.manifest.v1+json,"
+     "application/vnd.docker.distribution.manifest.v1+prettyjws"
+    }
   };
 
   return curl(manifestUri, manifestHeaders + basicAuthHeaders, stallTimeout)


[mesos] 08/17: Refactored the UCR docker store to construct 'Image' proto at pullers.

Posted by qi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

qianzhang pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit e1ce3d053aad9ddc6e370a44abb82a32a013ee90
Author: Gilbert Song <so...@gmail.com>
AuthorDate: Mon Apr 1 16:11:43 2019 -0700

    Refactored the UCR docker store to construct 'Image' proto at pullers.
    
    This refactoring is needed for supporting docker manifest v2s2 because
    the puller has to let the docker store knows the v1 config, and this
    is also needed for garbage collecting the v1 config in the image layer
    store.
    
    Review: https://reviews.apache.org/r/70354
---
 .../mesos/provisioner/docker/image_tar_puller.cpp  | 22 +++++++++++------
 .../mesos/provisioner/docker/image_tar_puller.hpp  |  2 +-
 .../mesos/provisioner/docker/message.proto         |  4 +++-
 .../mesos/provisioner/docker/metadata_manager.cpp  | 28 ++++++----------------
 .../mesos/provisioner/docker/metadata_manager.hpp  | 12 ++--------
 .../mesos/provisioner/docker/puller.hpp            |  4 +++-
 .../mesos/provisioner/docker/registry_puller.cpp   | 28 +++++++++++++---------
 .../mesos/provisioner/docker/registry_puller.hpp   |  2 +-
 .../mesos/provisioner/docker/store.cpp             | 16 ++++++-------
 .../containerizer/provisioner_docker_tests.cpp     | 20 +++++++++++-----
 10 files changed, 71 insertions(+), 67 deletions(-)

diff --git a/src/slave/containerizer/mesos/provisioner/docker/image_tar_puller.cpp b/src/slave/containerizer/mesos/provisioner/docker/image_tar_puller.cpp
index 3a33388..3e6741d 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/image_tar_puller.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/image_tar_puller.cpp
@@ -66,13 +66,13 @@ public:
 
   ~ImageTarPullerProcess() override {}
 
-  Future<vector<string>> pull(
+  Future<Image> pull(
       const spec::ImageReference& reference,
       const string& directory,
       const string& backend);
 
 private:
-  Future<vector<string>> _pull(
+  Future<Image> _pull(
       const spec::ImageReference& reference,
       const string& directory,
       const string& backend);
@@ -152,7 +152,7 @@ ImageTarPuller::~ImageTarPuller()
 }
 
 
-Future<vector<string>> ImageTarPuller::pull(
+Future<Image> ImageTarPuller::pull(
     const spec::ImageReference& reference,
     const string& directory,
     const string& backend,
@@ -167,7 +167,7 @@ Future<vector<string>> ImageTarPuller::pull(
 }
 
 
-Future<vector<string>> ImageTarPullerProcess::pull(
+Future<Image> ImageTarPullerProcess::pull(
     const spec::ImageReference& reference,
     const string& directory,
     const string& backend)
@@ -186,7 +186,7 @@ Future<vector<string>> ImageTarPullerProcess::pull(
             << "' to '" << directory << "' using HDFS uri fetcher";
 
     return fetcher->fetch(uri, directory)
-      .then(defer(self(), [=]() -> Future<vector<string>> {
+      .then(defer(self(), [=]() -> Future<Image> {
         const string source = paths::getImageArchiveTarPath(directory, image);
 
         VLOG(1) << "Untarring image '" << reference
@@ -216,7 +216,7 @@ Future<vector<string>> ImageTarPullerProcess::pull(
 }
 
 
-Future<vector<string>> ImageTarPullerProcess::_pull(
+Future<Image> ImageTarPullerProcess::_pull(
     const spec::ImageReference& reference,
     const string& directory,
     const string& backend)
@@ -294,7 +294,15 @@ Future<vector<string>> ImageTarPullerProcess::_pull(
   }
 
   return extractLayers(directory, layerIds, backend)
-    .then([layerIds]() -> vector<string> { return layerIds; });
+    .then([reference, layerIds]() -> Image {
+      Image image;
+      image.mutable_reference()->CopyFrom(reference);
+      foreach (const string& layerId, layerIds) {
+        image.add_layer_ids(layerId);
+      }
+
+      return image;
+    });
 }
 
 
diff --git a/src/slave/containerizer/mesos/provisioner/docker/image_tar_puller.hpp b/src/slave/containerizer/mesos/provisioner/docker/image_tar_puller.hpp
index b5ec924..6c0fa83 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/image_tar_puller.hpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/image_tar_puller.hpp
@@ -50,7 +50,7 @@ public:
 
   ~ImageTarPuller() override;
 
-  process::Future<std::vector<std::string>> pull(
+  process::Future<Image> pull(
       const ::docker::spec::ImageReference& reference,
       const std::string& directory,
       const std::string& backend,
diff --git a/src/slave/containerizer/mesos/provisioner/docker/message.proto b/src/slave/containerizer/mesos/provisioner/docker/message.proto
index a55ac90..612c591 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/message.proto
+++ b/src/slave/containerizer/mesos/provisioner/docker/message.proto
@@ -28,7 +28,9 @@ package mesos.internal.slave.docker;
 message Image {
   required .docker.spec.ImageReference reference = 1;
 
-  // The order of the layers represents the dependency between layers.
+  // The order of the layers represents the dependency between layers,
+  // where the root layer's id (no parent layer) is first and the leaf
+  // layer's id is last.
   repeated string layer_ids = 2;
 }
 
diff --git a/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp b/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp
index 7b8030a..2003d7d 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp
@@ -59,9 +59,7 @@ public:
 
   Future<Nothing> recover();
 
-  Future<Image> put(
-      const spec::ImageReference& reference,
-      const vector<string>& layerIds);
+  Future<Image> put(const Image& image);
 
   Future<Option<Image>> get(
       const spec::ImageReference& reference,
@@ -111,15 +109,12 @@ Future<Nothing> MetadataManager::recover()
 }
 
 
-Future<Image> MetadataManager::put(
-    const spec::ImageReference& reference,
-    const vector<string>& layerIds)
+Future<Image> MetadataManager::put(const Image& image)
 {
   return dispatch(
       process.get(),
       &MetadataManagerProcess::put,
-      reference,
-      layerIds);
+      image);
 }
 
 
@@ -145,19 +140,10 @@ Future<hashset<string>> MetadataManager::prune(
 }
 
 
-Future<Image> MetadataManagerProcess::put(
-    const spec::ImageReference& reference,
-    const vector<string>& layerIds)
+Future<Image> MetadataManagerProcess::put(const Image& image)
 {
-  const string imageReference = stringify(reference);
-
-  Image dockerImage;
-  dockerImage.mutable_reference()->CopyFrom(reference);
-  foreach (const string& layerId, layerIds) {
-    dockerImage.add_layer_ids(layerId);
-  }
-
-  storedImages[imageReference] = dockerImage;
+  const string imageReference = stringify(image.reference());
+  storedImages[imageReference] = image;
 
   Try<Nothing> status = persist();
   if (status.isError()) {
@@ -166,7 +152,7 @@ Future<Image> MetadataManagerProcess::put(
 
   VLOG(1) << "Successfully cached image '" << imageReference << "'";
 
-  return dockerImage;
+  return image;
 }
 
 
diff --git a/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp b/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp
index cfafd44..445363f 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp
@@ -68,18 +68,10 @@ public:
   process::Future<Nothing> recover();
 
   /**
-   * Create an Image, put it in metadata manager and persist the reference
+   * Put the image in metadata manager and persist the reference
    * store state to disk.
-   *
-   * @param reference the reference of the Docker image to place in the
-   *                  reference store.
-   * @param layerIds the list of layer ids that comprise the Docker image in
-   *                 order where the root layer's id (no parent layer) is first
-   *                 and the leaf layer's id is last.
    */
-  process::Future<Image> put(
-      const ::docker::spec::ImageReference& reference,
-      const std::vector<std::string>& layerIds);
+  process::Future<Image> put(const Image& image);
 
   /**
    * Retrieve Image based on image reference if it is among the Images
diff --git a/src/slave/containerizer/mesos/provisioner/docker/puller.hpp b/src/slave/containerizer/mesos/provisioner/docker/puller.hpp
index a79f2ad..c793fd1 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/puller.hpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/puller.hpp
@@ -32,6 +32,8 @@
 
 #include <mesos/secret/resolver.hpp>
 
+#include "slave/containerizer/mesos/provisioner/docker/message.hpp"
+
 #include "slave/flags.hpp"
 
 namespace mesos {
@@ -58,7 +60,7 @@ public:
    * @param directory The target directory to store the layers.
    * @return an ordered list of layer ids.
    */
-  virtual process::Future<std::vector<std::string>> pull(
+  virtual process::Future<Image> pull(
       const ::docker::spec::ImageReference& reference,
       const std::string& directory,
       const std::string& backend,
diff --git a/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp b/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
index a5683e3..7778976 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
@@ -66,26 +66,26 @@ public:
       const Shared<uri::Fetcher>& _fetcher,
       SecretResolver* _secretResolver);
 
-  Future<vector<string>> pull(
+  Future<Image> pull(
       const spec::ImageReference& reference,
       const string& directory,
       const string& backend,
       const Option<Secret>& config);
 
 private:
-  Future<vector<string>> _pull(
+  Future<Image> _pull(
       const spec::ImageReference& reference,
       const string& directory,
       const string& backend,
       const Option<Secret::Value>& config = None());
 
-  Future<vector<string>> __pull(
+  Future<Image> __pull(
       const spec::ImageReference& reference,
       const string& directory,
       const string& backend,
       const Option<Secret::Value>& config);
 
-  Future<vector<string>> ___pull(
+  Future<Image> ___pull(
     const spec::ImageReference& reference,
     const string& directory,
     const spec::v2::ImageManifest& manifest,
@@ -153,7 +153,7 @@ RegistryPuller::~RegistryPuller()
 }
 
 
-Future<vector<string>> RegistryPuller::pull(
+Future<Image> RegistryPuller::pull(
     const spec::ImageReference& reference,
     const string& directory,
     const string& backend,
@@ -216,7 +216,7 @@ static spec::ImageReference normalize(
 }
 
 
-Future<vector<string>> RegistryPullerProcess::pull(
+Future<Image> RegistryPullerProcess::pull(
     const spec::ImageReference& reference,
     const string& directory,
     const string& backend,
@@ -236,7 +236,7 @@ Future<vector<string>> RegistryPullerProcess::pull(
 }
 
 
-Future<vector<string>> RegistryPullerProcess::_pull(
+Future<Image> RegistryPullerProcess::_pull(
     const spec::ImageReference& _reference,
     const string& directory,
     const string& backend,
@@ -295,7 +295,7 @@ Future<vector<string>> RegistryPullerProcess::_pull(
 }
 
 
-Future<vector<string>> RegistryPullerProcess::__pull(
+Future<Image> RegistryPullerProcess::__pull(
     const spec::ImageReference& reference,
     const string& directory,
     const string& backend,
@@ -331,7 +331,7 @@ Future<vector<string>> RegistryPullerProcess::__pull(
 }
 
 
-Future<vector<string>> RegistryPullerProcess::___pull(
+Future<Image> RegistryPullerProcess::___pull(
     const spec::ImageReference& reference,
     const string& directory,
     const spec::v2::ImageManifest& manifest,
@@ -413,7 +413,7 @@ Future<vector<string>> RegistryPullerProcess::___pull(
   }
 
   return collect(futures)
-    .then([=]() -> Future<vector<string>> {
+    .then([=]() -> Future<Image> {
       // Remove the tarballs after the extraction.
       foreach (const string& blobSum, blobSums) {
         const string tar = path::join(directory, blobSum);
@@ -426,7 +426,13 @@ Future<vector<string>> RegistryPullerProcess::___pull(
         }
       }
 
-      return layerIds;
+      Image image;
+      image.mutable_reference()->CopyFrom(reference);
+      foreach (const string& layerId, layerIds) {
+        image.add_layer_ids(layerId);
+      }
+
+      return image;
     });
 }
 
diff --git a/src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp b/src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp
index 926fab1..c2ceda2 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp
@@ -58,7 +58,7 @@ public:
    * @param reference local name of the image.
    * @param directory path to which the layers will be downloaded.
    */
-  process::Future<std::vector<std::string>> pull(
+  process::Future<Image> pull(
       const ::docker::spec::ImageReference& reference,
       const std::string& directory,
       const std::string& backend,
diff --git a/src/slave/containerizer/mesos/provisioner/docker/store.cpp b/src/slave/containerizer/mesos/provisioner/docker/store.cpp
index 60507aa..e0f2371 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/store.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/store.cpp
@@ -124,9 +124,9 @@ private:
       const Image& image,
       const string& backend);
 
-  Future<vector<string>> moveLayers(
+  Future<Image> moveLayers(
       const string& staging,
-      const vector<string>& layerIds,
+      const Image& image,
       const string& backend);
 
   Future<Nothing> moveLayer(
@@ -356,8 +356,8 @@ Future<Image> StoreProcess::_get(
                   staging.get(),
                   lambda::_1,
                   backend))
-      .then(defer(self(), [=](const vector<string>& layerIds) {
-        return metadataManager->put(reference, layerIds);
+      .then(defer(self(), [=](const Image& image) {
+        return metadataManager->put(image);
       }))
       .onAny(defer(self(), [=](const Future<Image>&) {
         pulling.erase(name);
@@ -419,18 +419,18 @@ Future<ImageInfo> StoreProcess::__get(
 }
 
 
-Future<vector<string>> StoreProcess::moveLayers(
+Future<Image> StoreProcess::moveLayers(
     const string& staging,
-    const vector<string>& layerIds,
+    const Image& image,
     const string& backend)
 {
   vector<Future<Nothing>> futures;
-  foreach (const string& layerId, layerIds) {
+  foreach (const string& layerId, image.layer_ids()) {
     futures.push_back(moveLayer(staging, layerId, backend));
   }
 
   return collect(futures)
-    .then([layerIds]() -> vector<string> { return layerIds; });
+    .then([image]() -> Image { return image; });
 }
 
 
diff --git a/src/tests/containerizer/provisioner_docker_tests.cpp b/src/tests/containerizer/provisioner_docker_tests.cpp
index bf56d60..d3954c4 100644
--- a/src/tests/containerizer/provisioner_docker_tests.cpp
+++ b/src/tests/containerizer/provisioner_docker_tests.cpp
@@ -36,6 +36,7 @@
 #include "slave/containerizer/mesos/provisioner/constants.hpp"
 #include "slave/containerizer/mesos/provisioner/paths.hpp"
 
+#include "slave/containerizer/mesos/provisioner/docker/message.hpp"
 #include "slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp"
 #include "slave/containerizer/mesos/provisioner/docker/paths.hpp"
 #include "slave/containerizer/mesos/provisioner/docker/puller.hpp"
@@ -308,20 +309,20 @@ public:
 
   MOCK_METHOD4(
       pull,
-      Future<vector<string>>(
+      Future<slave::docker::Image>(
           const spec::ImageReference&,
           const string&,
           const string&,
           const Option<Secret>&));
 
-  Future<vector<string>> unmocked_pull(
+  Future<slave::docker::Image> unmocked_pull(
       const spec::ImageReference& reference,
       const string& directory,
       const string& backend,
       const Option<Secret>& config)
   {
-    // TODO(gilbert): Allow return list to be overridden.
-    return vector<string>();
+    // TODO(gilbert): Allow return Image to be overridden.
+    return slave::docker::Image();
   }
 };
 
@@ -338,7 +339,7 @@ TEST_F(ProvisionerDockerLocalStoreTest, PullingSameImageSimutanuously)
   MockPuller* puller = new MockPuller();
   Future<Nothing> pull;
   Future<string> directory;
-  Promise<vector<string>> promise;
+  Promise<slave::docker::Image> promise;
 
   EXPECT_CALL(*puller, pull(_, _, _, _))
     .WillOnce(testing::DoAll(FutureSatisfy(&pull),
@@ -379,7 +380,14 @@ TEST_F(ProvisionerDockerLocalStoreTest, PullingSameImageSimutanuously)
   Future<slave::ImageInfo> imageInfo2 =
     store.get()->get(mesosImage, COPY_BACKEND);
 
-  const vector<string> result = {"456"};
+  Try<spec::ImageReference> reference =
+    spec::parseImageReference(mesosImage.docker().name());
+
+  ASSERT_SOME(reference);
+
+  slave::docker::Image result;
+  result.mutable_reference()->CopyFrom(reference.get());
+  result.add_layer_ids("456");
 
   ASSERT_TRUE(imageInfo2.isPending());
   promise.set(result);


[mesos] 17/17: Added MESOS-9159 and MESOS-9675 to the 1.7.3 CHANGELOG.

Posted by qi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

qianzhang pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 62ac762345e6b192b03a695201b7f1ee81c3420c
Author: Qian Zhang <zh...@gmail.com>
AuthorDate: Mon Apr 15 16:13:11 2019 +0800

    Added MESOS-9159 and MESOS-9675 to the 1.7.3 CHANGELOG.
---
 CHANGELOG | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/CHANGELOG b/CHANGELOG
index e855cfb..ef1bcea 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -17,7 +17,9 @@ Release Notes - Mesos - Version 1.7.3 (WIP)
 
 ** Improvements
   * [MESOS-8880] - Add minimum capabilities in the master.
+  * [MESOS-9159] - Support Foreign URLs in docker registry puller.
   * [MESOS-9540] - Support `DESTROY_DISK` on preprovisioned CSI volumes.
+  * [MESOS-9675] - Docker Manifest V2 Schema2 Support.
 
 
 Release Notes - Mesos - Version 1.7.2


[mesos] 01/17: Windows: Update curl version to 7.61.0.

Posted by qi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

qianzhang pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 9abc873d2928119e44234ad57549d8aa0b4d5f14
Author: Liangyu Zhao <t-...@microsoft.com>
AuthorDate: Tue Aug 28 13:30:42 2018 -0700

    Windows: Update curl version to 7.61.0.
    
    A bug is encountered in version 7.57.0, and is fixed in 7.61.0.
    
    Review: https://reviews.apache.org/r/68450/
---
 3rdparty/CMakeLists.txt       | 4 ++--
 3rdparty/cmake/Versions.cmake | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/3rdparty/CMakeLists.txt b/3rdparty/CMakeLists.txt
index 23ba58f..e65f8dc 100644
--- a/3rdparty/CMakeLists.txt
+++ b/3rdparty/CMakeLists.txt
@@ -743,7 +743,7 @@ if (WIN32)
 
     set_target_properties(
       curl PROPERTIES
-      IMPORTED_LOCATION_DEBUG ${CURL_ROOT}-build/src/Debug/curl-d.exe
+      IMPORTED_LOCATION_DEBUG ${CURL_ROOT}-build/src/Debug/curl.exe
       IMPORTED_LOCATION_RELEASE ${CURL_ROOT}-build/src/Release/curl.exe)
   else ()
     # This is for single-configuration generators such as Ninja.
@@ -758,7 +758,7 @@ if (WIN32)
 
     set_target_properties(
       curl PROPERTIES
-      IMPORTED_LOCATION ${CURL_ROOT}-build/src/curl${CURL_SUFFIX}.exe)
+      IMPORTED_LOCATION ${CURL_ROOT}-build/src/curl.exe)
   endif ()
 
   MAKE_INCLUDE_DIR(libcurl)
diff --git a/3rdparty/cmake/Versions.cmake b/3rdparty/cmake/Versions.cmake
index a1c8184..69fc594 100644
--- a/3rdparty/cmake/Versions.cmake
+++ b/3rdparty/cmake/Versions.cmake
@@ -4,8 +4,8 @@ set(CONCURRENTQUEUE_VERSION "7b69a8f")
 set(CONCURRENTQUEUE_HASH    "SHA256=B2741A1FB2172C2A829503A85D5EE7548BE7ED04236A3FD1EFD2B6088E065CB7")
 set(CSI_VERSION             "0.2.0")
 set(CSI_HASH                "SHA256=CB75FC99B03F3C7C30E407AE86BA63EB069AE4A167A26C94FE97F09CB7FF8771")
-set(CURL_VERSION            "7.57.0")
-set(CURL_HASH               "SHA256=7CE35F207562674E71DBADA6891B37E3F043C1E7A82915CB9C2A17AD3A9D659B")
+set(CURL_VERSION            "7.61.0")
+set(CURL_HASH               "SHA256=64141F0DB4945268A21B490D58806B97C615D3D0C75BF8C335BBE0EFD13B45B5")
 set(ELFIO_VERSION           "3.2")
 set(ELFIO_HASH              "SHA256=964BE1D401F98FA7A1242BCF048DF32B7D56DBAAAE5D02834900499073AC2E95")
 set(GOOGLETEST_VERSION      "1.8.0")


[mesos] 14/17: Added a unit test for Mesos containerizer image force pulling.

Posted by qi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

qianzhang pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 2a4b2d5879df4d02218d13cbc3d7d632bb3eb034
Author: Gilbert Song <so...@gmail.com>
AuthorDate: Wed Mar 27 17:33:19 2019 -0700

    Added a unit test for Mesos containerizer image force pulling.
    
    Review: https://reviews.apache.org/r/70366
---
 .../containerizer/provisioner_docker_tests.cpp     | 127 +++++++++++++++++++++
 1 file changed, 127 insertions(+)

diff --git a/src/tests/containerizer/provisioner_docker_tests.cpp b/src/tests/containerizer/provisioner_docker_tests.cpp
index 6e035c4..76ba5f1 100644
--- a/src/tests/containerizer/provisioner_docker_tests.cpp
+++ b/src/tests/containerizer/provisioner_docker_tests.cpp
@@ -735,6 +735,133 @@ TEST_P(ProvisionerDockerTest, ROOT_INTERNET_CURL_SimpleCommand)
 }
 
 
+// This test verifies the functionality of image `cached` option
+// for image force pulling.
+TEST_F(ProvisionerDockerTest, ROOT_INTERNET_CURL_ImageForcePulling)
+{
+  Try<Owned<cluster::Master>> master = StartMaster();
+  ASSERT_SOME(master);
+
+  slave::Flags flags = CreateSlaveFlags();
+  flags.isolation = "docker/runtime,filesystem/linux";
+  flags.image_providers = "docker";
+
+  // Image pulling time may be long, depending on the location of
+  // the registry server.
+  flags.executor_registration_timeout = Minutes(10);
+
+  Owned<MasterDetector> detector = master.get()->createDetector();
+  Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(), flags);
+  ASSERT_SOME(slave);
+
+  MockScheduler sched;
+  MesosSchedulerDriver driver(
+      &sched, DEFAULT_FRAMEWORK_INFO, master.get()->pid, DEFAULT_CREDENTIAL);
+
+  EXPECT_CALL(sched, registered(&driver, _, _));
+
+  Future<vector<Offer>> offers1, offers2;
+  EXPECT_CALL(sched, resourceOffers(&driver, _))
+    .WillOnce(FutureArg<1>(&offers1))
+    .WillOnce(FutureArg<1>(&offers2))
+    .WillRepeatedly(Return()); // Ignore subsequent offers.
+
+  driver.start();
+
+  AWAIT_READY(offers1);
+  ASSERT_EQ(1u, offers1->size());
+
+  const Offer& offer1 = offers1.get()[0];
+
+  // NOTE: We use a non-shell command here because 'sh' might not be
+  // in the PATH. 'alpine' does not specify env PATH in the image. On
+  // some linux distribution, '/bin' is not in the PATH by default.
+  CommandInfo command;
+  command.set_shell(false);
+  command.set_value("/bin/ls");
+  command.add_arguments("ls");
+  command.add_arguments("-al");
+  command.add_arguments("/");
+
+  TaskInfo task = createTask(
+      offer1.slave_id(),
+      Resources::parse("cpus:1;mem:128").get(),
+      command);
+
+  Image image;
+  image.set_type(Image::DOCKER);
+  image.mutable_docker()->set_name("alpine");
+
+  ContainerInfo* container = task.mutable_container();
+  container->set_type(ContainerInfo::MESOS);
+  container->mutable_mesos()->mutable_image()->CopyFrom(image);
+
+  Future<TaskStatus> statusStarting1;
+  Future<TaskStatus> statusRunning1;
+  Future<TaskStatus> statusFinished1;
+  EXPECT_CALL(sched, statusUpdate(&driver, _))
+    .WillOnce(FutureArg<1>(&statusStarting1))
+    .WillOnce(FutureArg<1>(&statusRunning1))
+    .WillOnce(FutureArg<1>(&statusFinished1));
+
+  driver.launchTasks(offer1.id(), {task});
+
+  AWAIT_READY_FOR(statusStarting1, Minutes(10));
+  EXPECT_EQ(task.task_id(), statusStarting1->task_id());
+  EXPECT_EQ(TASK_STARTING, statusStarting1->state());
+
+  AWAIT_READY(statusRunning1);
+  EXPECT_EQ(task.task_id(), statusRunning1->task_id());
+  EXPECT_EQ(TASK_RUNNING, statusRunning1->state());
+
+  AWAIT_READY(statusFinished1);
+  EXPECT_EQ(task.task_id(), statusFinished1->task_id());
+  EXPECT_EQ(TASK_FINISHED, statusFinished1->state());
+
+  AWAIT_READY(offers2);
+  ASSERT_EQ(1u, offers2->size());
+
+  const Offer& offer2 = offers2.get()[0];
+
+  task = createTask(
+      offer2.slave_id(),
+      Resources::parse("cpus:1;mem:128").get(),
+      command);
+
+  // Image force pulling.
+  image.set_cached(false);
+
+  container = task.mutable_container();
+  container->set_type(ContainerInfo::MESOS);
+  container->mutable_mesos()->mutable_image()->CopyFrom(image);
+
+  Future<TaskStatus> statusStarting2;
+  Future<TaskStatus> statusRunning2;
+  Future<TaskStatus> statusFinished2;
+  EXPECT_CALL(sched, statusUpdate(&driver, _))
+    .WillOnce(FutureArg<1>(&statusStarting2))
+    .WillOnce(FutureArg<1>(&statusRunning2))
+    .WillOnce(FutureArg<1>(&statusFinished2));
+
+  driver.launchTasks(offer2.id(), {task});
+
+  AWAIT_READY_FOR(statusStarting2, Minutes(10));
+  EXPECT_EQ(task.task_id(), statusStarting2->task_id());
+  EXPECT_EQ(TASK_STARTING, statusStarting2->state());
+
+  AWAIT_READY(statusRunning2);
+  EXPECT_EQ(task.task_id(), statusRunning2->task_id());
+  EXPECT_EQ(TASK_RUNNING, statusRunning2->state());
+
+  AWAIT_READY(statusFinished2);
+  EXPECT_EQ(task.task_id(), statusFinished2->task_id());
+  EXPECT_EQ(TASK_FINISHED, statusFinished2->state());
+
+  driver.stop();
+  driver.join();
+}
+
+
 // This test verifies that the scratch based docker image (that
 // only contain a single binary and its dependencies) can be
 // launched correctly.


[mesos] 15/17: Fixed use-after-free bug in Docker provisioner store.

Posted by qi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

qianzhang pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit b3c1adf87bbd1408514d485a3f2ae7773930f543
Author: Andrei Budnik <ab...@mesosphere.com>
AuthorDate: Fri Apr 5 13:06:49 2019 +0200

    Fixed use-after-free bug in Docker provisioner store.
    
    Deferred lambda callback of the `moveLayers()` to the `StoreProcess`
    to prevent use-after-free of the process object since the callback
    refers to the `StoreProcess` class variable `flags`.
    
    Review: https://reviews.apache.org/r/70405
---
 src/slave/containerizer/mesos/provisioner/docker/store.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/slave/containerizer/mesos/provisioner/docker/store.cpp b/src/slave/containerizer/mesos/provisioner/docker/store.cpp
index 909364d..36b2c7d 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/store.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/store.cpp
@@ -446,7 +446,7 @@ Future<Image> StoreProcess::moveLayers(
   }
 
   return collect(futures)
-    .then([=]() -> Future<Image> {
+    .then(defer(self(), [=]() -> Future<Image> {
       if (image.has_config_digest()) {
         const string configSource = path::join(staging, image.config_digest());
         const string configTarget = paths::getImageLayerPath(
@@ -464,7 +464,7 @@ Future<Image> StoreProcess::moveLayers(
       }
 
       return image;
-    });
+    }));
 }
 
 


[mesos] 06/17: Windows: Add comment about V2S2 Docker image Manifest.

Posted by qi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

qianzhang pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 88936159fd4700bd064d89165f192fb222361491
Author: Liangyu Zhao <t-...@microsoft.com>
AuthorDate: Wed Aug 29 13:52:18 2018 -0700

    Windows: Add comment about V2S2 Docker image Manifest.
    
    Review: https://reviews.apache.org/r/68562/
---
 src/uri/fetchers/docker.cpp | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/uri/fetchers/docker.cpp b/src/uri/fetchers/docker.cpp
index c1012cb..a87d7f0 100644
--- a/src/uri/fetchers/docker.cpp
+++ b/src/uri/fetchers/docker.cpp
@@ -749,6 +749,11 @@ Future<Nothing> DockerFetcherPluginProcess::__fetch(
   // https://docs.docker.com/registry/spec/manifest-v2-2/
   //
   // If fetch is failed, program continues without schema 2 manifest.
+  //
+  // Schema 2 manifest may have foreign URLs to fetch blobs from servers
+  // other than Docker Hub. Some file layers, for example, Windows OS
+  // layers are only stored on Microsoft servers, so only with schema 2
+  // manifest, such layers can be successfully fetched.
   http::Headers s2ManifestHeaders = {
     {"Accept", "application/vnd.docker.distribution.manifest.v2+json"}
   };


[mesos] 09/17: Added protobuf for docker v2 schema2 config_digest in 'Image'.

Posted by qi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

qianzhang pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 14857c388cb4b25519f75017956affcba21156f2
Author: Gilbert Song <so...@gmail.com>
AuthorDate: Mon Apr 1 22:42:53 2019 -0700

    Added protobuf for docker v2 schema2 config_digest in 'Image'.
    
    Review: https://reviews.apache.org/r/70365
---
 src/slave/containerizer/mesos/provisioner/docker/message.proto | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/slave/containerizer/mesos/provisioner/docker/message.proto b/src/slave/containerizer/mesos/provisioner/docker/message.proto
index 612c591..42fc4fa 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/message.proto
+++ b/src/slave/containerizer/mesos/provisioner/docker/message.proto
@@ -32,6 +32,10 @@ message Image {
   // where the root layer's id (no parent layer) is first and the leaf
   // layer's id is last.
   repeated string layer_ids = 2;
+
+  // The digest of Docker V2 Schema2 manifest config. Only exists when
+  // pulling an image via V2 Schema2 manifest.
+  optional string config_digest = 3;
 }
 
 


[mesos] 02/17: Windows: Parse version 2 schema 2 Docker image manifest.

Posted by qi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

qianzhang pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit ee22ae2bc82786ec5a025ac389b4e608e16d7d16
Author: Liangyu Zhao <t-...@microsoft.com>
AuthorDate: Tue Aug 28 13:32:17 2018 -0700

    Windows: Parse version 2 schema 2 Docker image manifest.
    
    Added support to parse V2S2 Docker image manifest
    (https://docs.docker.com/registry/spec/manifest-v2-2/). Adopted the
    validation code from patch 53850.
    
    Review: https://reviews.apache.org/r/68451/
---
 include/mesos/docker/spec.hpp                 |  18 ++++
 include/mesos/docker/v2_2.hpp                 |  23 +++++
 include/mesos/docker/v2_2.proto               |  45 ++++++++++
 src/CMakeLists.txt                            |   1 +
 src/Makefile.am                               |  11 ++-
 src/docker/spec.cpp                           |  66 ++++++++++++++
 src/tests/CMakeLists.txt                      |   2 +-
 src/tests/containerizer/docker_spec_tests.cpp | 121 ++++++++++++++++++++++++++
 8 files changed, 284 insertions(+), 3 deletions(-)

diff --git a/include/mesos/docker/spec.hpp b/include/mesos/docker/spec.hpp
index 2879414..56827fe 100644
--- a/include/mesos/docker/spec.hpp
+++ b/include/mesos/docker/spec.hpp
@@ -30,6 +30,7 @@
 
 #include <mesos/docker/v1.hpp>
 #include <mesos/docker/v2.hpp>
+#include <mesos/docker/v2_2.hpp>
 
 namespace docker {
 namespace spec {
@@ -119,6 +120,23 @@ Try<ImageManifest> parse(const JSON::Object& json);
 Try<ImageManifest> parse(const std::string& s);
 
 } // namespace v2 {
+
+
+namespace v2_2 {
+
+// Validates if the specified docker v2 s2 image manifest conforms to the
+// Docker v2 s2 spec. Returns the error if the validation fails.
+Option<Error> validate(const ImageManifest& manifest);
+
+
+// Returns the docker v2 s2 image manifest from the given JSON object.
+Try<ImageManifest> parse(const JSON::Object& json);
+
+
+// Returns the docker v2 s2 image manifest from the given string.
+Try<ImageManifest> parse(const std::string& s);
+
+} // namespace v2_2 {
 } // namespace spec {
 } // namespace docker {
 
diff --git a/include/mesos/docker/v2_2.hpp b/include/mesos/docker/v2_2.hpp
new file mode 100644
index 0000000..19f710f
--- /dev/null
+++ b/include/mesos/docker/v2_2.hpp
@@ -0,0 +1,23 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef __MESOS_DOCKER_V2_2_HPP__
+#define __MESOS_DOCKER_V2_2_HPP__
+
+// ONLY USEFUL AFTER RUNNING PROTOC.
+#include <mesos/docker/v2_2.pb.h>
+
+#endif // __MESOS_DOCKER_V2_2_HPP__
diff --git a/include/mesos/docker/v2_2.proto b/include/mesos/docker/v2_2.proto
new file mode 100644
index 0000000..17bb11a
--- /dev/null
+++ b/include/mesos/docker/v2_2.proto
@@ -0,0 +1,45 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+syntax = "proto2";
+
+package docker.spec.v2_2;
+
+/**
+ * Protobuf for the Docker version 2 schema 2 image manifest JSON schema:
+ * https://github.com/docker/distribution/blob/master/docs/spec/manifest-v2-2.md
+ */
+message ImageManifest {
+  required uint32 schemaVersion = 1;
+  required string mediaType = 2;
+
+  message Config {
+    required string mediaType = 1;
+    required uint32 size = 2;
+    required string digest = 3;
+  }
+
+  required Config config = 3;
+
+  message Layer {
+    required string mediaType = 1;
+    required uint32 size = 2;
+    required string digest = 3;
+    repeated string urls = 4;
+  }
+
+  repeated Layer layers = 4;
+}
\ No newline at end of file
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 936e782..5f9bc31 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -44,6 +44,7 @@ PROTOC_GENERATE(TARGET mesos/authorizer/authorizer)
 PROTOC_GENERATE(TARGET mesos/docker/spec)
 PROTOC_GENERATE(TARGET mesos/docker/v1)
 PROTOC_GENERATE(TARGET mesos/docker/v2)
+PROTOC_GENERATE(TARGET mesos/docker/v2_2)
 PROTOC_GENERATE(TARGET mesos/maintenance/maintenance)
 PROTOC_GENERATE(TARGET mesos/master/master)
 PROTOC_GENERATE(TARGET mesos/module/hook)
diff --git a/src/Makefile.am b/src/Makefile.am
index aa343ab..fb2a27e 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -287,6 +287,7 @@ CONTAINERIZER_PROTO = $(top_srcdir)/include/mesos/slave/containerizer.proto
 DOCKER_SPEC_PROTO = $(top_srcdir)/include/mesos/docker/spec.proto
 DOCKER_V1_PROTO = $(top_srcdir)/include/mesos/docker/v1.proto
 DOCKER_V2_PROTO = $(top_srcdir)/include/mesos/docker/v2.proto
+DOCKER_V2_2_PROTO = $(top_srcdir)/include/mesos/docker/v2_2.proto
 EXECUTOR_PROTO = $(top_srcdir)/include/mesos/executor/executor.proto
 FETCHER_PROTO = $(top_srcdir)/include/mesos/fetcher/fetcher.proto
 HOOK_PROTO = $(top_srcdir)/include/mesos/module/hook.proto
@@ -333,6 +334,8 @@ CXX_PROTOS =								\
   ../include/mesos/docker/v1.pb.h					\
   ../include/mesos/docker/v2.pb.cc					\
   ../include/mesos/docker/v2.pb.h					\
+  ../include/mesos/docker/v2_2.pb.cc					\
+  ../include/mesos/docker/v2_2.pb.h					\
   ../include/mesos/executor/executor.pb.cc				\
   ../include/mesos/executor/executor.pb.h				\
   ../include/mesos/fetcher/fetcher.pb.cc				\
@@ -688,12 +691,15 @@ docker_HEADERS =							\
   $(top_srcdir)/include/mesos/docker/v1.hpp				\
   $(top_srcdir)/include/mesos/docker/v1.proto				\
   $(top_srcdir)/include/mesos/docker/v2.hpp				\
-  $(top_srcdir)/include/mesos/docker/v2.proto
+  $(top_srcdir)/include/mesos/docker/v2.proto       \
+  $(top_srcdir)/include/mesos/docker/v2_2.hpp				\
+  $(top_srcdir)/include/mesos/docker/v2_2.proto
 
 nodist_docker_HEADERS =							\
   ../include/mesos/docker/spec.pb.h					\
   ../include/mesos/docker/v1.pb.h					\
-  ../include/mesos/docker/v2.pb.h
+  ../include/mesos/docker/v2.pb.h         \
+  ../include/mesos/docker/v2_2.pb.h
 
 executordir = $(pkgincludedir)/executor
 
@@ -1616,6 +1622,7 @@ libmesos_la_SOURCES =							\
   $(DOCKER_SPEC_PROTO)							\
   $(DOCKER_V1_PROTO)							\
   $(DOCKER_V2_PROTO)							\
+  $(DOCKER_V2_2_PROTO)							\
   $(FETCHER_PROTO)							\
   $(HOOK_PROTO)								\
   $(MAINTENANCE_PROTO)							\
diff --git a/src/docker/spec.cpp b/src/docker/spec.cpp
index 96fbf1f..79ddcdb 100644
--- a/src/docker/spec.cpp
+++ b/src/docker/spec.cpp
@@ -341,5 +341,71 @@ Try<ImageManifest> parse(const string& s)
 }
 
 } // namespace v2 {
+
+namespace v2_2 {
+
+Option<Error> validate(const ImageManifest& manifest)
+{
+  // Validate required fields are present,
+  // e.g., repeated fields that has to be >= 1.
+  if (manifest.layers_size() <= 0) {
+    return Error("'layers' field size must be at least one");
+  }
+
+  // Verify 'config' field.
+  if (!strings::contains(manifest.config().digest(), ":")) {
+    return Error("Incorrect 'digest' format: " + manifest.config().digest());
+  }
+
+  // Verify 'layers' field.
+  for (int i = 0; i < manifest.layers_size(); ++i) {
+    if (!strings::contains(manifest.layers(i).digest(), ":")) {
+      return Error("Incorrect 'digest' format: " + manifest.layers(i).digest());
+    }
+  }
+
+  if (manifest.schemaversion() != 2) {
+    return Error("'schemaVersion' field must be 2");
+  }
+
+  if (manifest.mediatype() !=
+      "application/vnd.docker.distribution.manifest.v2+json") {
+    return Error(
+        "'mediaType' field must be "
+        "'application/vnd.docker.distribution.manifest.v2+json'");
+  }
+
+  return None();
+}
+
+
+Try<ImageManifest> parse(const JSON::Object& json)
+{
+  Try<ImageManifest> manifest = protobuf::parse<ImageManifest>(json);
+  if (manifest.isError()) {
+    return Error("Protobuf parse failed: " + manifest.error());
+  }
+
+  Option<Error> error = validate(manifest.get());
+  if (error.isSome()) {
+    return Error(
+        "Docker v2 s2 image manifest validation failed: " + error->message);
+  }
+
+  return manifest.get();
+}
+
+
+Try<ImageManifest> parse(const string& s)
+{
+  Try<JSON::Object> json = JSON::parse<JSON::Object>(s);
+  if (json.isError()) {
+    return Error("JSON parse failed: " + json.error());
+  }
+
+  return parse(json.get());
+}
+
+} // namespace v2_2 {
 } // namespace spec {
 } // namespace docker {
diff --git a/src/tests/CMakeLists.txt b/src/tests/CMakeLists.txt
index ae0ed58..7c75572 100644
--- a/src/tests/CMakeLists.txt
+++ b/src/tests/CMakeLists.txt
@@ -154,6 +154,7 @@ list(APPEND MESOS_TESTS_SRC
 list(APPEND MESOS_TESTS_SRC
   containerizer/containerizer_tests.cpp
   containerizer/cpu_isolator_tests.cpp
+  containerizer/docker_spec_tests.cpp
   containerizer/docker_tests.cpp
   containerizer/memory_isolator_tests.cpp)
 
@@ -199,7 +200,6 @@ if (NOT WIN32)
   list(APPEND MESOS_TESTS_SRC
     containerizer/appc_spec_tests.cpp
     containerizer/composing_containerizer_tests.cpp
-    containerizer/docker_spec_tests.cpp
     containerizer/environment_secret_isolator_tests.cpp
     containerizer/io_switchboard_tests.cpp
     containerizer/isolator_tests.cpp
diff --git a/src/tests/containerizer/docker_spec_tests.cpp b/src/tests/containerizer/docker_spec_tests.cpp
index 8f2fa4e..4be0716 100644
--- a/src/tests/containerizer/docker_spec_tests.cpp
+++ b/src/tests/containerizer/docker_spec_tests.cpp
@@ -603,6 +603,127 @@ TEST_F(DockerSpecTest, ValidateV2ImageManifestSignaturesNonEmpty)
   EXPECT_ERROR(manifest);
 }
 
+
+TEST_F(DockerSpecTest, ParseV2_2ImageManifest)
+{
+  Try<JSON::Object> json = JSON::parse<JSON::Object>(
+      R"~(
+      {
+        "schemaVersion": 2,
+        "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
+        "config": {
+            "mediaType": "application/vnd.docker.container.image.v1+json",
+            "size": 7023,
+            "digest": "sha256:b5b2b2c507a0944348e0303114d8d93aaaa081732b86451d9bce1f432a537bc7"
+        },
+        "layers": [
+            {
+                "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip",
+                "size": 32654,
+                "digest": "sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f"
+            },
+            {
+                "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip",
+                "size": 16724,
+                "digest": "sha256:3c3a4604a545cdc127456d94e421cd355bca5b528f4a9c1905b15da2eb4a4c6b"
+            },
+            {
+                "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip",
+                "size": 73109,
+                "digest": "sha256:ec4b8955958665577945c89419d1af06b5f7636b4ac3da7f12184802ad867736"
+            }
+        ]
+      })~");
+
+  ASSERT_SOME(json);
+
+  Try<spec::v2_2::ImageManifest> manifest = spec::v2_2::parse(json.get());
+  ASSERT_SOME(manifest);
+
+  EXPECT_EQ(2u, manifest->schemaversion());
+  EXPECT_EQ(
+      "application/vnd.docker.distribution.manifest.v2+json",
+      manifest->mediatype());
+
+  EXPECT_EQ(
+      "application/vnd.docker.container.image.v1+json",
+      manifest->config().mediatype());
+  EXPECT_EQ(7023u, manifest->config().size());
+  EXPECT_EQ(
+      "sha256:b5b2b2c507a0944348e0303114d8d93aaaa081732b86451d9bce1f432a537bc7",
+      manifest->config().digest());
+
+  EXPECT_EQ(3, manifest->layers_size());
+
+  EXPECT_EQ(
+      "application/vnd.docker.image.rootfs.diff.tar.gzip",
+      manifest->layers(0).mediatype());
+  EXPECT_EQ(32654u, manifest->layers(0).size());
+  EXPECT_EQ(
+      "sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f",
+      manifest->layers(0).digest());
+
+  EXPECT_EQ(
+      "application/vnd.docker.image.rootfs.diff.tar.gzip",
+      manifest->layers(1).mediatype());
+  EXPECT_EQ(16724u, manifest->layers(1).size());
+  EXPECT_EQ(
+      "sha256:3c3a4604a545cdc127456d94e421cd355bca5b528f4a9c1905b15da2eb4a4c6b",
+      manifest->layers(1).digest());
+
+  EXPECT_EQ(
+      "application/vnd.docker.image.rootfs.diff.tar.gzip",
+      manifest->layers(2).mediatype());
+  EXPECT_EQ(73109u, manifest->layers(2).size());
+  EXPECT_EQ(
+      "sha256:ec4b8955958665577945c89419d1af06b5f7636b4ac3da7f12184802ad867736",
+      manifest->layers(2).digest());
+}
+
+
+TEST_F(DockerSpecTest, ParseInvalidV2_2ImageManifest)
+{
+  // This is an invalid manifest. The size of the repeated fields
+  // 'layers' must be >= 1. The 'signatures' and
+  // 'schemaVersion' fields are not set.
+  Try<JSON::Object> json = JSON::parse<JSON::Object>(
+      R"~(
+      {
+        "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
+        "config": {
+            "mediaType": "application/vnd.docker.container.image.v1+json",
+            "size": 7023,
+            "digest": "sha256:b5b2b2c507a0944348e0303114d8d93aaaa081732b86451d9bce1f432a537bc7"
+        }
+      })~");
+
+  ASSERT_SOME(json);
+
+  Try<spec::v2_2::ImageManifest> manifest = spec::v2_2::parse(json.get());
+  EXPECT_ERROR(manifest);
+}
+
+
+TEST_F(DockerSpecTest, ValidateV2_2ImageManifestLayersNonEmpty)
+{
+  Try<JSON::Object> json = JSON::parse<JSON::Object>(
+      R"~(
+      {
+        "schemaVersion": 2,
+        "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
+        "config": {
+            "mediaType": "application/vnd.docker.container.image.v1+json",
+            "size": 7023,
+            "digest": "sha256:b5b2b2c507a0944348e0303114d8d93aaaa081732b86451d9bce1f432a537bc7"
+        }
+      })~");
+
+  ASSERT_SOME(json);
+
+  Try<spec::v2_2::ImageManifest> manifest = spec::v2_2::parse(json.get());
+  EXPECT_ERROR(manifest);
+}
+
 } // namespace tests {
 } // namespace internal {
 } // namespace mesos {


[mesos] 04/17: Windows: Enable DockerFetcher in Windows agent.

Posted by qi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

qianzhang pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 5699a39607a64e58fa8074a786b180d052904096
Author: Liangyu Zhao <t-...@microsoft.com>
AuthorDate: Wed Aug 29 11:54:25 2018 -0700

    Windows: Enable DockerFetcher in Windows agent.
    
    Review: https://reviews.apache.org/r/68455/
---
 src/CMakeLists.txt                              | 15 +++++----------
 src/slave/containerizer/mesos/containerizer.cpp |  2 ++
 src/uri/fetcher.cpp                             |  3 ---
 src/uri/fetcher.hpp                             |  5 -----
 4 files changed, 7 insertions(+), 18 deletions(-)

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 5f9bc31..5bfc8db 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -184,11 +184,7 @@ set(AGENT_SRC
   slave/containerizer/mesos/provisioner/docker/store.cpp
   slave/resource_estimators/noop.cpp)
 
-if (WIN32)
-  list(APPEND AGENT_SRC
-    slave/containerizer/mesos/isolators/windows/cpu.cpp
-    slave/containerizer/mesos/isolators/windows/mem.cpp)
-else ()
+if (NOT WIN32)
   list(APPEND AGENT_SRC
     slave/containerizer/mesos/utils.cpp
     slave/containerizer/mesos/isolators/environment_secret.cpp
@@ -455,13 +451,9 @@ set(URI_SRC
   uri/utils.cpp
   uri/fetchers/copy.cpp
   uri/fetchers/curl.cpp
+  uri/fetchers/docker.cpp
   uri/fetchers/hadoop.cpp)
 
-if (NOT WIN32)
-  list(APPEND URI_SRC
-    uri/fetchers/docker.cpp)
-endif ()
-
 set(USAGE_SRC
   usage/usage.cpp)
 
@@ -478,6 +470,9 @@ set(WATCHER_SRC
   watcher/whitelist_watcher.cpp)
 
 set(WIN32_SRC
+  slave/containerizer/mesos/isolators/docker/runtime.cpp
+  slave/containerizer/mesos/isolators/windows/cpu.cpp
+  slave/containerizer/mesos/isolators/windows/mem.cpp
   slave/containerizer/mesos/isolators/environment_secret.cpp
   slave/containerizer/mesos/isolators/filesystem/posix.cpp
   slave/containerizer/mesos/isolators/filesystem/windows.cpp)
diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp
index 321d04c..e8a4ab3 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -88,6 +88,7 @@
 #include "slave/containerizer/mesos/provisioner/provisioner.hpp"
 
 #ifdef __WINDOWS__
+#include "slave/containerizer/mesos/isolators/docker/runtime.hpp"
 #include "slave/containerizer/mesos/isolators/windows/cpu.hpp"
 #include "slave/containerizer/mesos/isolators/windows/mem.hpp"
 #include "slave/containerizer/mesos/isolators/filesystem/windows.hpp"
@@ -385,6 +386,7 @@ Try<MesosContainerizer*> MesosContainerizer::create(
 #endif // __WINDOWS__
 
 #ifdef __WINDOWS__
+    {"docker/runtime", &DockerRuntimeIsolatorProcess::create},
     {"windows/cpu", &WindowsCpuIsolatorProcess::create},
     {"windows/mem", &WindowsMemIsolatorProcess::create},
 #endif // __WINDOWS__
diff --git a/src/uri/fetcher.cpp b/src/uri/fetcher.cpp
index 489c7ce..3147e41 100644
--- a/src/uri/fetcher.cpp
+++ b/src/uri/fetcher.cpp
@@ -52,11 +52,8 @@ Try<Owned<Fetcher>> create(const Option<Flags>& _flags)
        [flags]() { return CopyFetcherPlugin::create(flags); }},
     {HadoopFetcherPlugin::NAME,
        [flags]() { return HadoopFetcherPlugin::create(flags); }},
-#ifndef __WINDOWS__
-    // TODO(coffler): Enable Docker plugin. See MESOS-8570.
     {DockerFetcherPlugin::NAME,
        [flags]() { return DockerFetcherPlugin::create(flags); }},
-#endif // __WINDOWS__
   };
 
   vector<Owned<Fetcher::Plugin>> plugins;
diff --git a/src/uri/fetcher.hpp b/src/uri/fetcher.hpp
index fbf64c6..cc4bd93 100644
--- a/src/uri/fetcher.hpp
+++ b/src/uri/fetcher.hpp
@@ -40,13 +40,8 @@ namespace fetcher {
 class Flags :
   public virtual CopyFetcherPlugin::Flags,
   public virtual CurlFetcherPlugin::Flags,
-#ifdef __WINDOWS__
-  public virtual HadoopFetcherPlugin::Flags {};
-#else
-  // TODO(coffler): Add support for Docker plugins. See MESOS-8570.
   public virtual HadoopFetcherPlugin::Flags,
   public virtual DockerFetcherPlugin::Flags {};
-#endif // __WINDOWS__
 
 
 /**


[mesos] 05/17: Windows: Enabled `DockerFetcherPluginTest` suite.

Posted by qi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

qianzhang pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit b4b03371398a5f2ba33d2a141bc1aa3d1c15a4bc
Author: Liangyu Zhao <t-...@microsoft.com>
AuthorDate: Wed Aug 29 11:54:47 2018 -0700

    Windows: Enabled `DockerFetcherPluginTest` suite.
    
    Enabled `Internet` test environment on Windows. Disabled `Internet`
    `HealthCheckTests` on Windows, since they require complete
    development. Modified `DockerFetcherPluginTest` to fetch
    `microsoft/nanoserver` for more extensive test for fetcher on Windows.
    
    Review: https://reviews.apache.org/r/67930/
---
 src/tests/environment.cpp        |  5 ++-
 src/tests/health_check_tests.cpp | 10 +++--
 src/tests/uri_fetcher_tests.cpp  | 81 ++++++++++++++++++++++++++--------------
 3 files changed, 62 insertions(+), 34 deletions(-)

diff --git a/src/tests/environment.cpp b/src/tests/environment.cpp
index 8c6ec58..0677b31 100644
--- a/src/tests/environment.cpp
+++ b/src/tests/environment.cpp
@@ -523,8 +523,11 @@ class InternetFilter : public TestFilter
 public:
   InternetFilter()
   {
+#ifdef __WINDOWS__
+    error = os::system("ping -n 1 -w 1000 google.com") != 0;
+#else
     error = os::system("ping -c 1 -W 1 google.com") != 0;
-    // TODO(andschwa): Make ping command cross-platform.
+#endif // __WINDOWS__
     if (error) {
       std::cerr
         << "-------------------------------------------------------------\n"
diff --git a/src/tests/health_check_tests.cpp b/src/tests/health_check_tests.cpp
index 15ac700..4362c45 100644
--- a/src/tests/health_check_tests.cpp
+++ b/src/tests/health_check_tests.cpp
@@ -1358,7 +1358,8 @@ TEST_F(HealthCheckTest, HealthyTaskViaTCP)
 // Tests a healthy task via HTTP with a container image using mesos
 // containerizer. To emulate a task responsive to HTTP health checks,
 // starts Netcat in the docker "alpine" image.
-TEST_F(HealthCheckTest, ROOT_INTERNET_CURL_HealthyTaskViaHTTPWithContainerImage)
+TEST_F_TEMP_DISABLED_ON_WINDOWS(
+    HealthCheckTest, ROOT_INTERNET_CURL_HealthyTaskViaHTTPWithContainerImage)
 {
   master::Flags masterFlags = CreateMasterFlags();
   masterFlags.allocation_interval = Milliseconds(50);
@@ -1451,8 +1452,8 @@ TEST_F(HealthCheckTest, ROOT_INTERNET_CURL_HealthyTaskViaHTTPWithContainerImage)
 // Tests a healthy task via HTTPS with a container image using mesos
 // containerizer. To emulate a task responsive to HTTPS health checks,
 // starts an HTTPS server in the docker "haosdent/https-server" image.
-TEST_F(HealthCheckTest,
-       ROOT_INTERNET_CURL_HealthyTaskViaHTTPSWithContainerImage)
+TEST_F_TEMP_DISABLED_ON_WINDOWS(
+    HealthCheckTest, ROOT_INTERNET_CURL_HealthyTaskViaHTTPSWithContainerImage)
 {
   master::Flags masterFlags = CreateMasterFlags();
   Try<Owned<cluster::Master>> master = StartMaster(masterFlags);
@@ -1551,7 +1552,8 @@ TEST_F(HealthCheckTest,
 // NOTE: This test is almost identical to
 // ROOT_INTERNET_CURL_HealthyTaskViaHTTPWithContainerImage
 // with the difference being TCP health check.
-TEST_F(HealthCheckTest, ROOT_INTERNET_CURL_HealthyTaskViaTCPWithContainerImage)
+TEST_F_TEMP_DISABLED_ON_WINDOWS(
+    HealthCheckTest, ROOT_INTERNET_CURL_HealthyTaskViaTCPWithContainerImage)
 {
   master::Flags masterFlags = CreateMasterFlags();
   masterFlags.allocation_interval = Milliseconds(50);
diff --git a/src/tests/uri_fetcher_tests.cpp b/src/tests/uri_fetcher_tests.cpp
index 260ae9c..f8bacc0 100644
--- a/src/tests/uri_fetcher_tests.cpp
+++ b/src/tests/uri_fetcher_tests.cpp
@@ -49,6 +49,8 @@ namespace http = process::http;
 using std::list;
 using std::string;
 
+using mesos::uri::DockerFetcherPlugin;
+
 using process::Future;
 using process::Owned;
 using process::Process;
@@ -274,6 +276,16 @@ TEST_F(HadoopFetcherPluginTest, InvokeFetchByName)
 // tests can use this as well.
 static constexpr char DOCKER_REGISTRY_HOST[] = "registry-1.docker.io";
 
+#ifdef __WINDOWS__
+static constexpr char TEST_REPOSITORY[] = "microsoft/nanoserver";
+static constexpr char TEST_DIGEST[] = "sha256:54389c2d19b423943102864aaf3fc"
+                                      "1296e5dd140a074b5bd6700de858a8e5479";
+#else
+static constexpr char TEST_REPOSITORY[] = "library/busybox";
+static constexpr char TEST_DIGEST[] = "sha256:a3ed95caeb02ffe68cdd9fd844066"
+                                      "80ae93d633cb16422d00e8a7c22955b46d4";
+#endif // __WINDOWS__
+
 
 class DockerFetcherPluginTest : public TemporaryDirectoryTest {};
 
@@ -281,9 +293,7 @@ class DockerFetcherPluginTest : public TemporaryDirectoryTest {};
 TEST_F(DockerFetcherPluginTest, INTERNET_CURL_FetchManifest)
 {
   URI uri = uri::docker::manifest(
-      "library/busybox",
-      "latest",
-      DOCKER_REGISTRY_HOST);
+      TEST_REPOSITORY, "latest", DOCKER_REGISTRY_HOST);
 
   Try<Owned<uri::Fetcher>> fetcher = uri::fetcher::create();
   ASSERT_SOME(fetcher);
@@ -292,27 +302,36 @@ TEST_F(DockerFetcherPluginTest, INTERNET_CURL_FetchManifest)
 
   AWAIT_READY_FOR(fetcher.get()->fetch(uri, dir), Seconds(60));
 
-  Try<string> _manifest = os::read(path::join(dir, "manifest"));
-  ASSERT_SOME(_manifest);
+  // Version 2 schema 1 image manifest test
+  Try<string> _s1Manifest = os::read(path::join(dir, "manifest"));
+  ASSERT_SOME(_s1Manifest);
 
-  Try<docker::spec::v2::ImageManifest> manifest =
-    docker::spec::v2::parse(_manifest.get());
+  Try<docker::spec::v2::ImageManifest> s1Manifest =
+    docker::spec::v2::parse(_s1Manifest.get());
 
-  ASSERT_SOME(manifest);
-  EXPECT_EQ("library/busybox", manifest->name());
-  EXPECT_EQ("latest", manifest->tag());
+  ASSERT_SOME(s1Manifest);
+  EXPECT_EQ(1u, s1Manifest->schemaversion());
+  EXPECT_EQ(TEST_REPOSITORY, s1Manifest->name());
+  EXPECT_EQ("latest", s1Manifest->tag());
+
+#ifdef __WINDOWS__
+  // Version 2 schema 2 image manifest test
+  Try<string> _s2Manifest = os::read(path::join(dir, "manifest_v2s2"));
+  ASSERT_SOME(_s2Manifest);
+
+  Try<docker::spec::v2_2::ImageManifest> s2Manifest =
+    docker::spec::v2_2::parse(_s2Manifest.get());
+
+  ASSERT_SOME(s2Manifest);
+  EXPECT_EQ(2u, s2Manifest->schemaversion());
+#endif // __WINDOWS__
 }
 
 
 TEST_F(DockerFetcherPluginTest, INTERNET_CURL_FetchBlob)
 {
-  const string digest =
-    "sha256:a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4";
-
   URI uri = uri::docker::blob(
-      "library/busybox",
-      digest,
-      DOCKER_REGISTRY_HOST);
+      TEST_REPOSITORY, TEST_DIGEST, DOCKER_REGISTRY_HOST);
 
   Try<Owned<uri::Fetcher>> fetcher = uri::fetcher::create();
   ASSERT_SOME(fetcher);
@@ -321,7 +340,7 @@ TEST_F(DockerFetcherPluginTest, INTERNET_CURL_FetchBlob)
 
   AWAIT_READY_FOR(fetcher.get()->fetch(uri, dir), Seconds(60));
 
-  EXPECT_TRUE(os::exists(path::join(dir, digest)));
+  EXPECT_TRUE(os::exists(DockerFetcherPlugin::getBlobPath(dir, TEST_DIGEST)));
 }
 
 
@@ -329,9 +348,7 @@ TEST_F(DockerFetcherPluginTest, INTERNET_CURL_FetchBlob)
 TEST_F(DockerFetcherPluginTest, INTERNET_CURL_FetchImage)
 {
   URI uri = uri::docker::image(
-      "library/busybox",
-      "latest",
-      DOCKER_REGISTRY_HOST);
+      TEST_REPOSITORY, "latest", DOCKER_REGISTRY_HOST);
 
   Try<Owned<uri::Fetcher>> fetcher = uri::fetcher::create();
   ASSERT_SOME(fetcher);
@@ -344,14 +361,18 @@ TEST_F(DockerFetcherPluginTest, INTERNET_CURL_FetchImage)
   ASSERT_SOME(_manifest);
 
   Try<docker::spec::v2::ImageManifest> manifest =
-    docker::spec::v2::parse(_manifest.get());
+      docker::spec::v2::parse(_manifest.get());
 
   ASSERT_SOME(manifest);
-  EXPECT_EQ("library/busybox", manifest->name());
+  EXPECT_EQ(1u, manifest->schemaversion());
+  EXPECT_EQ(TEST_REPOSITORY, manifest->name());
   EXPECT_EQ("latest", manifest->tag());
 
   for (int i = 0; i < manifest->fslayers_size(); i++) {
-    EXPECT_TRUE(os::exists(path::join(dir, manifest->fslayers(i).blobsum())));
+    EXPECT_TRUE(os::exists(
+        DockerFetcherPlugin::getBlobPath(
+            dir,
+            manifest->fslayers(i).blobsum())));
   }
 }
 
@@ -360,9 +381,7 @@ TEST_F(DockerFetcherPluginTest, INTERNET_CURL_FetchImage)
 TEST_F(DockerFetcherPluginTest, INTERNET_CURL_InvokeFetchByName)
 {
   URI uri = uri::docker::image(
-      "library/busybox",
-      "latest",
-      DOCKER_REGISTRY_HOST);
+      TEST_REPOSITORY, "latest", DOCKER_REGISTRY_HOST);
 
   Try<Owned<uri::Fetcher>> fetcher = uri::fetcher::create();
   ASSERT_SOME(fetcher);
@@ -377,14 +396,18 @@ TEST_F(DockerFetcherPluginTest, INTERNET_CURL_InvokeFetchByName)
   ASSERT_SOME(_manifest);
 
   Try<docker::spec::v2::ImageManifest> manifest =
-    docker::spec::v2::parse(_manifest.get());
+      docker::spec::v2::parse(_manifest.get());
 
   ASSERT_SOME(manifest);
-  EXPECT_EQ("library/busybox", manifest->name());
+  EXPECT_EQ(1u, manifest->schemaversion());
+  EXPECT_EQ(TEST_REPOSITORY, manifest->name());
   EXPECT_EQ("latest", manifest->tag());
 
   for (int i = 0; i < manifest->fslayers_size(); i++) {
-    EXPECT_TRUE(os::exists(path::join(dir, manifest->fslayers(i).blobsum())));
+    EXPECT_TRUE(os::exists(
+        DockerFetcherPlugin::getBlobPath(
+            dir,
+            manifest->fslayers(i).blobsum())));
   }
 }
 


[mesos] 16/17: Fixed the URI fetcher image fetch test failure on windows.

Posted by qi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

qianzhang pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit f8b64a407237ff21f9cce6c87356ca0a1623692b
Author: Gilbert Song <so...@gmail.com>
AuthorDate: Fri Apr 12 17:13:58 2019 +0800

    Fixed the URI fetcher image fetch test failure on windows.
    
    Review: https://reviews.apache.org/r/70398/
---
 src/tests/uri_fetcher_tests.cpp | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/tests/uri_fetcher_tests.cpp b/src/tests/uri_fetcher_tests.cpp
index bb224d8..96175f4 100644
--- a/src/tests/uri_fetcher_tests.cpp
+++ b/src/tests/uri_fetcher_tests.cpp
@@ -356,7 +356,8 @@ TEST_F(DockerFetcherPluginTest, INTERNET_CURL_FetchImage)
   EXPECT_EQ("application/vnd.docker.distribution.manifest.v2+json",
             manifest->mediatype());
 
-  EXPECT_TRUE(os::exists(path::join(dir, manifest->config().digest())));
+  EXPECT_TRUE(os::exists(DockerFetcherPlugin::getBlobPath(
+      dir, manifest->config().digest())));
 
   for (int i = 0; i < manifest->layers_size(); i++) {
     EXPECT_TRUE(os::exists(
@@ -393,7 +394,8 @@ TEST_F(DockerFetcherPluginTest, INTERNET_CURL_InvokeFetchByName)
   EXPECT_EQ("application/vnd.docker.distribution.manifest.v2+json",
             manifest->mediatype());
 
-  EXPECT_TRUE(os::exists(path::join(dir, manifest->config().digest())));
+  EXPECT_TRUE(os::exists(DockerFetcherPlugin::getBlobPath(
+      dir, manifest->config().digest())));
 
   for (int i = 0; i < manifest->layers_size(); i++) {
     EXPECT_TRUE(os::exists(


[mesos] 12/17: Fixed docker fetcher plugin unit test for v2s2 change.

Posted by qi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

qianzhang pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit e750b7aea06d68138997cb0c96eb7654cb87d353
Author: Gilbert Song <so...@gmail.com>
AuthorDate: Wed Mar 27 12:03:00 2019 -0700

    Fixed docker fetcher plugin unit test for v2s2 change.
    
    Review: https://reviews.apache.org/r/70290
---
 src/tests/uri_fetcher_tests.cpp | 60 ++++++++++++++++++-----------------------
 1 file changed, 26 insertions(+), 34 deletions(-)

diff --git a/src/tests/uri_fetcher_tests.cpp b/src/tests/uri_fetcher_tests.cpp
index f8bacc0..bb224d8 100644
--- a/src/tests/uri_fetcher_tests.cpp
+++ b/src/tests/uri_fetcher_tests.cpp
@@ -302,29 +302,17 @@ TEST_F(DockerFetcherPluginTest, INTERNET_CURL_FetchManifest)
 
   AWAIT_READY_FOR(fetcher.get()->fetch(uri, dir), Seconds(60));
 
-  // Version 2 schema 1 image manifest test
-  Try<string> _s1Manifest = os::read(path::join(dir, "manifest"));
-  ASSERT_SOME(_s1Manifest);
-
-  Try<docker::spec::v2::ImageManifest> s1Manifest =
-    docker::spec::v2::parse(_s1Manifest.get());
-
-  ASSERT_SOME(s1Manifest);
-  EXPECT_EQ(1u, s1Manifest->schemaversion());
-  EXPECT_EQ(TEST_REPOSITORY, s1Manifest->name());
-  EXPECT_EQ("latest", s1Manifest->tag());
-
-#ifdef __WINDOWS__
   // Version 2 schema 2 image manifest test
-  Try<string> _s2Manifest = os::read(path::join(dir, "manifest_v2s2"));
-  ASSERT_SOME(_s2Manifest);
+  Try<string> _manifest = os::read(path::join(dir, "manifest"));
+  ASSERT_SOME(_manifest);
 
-  Try<docker::spec::v2_2::ImageManifest> s2Manifest =
-    docker::spec::v2_2::parse(_s2Manifest.get());
+  Try<docker::spec::v2_2::ImageManifest> manifest =
+    docker::spec::v2_2::parse(_manifest.get());
 
-  ASSERT_SOME(s2Manifest);
-  EXPECT_EQ(2u, s2Manifest->schemaversion());
-#endif // __WINDOWS__
+  ASSERT_SOME(manifest);
+  EXPECT_EQ(2u, manifest->schemaversion());
+  EXPECT_EQ("application/vnd.docker.distribution.manifest.v2+json",
+            manifest->mediatype());
 }
 
 
@@ -360,19 +348,21 @@ TEST_F(DockerFetcherPluginTest, INTERNET_CURL_FetchImage)
   Try<string> _manifest = os::read(path::join(dir, "manifest"));
   ASSERT_SOME(_manifest);
 
-  Try<docker::spec::v2::ImageManifest> manifest =
-      docker::spec::v2::parse(_manifest.get());
+  Try<docker::spec::v2_2::ImageManifest> manifest =
+      docker::spec::v2_2::parse(_manifest.get());
 
   ASSERT_SOME(manifest);
-  EXPECT_EQ(1u, manifest->schemaversion());
-  EXPECT_EQ(TEST_REPOSITORY, manifest->name());
-  EXPECT_EQ("latest", manifest->tag());
+  EXPECT_EQ(2u, manifest->schemaversion());
+  EXPECT_EQ("application/vnd.docker.distribution.manifest.v2+json",
+            manifest->mediatype());
 
-  for (int i = 0; i < manifest->fslayers_size(); i++) {
+  EXPECT_TRUE(os::exists(path::join(dir, manifest->config().digest())));
+
+  for (int i = 0; i < manifest->layers_size(); i++) {
     EXPECT_TRUE(os::exists(
         DockerFetcherPlugin::getBlobPath(
             dir,
-            manifest->fslayers(i).blobsum())));
+            manifest->layers(i).digest())));
   }
 }
 
@@ -395,19 +385,21 @@ TEST_F(DockerFetcherPluginTest, INTERNET_CURL_InvokeFetchByName)
   Try<string> _manifest = os::read(path::join(dir, "manifest"));
   ASSERT_SOME(_manifest);
 
-  Try<docker::spec::v2::ImageManifest> manifest =
-      docker::spec::v2::parse(_manifest.get());
+  Try<docker::spec::v2_2::ImageManifest> manifest =
+      docker::spec::v2_2::parse(_manifest.get());
 
   ASSERT_SOME(manifest);
-  EXPECT_EQ(1u, manifest->schemaversion());
-  EXPECT_EQ(TEST_REPOSITORY, manifest->name());
-  EXPECT_EQ("latest", manifest->tag());
+  EXPECT_EQ(2u, manifest->schemaversion());
+  EXPECT_EQ("application/vnd.docker.distribution.manifest.v2+json",
+            manifest->mediatype());
+
+  EXPECT_TRUE(os::exists(path::join(dir, manifest->config().digest())));
 
-  for (int i = 0; i < manifest->fslayers_size(); i++) {
+  for (int i = 0; i < manifest->layers_size(); i++) {
     EXPECT_TRUE(os::exists(
         DockerFetcherPlugin::getBlobPath(
             dir,
-            manifest->fslayers(i).blobsum())));
+            manifest->layers(i).digest())));
   }
 }
 


[mesos] 13/17: Added gcr registry test.

Posted by qi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

qianzhang pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 227222878c636dc0a5531e5bc7d6b7c29978c23f
Author: Gilbert Song <so...@gmail.com>
AuthorDate: Fri Mar 22 10:37:46 2019 -0700

    Added gcr registry test.
    
    Review: https://reviews.apache.org/r/70291
---
 src/tests/containerizer/provisioner_docker_tests.cpp | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/tests/containerizer/provisioner_docker_tests.cpp b/src/tests/containerizer/provisioner_docker_tests.cpp
index d3954c4..6e035c4 100644
--- a/src/tests/containerizer/provisioner_docker_tests.cpp
+++ b/src/tests/containerizer/provisioner_docker_tests.cpp
@@ -634,15 +634,18 @@ TEST_P(ProvisionerDockerHdfsTest, ROOT_ImageTarPullerHdfsFetcherSimpleCommand)
 // when specifying the repository name (e.g., 'busybox'). The registry
 // puller normalize docker official images if necessary.
 INSTANTIATE_TEST_CASE_P(
-    ImageAlpine,
+    ContainerImage,
     ProvisionerDockerTest,
     ::testing::ValuesIn(vector<string>({
         "alpine", // Verifies the normalization of the Docker repository name.
         "library/alpine",
+        "gcr.io/google-containers/busybox:1.24", // manifest.v1+prettyjws
+        "gcr.io/google-containers/busybox:1.27", // manifest.v2+json
         // TODO(alexr): The registry below is unreliable and hence disabled.
         // Consider re-enabling shall it become more stable.
         // "registry.cn-hangzhou.aliyuncs.com/acs-sample/alpine",
-        "quay.io/coreos/alpine-sh"})));
+        "quay.io/coreos/alpine-sh" // manifest.v1+prettyjws
+      })));
 
 
 // TODO(jieyu): This is a ROOT test because of MESOS-4757. Remove the


[mesos] 03/17: Windows: Fetch blobs with V2S2 Docker image manifest.

Posted by qi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

qianzhang pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 53979cc77878b546d2fa248b0a71af7beb593094
Author: Liangyu Zhao <t-...@microsoft.com>
AuthorDate: Tue Aug 28 17:08:59 2018 -0700

    Windows: Fetch blobs with V2S2 Docker image manifest.
    
    DockerFetcher now fetches both V2S1 and V2S2 manifests to save on
    disk when agent is running on Windows. Linux part of the code in
    agent is unchanged. In addition to fetching from DockerHub,
    DockerFetcher now supports fetching from foreign URLs provided in
    V2S2 Docker image manifest.
    
    Review: https://reviews.apache.org/r/68454/
---
 src/uri/fetchers/docker.cpp | 364 ++++++++++++++++++++++++++++++++++++++------
 src/uri/fetchers/docker.hpp |   4 +
 2 files changed, 319 insertions(+), 49 deletions(-)

diff --git a/src/uri/fetchers/docker.cpp b/src/uri/fetchers/docker.cpp
index 49dfda4..c1012cb 100644
--- a/src/uri/fetchers/docker.cpp
+++ b/src/uri/fetchers/docker.cpp
@@ -315,7 +315,11 @@ static Future<int> download(
             (output.isFailed() ? output.failure() : "discarded"));
       }
 
+#ifdef __WINDOWS__
+      vector<string> tokens = strings::tokenize(output.get(), "\r\n", 2);
+#else
       vector<string> tokens = strings::tokenize(output.get(), "\n", 2);
+#endif // __WINDOWS__
       if (tokens.empty()) {
         return Failure("Unexpected 'curl' output: " + output.get());
       }
@@ -343,13 +347,25 @@ static Future<int> download(
 
 static Future<int> download(
     const URI& uri,
+    const string& url,
     const string& directory,
     const http::Headers& headers,
     const Option<Duration>& stallTimeout)
 {
-  const string blobPath = path::join(directory, Path(uri.path()).basename());
+  string blobSum;
+
+  auto lastSlash = uri.path().find_last_of('/');
+  if (lastSlash == string::npos) {
+    blobSum = uri.path();
+  } else {
+    blobSum = uri.path().substr(lastSlash + 1);
+  }
+
   return download(
-      strings::trim(stringify(uri)), blobPath, headers, stallTimeout);
+      url,
+      DockerFetcherPlugin::getBlobPath(directory, blobSum),
+      headers,
+      stallTimeout);
 }
 
 
@@ -442,6 +458,20 @@ private:
       const http::Headers& authHeaders,
       const http::Response& response);
 
+  Future<Nothing> ___fetch(
+      const URI& uri,
+      const string& directory,
+      const http::Headers& authHeaders,
+      const spec::v2::ImageManifest& manifest);
+
+  Try<spec::v2::ImageManifest> saveV2S1Manifest(
+      const string& directory,
+      const http::Response& response);
+
+  Try<spec::v2_2::ImageManifest> saveV2S2Manifest(
+      const string& directory,
+      const http::Response& response);
+
   Future<Nothing> fetchBlob(
       const URI& uri,
       const string& directory,
@@ -453,7 +483,19 @@ private:
       const URI& blobUri,
       const http::Headers& basicAuthHeaders);
 
-  Future<Nothing> __fetchBlob(int code);
+#ifdef __WINDOWS__
+  Future<Nothing> urlFetchBlob(
+      const URI& uri,
+      const string& directory,
+      const URI& blobUri,
+      const http::Headers& authHeaders);
+
+  Future<Nothing> _urlFetchBlob(
+      const string& directory,
+      const URI& blobUri,
+      const http::Headers& authHeaders,
+      vector<string> urls);
+#endif
 
   // Returns a token-based authorization header. Basic authorization
   // header may be required to get a proper authorization token.
@@ -516,6 +558,32 @@ Try<Owned<Fetcher::Plugin>> DockerFetcherPlugin::create(const Flags& flags)
 }
 
 
+string DockerFetcherPlugin::getBlobPath(
+    const string& directory,
+    const string& blobSum)
+{
+#ifdef __WINDOWS__
+  std::string path = path::join(directory, blobSum);
+
+  // The colon in disk designator is preserved.
+  auto i = 0;
+  if (path::absolute(path)) {
+    i = path.find_first_of(':') + 1;
+  }
+
+  for (; i < path.size(); ++i) {
+    if (path[i] == ':') {
+      path[i] = '_';
+    }
+  }
+
+  return path;
+#else
+  return path::join(directory, blobSum);
+#endif
+}
+
+
 DockerFetcherPlugin::DockerFetcherPlugin(
     Owned<DockerFetcherPluginProcess> _process)
   : process(_process)
@@ -667,10 +735,94 @@ Future<Nothing> DockerFetcherPluginProcess::__fetch(
     const http::Headers& authHeaders,
     const http::Response& response)
 {
+  Try<spec::v2::ImageManifest> manifest =
+      saveV2S1Manifest(directory, response);
+
+  if (manifest.isError()) {
+    return Failure(manifest.error());
+  }
+
+#ifdef __WINDOWS__
+  URI manifestUri = getManifestUri(uri);
+
+  // Fetching version 2 schema 2 manifest:
+  // https://docs.docker.com/registry/spec/manifest-v2-2/
+  //
+  // If fetch is failed, program continues without schema 2 manifest.
+  http::Headers s2ManifestHeaders = {
+    {"Accept", "application/vnd.docker.distribution.manifest.v2+json"}
+  };
+
+  return curl(manifestUri, s2ManifestHeaders + authHeaders, stallTimeout)
+      .then(defer(self(), [=](const http::Response& response)
+          -> Future<Nothing> {
+        Try<spec::v2_2::ImageManifest> manifest =
+            saveV2S2Manifest(directory, response);
+
+        if (manifest.isError()) {
+          LOG(WARNING) << "Failed to fetch schema 2 manifest: "
+                       << manifest.error();
+        }
+
+        return Nothing();
+      }))
+      .then(defer(self(),
+                  &Self::___fetch,
+                  uri,
+                  directory,
+                  authHeaders,
+                  manifest.get()));
+#else
+  return ___fetch(uri, directory, authHeaders, manifest.get());
+#endif
+}
+
+
+Future<Nothing> DockerFetcherPluginProcess::___fetch(
+    const URI& uri,
+    const string& directory,
+    const http::Headers& authHeaders,
+    const spec::v2::ImageManifest& manifest)
+{
+  // No need to proceed if we only want manifest.
+  if (uri.scheme() == "docker-manifest") {
+    return Nothing();
+  }
+
+  // Download all the filesystem layers.
+  vector<Future<Nothing>> futures;
+  for (int i = 0; i < manifest.fslayers_size(); i++) {
+    URI blob = uri::docker::blob(
+        uri.path(),                         // The 'repository'.
+        manifest.fslayers(i).blobsum(),    // The 'digest'.
+        uri.host(),                         // The 'registry'.
+        (uri.has_fragment()                 // The 'scheme'.
+          ? Option<string>(uri.fragment())
+          : None()),
+        (uri.has_port()                     // The 'port'.
+          ? Option<int>(uri.port())
+          : None()));
+
+    // Use the same 'authHeaders' as for the manifest to pull the blobs.
+    futures.push_back(fetchBlob(
+        blob,
+        directory,
+        authHeaders));
+  }
+
+  return collect(futures)
+    .then([]() -> Future<Nothing> { return Nothing(); });
+}
+
+
+Try<spec::v2::ImageManifest> DockerFetcherPluginProcess::saveV2S1Manifest(
+    const string& directory,
+    const http::Response& response)
+{
   if (response.code != http::Status::OK) {
-    return Failure(
+    return Error(
         "Unexpected HTTP response '" + response.status + "' "
-        "when trying to get the manifest");
+        "when trying to get the schema 1 manifest");
   }
 
   CHECK_EQ(response.type, http::Response::BODY);
@@ -697,54 +849,78 @@ Future<Nothing> DockerFetcherPluginProcess::__fetch(
           "application/json");
 
     if (!isV2Schema1) {
-      return Failure("Unsupported manifest MIME type: " + contentType.get());
+      return Error(
+          "Unsupported schema 1 manifest MIME type: " +
+          contentType.get());
     }
   }
 
   Try<spec::v2::ImageManifest> manifest = spec::v2::parse(response.body);
   if (manifest.isError()) {
-    return Failure("Failed to parse the image manifest: " + manifest.error());
+    return Error(
+        "Failed to parse the schema 1 image manifest: " +
+        manifest.error());
   }
 
   // Save manifest to 'directory'.
   Try<Nothing> write = os::write(
-      path::join(directory, "manifest"),
-      response.body);
+      path::join(directory, "manifest"), response.body);
 
   if (write.isError()) {
-    return Failure(
-        "Failed to write the image manifest to "
-        "'" + directory + "': " + write.error());
+    return Error(
+        "Failed to write the schema 1 image manifest to '" +
+        directory + "': " + write.error());
   }
 
-  // No need to proceed if we only want manifest.
-  if (uri.scheme() == "docker-manifest") {
-    return Nothing();
+  return manifest;
+}
+
+
+Try<spec::v2_2::ImageManifest> DockerFetcherPluginProcess::saveV2S2Manifest(
+    const string& directory,
+    const http::Response& response)
+{
+  if (response.code != http::Status::OK) {
+    return Error(
+        "Unexpected HTTP response '" + response.status +
+        "' when trying to get the schema 2 manifest");
   }
 
-  // Download all the filesystem layers.
-  vector<Future<Nothing>> futures;
-  for (int i = 0; i < manifest->fslayers_size(); i++) {
-    URI blob = uri::docker::blob(
-        uri.path(),                         // The 'repository'.
-        manifest->fslayers(i).blobsum(),    // The 'digest'.
-        uri.host(),                         // The 'registry'.
-        (uri.has_fragment()                 // The 'scheme'.
-          ? Option<string>(uri.fragment())
-          : None()),
-        (uri.has_port()                     // The 'port'.
-          ? Option<int>(uri.port())
-          : None()));
+  Option<string> contentType = response.headers.get("Content-Type");
+  if (contentType.isSome()) {
+    bool isV2Schema2 =
+      strings::startsWith(
+          contentType.get(),
+          "application/vnd.docker.distribution.manifest.v2") ||
+      strings::startsWith(
+          contentType.get(),
+          "application/json");
 
-    // Use the same 'authHeaders' as for the manifest to pull the blobs.
-    futures.push_back(fetchBlob(
-        blob,
-        directory,
-        authHeaders));
+    if (!isV2Schema2) {
+      return Error(
+          "Unsupported schema 2 manifest MIME type: " +
+          contentType.get());
+    }
   }
 
-  return collect(futures)
-    .then([]() -> Future<Nothing> { return Nothing(); });
+  Try<spec::v2_2::ImageManifest> manifest = spec::v2_2::parse(response.body);
+  if (manifest.isError()) {
+    return Error(
+        "Failed to parse the schema 2 manifest: " +
+        manifest.error());
+  }
+
+  // Save manifest to 'directory'.
+  Try<Nothing> write = os::write(
+      path::join(directory, "manifest_v2s2"), response.body);
+
+  if (write.isError()) {
+    return Error(
+        "Failed to write the schema 2 image manifest to '" +
+        directory + "': " + write.error());
+  }
+
+  return manifest;
 }
 
 
@@ -755,7 +931,12 @@ Future<Nothing> DockerFetcherPluginProcess::fetchBlob(
 {
   URI blobUri = getBlobUri(uri);
 
-  return download(blobUri, directory, authHeaders, stallTimeout)
+  return download(
+      blobUri,
+      strings::trim(stringify(blobUri)),
+      directory,
+      authHeaders,
+      stallTimeout)
     .then(defer(self(), [=](int code) -> Future<Nothing> {
       if (code == http::Status::UNAUTHORIZED) {
         // If we get a '401 Unauthorized', we assume that 'authHeaders'
@@ -765,7 +946,17 @@ Future<Nothing> DockerFetcherPluginProcess::fetchBlob(
         return _fetchBlob(uri, directory, blobUri, authHeaders);
       }
 
-      return __fetchBlob(code);
+      if (code == http::Status::OK) {
+        return Nothing();
+      }
+
+#ifdef __WINDOWS__
+      return urlFetchBlob(uri, directory, blobUri, authHeaders);
+#else
+      return Failure(
+          "Unexpected HTTP response '" + http::Status::string(code) + "' "
+          "when trying to download the blob");
+#endif
     }));
 }
 
@@ -793,25 +984,100 @@ Future<Nothing> DockerFetcherPluginProcess::_fetchBlob(
       return getAuthHeader(blobUri, basicAuthHeaders, response)
         .then(defer(self(), [=](
             const http::Headers& authHeaders) -> Future<Nothing> {
-          return download(blobUri, directory, authHeaders, stallTimeout)
-            .then(defer(self(),
-                        &Self::__fetchBlob,
-                        lambda::_1));
+          return download(
+              blobUri,
+              strings::trim(stringify(blobUri)),
+              directory,
+              authHeaders,
+              stallTimeout)
+            .then(defer(self(), [=](int code) -> Future<Nothing> {
+              if (code == http::Status::OK) {
+                return Nothing();
+              }
+
+#ifdef __WINDOWS__
+              return urlFetchBlob(uri, directory, blobUri, authHeaders);
+#else
+              return Failure(
+                  "Unexpected HTTP response '" + http::Status::string(code) +
+                  "' when trying to download blob '" +
+                  strings::trim(stringify(blobUri)) +
+                  "' with schema 1 manifest");
+#endif
+            }));
         }));
     }));
 }
 
 
-Future<Nothing> DockerFetcherPluginProcess::__fetchBlob(int code)
+#ifdef __WINDOWS__
+Future<Nothing> DockerFetcherPluginProcess::urlFetchBlob(
+      const URI& uri,
+      const string& directory,
+      const URI& blobUri,
+      const http::Headers& authHeaders)
 {
-  if (code == http::Status::OK) {
-    return Nothing();
+  Try<string> _manifest = os::read(path::join(directory, "manifest_v2s2"));
+  if (_manifest.isError()) {
+    return Failure("Schema 2 manifest does not exist");
+  }
+
+  Try<spec::v2_2::ImageManifest> manifest = spec::v2_2::parse(_manifest.get());
+  if (manifest.isError()) {
+    return Failure(
+        "Failed to parse the schema 2 manifest: " +
+        manifest.error());
+  }
+
+  const string& blobsum = uri.query(); // blobsum or digest of blob
+  vector<string> urls;
+  for (int i = 0; i < manifest->layers_size(); i++) {
+    if (blobsum != manifest->layers(i).digest()) {
+      continue;
+    }
+    for (int j = 0; j < manifest->layers(i).urls_size(); j++) {
+      urls.emplace_back(manifest->layers(i).urls(j));
+    }
+    break;
+  }
+
+  if (urls.empty()) {
+    return Failure("No foreign url found from schema 2 manifest");
+  }
+
+  return _urlFetchBlob(directory, blobUri, authHeaders, urls);
+}
+
+
+Future<Nothing> DockerFetcherPluginProcess::_urlFetchBlob(
+      const string& directory,
+      const URI& blobUri,
+      const http::Headers& authHeaders,
+      vector<string> urls)
+{
+  if (urls.empty()) {
+    return Failure("Failed to fetch with foreign urls");
   }
 
-  return Failure(
-      "Unexpected HTTP response '" + http::Status::string(code) + "' "
-      "when trying to download the blob");
+  string url = urls.back();
+  urls.pop_back();
+  return download(blobUri, url, directory, authHeaders, stallTimeout)
+      .then(defer(self(), [=](int code) -> Future<Nothing> {
+        if (code == http::Status::OK) {
+          return Nothing();
+        }
+
+        LOG(WARNING) << "Unexpected HTTP response '"
+                      << http::Status::string(code)
+                      << "' when trying to download blob '"
+                      << strings::trim(stringify(blobUri))
+                      << "' from '" << url
+                      << "' in schema 2 manifest";
+
+        return _urlFetchBlob(directory, blobUri, authHeaders, urls);
+      }));
 }
+#endif
 
 
 Future<http::Headers> DockerFetcherPluginProcess::getAuthHeader(
@@ -911,7 +1177,7 @@ URI DockerFetcherPluginProcess::getManifestUri(const URI& uri)
 
   return uri::construct(
       scheme,
-      path::join("/v2", uri.path(), "manifests", uri.query()),
+      strings::join("/", "/v2", uri.path(), "manifests", uri.query()),
       uri.host(),
       (uri.has_port() ? Option<int>(uri.port()) : None()));
 }
@@ -926,7 +1192,7 @@ URI DockerFetcherPluginProcess::getBlobUri(const URI& uri)
 
   return uri::construct(
       scheme,
-      path::join("/v2", uri.path(), "blobs", uri.query()),
+      strings::join("/", "/v2", uri.path(), "blobs", uri.query()),
       uri.host(),
       (uri.has_port() ? Option<int>(uri.port()) : None()));
 }
diff --git a/src/uri/fetchers/docker.hpp b/src/uri/fetchers/docker.hpp
index 7a6193d..2abe735 100644
--- a/src/uri/fetchers/docker.hpp
+++ b/src/uri/fetchers/docker.hpp
@@ -47,6 +47,10 @@ public:
 
   static Try<process::Owned<Fetcher::Plugin>> create(const Flags& flags);
 
+  static std::string getBlobPath(
+      const std::string& directory,
+      const std::string& blobSum);
+
   ~DockerFetcherPlugin() override;
 
   std::set<std::string> schemes() const override;


[mesos] 10/17: Supported docker manifest v2 schema2.

Posted by qi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

qianzhang pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit adc699d04cc7548fac9ce3b983df2faf52a57ff6
Author: Gilbert Song <so...@gmail.com>
AuthorDate: Fri Mar 22 00:32:30 2019 -0700

    Supported docker manifest v2 schema2.
    
    Review: https://reviews.apache.org/r/70288
---
 .../mesos/provisioner/docker/registry_puller.cpp   | 266 +++++++++++++++---
 .../mesos/provisioner/docker/store.cpp             |  52 +++-
 src/uri/fetchers/docker.cpp                        | 296 ++++++++-------------
 3 files changed, 392 insertions(+), 222 deletions(-)

diff --git a/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp b/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
index 7778976..35b6afb 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
@@ -25,6 +25,7 @@
 
 #include <stout/os/exists.hpp>
 #include <stout/os/mkdir.hpp>
+#include <stout/os/rename.hpp>
 #include <stout/os/rm.hpp>
 #include <stout/os/write.hpp>
 
@@ -92,13 +93,34 @@ private:
     const hashset<string>& blobSums,
     const string& backend);
 
-  Future<hashset<string>> fetchBlobs(
+  Future<Image> ____pull(
     const spec::ImageReference& reference,
     const string& directory,
+    const spec::v2_2::ImageManifest& manifest,
+    const hashset<string>& digests,
+    const string& backend);
+
+  Future<hashset<string>> fetchBlobs(
+    const spec::ImageReference& normalizedRef,
+    const string& directory,
     const spec::v2::ImageManifest& manifest,
     const string& backend,
     const Option<Secret::Value>& config);
 
+  Future<hashset<string>> fetchBlobs(
+      const spec::ImageReference& normalizedRef,
+      const string& directory,
+      const spec::v2_2::ImageManifest& manifest,
+      const string& backend,
+      const Option<Secret::Value>& config);
+
+  Future<hashset<string>> fetchBlobs(
+      const spec::ImageReference& normalizedRef,
+      const string& directory,
+      const hashset<string>& digests,
+      const string& backend,
+      const Option<Secret::Value>& config);
+
   RegistryPullerProcess(const RegistryPullerProcess&) = delete;
   RegistryPullerProcess& operator=(const RegistryPullerProcess&) = delete;
 
@@ -237,31 +259,31 @@ Future<Image> RegistryPullerProcess::pull(
 
 
 Future<Image> RegistryPullerProcess::_pull(
-    const spec::ImageReference& _reference,
+    const spec::ImageReference& reference,
     const string& directory,
     const string& backend,
     const Option<Secret::Value>& config)
 {
-  spec::ImageReference reference = normalize(_reference, defaultRegistryUrl);
+  spec::ImageReference normalizedRef = normalize(reference, defaultRegistryUrl);
 
   URI manifestUri;
-  if (reference.has_registry()) {
-    Result<int> port = spec::getRegistryPort(reference.registry());
+  if (normalizedRef.has_registry()) {
+    Result<int> port = spec::getRegistryPort(normalizedRef.registry());
     if (port.isError()) {
       return Failure("Failed to get registry port: " + port.error());
     }
 
-    Try<string> scheme = spec::getRegistryScheme(reference.registry());
+    Try<string> scheme = spec::getRegistryScheme(normalizedRef.registry());
     if (scheme.isError()) {
       return Failure("Failed to get registry scheme: " + scheme.error());
     }
 
     manifestUri = uri::docker::manifest(
-        reference.repository(),
-        (reference.has_digest()
-          ? reference.digest()
-          : (reference.has_tag() ? reference.tag() : "latest")),
-        spec::getRegistryHost(reference.registry()),
+        normalizedRef.repository(),
+        (normalizedRef.has_digest()
+          ? normalizedRef.digest()
+          : (normalizedRef.has_tag() ? normalizedRef.tag() : "latest")),
+        spec::getRegistryHost(normalizedRef.registry()),
         scheme.get(),
         port.isSome() ? port.get() : Option<int>());
   } else {
@@ -274,19 +296,24 @@ Future<Image> RegistryPullerProcess::_pull(
       : Option<int>();
 
     manifestUri = uri::docker::manifest(
-        reference.repository(),
-        (reference.has_digest()
-          ? reference.digest()
-          : (reference.has_tag() ? reference.tag() : "latest")),
+        normalizedRef.repository(),
+        (normalizedRef.has_digest()
+          ? normalizedRef.digest()
+          : (normalizedRef.has_tag() ? normalizedRef.tag() : "latest")),
         registry,
         defaultRegistryUrl.scheme,
         port);
   }
 
-  VLOG(1) << "Pulling image '" << reference
+  VLOG(1) << "Pulling image '" << normalizedRef
           << "' from '" << manifestUri
           << "' to '" << directory << "'";
 
+  // Pass the original 'reference' along to subsequent methods
+  // because metadata manager may already has this reference in
+  // cache. This is necessary to ensure the backward compatibility
+  // after upgrading to the version including MESOS-9675 for
+  // docker manifest v2 schema2 support.
   return fetcher->fetch(
       manifestUri,
       directory,
@@ -306,21 +333,57 @@ Future<Image> RegistryPullerProcess::__pull(
     return Failure("Failed to read the manifest: " + _manifest.error());
   }
 
-  Try<spec::v2::ImageManifest> manifest = spec::v2::parse(_manifest.get());
+  VLOG(1) << "The manifest for image '" << reference << "' is '"
+          << _manifest.get() << "'";
+
+  // To ensure backward compatibility in upgrade case for docker
+  // manifest v2 schema2 support, it is unavoidable to call
+  // 'normalize()' twice because some existing image may already
+  // be cached by metadata manager before upgrade and now metadata
+  // persists the cache from the image information constructed at
+  // registry puller. Please see MESOS-9675 for details.
+  spec::ImageReference normalizedRef = normalize(reference, defaultRegistryUrl);
+
+  Try<JSON::Object> json = JSON::parse<JSON::Object>(_manifest.get());
+  if (json.isError()) {
+    return Failure("Failed to parse the manifest JSON: " + json.error());
+  }
+
+  Result<JSON::Number> schemaVersion = json->at<JSON::Number>("schemaVersion");
+  if (schemaVersion.isError()) {
+    return Failure(
+        "Failed to find manifest schema version: " + schemaVersion.error());
+  }
+
+  if (schemaVersion.isSome() && schemaVersion->as<int>() == 2) {
+    Try<spec::v2_2::ImageManifest> manifest = spec::v2_2::parse(json.get());
+    if (manifest.isError()) {
+      return Failure("Failed to parse the manifest: " + manifest.error());
+    }
+
+    return fetchBlobs(normalizedRef, directory, manifest.get(), backend, config)
+      .then(defer(self(),
+                  &Self::____pull,
+                  reference,
+                  directory,
+                  manifest.get(),
+                  lambda::_1,
+                  backend));
+  }
+
+  // By default treat the manifest format as schema 1.
+  Try<spec::v2::ImageManifest> manifest = spec::v2::parse(json.get());
   if (manifest.isError()) {
     return Failure("Failed to parse the manifest: " + manifest.error());
   }
 
-  VLOG(1) << "The manifest for image '" << reference << "' is '"
-          << _manifest.get() << "'";
-
   // NOTE: This can be a CHECK (i.e., shouldn't happen). However, in
   // case docker has bugs, we return a Failure instead.
   if (manifest->fslayers_size() != manifest->history_size()) {
     return Failure("'fsLayers' and 'history' have different size in manifest");
   }
 
-  return fetchBlobs(reference, directory, manifest.get(), backend, config)
+  return fetchBlobs(normalizedRef, directory, manifest.get(), backend, config)
     .then(defer(self(),
                 &Self::___pull,
                 reference,
@@ -437,9 +500,94 @@ Future<Image> RegistryPullerProcess::___pull(
 }
 
 
-Future<hashset<string>> RegistryPullerProcess::fetchBlobs(
+Future<Image> RegistryPullerProcess::____pull(
     const spec::ImageReference& reference,
     const string& directory,
+    const spec::v2_2::ImageManifest& manifest,
+    const hashset<string>& digests,
+    const string& backend)
+{
+  hashset<string> uniqueIds;
+  vector<string> layerIds;
+  vector<Future<Nothing>> futures;
+
+  for (int i = 0; i < manifest.layers_size(); i++) {
+    const string& digest = manifest.layers(i).digest();
+    if (uniqueIds.contains(digest)) {
+      continue;
+    }
+
+    layerIds.push_back(digest);
+    uniqueIds.insert(digest);
+
+    // Skip if the layer is already in the store.
+    if (os::exists(paths::getImageLayerRootfsPath(storeDir, digest, backend))) {
+      continue;
+    }
+
+    const string layerPath = path::join(directory, digest);
+    const string originalTar = path::join(directory, digest);
+    const string tar = path::join(directory, digest + "-archive");
+    const string rootfs = paths::getImageLayerRootfsPath(layerPath, backend);
+
+    VLOG(1) << "Moving layer tar ball '" << originalTar
+            << "' to '" << tar << "'";
+
+    // Move layer tar ball to use its name for the extracted layer directory.
+    Try<Nothing> rename = os::rename(originalTar, tar);
+    if (rename.isError()) {
+      return Failure(
+          "Failed to move the layer tar ball from '" + originalTar +
+          "' to '" + tar + "': " + rename.error());
+    }
+
+    VLOG(1) << "Extracting layer tar ball '" << tar
+            << "' to rootfs '" << rootfs << "'";
+
+    // NOTE: This will create 'layerPath' as well.
+    Try<Nothing> mkdir = os::mkdir(rootfs, true);
+    if (mkdir.isError()) {
+      return Failure(
+          "Failed to create rootfs directory '" + rootfs + "' "
+          "for layer '" + digest + "': " + mkdir.error());
+    }
+
+    futures.push_back(command::untar(Path(tar), Path(rootfs)));
+  }
+
+  return collect(futures)
+    .then([=]() -> Future<Image> {
+      // Remove the tarballs after the extraction.
+      foreach (const string& digest, digests) {
+        // Skip if the digest represents the image manifest config.
+        if (digest == manifest.config().digest()) {
+          continue;
+        }
+
+        const string tar = path::join(directory, digest + "-archive");
+
+        Try<Nothing> rm = os::rm(tar);
+        if (rm.isError()) {
+          return Failure(
+              "Failed to remove '" + tar + "' after extraction: " + rm.error());
+        }
+      }
+
+      Image image;
+      image.set_config_digest(manifest.config().digest());
+      image.mutable_reference()->CopyFrom(reference);
+      foreach (const string& layerId, layerIds) {
+        image.add_layer_ids(layerId);
+      }
+
+      return image;
+    });
+}
+
+
+Future<hashset<string>> RegistryPullerProcess::fetchBlobs(
+    const spec::ImageReference& normalizedRef,
+    const string& directory,
     const spec::v2::ImageManifest& manifest,
     const string& backend,
     const Option<Secret::Value>& config)
@@ -464,24 +612,74 @@ Future<hashset<string>> RegistryPullerProcess::fetchBlobs(
     const string& blobSum = manifest.fslayers(i).blobsum();
 
     VLOG(1) << "Fetching blob '" << blobSum << "' for layer '"
-            << v1.id() << "' of image '" << reference << "'";
+            << v1.id() << "' of image '" << normalizedRef << "'";
 
     blobSums.insert(blobSum);
   }
 
-  // Now, actually fetch the blobs.
+  return fetchBlobs(normalizedRef, directory, blobSums, backend, config);
+}
+
+
+Future<hashset<string>> RegistryPullerProcess::fetchBlobs(
+    const spec::ImageReference& normalizedRef,
+    const string& directory,
+    const spec::v2_2::ImageManifest& manifest,
+    const string& backend,
+    const Option<Secret::Value>& config)
+{
+  // First, find all the blobs that need to be fetched.
+  //
+  // NOTE: There might exist duplicated digests in 'layers'. We
+  // just need to fetch one of them.
+  hashset<string> digests;
+
+  const string& configDigest = manifest.config().digest();
+  if (!os::exists(paths::getImageLayerPath(storeDir, configDigest))) {
+    VLOG(1) << "Fetching config '" << configDigest << "' for image '"
+            << normalizedRef << "'";
+
+    digests.insert(configDigest);
+  }
+
+  for (int i = 0; i < manifest.layers_size(); i++) {
+    const string& digest = manifest.layers(i).digest();
+
+    // Check if the layer is in the store or not. If yes, skip the unnecessary
+    // fetching.
+    if (os::exists(paths::getImageLayerRootfsPath(storeDir, digest, backend))) {
+      continue;
+    }
+
+    VLOG(1) << "Fetching layer '" << digest << "' for image '"
+            << normalizedRef << "'";
+
+    digests.insert(digest);
+  }
+
+  return fetchBlobs(normalizedRef, directory, digests, backend, config);
+}
+
+
+Future<hashset<string>> RegistryPullerProcess::fetchBlobs(
+    const spec::ImageReference& normalizedRef,
+    const string& directory,
+    const hashset<string>& digests,
+    const string& backend,
+    const Option<Secret::Value>& config)
+{
   vector<Future<Nothing>> futures;
 
-  foreach (const string& blobSum, blobSums) {
+  foreach (const string& digest, digests) {
     URI blobUri;
 
-    if (reference.has_registry()) {
-      Result<int> port = spec::getRegistryPort(reference.registry());
+    if (normalizedRef.has_registry()) {
+      Result<int> port = spec::getRegistryPort(normalizedRef.registry());
       if (port.isError()) {
         return Failure("Failed to get registry port: " + port.error());
       }
 
-      Try<string> scheme = spec::getRegistryScheme(reference.registry());
+      Try<string> scheme = spec::getRegistryScheme(normalizedRef.registry());
       if (scheme.isError()) {
         return Failure("Failed to get registry scheme: " + scheme.error());
       }
@@ -490,9 +688,9 @@ Future<hashset<string>> RegistryPullerProcess::fetchBlobs(
       // an URL scheme must be specified in '--docker_registry', because
       // there is no scheme allowed in docker image name.
       blobUri = uri::docker::blob(
-          reference.repository(),
-          blobSum,
-          spec::getRegistryHost(reference.registry()),
+          normalizedRef.repository(),
+          digest,
+          spec::getRegistryHost(normalizedRef.registry()),
           scheme.get(),
           port.isSome() ? port.get() : Option<int>());
     } else {
@@ -505,8 +703,8 @@ Future<hashset<string>> RegistryPullerProcess::fetchBlobs(
         : Option<int>();
 
       blobUri = uri::docker::blob(
-          reference.repository(),
-          blobSum,
+          normalizedRef.repository(),
+          digest,
           registry,
           defaultRegistryUrl.scheme,
           port);
@@ -519,7 +717,7 @@ Future<hashset<string>> RegistryPullerProcess::fetchBlobs(
   }
 
   return collect(futures)
-    .then([blobSums]() -> hashset<string> { return blobSums; });
+    .then([digests]() -> hashset<string> { return digests; });
 }
 
 } // namespace docker {
diff --git a/src/slave/containerizer/mesos/provisioner/docker/store.cpp b/src/slave/containerizer/mesos/provisioner/docker/store.cpp
index e0f2371..909364d 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/store.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/store.cpp
@@ -326,6 +326,13 @@ Future<Image> StoreProcess::_get(
       }
     }
 
+    if (image->has_config_digest() &&
+        !os::exists(paths::getImageLayerPath(
+            flags.docker_store_dir,
+            image->config_digest()))) {
+      layerMissed = true;
+    }
+
     if (!layerMissed) {
       return image.get();
     }
@@ -393,16 +400,25 @@ Future<ImageInfo> StoreProcess::__get(
         backend));
   }
 
-  const string path = paths::getImageLayerManifestPath(
-      flags.docker_store_dir,
-      image.layer_ids(image.layer_ids_size() - 1));
+  string configPath;
+  if (image.has_config_digest()) {
+    // Optional 'config_digest' will only be set for docker manifest
+    // V2 Schema2 case.
+    configPath = paths::getImageLayerPath(
+        flags.docker_store_dir,
+        image.config_digest());
+  } else {
+    // Read the manifest from the last layer because all runtime config
+    // are merged at the leaf already.
+    configPath = paths::getImageLayerManifestPath(
+        flags.docker_store_dir,
+        image.layer_ids(image.layer_ids_size() - 1));
+  }
 
-  // Read the manifest from the last layer because all runtime config
-  // are merged at the leaf already.
-  Try<string> manifest = os::read(path);
+  Try<string> manifest = os::read(configPath);
   if (manifest.isError()) {
     return Failure(
-        "Failed to read manifest from '" + path + "': " +
+        "Failed to read manifest from '" + configPath + "': " +
         manifest.error());
   }
 
@@ -411,7 +427,7 @@ Future<ImageInfo> StoreProcess::__get(
 
   if (v1.isError()) {
     return Failure(
-        "Failed to parse docker v1 manifest from '" + path + "': " +
+        "Failed to parse docker v1 manifest from '" + configPath + "': " +
         v1.error());
   }
 
@@ -430,7 +446,25 @@ Future<Image> StoreProcess::moveLayers(
   }
 
   return collect(futures)
-    .then([image]() -> Image { return image; });
+    .then([=]() -> Future<Image> {
+      if (image.has_config_digest()) {
+        const string configSource = path::join(staging, image.config_digest());
+        const string configTarget = paths::getImageLayerPath(
+            flags.docker_store_dir,
+            image.config_digest());
+
+        if (!os::exists(configTarget)) {
+          Try<Nothing> rename = os::rename(configSource, configTarget);
+          if (rename.isError()) {
+            return Failure(
+                "Failed to move image manifest config from '" + configSource +
+                "' to '" + configTarget + "': " + rename.error());
+          }
+        }
+      }
+
+      return image;
+    });
 }
 
 
diff --git a/src/uri/fetchers/docker.cpp b/src/uri/fetchers/docker.cpp
index ffb5194..e11e54f 100644
--- a/src/uri/fetchers/docker.cpp
+++ b/src/uri/fetchers/docker.cpp
@@ -458,19 +458,11 @@ private:
       const http::Headers& authHeaders,
       const http::Response& response);
 
-  Future<Nothing> ___fetch(
+  Future<Nothing> fetchBlobs(
       const URI& uri,
       const string& directory,
-      const http::Headers& authHeaders,
-      const spec::v2::ImageManifest& manifest);
-
-  Try<spec::v2::ImageManifest> saveV2S1Manifest(
-      const string& directory,
-      const http::Response& response);
-
-  Try<spec::v2_2::ImageManifest> saveV2S2Manifest(
-      const string& directory,
-      const http::Response& response);
+      const hashset<string>& digests,
+      const http::Headers& authHeaders);
 
   Future<Nothing> fetchBlob(
       const URI& uri,
@@ -681,13 +673,14 @@ Future<Nothing> DockerFetcherPluginProcess::fetch(
 
   URI manifestUri = getManifestUri(uri);
 
-  // Request a Version 2 Schema 1 manifest. The MIME type of a Schema 1
-  // manifest is described in the following link:
-  // https://docs.docker.com/registry/spec/manifest-v2-1/
-  // Note: The 'Accept' header is required for Amazon ECR. See:
-  // https://forums.aws.amazon.com/message.jspa?messageID=780440
+  // Both docker manifest v2s1 and v2s2 are supported. We put all
+  // accept headers to the curl request for manifest because:
+  // 1. v2+json is needed since some registries start to deprecate
+  //    schema 1 support.
+  // 2. Some registries only support one schema type.
   http::Headers manifestHeaders = {
     {"Accept",
+     "application/vnd.docker.distribution.manifest.v2+json,"
      "application/vnd.docker.distribution.manifest.v1+json,"
      "application/vnd.docker.distribution.manifest.v1+prettyjws"
     }
@@ -738,71 +731,129 @@ Future<Nothing> DockerFetcherPluginProcess::__fetch(
     const http::Headers& authHeaders,
     const http::Response& response)
 {
-  Try<spec::v2::ImageManifest> manifest =
-      saveV2S1Manifest(directory, response);
-
-  if (manifest.isError()) {
-    return Failure(manifest.error());
+  if (response.code != http::Status::OK) {
+    return Failure(
+        "Unexpected HTTP response '" + response.status + "' "
+        "when trying to get the manifest");
   }
 
-#ifdef __WINDOWS__
-  URI manifestUri = getManifestUri(uri);
+  CHECK_EQ(response.type, http::Response::BODY);
 
-  // Fetching version 2 schema 2 manifest:
-  // https://docs.docker.com/registry/spec/manifest-v2-2/
+  Option<string> contentType = response.headers.get("Content-Type");
+  if (contentType.isNone()) {
+    return Failure("No Content-Type present");
+  }
+
+  // NOTE: Docker supports the following five media types.
   //
-  // If fetch is failed, program continues without schema 2 manifest.
+  // V2 schema 1 manifest:
+  // 1. application/vnd.docker.distribution.manifest.v1+json
+  // 2. application/vnd.docker.distribution.manifest.v1+prettyjws
+  // 3. application/json
   //
-  // Schema 2 manifest may have foreign URLs to fetch blobs from servers
-  // other than Docker Hub. Some file layers, for example, Windows OS
-  // layers are only stored on Microsoft servers, so only with schema 2
-  // manifest, such layers can be successfully fetched.
-  http::Headers s2ManifestHeaders = {
-    {"Accept", "application/vnd.docker.distribution.manifest.v2+json"}
-  };
+  // For more details, see:
+  // https://docs.docker.com/registry/spec/manifest-v2-1/
+  //
+  // V2 schema 2 manifest:
+  // 1. application/vnd.docker.distribution.manifest.v2+json
+  // 2. application/vnd.docker.distribution.manifest.list.v2+json
+  //    (manifest list is not supported yet)
+  //
+  // For more details, see:
+  // https://docs.docker.com/registry/spec/manifest-v2-2/
+  bool isV2Schema1 =
+    strings::startsWith(
+        contentType.get(),
+        "application/vnd.docker.distribution.manifest.v1") ||
+    strings::startsWith(
+        contentType.get(),
+        "application/json");
+
+  // TODO(gilbert): Support manifest list (fat manifest) in V2 Schema2.
+  bool isV2Schema2 =
+    contentType.get() == "application/vnd.docker.distribution.manifest.v2+json";
+
+  if (isV2Schema1) {
+    // Parse V2 schema 1 image manifest.
+    Try<spec::v2::ImageManifest> manifest = spec::v2::parse(response.body);
+    if (manifest.isError()) {
+      return Failure(
+          "Failed to parse the V2 Schema 1 image manifest: " +
+          manifest.error());
+    }
 
-  return curl(manifestUri, s2ManifestHeaders + authHeaders, stallTimeout)
-      .then(defer(self(), [=](const http::Response& response)
-          -> Future<Nothing> {
-        Try<spec::v2_2::ImageManifest> manifest =
-            saveV2S2Manifest(directory, response);
+    // Save manifest to 'directory'.
+    Try<Nothing> write = os::write(
+        path::join(directory, "manifest"), response.body);
 
-        if (manifest.isError()) {
-          LOG(WARNING) << "Failed to fetch schema 2 manifest: "
-                       << manifest.error();
-        }
+    if (write.isError()) {
+      return Failure(
+          "Failed to write the V2 Schema 1 image manifest to "
+          "'" + directory + "': " + write.error());
+    }
 
-        return Nothing();
-      }))
-      .then(defer(self(),
-                  &Self::___fetch,
-                  uri,
-                  directory,
-                  authHeaders,
-                  manifest.get()));
-#else
-  return ___fetch(uri, directory, authHeaders, manifest.get());
-#endif
+    // No need to proceed if we only want manifest.
+    if (uri.scheme() == "docker-manifest") {
+      return Nothing();
+    }
+
+    hashset<string> digests;
+    for (int i = 0; i < manifest->fslayers_size(); i++) {
+      digests.insert(manifest->fslayers(i).blobsum());
+    }
+
+    return fetchBlobs(uri, directory, digests, authHeaders);
+  } else if (isV2Schema2) {
+    // Parse V2 schema 2 manifest.
+    Try<spec::v2_2::ImageManifest> manifest =
+      spec::v2_2::parse(response.body);
+
+    if (manifest.isError()) {
+      return Failure(
+          "Failed to parse the V2 Schema 2 image manifest: " +
+          manifest.error());
+    }
+
+    // Save manifest to 'directory'.
+    Try<Nothing> write = os::write(
+        path::join(directory, "manifest"), response.body);
+
+    if (write.isError()) {
+      return Failure(
+          "Failed to write the V2 Schema 2 image manifest to "
+          "'" + directory + "': " + write.error());
+    }
+
+    // No need to proceed if we only want manifest.
+    if (uri.scheme() == "docker-manifest") {
+      return Nothing();
+    }
+
+    hashset<string> digests{manifest->config().digest()};
+    for (int i = 0; i < manifest->layers_size(); i++) {
+      digests.insert(manifest->layers(i).digest());
+    }
+
+    // TODO(gilbert): Verify the digest after contents are fetched.
+    return fetchBlobs(uri, directory, digests, authHeaders);
+  }
+
+  return Failure("Unsupported manifest MIME type: " + contentType.get());
 }
 
 
-Future<Nothing> DockerFetcherPluginProcess::___fetch(
+Future<Nothing> DockerFetcherPluginProcess::fetchBlobs(
     const URI& uri,
     const string& directory,
-    const http::Headers& authHeaders,
-    const spec::v2::ImageManifest& manifest)
+    const hashset<string>& digests,
+    const http::Headers& authHeaders)
 {
-  // No need to proceed if we only want manifest.
-  if (uri.scheme() == "docker-manifest") {
-    return Nothing();
-  }
-
-  // Download all the filesystem layers.
   vector<Future<Nothing>> futures;
-  for (int i = 0; i < manifest.fslayers_size(); i++) {
+
+  foreach (const string& digest, digests) {
     URI blob = uri::docker::blob(
         uri.path(),                         // The 'repository'.
-        manifest.fslayers(i).blobsum(),    // The 'digest'.
+        digest,                             // The 'digest'.
         uri.host(),                         // The 'registry'.
         (uri.has_fragment()                 // The 'scheme'.
           ? Option<string>(uri.fragment())
@@ -811,11 +862,7 @@ Future<Nothing> DockerFetcherPluginProcess::___fetch(
           ? Option<int>(uri.port())
           : None()));
 
-    // Use the same 'authHeaders' as for the manifest to pull the blobs.
-    futures.push_back(fetchBlob(
-        blob,
-        directory,
-        authHeaders));
+    futures.push_back(fetchBlob(blob, directory, authHeaders));
   }
 
   return collect(futures)
@@ -823,115 +870,6 @@ Future<Nothing> DockerFetcherPluginProcess::___fetch(
 }
 
 
-Try<spec::v2::ImageManifest> DockerFetcherPluginProcess::saveV2S1Manifest(
-    const string& directory,
-    const http::Response& response)
-{
-  if (response.code != http::Status::OK) {
-    return Error(
-        "Unexpected HTTP response '" + response.status + "' "
-        "when trying to get the schema 1 manifest");
-  }
-
-  CHECK_EQ(response.type, http::Response::BODY);
-
-  // Check if we got a V2 Schema 1 manifest.
-  // TODO(ipronin): We have to support Schema 2 manifests to be able to use
-  // digests for pulling images that were pushed with Docker 1.10+ to
-  // Registry 2.3+.
-  Option<string> contentType = response.headers.get("Content-Type");
-  if (contentType.isSome()) {
-    // NOTE: Docker support the following three media type for V2
-    // schema 1 manifest:
-    // 1. application/vnd.docker.distribution.manifest.v1+json
-    // 2. application/vnd.docker.distribution.manifest.v1+prettyjws
-    // 3. application/json
-    // For more details, see:
-    // https://docs.docker.com/registry/spec/manifest-v2-1/
-    bool isV2Schema1 =
-      strings::startsWith(
-          contentType.get(),
-          "application/vnd.docker.distribution.manifest.v1") ||
-      strings::startsWith(
-          contentType.get(),
-          "application/json");
-
-    if (!isV2Schema1) {
-      return Error(
-          "Unsupported schema 1 manifest MIME type: " +
-          contentType.get());
-    }
-  }
-
-  Try<spec::v2::ImageManifest> manifest = spec::v2::parse(response.body);
-  if (manifest.isError()) {
-    return Error(
-        "Failed to parse the schema 1 image manifest: " +
-        manifest.error());
-  }
-
-  // Save manifest to 'directory'.
-  Try<Nothing> write = os::write(
-      path::join(directory, "manifest"), response.body);
-
-  if (write.isError()) {
-    return Error(
-        "Failed to write the schema 1 image manifest to '" +
-        directory + "': " + write.error());
-  }
-
-  return manifest;
-}
-
-
-Try<spec::v2_2::ImageManifest> DockerFetcherPluginProcess::saveV2S2Manifest(
-    const string& directory,
-    const http::Response& response)
-{
-  if (response.code != http::Status::OK) {
-    return Error(
-        "Unexpected HTTP response '" + response.status +
-        "' when trying to get the schema 2 manifest");
-  }
-
-  Option<string> contentType = response.headers.get("Content-Type");
-  if (contentType.isSome()) {
-    bool isV2Schema2 =
-      strings::startsWith(
-          contentType.get(),
-          "application/vnd.docker.distribution.manifest.v2") ||
-      strings::startsWith(
-          contentType.get(),
-          "application/json");
-
-    if (!isV2Schema2) {
-      return Error(
-          "Unsupported schema 2 manifest MIME type: " +
-          contentType.get());
-    }
-  }
-
-  Try<spec::v2_2::ImageManifest> manifest = spec::v2_2::parse(response.body);
-  if (manifest.isError()) {
-    return Error(
-        "Failed to parse the schema 2 manifest: " +
-        manifest.error());
-  }
-
-  // Save manifest to 'directory'.
-  Try<Nothing> write = os::write(
-      path::join(directory, "manifest_v2s2"), response.body);
-
-  if (write.isError()) {
-    return Error(
-        "Failed to write the schema 2 image manifest to '" +
-        directory + "': " + write.error());
-  }
-
-  return manifest;
-}
-
-
 Future<Nothing> DockerFetcherPluginProcess::fetchBlob(
     const URI& uri,
     const string& directory,
@@ -1025,7 +963,7 @@ Future<Nothing> DockerFetcherPluginProcess::urlFetchBlob(
       const URI& blobUri,
       const http::Headers& authHeaders)
 {
-  Try<string> _manifest = os::read(path::join(directory, "manifest_v2s2"));
+  Try<string> _manifest = os::read(path::join(directory, "manifest"));
   if (_manifest.isError()) {
     return Failure("Schema 2 manifest does not exist");
   }