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:37:23 UTC

[mesos] branch 1.4.x updated (177bc52 -> 1a76202)

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

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


    from 177bc52  Bumped the Mesos version to 1.4.4.
     new 6bcaadf  Windows: Update curl version to 7.61.0.
     new 3d3dd70  Windows: Parse version 2 schema 2 Docker image manifest.
     new 2b3ad79  Windows: Fetch blobs with V2S2 Docker image manifest.
     new 39af40b  Windows: Enable DockerFetcher in Windows agent.
     new 137899c  Windows: Enabled `DockerFetcherPluginTest` suite.
     new e1360f5  Windows: Add comment about V2S2 Docker image Manifest.
     new a366022  Added 'prettyjws' option to docker manifest V2 Schema1 accept header.
     new 0161d5d  Refactored the UCR docker store to construct 'Image' proto at pullers.
     new 7e2e044  Added protobuf for docker v2 schema2 config_digest in 'Image'.
     new bc23389  Supported docker manifest v2 schema2.
     new 46b1a62  Added a TODO for additional URLs support.
     new 29c2663  Fixed docker fetcher plugin unit test for v2s2 change.
     new 71ba7a8  Added gcr registry test.
     new df4babf  Added a unit test for Mesos containerizer image force pulling.
     new 1b6568f  Fixed use-after-free bug in Docker provisioner store.
     new cb492cc  Fixed the URI fetcher image fetch test failure on windows.
     new 1a76202  Added MESOS-9159 and MESOS-9675 to the 1.4.4 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/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                                 |   5 +-
 src/Makefile.am                                    |  11 +-
 src/docker/spec.cpp                                |  66 ++++
 src/slave/containerizer/mesos/containerizer.cpp    |   2 +
 .../mesos/provisioner/docker/local_puller.cpp      |  20 +-
 .../mesos/provisioner/docker/local_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             |  70 +++-
 src/tests/CMakeLists.txt                           |   2 +-
 src/tests/containerizer/docker_spec_tests.cpp      | 121 +++++++
 .../containerizer/provisioner_docker_tests.cpp     | 141 +++++++-
 src/tests/environment.cpp                          |   4 +
 src/tests/health_check_tests.cpp                   |  10 +-
 src/tests/uri_fetcher_tests.cpp                    |  85 +++--
 src/uri/fetcher.cpp                                |   6 +-
 src/uri/fetcher.hpp                                |  10 +-
 src/uri/fetchers/docker.cpp                        | 354 +++++++++++++++++----
 src/uri/fetchers/docker.hpp                        |   4 +
 28 files changed, 1078 insertions(+), 250 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] 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.4.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 2b3ad791670a0099e38cb1211ed1fd4b06859aa9
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 aa28161..a60fbea 100644
--- a/src/uri/fetchers/docker.cpp
+++ b/src/uri/fetchers/docker.cpp
@@ -294,7 +294,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());
       }
@@ -322,13 +326,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);
 }
 
 
@@ -421,6 +437,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,
@@ -432,7 +462,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.
@@ -495,6 +537,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)
@@ -646,10 +714,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.
+  list<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);
@@ -676,54 +828,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.
-  list<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;
 }
 
 
@@ -734,7 +910,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'
@@ -744,7 +925,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
     }));
 }
 
@@ -772,25 +963,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(
@@ -890,7 +1156,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()));
 }
@@ -905,7 +1171,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 cdbab9a..1bf3757 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);
+
   virtual ~DockerFetcherPlugin();
 
   virtual std::set<std::string> schemes() const;


[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.4.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit e1360f561c81cf52449b1abc636bcd7a614412b7
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 a60fbea..56833fe 100644
--- a/src/uri/fetchers/docker.cpp
+++ b/src/uri/fetchers/docker.cpp
@@ -728,6 +728,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.4.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 7e2e044694a69017e28c961a90b9f65e3dc59653
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] 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.4.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 137899c652686034fb76ed89b57727fd54fcb761
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        |  4 ++
 src/tests/health_check_tests.cpp | 10 +++--
 src/tests/uri_fetcher_tests.cpp  | 81 ++++++++++++++++++++++++++--------------
 3 files changed, 62 insertions(+), 33 deletions(-)

