You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ji...@apache.org on 2016/10/14 19:55:46 UTC

mesos git commit: Added backend suffix to image layer rootfs path.

Repository: mesos
Updated Branches:
  refs/heads/master 9afaaffd8 -> 3f6503861


Added backend suffix to image layer rootfs path.

Previously image layer rootfs path is in the format below regardless
of which backend is used.
  <image_store_dir>/layers/<layer_id>/rootfs
This introduced an issue: when agent is restarted with a different
backend, we will wrongly handle the whiteout files since different
backends(e.g., aufs and overlay) have different whiteout standard.

In this commit, we added backend suffix to image layer rootfs path
for overlay backend like below.
  <image_store_dir>/layers/<layer_id>/rootfs.overlay
For non-overlay backends, it is still in the previous format, this
is because they share the same whiteout standard (aufs standard),
and also used to handle backward compatibility.

So the expected result of this commit is:
1. If user switches backend from overlay to a non-overlay (or vice
versa) when restarting agent, all the image layers of the previous
backend in the store will just be ignored.
2. In the upgrade case, if user starts a new version of Mesos agent
with overlay backend, then all the image layers in the store pulled
by the old agent will just be ignored, but if user starts the new
agent with a non-overlay backend, then all the image layers in the
store can still be used.

Review: https://reviews.apache.org/r/52827/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/3f650386
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/3f650386
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/3f650386

Branch: refs/heads/master
Commit: 3f6503861e92896a619e3b26a65f9b679a2dd3a9
Parents: 9afaaff
Author: Qian Zhang <zh...@cn.ibm.com>
Authored: Fri Oct 14 11:41:02 2016 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Fri Oct 14 12:55:43 2016 -0700

----------------------------------------------------------------------
 .../mesos/provisioner/docker/local_puller.cpp   | 13 +++--
 .../provisioner/docker/metadata_manager.cpp     |  6 ++-
 .../mesos/provisioner/docker/paths.cpp          | 13 +++--
 .../mesos/provisioner/docker/paths.hpp          |  6 ++-
 .../provisioner/docker/registry_puller.cpp      | 13 +++--
 .../mesos/provisioner/docker/store.cpp          | 55 ++++++++++++++------
 .../containerizer/provisioner_docker_tests.cpp  |  6 ++-
 7 files changed, 79 insertions(+), 33 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/3f650386/src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp b/src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp
index 9b09dca..817e30c 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp
@@ -51,9 +51,10 @@ namespace docker {
 class LocalPullerProcess : public Process<LocalPullerProcess>
 {
 public:
-  LocalPullerProcess(const string& _archivesDir)
+  LocalPullerProcess(const string& _archivesDir, const string& _backend)
     : ProcessBase(process::ID::generate("docker-provisioner-local-puller")),
-      archivesDir(_archivesDir) {}
+      archivesDir(_archivesDir),
+      backend(_backend){}
 
   ~LocalPullerProcess() {}
 
@@ -79,6 +80,7 @@ private:
       const string& layerId);
 
   const string archivesDir;
+  const string backend;
 };
 
 
@@ -92,8 +94,9 @@ Try<Owned<Puller>> LocalPuller::create(const Flags& flags)
   VLOG(1) << "Creating local puller with docker registry '"
           << flags.docker_registry << "'";
 
-  Owned<LocalPullerProcess> process(
-      new LocalPullerProcess(flags.docker_registry));
+  Owned<LocalPullerProcess> process(new LocalPullerProcess(
+      flags.docker_registry,
+      flags.image_provisioner_backend));
 
   return Owned<Puller>(new LocalPuller(process));
 }
