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/02/12 00:24:47 UTC
[2/3] mesos git commit: Introduced Appc image cache.
Introduced Appc image cache.
Image cache will cache metadata about all Appc images stores in the
store's image directory. This cache could be useful to quickly query if
an image is present in the store.
Review: https://reviews.apache.org/r/43127/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/ec20fc4b
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/ec20fc4b
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/ec20fc4b
Branch: refs/heads/master
Commit: ec20fc4bf7b128f7c0603c356f9000d3f3699ac8
Parents: 1d276ac
Author: Jojy Varghese <jo...@mesosphere.io>
Authored: Thu Feb 11 15:11:15 2016 -0800
Committer: Jie Yu <yu...@gmail.com>
Committed: Thu Feb 11 15:11:15 2016 -0800
----------------------------------------------------------------------
.../mesos/provisioner/appc/cache.cpp | 140 +++++++++++++++++--
.../mesos/provisioner/appc/cache.hpp | 91 +++++++++---
.../mesos/provisioner/appc/store.cpp | 140 ++++++-------------
.../containerizer/provisioner_appc_tests.cpp | 37 ++++-
4 files changed, 281 insertions(+), 127 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/ec20fc4b/src/slave/containerizer/mesos/provisioner/appc/cache.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/provisioner/appc/cache.cpp b/src/slave/containerizer/mesos/provisioner/appc/cache.cpp
index af69db3..ff392ef 100644
--- a/src/slave/containerizer/mesos/provisioner/appc/cache.cpp
+++ b/src/slave/containerizer/mesos/provisioner/appc/cache.cpp
@@ -14,12 +14,20 @@
// See the License for the specific language governing permissions and
// limitations under the License.
+#include <list>
+
+#include <boost/functional/hash.hpp>
+
#include <stout/os.hpp>
#include <mesos/appc/spec.hpp>
#include "slave/containerizer/mesos/provisioner/appc/cache.hpp"
+#include "slave/containerizer/mesos/provisioner/appc/paths.hpp"
+using std::list;
+using std::map;
+using std::pair;
using std::string;
namespace spec = appc::spec;
@@ -29,21 +37,52 @@ namespace internal {
namespace slave {
namespace appc {
-Try<CachedImage> CachedImage::create(const string& imagePath)
+Try<process::Owned<Cache>> Cache::create(const Path& storeDir)
+{
+ // TODO(jojy): Should we create a directory if it does not exist ?
+ if (!os::exists(storeDir)) {
+ return Error(
+ "Failed to find store directory '" + stringify(storeDir) + "'");
+ }
+
+ return new Cache(storeDir);
+}
+
+
+Cache::Cache(const Path& _storeDir)
+ : storeDir(_storeDir) {}
+
+
+Try<Nothing> Cache::recover()
{
- Option<Error> error = spec::validateLayout(imagePath);
- if (error.isSome()) {
- return Error("Invalid image layout: " + error.get().message);
+ Try<list<string>> imageDirs = os::ls(paths::getImagesDir(storeDir));
+ if (imageDirs.isError()) {
+ return Error(
+ "Failed to list images under '" +
+ paths::getImagesDir(storeDir) + "': " +
+ imageDirs.error());
}
- string imageId = Path(imagePath).basename();
+ foreach (const string& imageId, imageDirs.get()) {
+ Try<Nothing> adding = add(imageId);
+ if (adding.isError()) {
+ LOG(WARNING) << "Failed to add image with id '" << imageId
+ << "' to cache: " << adding.error();
+ continue;
+ }
- error = spec::validateImageID(imageId);
- if (error.isSome()) {
- return Error("Invalid image ID: " + error.get().message);
+ LOG(INFO) << "Restored image with id '" << imageId << "'";
}
- Try<string> read = os::read(spec::getImageManifestPath(imagePath));
+ return Nothing();
+}
+
+
+Try<Nothing> Cache::add(const string& imageId)
+{
+ const Path imageDir(paths::getImagePath(storeDir, imageId));
+
+ Try<string> read = os::read(spec::getImageManifestPath(imageDir));
if (read.isError()) {
return Error("Failed to read manifest: " + read.error());
}
@@ -53,13 +92,90 @@ Try<CachedImage> CachedImage::create(const string& imagePath)
return Error("Failed to parse manifest: " + manifest.error());
}
- return CachedImage(manifest.get(), imageId, imagePath);
+ map<string, string> labels;
+ foreach (const spec::ImageManifest::Label& label, manifest.get().labels()) {
+ labels.insert({label.name(), label.value()});
+ }
+
+ imageIds.put(Key(manifest.get().name(), labels), imageId);
+
+ VLOG(1) << "Added image with id '" << imageId << "' to cache";
+
+ return Nothing();
+}
+
+
+Option<string> Cache::find(const Image::Appc& image) const
+{
+ // Create a cache key from image.
+ Cache::Key key(image);
+
+ if (imageIds.contains(key)) {
+ return None();
+ }
+
+ return imageIds.at(key);
+}
+
+
+Cache::Key::Key(const Image::Appc& image)
+ : name(image.name())
+{
+ // Extract labels for easier comparison, this also weeds out duplicates.
+ // TODO(xujyan): Detect duplicate labels in image manifest validation
+ // and Image::Appc validation.
+
+ // TODO(jojy): Do we need to normalize the labels by adding default labels
+ // like os, version and arch if they are missing?
+ foreach (const Label& label, image.labels().labels()) {
+ labels.insert({label.key(), label.value()});
+ }
+}
+
+
+Cache::Key::Key(
+ const string& _name,
+ const map<string, std::string> _labels)
+ : name(_name),
+ labels(_labels) {}
+
+
+bool Cache::Key::operator==(const Cache::Key& other) const
+{
+ if (name != other.name) {
+ return false;
+ }
+
+ foreachpair (const string& name, const string& value, other.labels) {
+ map<string, string>::const_iterator itr = labels.find(name);
+ if ((itr == labels.end()) || (labels.at(name) != value)) {
+ return false;
+ }
+ }
+
+ foreachpair (const string& name, const string& value, labels) {
+ map<string, string>::const_iterator itr = other.labels.find(name);
+ if ((itr == other.labels.end()) || (other.labels.at(name) != value)) {
+ return false;
+ }
+ }
+
+ return true;
}
-string CachedImage::rootfs() const
+size_t Cache::KeyHasher::operator()(const Cache::Key& key) const
{
- return path::join(path, "rootfs");
+ size_t seed = 0;
+
+ boost::hash_combine(seed, key.name);
+
+ foreachpair (const string& name, const string& value, key.labels) {
+ boost::hash_combine(seed, name);
+ boost::hash_combine(seed, value);
+ }
+
+ return seed;
}
} // namespace appc {
http://git-wip-us.apache.org/repos/asf/mesos/blob/ec20fc4b/src/slave/containerizer/mesos/provisioner/appc/cache.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/provisioner/appc/cache.hpp b/src/slave/containerizer/mesos/provisioner/appc/cache.hpp
index 4a63d47..7f888cc 100644
--- a/src/slave/containerizer/mesos/provisioner/appc/cache.hpp
+++ b/src/slave/containerizer/mesos/provisioner/appc/cache.hpp
@@ -17,38 +17,95 @@
#ifndef __APPC_PROVISIONER_CACHE_HPP__
#define __APPC_PROVISIONER_CACHE_HPP__
+#include <map>
#include <string>
+#include <process/owned.hpp>
+
+#include <stout/hashmap.hpp>
+
+#include <mesos/mesos.hpp>
#include <mesos/appc/spec.hpp>
-#include <mesos/mesos.pb.h>
namespace mesos {
namespace internal {
namespace slave {
namespace appc {
-// Defines a locally cached image (which has passed validation).
-struct CachedImage
+/**
+ * Encapsulates Appc image cache.
+ * Note: We only keep 1 level image information and do not bother to keep
+ * dependency information. This is because dependency graph will be resolved at
+ * runtime (say when store->get is called).
+ */
+class Cache
{
- static Try<CachedImage> create(const std::string& imagePath);
+public:
+ /**
+ * Factory method for creating cache.
+ *
+ * @param storeDir Path to the image store.
+ * @returns Owned Cache pointer on success.
+ * Error on failure.
+ */
+ static Try<process::Owned<Cache>> create(const Path& storeDir);
+
+
+ /**
+ * Recovers/rebuilds the cache from its image store directory.
+ */
+ Try<Nothing> recover();
+
+
+ /**
+ * Adds an image to the cache by the image's id.
+ *
+ * Add is done in two steps:
+ * 1. A cache entry is created using the path constructed from store
+ * directory and the imageId parameter.
+ * 2. The cache entry is inserted into the cache.
+ *
+ * @param imageId Image id for the image that has to be added to the cache.
+ * @returns Nothing on success.
+ * Error on failure to add.
+ */
+ Try<Nothing> add(const std::string& imageId);
+
+
+ /**
+ * Finds image id of an image if it is present in the cache/store.
+ *
+ * @param appc Appc image data.
+ * @returns Image id of the image if its found in the cache.
+ * Error otherwise.
+ */
+ Option<std::string> find(const Image::Appc& image) const;
+
+private:
+ struct Key
+ {
+ Key(const Image::Appc& image);
+
+ Key(const std::string& name,
+ const std::map<std::string, std::string> labels);
+
+ bool operator==(const Key& other) const;
- CachedImage(
- const ::appc::spec::ImageManifest& _manifest,
- const std::string& _id,
- const std::string& _path)
- : manifest(_manifest), id(_id), path(_path) {}
+ std::string name;
+ std::map<std::string, std::string> labels;
+ };
- std::string rootfs() const;
+ struct KeyHasher
+ {
+ size_t operator()(const Key& key) const;
+ };
- const ::appc::spec::ImageManifest manifest;
+ Cache(const Path& imagesDir);
- // Image ID of the format "sha512-value" where "value" is the hex
- // encoded string of the sha512 digest of the uncompressed tar file
- // of the image.
- const std::string id;
+ const Path storeDir;
- // Absolute path to the extracted image.
- const std::string path;
+ // Mappings: Key -> image id.
+ hashmap<Cache::Key, std::string, KeyHasher> imageIds;
};
} // namespace appc {
http://git-wip-us.apache.org/repos/asf/mesos/blob/ec20fc4b/src/slave/containerizer/mesos/provisioner/appc/store.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/provisioner/appc/store.cpp b/src/slave/containerizer/mesos/provisioner/appc/store.cpp
index 1f9b573..4b38291 100644
--- a/src/slave/containerizer/mesos/provisioner/appc/store.cpp
+++ b/src/slave/containerizer/mesos/provisioner/appc/store.cpp
@@ -40,57 +40,17 @@ using std::list;
using std::string;
using std::vector;
+using process::Owned;
+
namespace mesos {
namespace internal {
namespace slave {
namespace appc {
-// Helper that implements this:
-// https://github.com/appc/spec/blob/master/spec/aci.md#dependency-matching
-static bool matches(Image::Appc requirements, const CachedImage& candidate)
-{
- // The name must match.
- if (candidate.manifest.name() != requirements.name()) {
- return false;
- }
-
- // If an id is specified the candidate must match.
- if (requirements.has_id() && (candidate.id != requirements.id())) {
- return false;
- }
-
- // Extract labels for easier comparison, this also weeds out duplicates.
- // TODO(xujyan): Detect duplicate labels in image manifest validation
- // and Image::Appc validation.
- hashmap<string, string> requiredLabels;
- foreach (const Label& label, requirements.labels().labels()) {
- requiredLabels[label.key()] = label.value();
- }
-
- hashmap<string, string> candidateLabels;
- foreach (const spec::ImageManifest::Label& label,
- candidate.manifest.labels()) {
- candidateLabels[label.name()] = label.value();
- }
-
- // Any label specified must be present and match in the candidate.
- foreachpair (const string& name,
- const string& value,
- requiredLabels) {
- if (!candidateLabels.contains(name) ||
- candidateLabels.get(name).get() != value) {
- return false;
- }
- }
-
- return true;
-}
-
-
class StoreProcess : public Process<StoreProcess>
{
public:
- StoreProcess(const string& rootDir);
+ StoreProcess(const string& rootDir, Owned<Cache> cache);
~StoreProcess() {}
@@ -103,8 +63,7 @@ private:
// --appc_store_dir.
const string rootDir;
- // Mappings: name -> id -> image.
- hashmap<string, hashmap<string, CachedImage>> images;
+ Owned<Cache> cache;
};
@@ -128,8 +87,13 @@ Try<Owned<slave::Store>> Store::create(const Flags& flags)
rootDir.error());
}
+ Try<Owned<Cache>> cache = Cache::create(Path(rootDir.get()));
+ if (cache.isError()) {
+ return Error("Failed to create image cache: " + cache.error());
+ }
+
return Owned<slave::Store>(new Store(
- Owned<StoreProcess>(new StoreProcess(rootDir.get()))));
+ Owned<StoreProcess>(new StoreProcess(rootDir.get(), cache.get()))));
}
@@ -159,36 +123,16 @@ Future<ImageInfo> Store::get(const Image& image)
}
-StoreProcess::StoreProcess(const string& _rootDir) : rootDir(_rootDir) {}
+StoreProcess::StoreProcess(const string& _rootDir, Owned<Cache> _cache)
+ : rootDir(_rootDir),
+ cache(_cache) {}
Future<Nothing> StoreProcess::recover()
{
- // Recover everything in the store.
- Try<list<string>> imageIds = os::ls(paths::getImagesDir(rootDir));
- if (imageIds.isError()) {
- return Failure(
- "Failed to list images under '" +
- paths::getImagesDir(rootDir) + "': " +
- imageIds.error());
- }
-
- foreach (const string& imageId, imageIds.get()) {
- string path = paths::getImagePath(rootDir, imageId);
- if (!os::stat::isdir(path)) {
- LOG(WARNING) << "Unexpected entry in storage: " << imageId;
- continue;
- }
-
- Try<CachedImage> image = CachedImage::create(path);
- if (image.isError()) {
- LOG(WARNING) << "Unexpected entry in storage: " << image.error();
- continue;
- }
-
- LOG(INFO) << "Restored image '" << image.get().manifest.name() << "'";
-
- images[image.get().manifest.name()].put(image.get().id, image.get());
+ Try<Nothing> recover = cache->recover();
+ if (recover.isError()) {
+ return Failure("Failed to recover cache: " + recover.error());
}
return Nothing();
@@ -203,33 +147,39 @@ Future<ImageInfo> StoreProcess::get(const Image& image)
const Image::Appc& appc = image.appc();
- if (!images.contains(appc.name())) {
- return Failure("No Appc image named '" + appc.name() + "' can be found");
+ Option<string> imageId = None();
+
+ // If the image specifies an id, use that. If not, then find the image in the
+ // cache by its name and labels. Note that if store has the image, it has to
+ // be in the cache. It is possible that an image could be found in cache but
+ // not in the store due to eviction of the image from the store in between
+ // cache restoration and now.
+
+ if (appc.has_id()) {
+ imageId = appc.id();
+ } else {
+ imageId = cache->find(appc);
}
- // Get local candidates.
- vector<CachedImage> candidates;
- foreach (const CachedImage& candidate, images[appc.name()].values()) {
- // The first match is returned.
- // TODO(xujyan): Some tie-breaking rules are necessary.
- if (matches(appc, candidate)) {
- LOG(INFO) << "Found match for Appc image '" << appc.name()
- << "' in the store";
-
- // TODO(gilbert): Get Appc runtime config from manifest.
-
- // The Appc store current doesn't support dependencies and this
- // is enforced by manifest validation: if the image's manifest
- // contains dependencies it would fail the validation and
- // wouldn't be stored in the store.
- return ImageInfo{
- vector<string>({candidate.rootfs()}),
- None()};
- }
+ if (imageId.isNone()) {
+ return Failure("Failed to find image '" + appc.name() + "' in cache");
+ }
+
+ // Now validate the image path for the image. This will also check for the
+ // existence of the directory.
+ Option<Error> error =
+ spec::validateLayout(paths::getImagePath(rootDir, imageId.get()));
+
+ if (error.isSome()) {
+ return Failure(
+ "Failed to validate directory for image '" + appc.name() + "': " +
+ error.get().message);
}
- return Failure("No Appc image named '" + appc.name() +
- "' can match the requirements");
+ return ImageInfo{
+ vector<string>({paths::getImageRootfsPath(rootDir, imageId.get())}),
+ None()
+ };
}
} // namespace appc {
http://git-wip-us.apache.org/repos/asf/mesos/blob/ec20fc4b/src/tests/containerizer/provisioner_appc_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/provisioner_appc_tests.cpp b/src/tests/containerizer/provisioner_appc_tests.cpp
index e0aa856..9d9779a 100644
--- a/src/tests/containerizer/provisioner_appc_tests.cpp
+++ b/src/tests/containerizer/provisioner_appc_tests.cpp
@@ -217,6 +217,36 @@ protected:
return paths::getImagePath(storeDir, imageId);
}
+
+ Image::Appc getTestImage() const
+ {
+ Image::Appc appc;
+ appc.set_name("foo.com/bar");
+ appc.set_id(
+ "sha512-e77d96aa0240eedf134b8c90baeaf76dca8e78691836301d7498c8402044604"
+ "2e797b296d6ab296e0954c2626bfb264322ebeb8f447dac4fac6511ea06bc61f0");
+
+ Label version;
+ version.set_key("version");
+ version.set_value("1.0.0");
+
+ Label arch;
+ arch.set_key("arch");
+ arch.set_value("amd64");
+
+ Label os;
+ os.set_key("os");
+ os.set_value("linux");
+
+ Labels labels;
+ labels.add_labels()->CopyFrom(version);
+ labels.add_labels()->CopyFrom(arch);
+ labels.add_labels()->CopyFrom(os);
+
+ appc.mutable_labels()->CopyFrom(labels);
+
+ return appc;
+ }
};
@@ -238,7 +268,8 @@ TEST_F(AppcStoreTest, Recover)
AWAIT_READY(store.get()->recover());
Image image;
- image.mutable_appc()->set_name("foo.com/bar");
+ image.mutable_appc()->CopyFrom(getTestImage());
+
Future<slave::ImageInfo> ImageInfo = store.get()->get(image);
AWAIT_READY(ImageInfo);
@@ -279,7 +310,7 @@ TEST_F(ProvisionerAppcTest, ROOT_Provision)
// Simulate a task that requires an image.
Image image;
- image.mutable_appc()->set_name("foo.com/bar");
+ image.mutable_appc()->CopyFrom(getTestImage());
ContainerID containerId;
containerId.set_value("12345");
@@ -344,7 +375,7 @@ TEST_F(ProvisionerAppcTest, Recover)
AWAIT_READY(provisioner1.get()->recover({}, {}));
Image image;
- image.mutable_appc()->set_name("foo.com/bar");
+ image.mutable_appc()->CopyFrom(getTestImage());
ContainerID containerId;
containerId.set_value(UUID::random().toString());