diff --git a/src/tests/environment.cpp b/src/tests/environment.cpp
index a7262cd..ac3b969 100644
--- a/src/tests/environment.cpp
+++ b/src/tests/environment.cpp
@@ -302,7 +302,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;
+#endif // __WINDOWS__
     if (error) {
       std::cerr
         << "-------------------------------------------------------------\n"
diff --git a/src/tests/health_check_tests.cpp b/src/tests/health_check_tests.cpp
index 8f7fe0a..a97fe07 100644
--- a/src/tests/health_check_tests.cpp
+++ b/src/tests/health_check_tests.cpp
@@ -1574,7 +1574,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);
@@ -1662,8 +1663,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);
@@ -1757,7 +1758,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 8a40f46..14b1ccf 100644
--- a/src/tests/uri_fetcher_tests.cpp
+++ b/src/tests/uri_fetcher_tests.cpp
@@ -48,6 +48,8 @@ namespace http = process::http;
 using std::list;
 using std::string;
 
+using mesos::uri::DockerFetcherPlugin;
+
 using process::Future;
 using process::Owned;
 using process::Process;
@@ -269,6 +271,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 {};
 
@@ -276,9 +288,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);
@@ -287,27 +297,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);
@@ -316,7 +335,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)));
 }
 
 
@@ -324,9 +343,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);
@@ -339,14 +356,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())));
   }
 }
 
@@ -355,9 +376,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);
@@ -372,14 +391,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] 17/17: Added MESOS-9159 and MESOS-9675 to the 1.4.4 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.4.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 1a762020ec9fdc46b55754afd955d81f1da8880f
Author: Qian Zhang <zh...@gmail.com>
AuthorDate: Mon Apr 15 16:20:54 2019 +0800

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

diff --git a/CHANGELOG b/CHANGELOG
index 0422529..0f32baa 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -6,6 +6,8 @@ Release Notes - Mesos - Version 1.4.4 (WIP)
   * [MESOS-9507] - Agent could not recover due to empty docker volume checkpointed files.
 
 ** Improvement:
+  * [MESOS-9159] - Support Foreign URLs in docker registry puller.
+  * [MESOS-9675] - Docker Manifest V2 Schema2 Support.
 
 
 Release Notes - Mesos - Version 1.4.3