@@ -287,7 +290,7 @@ Future<Nothing> LocalPullerProcess::extractLayer(
 {
   const string layerPath = path::join(directory, layerId);
   const string tar = paths::getImageLayerTarPath(layerPath);
-  const string rootfs = paths::getImageLayerRootfsPath(layerPath);
+  const string rootfs = paths::getImageLayerRootfsPath(layerPath, backend);
 
   VLOG(1) << "Extracting layer tar ball '" << tar
           << " to rootfs '" << rootfs << "'";

http://git-wip-us.apache.org/repos/asf/mesos/blob/3f650386/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp b/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp
index 6456d90..395c36b 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp
@@ -223,8 +223,10 @@ Future<Nothing> MetadataManagerProcess::recover()
     vector<string> missingLayerIds;
 
     foreach (const string& layerId, image.layer_ids()) {
-      const string rootfsPath =
-        paths::getImageLayerRootfsPath(flags.docker_store_dir, layerId);
+      const string rootfsPath = paths::getImageLayerRootfsPath(
+          flags.docker_store_dir,
+          layerId,
+          flags.image_provisioner_backend);
 
       if (!os::exists(rootfsPath)) {
         missingLayerIds.push_back(layerId);

http://git-wip-us.apache.org/repos/asf/mesos/blob/3f650386/src/slave/containerizer/mesos/provisioner/docker/paths.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/provisioner/docker/paths.cpp b/src/slave/containerizer/mesos/provisioner/docker/paths.cpp
index a5cc952..dea3756 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/paths.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/paths.cpp
@@ -56,15 +56,22 @@ string getImageLayerManifestPath(const string& storeDir, const string& layerId)
 }
 
 
-string getImageLayerRootfsPath(const string& layerPath)
+string getImageLayerRootfsPath(const string& layerPath, const string& backend)
 {
+  if (backend == "overlay") {
+    return path::join(layerPath, "rootfs." + backend);
+  }
+
   return path::join(layerPath, "rootfs");
 }
 
 
-string getImageLayerRootfsPath(const string& storeDir, const string& layerId)
+string getImageLayerRootfsPath(
+    const string& storeDir,
+    const string& layerId,
+    const string& backend)
 {
-  return getImageLayerRootfsPath(getImageLayerPath(storeDir, layerId));
+  return getImageLayerRootfsPath(getImageLayerPath(storeDir, layerId), backend);
 }
 
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/3f650386/src/slave/containerizer/mesos/provisioner/docker/paths.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/provisioner/docker/paths.hpp b/src/slave/containerizer/mesos/provisioner/docker/paths.hpp
index 949e0c1..232c027 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/paths.hpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/paths.hpp
@@ -64,12 +64,14 @@ std::string getImageLayerManifestPath(
 
 
 std::string getImageLayerRootfsPath(
-    const std::string& layerPath);
+    const std::string& layerPath,
+    const std::string& backend);
 
 
 std::string getImageLayerRootfsPath(
     const std::string& storeDir,
-    const std::string& layerId);
+    const std::string& layerId,
+    const std::string& backend);
 
 
 std::string getImageLayerTarPath(

http://git-wip-us.apache.org/repos/asf/mesos/blob/3f650386/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp b/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
index 1e2cc2d..b06ddff 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
@@ -61,6 +61,7 @@ class RegistryPullerProcess : public Process<RegistryPullerProcess>
 public:
   RegistryPullerProcess(
       const string& _storeDir,
+      const string& _backend,
       const http::URL& _defaultRegistryUrl,
       const Shared<uri::Fetcher>& _fetcher);
 
@@ -88,6 +89,7 @@ private:
   RegistryPullerProcess& operator=(const RegistryPullerProcess&) = delete;
 
   const string storeDir;
+  const string backend;
 
   // If the user does not specify the registry url in the image
   // reference, this registry url will be used as the default.
@@ -114,6 +116,7 @@ Try<Owned<Puller>> RegistryPuller::create(
   Owned<RegistryPullerProcess> process(
       new RegistryPullerProcess(
           flags.docker_store_dir,
+          flags.image_provisioner_backend,
           defaultRegistryUrl.get(),
           fetcher));
 
@@ -149,10 +152,12 @@ Future<vector<string>> RegistryPuller::pull(
 
 RegistryPullerProcess::RegistryPullerProcess(
     const string& _storeDir,
+    const string& _backend,
     const http::URL& _defaultRegistryUrl,
     const Shared<uri::Fetcher>& _fetcher)
   : ProcessBase(process::ID::generate("docker-provisioner-registry-puller")),
     storeDir(_storeDir),
+    backend(_backend),
     defaultRegistryUrl(_defaultRegistryUrl),
     fetcher(_fetcher) {}
 
@@ -294,13 +299,14 @@ Future<vector<string>> RegistryPullerProcess::__pull(
     layerIds.insert(layerIds.begin(), v1.id());
 
     // Skip if the layer is already in the store.
-    if (os::exists(paths::getImageLayerPath(storeDir, v1.id()))) {
+    if (os::exists(
+        paths::getImageLayerRootfsPath(storeDir, v1.id(), backend))) {
       continue;
     }
 
     const string layerPath = path::join(directory, v1.id());
     const string tar = path::join(directory, blobSum);
-    const string rootfs = paths::getImageLayerRootfsPath(layerPath);
+    const string rootfs = paths::getImageLayerRootfsPath(layerPath, backend);
     const string json = paths::getImageLayerManifestPath(layerPath);
 
     VLOG(1) << "Extracting layer tar ball '" << tar
@@ -360,7 +366,8 @@ Future<hashset<string>> RegistryPullerProcess::fetchBlobs(
 
     // Check if the layer is in the store or not. If yes, skip the
     // unnecessary fetching.
-    if (os::exists(paths::getImageLayerPath(storeDir, v1.id()))) {
+    if (os::exists(
+        paths::getImageLayerRootfsPath(storeDir, v1.id(), backend))) {
       continue;
     }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/3f650386/src/slave/containerizer/mesos/provisioner/docker/store.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/provisioner/docker/store.cpp b/src/slave/containerizer/mesos/provisioner/docker/store.cpp
index 52b9ea9..e192f86 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/store.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/store.cpp
@@ -259,8 +259,10 @@ Future<ImageInfo> StoreProcess::__get(const Image& image)
 
   vector<string> layerPaths;
   foreach (const string& layerId, image.layer_ids()) {
-    layerPaths.push_back(
-        paths::getImageLayerRootfsPath(flags.docker_store_dir, layerId));
+    layerPaths.push_back(paths::getImageLayerRootfsPath(
+        flags.docker_store_dir,
+        layerId,
+        flags.image_provisioner_backend));
   }
 
   // Read the manifest from the last layer because all runtime config
@@ -313,29 +315,50 @@ Future<Nothing> StoreProcess::moveLayer(
     return Nothing();
   }
 
-  const string target = paths::getImageLayerPath(
+  const string targetRootfs = paths::getImageLayerRootfsPath(
       flags.docker_store_dir,
-      layerId);
+      layerId,
+      flags.image_provisioner_backend);
 
   // NOTE: Since the layer id is supposed to be unique. If the layer
   // already exists in the store, we'll skip the moving since they are
   // expected to be the same.
-  if (os::exists(target)) {
+  if (os::exists(targetRootfs)) {
     return Nothing();
   }
 
-  Try<Nothing> mkdir = os::mkdir(target);
-  if (mkdir.isError()) {
-    return Failure(
-        "Failed to create directory in store for layer '" +
-        layerId + "': " + mkdir.error());
-  }
+  const string target = paths::getImageLayerPath(
+      flags.docker_store_dir,
+      layerId);
 
-  Try<Nothing> rename = os::rename(source, target);
-  if (rename.isError()) {
-    return Failure(
-        "Failed to move layer from '" + source +
-        "' to '" + target + "': " + rename.error());
+  if (!os::exists(target)) {
+    // This is the case that we pull the layer for the first time.
+    Try<Nothing> mkdir = os::mkdir(target);
+    if (mkdir.isError()) {
+      return Failure(
+          "Failed to create directory in store for layer '" +
+          layerId + "': " + mkdir.error());
+    }
+
+    Try<Nothing> rename = os::rename(source, target);
+    if (rename.isError()) {
+      return Failure(
+          "Failed to move layer from '" + source +
+          "' to '" + target + "': " + rename.error());
+    }
+  } else {
+    // This is the case where the layer has already been pulled with a
+    // different backend.
+    const string sourceRootfs = paths::getImageLayerRootfsPath(
+        source,
+        flags.image_provisioner_backend);
+
+    Try<Nothing> rename = os::rename(sourceRootfs, targetRootfs);
+    if (rename.isError()) {
+      return Failure(
+          "Failed to move rootfs from '" + sourceRootfs +
+          "' to '" + targetRootfs + "': " + rename.error());
+    }
   }
 
   return Nothing();

http://git-wip-us.apache.org/repos/asf/mesos/blob/3f650386/src/tests/containerizer/provisioner_docker_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/provisioner_docker_tests.cpp b/src/tests/containerizer/provisioner_docker_tests.cpp
index 10fbc41..d8fb7ab 100644
--- a/src/tests/containerizer/provisioner_docker_tests.cpp
+++ b/src/tests/containerizer/provisioner_docker_tests.cpp
@@ -78,11 +78,13 @@ public:
     // Verify contents of the image in store directory.
     const string layerPath1 = paths::getImageLayerRootfsPath(
         flags.docker_store_dir,
-        "123");
+        "123",
+        flags.image_provisioner_backend);
 
     const string layerPath2 = paths::getImageLayerRootfsPath(
         flags.docker_store_dir,
-        "456");
+        "456",
+        flags.image_provisioner_backend);
 
     EXPECT_TRUE(os::exists(layerPath1));
     EXPECT_TRUE(os::exists(layerPath2));