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 <[email protected]> Authored: Mon Sep 7 14:29:28 2015 -0700 Committer: Timothy Chen <[email protected]> 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");