[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.4.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 39af40b9ea89c90dc2f1a7247b418e06ac3fd565
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                              |  3 ++-
 src/slave/containerizer/mesos/containerizer.cpp |  2 ++
 src/uri/fetcher.cpp                             |  6 +++---
 src/uri/fetcher.hpp                             | 10 +++++-----
 4 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 34eafdf..9cef63b 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -438,12 +438,12 @@ set(URI_SRC
   uri/utils.cpp
   uri/fetchers/copy.cpp
   uri/fetchers/curl.cpp
+  uri/fetchers/docker.cpp
   )
 
 if (NOT WIN32)
   set(URI_SRC
     ${URI_SRC}
-    uri/fetchers/docker.cpp
     uri/fetchers/hadoop.cpp
     )
 endif ()
@@ -469,6 +469,7 @@ set(WATCHER_SRC
 
 set(WIN32_SRC
   ${WIN32_SRC}
+  slave/containerizer/mesos/isolators/docker/runtime.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 aef9a18..e54fc2d 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -80,6 +80,7 @@
 #include "slave/containerizer/mesos/provisioner/provisioner.hpp"
 
 #ifdef __WINDOWS__
+#include "slave/containerizer/mesos/isolators/docker/runtime.hpp"
 #include "slave/containerizer/mesos/isolators/windows.hpp"
 #include "slave/containerizer/mesos/isolators/filesystem/windows.hpp"
 #endif // __WINDOWS__
@@ -336,6 +337,7 @@ Try<MesosContainerizer*> MesosContainerizer::create(
     {"disk/xfs", &XfsDiskIsolatorProcess::create},
 #endif // ENABLE_XFS_DISK_ISOLATOR
 #else
+    {"docker/runtime", &DockerRuntimeIsolatorProcess::create},
     {"windows/cpu", &WindowsCpuIsolatorProcess::create},
 #endif // __WINDOWS__
 
diff --git a/src/uri/fetcher.cpp b/src/uri/fetcher.cpp
index 13c2d54..005b6f7 100644
--- a/src/uri/fetcher.cpp
+++ b/src/uri/fetcher.cpp
@@ -50,12 +50,12 @@ Try<Owned<Fetcher>> create(const Option<Flags>& _flags)
        [flags]() { return CurlFetcherPlugin::create(flags); }},
     {CopyFetcherPlugin::NAME,
        [flags]() { return CopyFetcherPlugin::create(flags); }},
+    {DockerFetcherPlugin::NAME,
+       [flags]() { return DockerFetcherPlugin::create(flags); }},
 #ifndef __WINDOWS__
-    // TODO(dpravat): Enable `Hadoop` and `Docker` plugins. See MESOS-5473.
+    // TODO(dpravat): Enable `Hadoop` plugin. See MESOS-5473.
     {HadoopFetcherPlugin::NAME,
        [flags]() { return HadoopFetcherPlugin::create(flags); }},
-    {DockerFetcherPlugin::NAME,
-       [flags]() { return DockerFetcherPlugin::create(flags); }},
 #endif // __WINDOWS__
   };
 
diff --git a/src/uri/fetcher.hpp b/src/uri/fetcher.hpp
index 7b91010..6997b41 100644
--- a/src/uri/fetcher.hpp
+++ b/src/uri/fetcher.hpp
@@ -39,13 +39,13 @@ namespace fetcher {
  */
 class Flags :
   public virtual CopyFetcherPlugin::Flags,
-#ifdef __WINDOWS__
-  // TODO(dpravat): Add support for Hadoop and Docker plugins. See MESOS-5473.
-  public virtual CurlFetcherPlugin::Flags {};
-#else
   public virtual CurlFetcherPlugin::Flags,
-  public virtual HadoopFetcherPlugin::Flags,
+#ifdef __WINDOWS__
+  // TODO(dpravat): Add support for Hadoop plugin. See MESOS-5473.
   public virtual DockerFetcherPlugin::Flags {};
+#else
+  public virtual DockerFetcherPlugin::Flags,
+  public virtual HadoopFetcherPlugin::Flags {};
 #endif // __WINDOWS__
 
 


[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.4.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit cb492ccdc949a5b2be07f41c42f9db1186596ed8
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 5a29c5a..b89e9bd 100644
--- a/src/tests/uri_fetcher_tests.cpp
+++ b/src/tests/uri_fetcher_tests.cpp
@@ -351,7 +351,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(
@@ -388,7 +389,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] 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.4.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 3d3dd703aec9e186aef8d484212baf44dfe3c794
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                            |   2 +
 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, 285 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 4c3dea9..34eafdf 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -29,6 +29,7 @@ PROTOC_TO_INCLUDE_DIR(CONTAINERIZER         mesos/slave/containerizer)
 PROTOC_TO_INCLUDE_DIR(DOCKER_SPEC           mesos/docker/spec)
 PROTOC_TO_INCLUDE_DIR(DOCKER_V1             mesos/docker/v1)
 PROTOC_TO_INCLUDE_DIR(DOCKER_V2             mesos/docker/v2)
+PROTOC_TO_INCLUDE_DIR(DOCKER_V2_2           mesos/docker/v2_2)
 PROTOC_TO_INCLUDE_DIR(EXECUTOR              mesos/executor/executor)
 PROTOC_TO_INCLUDE_DIR(FETCHER               mesos/fetcher/fetcher)
 PROTOC_TO_INCLUDE_DIR(HOOK                  mesos/module/hook)
@@ -73,6 +74,7 @@ set(PUBLIC_PROTOBUF_SRC
   ${DOCKER_SPEC_PROTO_CC}
   ${DOCKER_V1_PROTO_CC}
   ${DOCKER_V2_PROTO_CC}
+  ${DOCKER_V2_2_PROTO_CC}
   ${EXECUTOR_PROTO_CC}
   ${FETCHER_PROTO_CC}
   ${HOOK_PROTO_CC}
diff --git a/src/Makefile.am b/src/Makefile.am
index 1fd80b1..3fa0451 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -265,6 +265,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
@@ -311,6 +312,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				\
@@ -637,12 +640,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
 
@@ -1510,6 +1516,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 538cf18..4253520 100644
--- a/src/docker/spec.cpp
+++ b/src/docker/spec.cpp
@@ -408,5 +408,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 6dd2716..7ac34fc 100644
--- a/src/tests/CMakeLists.txt
+++ b/src/tests/CMakeLists.txt
@@ -150,6 +150,7 @@ set(MESOS_TESTS_SRC
 set(MESOS_TESTS_SRC
   ${MESOS_TESTS_SRC}
   containerizer/containerizer_tests.cpp
+  containerizer/docker_spec_tests.cpp
   containerizer/docker_tests.cpp
   )
 
@@ -200,7 +201,6 @@ if (NOT WIN32)
     containerizer/composing_containerizer_tests.cpp
     containerizer/cpu_isolator_tests.cpp
     containerizer/docker_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 5fde49a..ffd5bcc 100644
--- a/src/tests/containerizer/docker_spec_tests.cpp
+++ b/src/tests/containerizer/docker_spec_tests.cpp
@@ -619,6 +619,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] 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.4.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 71ba7a8a0da085f8b9b81ca5f9951b50fc328164
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 | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/tests/containerizer/provisioner_docker_tests.cpp b/src/tests/containerizer/provisioner_docker_tests.cpp
index 970a33d..8d060ce 100644
--- a/src/tests/containerizer/provisioner_docker_tests.cpp
+++ b/src/tests/containerizer/provisioner_docker_tests.cpp
@@ -486,12 +486,14 @@ TEST_F(ProvisionerDockerTest, ROOT_LocalPullerSimpleCommand)
 // 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",
-        "quay.io/coreos/alpine-sh",
+        "gcr.io/google-containers/busybox:1.24", // manifest.v1+prettyjws
+        "gcr.io/google-containers/busybox:1.27", // manifest.v2+json
+        "quay.io/coreos/alpine-sh", // manifest.v1+prettyjws
         "registry.cn-hangzhou.aliyuncs.com/acs-sample/alpine"})));
 
 


[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.4.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 6bcaadf76ab332e73680e6da53a357d2fd85c221
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/cmake/Versions.cmake | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/3rdparty/cmake/Versions.cmake b/3rdparty/cmake/Versions.cmake
index 9586296..30abf3e 100644
--- a/3rdparty/cmake/Versions.cmake
+++ b/3rdparty/cmake/Versions.cmake
@@ -2,8 +2,8 @@ set(BOOST_VERSION           "1.53.0")
 set(BOOST_HASH              "SHA256=CED7CE2ED8D7D34815AC9DB1D18D28FCD386FFBB3DE6DA45303E1CF193717038")
 set(CONCURRENTQUEUE_VERSION "1.0.0-beta")
 set(CONCURRENTQUEUE_HASH    "SHA256=D723E784E4D54F7519208BD795F2AA799A449B873DBB7A1251F288E04BE23465")
-set(CURL_VERSION            "7.43.0")
-set(CURL_HASH               "SHA256=1A084DA1EDBFC3BD632861358B26AF45BA91AAADFB15D6482DE55748B8DFC693")
+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] 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.4.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 29c2663f7ae365bae005dd0c31d22d26422de9ef
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 14b1ccf..5a29c5a 100644
--- a/src/tests/uri_fetcher_tests.cpp
+++ b/src/tests/uri_fetcher_tests.cpp
@@ -297,29 +297,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());
 }
 
 
