You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by tn...@apache.org on 2015/09/25 18:33:04 UTC
[12/17] mesos git commit: Fix Docker provisioners include guard and
comments.
Fix Docker provisioners include guard and comments.
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/11dc5842
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/11dc5842
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/11dc5842
Branch: refs/heads/master
Commit: 11dc5842ca7e1223918b462df15e14e267331043
Parents: 59404cd
Author: Timothy Chen <tn...@gmail.com>
Authored: Mon Sep 7 14:29:28 2015 -0700
Committer: Timothy Chen <tn...@apache.org>
Committed: Fri Sep 25 09:02:05 2015 -0700
----------------------------------------------------------------------
src/Makefile.am | 4 +-
src/slave/containerizer/provisioners/docker.hpp | 6 +-
.../provisioners/docker/local_store.cpp | 98 ++++++++++++++------
.../provisioners/docker/local_store.hpp | 6 +-
.../containerizer/provisioners/docker/paths.hpp | 6 +-
.../provisioners/docker/reference_store.cpp | 39 +++++++-
.../provisioners/docker/reference_store.hpp | 40 +-------
.../containerizer/provisioners/docker/store.hpp | 6 +-
.../containerizer/provisioner_docker_tests.cpp | 2 +-
9 files changed, 125 insertions(+), 82 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/11dc5842/src/Makefile.am
----------------------------------------------------------------------
diff --git a/src/Makefile.am b/src/Makefile.am
index fd367d3..65def70 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -249,7 +249,9 @@ STATE_PROTOS = messages/state.pb.cc messages/state.pb.h
BUILT_SOURCES += $(STATE_PROTOS)
CLEANFILES += $(STATE_PROTOS)
-DOCKER_PROVISIONER_PROTOS = messages/docker_provisioner.pb.cc messages/docker_provisioner.pb.h
+DOCKER_PROVISIONER_PROTOS = \
+ messages/docker_provisioner.pb.cc \
+ messages/docker_provisioner.pb.h
BUILT_SOURCES += $(DOCKER_PROVISIONER_PROTOS)
CLEANFILES += $(DOCKER_PROVISIONER_PROTOS)
http://git-wip-us.apache.org/repos/asf/mesos/blob/11dc5842/src/slave/containerizer/provisioners/docker.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/provisioners/docker.hpp b/src/slave/containerizer/provisioners/docker.hpp
index cda83cb..850ce85 100644
--- a/src/slave/containerizer/provisioners/docker.hpp
+++ b/src/slave/containerizer/provisioners/docker.hpp
@@ -16,8 +16,8 @@
* limitations under the License.
*/
-#ifndef __MESOS_DOCKER__
-#define __MESOS_DOCKER__
+#ifndef __MESOS_DOCKER_HPP__
+#define __MESOS_DOCKER_HPP__
#include <list>
#include <ostream>
@@ -130,4 +130,4 @@ private:
} // namespace internal {
} // namespace mesos {
-#endif // __MESOS_DOCKER__
+#endif // __MESOS_DOCKER_HPP__
http://git-wip-us.apache.org/repos/asf/mesos/blob/11dc5842/src/slave/containerizer/provisioners/docker/local_store.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/provisioners/docker/local_store.cpp b/src/slave/containerizer/provisioners/docker/local_store.cpp
index 58af655..aec7df9 100644
--- a/src/slave/containerizer/provisioners/docker/local_store.cpp
+++ b/src/slave/containerizer/provisioners/docker/local_store.cpp
@@ -63,7 +63,9 @@ public:
process::Future<DockerImage> get(const std::string& name);
private:
- LocalStoreProcess(const Flags& flags);
+ LocalStoreProcess(
+ const Flags& flags,
+ Owned<ReferenceStore> _refStore);
process::Future<Nothing> untarImage(
const std::string& tarPath,
@@ -81,7 +83,7 @@ private:
const std::string& staging,
const std::list<std::string>& layers)
- process::Future<Nothing> untarLayer(
+ process::Future<Nothing> putLayer(
const std::string& staging,
const std::string& id)
@@ -115,18 +117,42 @@ Try<Owned<Store>> LocalStore::create(
const Flags& flags,
Fetcher* fetcher)
{
+ if (!os::exists(flags.docker_store_dir)) {
+ Try<Nothing> mkdir = os::mkdir(flags.docker_store_dir);
+ if (mkdir.isError()) {
+ return Error("Failed to create Docker store directory: " + mkdir.error());
+ }
+ }
+
+ if (!os::exists(paths::getStagingDir(flags.docker_store_dir))) {
+ Try<Nothing> mkdir =
+ os::mkdir(paths::getStagingDir(flags.docker_store_dir));
+ if (mkdir.isError()) {
+ return Error("Failed to create Docker store staging directory: " +
+ mkdir.error());
+ }
+ }
+
Try<Owned<LocalStoreProcess>> process =
LocalStoreProcess::create(flags, fetcher);
if (process.isError()) {
- return Error("Failed to create store: " + process.error());
+ return Error(process.error());
}
- return Owned<Store>(new LocalStore(process.get()));
+ Try<Owned<ReferenceStore>> refStore = ReferenceStore::create(flags);
+ if (refStore.isError()) {
+ return Error(refStore);
+ }
+
+ return Owned<Store>(new LocalStore(process.get(), refStore.get()));
}
-LocalStore::LocalStore(Owned<LocalStoreProcess> process)
- : process(process)
+LocalStore::LocalStore(
+ Owned<LocalStoreProcess> process,
+ Owned<ReferenceStore> refStore)
+ : process(process),
+ _refStore(refStore)
{
process::spawn(CHECK_NOTNULL(process.get()));
}
@@ -170,10 +196,6 @@ Future<DockerImage> LocalStoreProcess::get(const string& name)
return Failure("No Docker image tar archive found");
}
- if (!os::exists(paths::getStagingDir(flags.docker_store_dir))) {
- os::mkdir(paths::getStagingDir(flags.docker_store_dir));
- }
-
// Create a temporary staging directory.
Try<string> staging =
os::mkdtemp(paths::getTempStaging(flags.docker_store_dir));
@@ -328,7 +350,7 @@ Future<Nothing> LocalStoreProcess::putLayers(
foreach (const string& layer, layers) {
futures.push_back(
futures.back().then(
- defer(self(), &Self::untarLayer, staging, layer)));
+ defer(self(), &Self::putLayer, staging, layer)));
}
return collect(futures)
@@ -336,26 +358,50 @@ Future<Nothing> LocalStoreProcess::putLayers(
}
-Future<Nothing> LocalStoreProcess::untarLayer(
+Future<Nothing> LocalStoreProcess::putLayer(
const string& staging,
const string& id)
{
- // Check if image layer is already in store.
- if (os::exists(paths::getImageLayerPath(flags.docker_store_dir, id))) {
- VLOG(1) << "Image layer: " << id << " already in store. Skipping untar"
- << " and putLayer.";
+ // We untar the layer from source into a staging directory, then
+ // move the layer into store. We do this instead of untarring
+ // directly to store to make sure we don't end up with partially
+ // untarred layer rootfs.
+
+ // Check if image layer rootfs is already in store.
+ if (os::exists(paths::getImageLayerRootfsPath(flags.docker_store_dir, id))) {
+ VLOG(1) << "Image layer '" << id << "' rootfs already in store. "
+ << "Skipping put.";
return Nothing();
}
+ const string imageLayerPath =
+ paths::getImageLayerPath(flags.docker_store_dir, id);
+ if (!os::exists()) {
+ Try<Nothing> mkdir = os::mkdir(imageLayerPath);
+ if (mkdir.isError()) {
+ return Failure("Failed to create Image layer directory '" +
+ imageLayerPath + "': " + mkdir.error());
+ }
+ }
+
// Image layer has been untarred but is not present in the store directory.
string localRootfsPath = paths::getLocalImageLayerRootfsPath(staging, id);
if (os::exists(localRootfsPath)) {
- LOG(WARNING) << "Image layer rootfs present at but not in store directory: "
- << localRootfsPath << "Skipping untarLayer.";
- return moveLayer(staging, id);
+ LOG(WARNING) << "Image layer '" << id << "' rootfs present at but not in "
+ << "store directory '" << localRootfsPath << "'. Removing "
+ << "staged rootfs and untarring layer again.";
+ Try<Nothing> rmdir = os::rmdir(localRootfsPath);
+ if (rmdir.isError()) {
+ return Failure("Failed to remove incomplete staged rootfs for layer '" +
+ id + "': " + rmdir.error());
+ }
}
- os::mkdir(localRootfsPath);
+ Try<Nothing> mkdir = os::mkdir(localRootfsPath);
+ if (mkdir.isError()) {
+ return Failure("Failed to create rootfs path '" + localRootfsPath + "': " +
+ mkdir.error());
+ }
// Untar staging/id/layer.tar into staging/id/rootfs.
vector<string> argv = {
"tar",
@@ -394,21 +440,13 @@ Future<Nothing> LocalStoreProcess::moveLayer(
const string& staging,
const string& id)
{
- if (!os::exists(flags.docker_store_dir)) {
- VLOG(1) << "Creating docker store directory";
- os::mkdir(flags.docker_store_dir);
- }
-
- if (!os::exists(paths::getImageLayerPath(flags.docker_store_dir, id))) {
- os::mkdir(paths::getImageLayerPath(flags.docker_store_dir, id));
- }
-
Try<Nothing> status = os::rename(
paths::getLocalImageLayerRootfsPath(staging, id),
paths::getImageLayerRootfsPath(flags.docker_store_dir, id));
if (status.isError()) {
- return Failure("Failed to move layer to store directory:" + status.error());
+ return Failure("Failed to move layer to store directory: "
+ + status.error());
}
return Nothing();
http://git-wip-us.apache.org/repos/asf/mesos/blob/11dc5842/src/slave/containerizer/provisioners/docker/local_store.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/provisioners/docker/local_store.hpp b/src/slave/containerizer/provisioners/docker/local_store.hpp
index 41a3559..2f0c9f1 100644
--- a/src/slave/containerizer/provisioners/docker/local_store.hpp
+++ b/src/slave/containerizer/provisioners/docker/local_store.hpp
@@ -16,8 +16,8 @@
* limitations under the License.
*/
-#ifndef __MESOS_DOCKER_LOCAL_STORE__
-#define __MESOS_DOCKER_LOCAL_STORE__
+#ifndef __MESOS_DOCKER_LOCAL_STORE_HPP__
+#define __MESOS_DOCKER_LOCAL_STORE_HPP__
#include "slave/containerizer/provisioners/docker/store.hpp"
@@ -62,4 +62,4 @@ private:
} // namespace internal {
} // namespace mesos {
-#endif // __MESOS_DOCKER_LOCAL_STORE__
+#endif // __MESOS_DOCKER_LOCAL_STORE_HPP__
http://git-wip-us.apache.org/repos/asf/mesos/blob/11dc5842/src/slave/containerizer/provisioners/docker/paths.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/provisioners/docker/paths.hpp b/src/slave/containerizer/provisioners/docker/paths.hpp
index 0ad3b74..02f129f 100644
--- a/src/slave/containerizer/provisioners/docker/paths.hpp
+++ b/src/slave/containerizer/provisioners/docker/paths.hpp
@@ -16,8 +16,8 @@
* limitations under the License.
*/
-#ifndef __MESOS_DOCKER_PATHS__
-#define __MESOS_DOCKER_PATHS__
+#ifndef __MESOS_DOCKER_PATHS_HPP__
+#define __MESOS_DOCKER_PATHS_HPP__
#include <list>
#include <string>
@@ -83,4 +83,4 @@ std::string getStoredImagesPath(const std::string& storeDir);
} // namespace internal {
} // namespace mesos {
-#endif // __MESOS_DOCKER_PATHS__
+#endif // __MESOS_DOCKER_PATHS_HPP__
http://git-wip-us.apache.org/repos/asf/mesos/blob/11dc5842/src/slave/containerizer/provisioners/docker/reference_store.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/provisioners/docker/reference_store.cpp b/src/slave/containerizer/provisioners/docker/reference_store.cpp
index e890b3c..1567248 100644
--- a/src/slave/containerizer/provisioners/docker/reference_store.cpp
+++ b/src/slave/containerizer/provisioners/docker/reference_store.cpp
@@ -48,6 +48,42 @@ namespace internal {
namespace slave {
namespace docker {
+
+class ReferenceStoreProcess : public process::Process<ReferenceStoreProcess>
+{
+public:
+ ~ReferenceStoreProcess() {}
+
+ // Explicitly use 'initialize' since we are overloading below.
+ using process::ProcessBase::initialize;
+
+ void initialize();
+
+ static Try<process::Owned<ReferenceStoreProcess>> create(const Flags& flags);
+
+ process::Future<DockerImage> put(
+ const std::string& name,
+ const std::list<std::string>& layers);
+
+ process::Future<Option<DockerImage>> get(const std::string& name);
+
+ // TODO(chenlily): Implement removal of unreferenced images.
+
+private:
+ ReferenceStoreProcess(const Flags& flags);
+
+ // Write out reference store state to persistent store.
+ Try<Nothing> persist();
+
+ const Flags flags;
+
+ // This is a lookup table for images that are stored in memory. It is keyed
+ // by the name of the DockerImage.
+ // For example, "ubuntu:14.04" -> ubuntu14:04 DockerImage.
+ hashmap<std::string, DockerImage> storedImages;
+};
+
+
Try<Owned<ReferenceStore>> ReferenceStore::create(const Flags& flags)
{
Try<Owned<ReferenceStoreProcess>> process =
@@ -203,9 +239,10 @@ void ReferenceStoreProcess::initialize()
continue;
}
- VLOG(1) << "Loaded Docker image: " << imageName << " from disk.";
storedImages[imageName] = DockerImage(imageName, layers);
}
+
+ LOG(INFO) << "Loaded " << storedImages.size() << " Docker images.";
}
} // namespace docker {
http://git-wip-us.apache.org/repos/asf/mesos/blob/11dc5842/src/slave/containerizer/provisioners/docker/reference_store.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/provisioners/docker/reference_store.hpp b/src/slave/containerizer/provisioners/docker/reference_store.hpp
index d9f7070..66b7573 100644
--- a/src/slave/containerizer/provisioners/docker/reference_store.hpp
+++ b/src/slave/containerizer/provisioners/docker/reference_store.hpp
@@ -16,8 +16,8 @@
* limitations under the License.
*/
-#ifndef __MESOS_DOCKER_REFERENCE_STORE__
-#define __MESOS_DOCKER_REFERENCE_STORE__
+#ifndef __MESOS_DOCKER_REFERENCE_STORE_HPP__
+#define __MESOS_DOCKER_REFERENCE_STORE_HPP__
#include <list>
#include <string>
@@ -95,43 +95,9 @@ private:
};
-class ReferenceStoreProcess : public process::Process<ReferenceStoreProcess>
-{
-public:
- ~ReferenceStoreProcess() {}
-
- // Explicitly use 'initialize' since we are overloading below.
- using process::ProcessBase::initialize;
-
- void initialize();
-
- static Try<process::Owned<ReferenceStoreProcess>> create(const Flags& flags);
-
- process::Future<DockerImage> put(
- const std::string& name,
- const std::list<std::string>& layers);
-
- process::Future<Option<DockerImage>> get(const std::string& name);
-
- // TODO(chenlily): Implement removal of unreferenced images.
-
-private:
- ReferenceStoreProcess(const Flags& flags);
-
- // Write out reference store state to persistent store.
- Try<Nothing> persist();
-
- const Flags flags;
-
- // This is a lookup table for images that are stored in memory. It is keyed
- // by the name of the DockerImage.
- // For example, "ubuntu:14.04" -> ubuntu14:04 DockerImage.
- hashmap<std::string, DockerImage> storedImages;
-};
-
} // namespace docker {
} // namespace slave {
} // namespace internal {
} // namespace mesos {
-#endif // __MESOS_DOCKER_REFERENCE_STORE__
+#endif // __MESOS_DOCKER_REFERENCE_STORE_HPP__
http://git-wip-us.apache.org/repos/asf/mesos/blob/11dc5842/src/slave/containerizer/provisioners/docker/store.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/provisioners/docker/store.hpp b/src/slave/containerizer/provisioners/docker/store.hpp
index 0520a2c..b9cb770 100644
--- a/src/slave/containerizer/provisioners/docker/store.hpp
+++ b/src/slave/containerizer/provisioners/docker/store.hpp
@@ -16,8 +16,8 @@
* limitations under the License.
*/
-#ifndef __MESOS_DOCKER_STORE__
-#define __MESOS_DOCKER_STORE__
+#ifndef __MESOS_DOCKER_STORE_HPP__
+#define __MESOS_DOCKER_STORE_HPP__
#include <string>
@@ -68,4 +68,4 @@ protected:
} // namespace internal {
} // namespace mesos {
-#endif // __MESOS_DOCKER_STORE__
+#endif // __MESOS_DOCKER_STORE_HPP__
http://git-wip-us.apache.org/repos/asf/mesos/blob/11dc5842/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 3927009..3a9a6ec 100644
--- a/src/tests/containerizer/provisioner_docker_tests.cpp
+++ b/src/tests/containerizer/provisioner_docker_tests.cpp
@@ -713,7 +713,7 @@ public:
"bar 456",
os::read(path::join(flags.docker_store_dir, "456", "rootfs", "temp")));
- // Verify the docker Image provided.
+ // Verify the Docker Image provided.
EXPECT_EQ(dockerImage.imageName, "abc");
list<string> expectedLayers;
expectedLayers.push_back("123");