@@ -355,19 +343,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())));
   }
 }
 
@@ -390,19 +380,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] 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.4.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit df4babfdd718bb9325eb6c1bac9b2995cffacc8e
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     | 115 +++++++++++++++++++++
 1 file changed, 115 insertions(+)

diff --git a/src/tests/containerizer/provisioner_docker_tests.cpp b/src/tests/containerizer/provisioner_docker_tests.cpp
index 8d060ce..cb032d3 100644
--- a/src/tests/containerizer/provisioner_docker_tests.cpp
+++ b/src/tests/containerizer/provisioner_docker_tests.cpp
@@ -578,6 +578,121 @@ 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> statusRunning1;
+  Future<TaskStatus> statusFinished1;
+  EXPECT_CALL(sched, statusUpdate(&driver, _))
+    .WillOnce(FutureArg<1>(&statusRunning1))
+    .WillOnce(FutureArg<1>(&statusFinished1));
+
+  driver.launchTasks(offer1.id(), {task});
+
+  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> statusRunning2;
+  Future<TaskStatus> statusFinished2;
+  EXPECT_CALL(sched, statusUpdate(&driver, _))
+    .WillOnce(FutureArg<1>(&statusRunning2))
+    .WillOnce(FutureArg<1>(&statusFinished2));
+
+  driver.launchTasks(offer2.id(), {task});
+
+  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] 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.4.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit a366022dbe3278854f12dd295e9158f4f89c31e5
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 56833fe..00091cb 100644
--- a/src/uri/fetchers/docker.cpp
+++ b/src/uri/fetchers/docker.cpp
@@ -666,7 +666,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.4.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 0161d5decf2df4f1fa12f63dfb31cf8e69a4aac9
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/local_puller.cpp      | 20 +++++++++++-----
 .../mesos/provisioner/docker/local_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, 70 insertions(+), 66 deletions(-)

diff --git a/src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp b/src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp
index c4879ec..4fe3b46 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp
@@ -58,13 +58,13 @@ public:
 
   ~LocalPullerProcess() {}
 
-  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);
@@ -119,7 +119,7 @@ LocalPuller::~LocalPuller()
 }
 
 
-Future<vector<string>> LocalPuller::pull(
+Future<Image> LocalPuller::pull(
     const spec::ImageReference& reference,
     const string& directory,
     const string& backend,
@@ -134,7 +134,7 @@ Future<vector<string>> LocalPuller::pull(
 }
 
 
-Future<vector<string>> LocalPullerProcess::pull(
+Future<Image> LocalPullerProcess::pull(
     const spec::ImageReference& reference,
     const string& directory,
     const string& backend)
@@ -160,7 +160,7 @@ Future<vector<string>> LocalPullerProcess::pull(
 }
 
 
-Future<vector<string>> LocalPullerProcess::_pull(
+Future<Image> LocalPullerProcess::_pull(
     const spec::ImageReference& reference,
     const string& directory,
     const string& backend)
@@ -238,7 +238,15 @@ Future<vector<string>> LocalPullerProcess::_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/local_puller.hpp b/src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp
index 4d2e497..fb4cdbc 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp
@@ -46,7 +46,7 @@ public:
 
   ~LocalPuller();
 
-  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 7dfeb65..1078109 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,
@@ -110,15 +108,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);
 }
 
 
@@ -134,19 +129,10 @@ Future<Option<Image>> MetadataManager::get(
 }
 
 
-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()) {
@@ -155,7 +141,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 954da16..a5e321a 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp
@@ -67,18 +67,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 693e841..e5abb68 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
@@ -67,26 +67,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,
@@ -154,7 +154,7 @@ RegistryPuller::~RegistryPuller()
 }
 
 
-Future<vector<string>> RegistryPuller::pull(
+Future<Image> RegistryPuller::pull(
     const spec::ImageReference& reference,
     const string& directory,
     const string& backend,
@@ -217,7 +217,7 @@ static spec::ImageReference normalize(
 }
 
 
-Future<vector<string>> RegistryPullerProcess::pull(
+Future<Image> RegistryPullerProcess::pull(
     const spec::ImageReference& reference,
     const string& directory,
     const string& backend,
@@ -237,7 +237,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,
@@ -296,7 +296,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,
@@ -332,7 +332,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,
@@ -414,7 +414,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);
@@ -427,7 +427,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 4df679b..7489d7d 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 34d36a7..8d12104 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/store.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/store.cpp
@@ -96,9 +96,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(
@@ -300,8 +300,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);
@@ -359,18 +359,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)
 {
   list<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 44e13a5..970a33d 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] 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.4.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 1b6568f150013ab5def600cd1047017b5855d431
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 ba4db0a..89d1a88 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/store.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/store.cpp
@@ -390,7 +390,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(
@@ -408,7 +408,7 @@ Future<Image> StoreProcess::moveLayers(
       }
 
       return image;
-    });
+    }));
 }
 
 


[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.4.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 46b1a62b4d3838d54d5d9ecba3ed76eb9b2bddda
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 da814d8..cb0124c 100644
--- a/src/uri/fetchers/docker.cpp
+++ b/src/uri/fetchers/docker.cpp
@@ -947,6 +947,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] 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.4.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit bc233896d69a4af772463355fc9e53c999b366c0
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             |  56 +++-
 src/uri/fetchers/docker.cpp                        | 296 ++++++++-------------
 3 files changed, 396 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 e5abb68..155c165 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>
 
@@ -93,13 +94,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;
 
@@ -238,31 +260,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 {
@@ -275,19 +297,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,
@@ -307,21 +334,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,
@@ -438,9 +501,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;
+  list<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)
@@ -465,24 +613,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)
+{
   list<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());
       }
@@ -491,9 +689,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 {
@@ -506,8 +704,8 @@ Future<hashset<string>> RegistryPullerProcess::fetchBlobs(
         : Option<int>();
 
       blobUri = uri::docker::blob(
-          reference.repository(),
-          blobSum,
+          normalizedRef.repository(),
+          digest,
           registry,
           defaultRegistryUrl.scheme,
           port);
@@ -520,7 +718,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 8d12104..ba4db0a 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/store.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/store.cpp
@@ -270,6 +270,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();
     }
@@ -337,22 +344,35 @@ Future<ImageInfo> StoreProcess::__get(
         backend));
   }
 
-  // Read the manifest from the last layer because all runtime config
-  // are merged at the leaf already.
-  Try<string> manifest = os::read(
-      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));
+  }
 
+  Try<string> manifest = os::read(configPath);
   if (manifest.isError()) {
-    return Failure("Failed to read manifest: " + manifest.error());
+    return Failure(
+        "Failed to read manifest from '" + configPath + "': " +
+        manifest.error());
   }
 
   Try<::docker::spec::v1::ImageManifest> v1 =
     ::docker::spec::v1::parse(manifest.get());
 
   if (v1.isError()) {
-    return Failure("Failed to parse docker v1 manifest: " + v1.error());
+    return Failure(
+        "Failed to parse docker v1 manifest from '" + configPath + "': " +
+        v1.error());
   }
 
   return ImageInfo{layerPaths, v1.get()};
@@ -370,7 +390,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 00091cb..da814d8 100644
--- a/src/uri/fetchers/docker.cpp
+++ b/src/uri/fetchers/docker.cpp
@@ -437,19 +437,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,
@@ -660,13 +652,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"
     }
@@ -717,71 +710,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.
   list<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())
@@ -790,11 +841,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)
@@ -802,115 +849,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,
@@ -1004,7 +942,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");
   }