You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ya...@apache.org on 2015/09/04 02:30:14 UTC
[3/3] mesos git commit: Changed provisioner Store API and
implementation so it works as a read-through cache.
Changed provisioner Store API and implementation so it works as a read-through cache.
- i.e., It fetches images transparently when it's not in the local cache.
- This way, the store doesn't have the sense of "localness" anymore but rather it's an abstraction that provides access to all discoverable images, no matter where they can be found.
Review: https://reviews.apache.org/r/37929
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/eeb4227a
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/eeb4227a
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/eeb4227a
Branch: refs/heads/master
Commit: eeb4227adc24d1a1966339b5c0966139f1c9611e
Parents: fb63151
Author: Jiang Yan Xu <ya...@jxu.me>
Authored: Sat Aug 29 21:19:34 2015 -0700
Committer: Jiang Yan Xu <ya...@jxu.me>
Committed: Thu Sep 3 17:29:16 2015 -0700
----------------------------------------------------------------------
.../containerizer/provisioners/appc/store.cpp | 106 ++++++++++++++++---
.../containerizer/provisioners/appc/store.hpp | 50 ++++-----
.../containerizer/appc_provisioner_tests.cpp | 12 +--
3 files changed, 120 insertions(+), 48 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/eeb4227a/src/slave/containerizer/provisioners/appc/store.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/provisioners/appc/store.cpp b/src/slave/containerizer/provisioners/appc/store.cpp
index fbd1c53..04ab2ad 100644
--- a/src/slave/containerizer/provisioners/appc/store.cpp
+++ b/src/slave/containerizer/provisioners/appc/store.cpp
@@ -43,6 +43,69 @@ namespace internal {
namespace slave {
namespace appc {
+// Defines a locally cached image (which has passed validation).
+struct CachedImage
+{
+ CachedImage(
+ const AppcImageManifest& _manifest,
+ const string& _id,
+ const string& _path)
+ : manifest(_manifest), id(_id), path(_path) {}
+
+ const AppcImageManifest manifest;
+
+ // 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 string id;
+
+ // Absolute path to the extracted image.
+ const string path;
+};
+
+
+// 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 AppcImageManifest::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:
@@ -52,7 +115,7 @@ public:
Future<Nothing> recover();
- Future<std::vector<Store::Image>> get(const string& name);
+ Future<vector<string>> get(const Image::Appc& image);
private:
// Absolute path to the root directory of the store as defined by
@@ -60,7 +123,7 @@ private:
const string root;
// Mappings: name -> id -> image.
- hashmap<string, hashmap<string, Store::Image>> images;
+ hashmap<string, hashmap<string, CachedImage>> images;
};
@@ -107,17 +170,18 @@ Future<Nothing> Store::recover()
}
-Future<vector<Store::Image>> Store::get(const string& name)
+Future<vector<string>> Store::get(const Image::Appc& image)
{
- return dispatch(process.get(), &StoreProcess::get, name);
+ return dispatch(process.get(), &StoreProcess::get, image);
}
StoreProcess::StoreProcess(const string& _root) : root(_root) {}
-// Implemented as a helper function because it's going to be used by 'put()'.
-static Try<Store::Image> createImage(const string& imagePath)
+// Implemented as a helper function because it's going to be used for a
+// newly downloaded image too.
+static Try<CachedImage> createImage(const string& imagePath)
{
Option<Error> error = spec::validateLayout(imagePath);
if (error.isSome()) {
@@ -141,22 +205,34 @@ static Try<Store::Image> createImage(const string& imagePath)
return Error("Failed to parse manifest: " + manifest.error());
}
- return Store::Image(manifest.get(), imageId, imagePath);
+ return CachedImage(manifest.get(), imageId, imagePath);
}
-Future<vector<Store::Image>> StoreProcess::get(const string& name)
+Future<vector<string>> StoreProcess::get(const Image::Appc& image)
{
- if (!images.contains(name)) {
- return vector<Store::Image>();
+ if (!images.contains(image.name())) {
+ return Failure("No image named '" + image.name() + "' can be found");
}
- vector<Store::Image> result;
- foreach (const Store::Image& image, images[name].values()) {
- result.push_back(image);
+ // Get local candidates.
+ vector<CachedImage> candidates;
+ foreach (const CachedImage& candidate, images[image.name()].values()) {
+ // The first match is returned.
+ // TODO(xujyan): Some tie-breaking rules are necessary.
+ if (matches(image, candidate)) {
+ LOG(INFO) << "Found match for image '" << image.name()
+ << "' in the store";
+ // 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 vector<string>({candidate.path});
+ }
}
- return result;
+ return Failure("No image named '" + image.name() +
+ "' can match the requirements");
}
@@ -178,7 +254,7 @@ Future<Nothing> StoreProcess::recover()
continue;
}
- Try<Store::Image> image = createImage(path);
+ Try<CachedImage> image = createImage(path);
if (image.isError()) {
LOG(WARNING) << "Unexpected entry in storage: " << image.error();
continue;
http://git-wip-us.apache.org/repos/asf/mesos/blob/eeb4227a/src/slave/containerizer/provisioners/appc/store.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/provisioners/appc/store.hpp b/src/slave/containerizer/provisioners/appc/store.hpp
index e48d91b..95067f8 100644
--- a/src/slave/containerizer/provisioners/appc/store.hpp
+++ b/src/slave/containerizer/provisioners/appc/store.hpp
@@ -40,42 +40,38 @@ namespace appc {
class StoreProcess;
-// Provides the provisioner with access to locally stored / cached Appc images.
+// An image store abstraction that "stores" images. It serves as a read-through
+// cache (cache misses are fetched remotely and transparently) for images.
+// TODO(xujyan): The store currently keeps cached images indefinitely and we
+// should introduce cache eviction policies.
class Store
{
public:
- // Defines an image in the store (which has passed validation).
- struct Image
- {
- Image(
- const AppcImageManifest& _manifest,
- const std::string& _id,
- const std::string& _path)
- : manifest(_manifest), id(_id), path(_path) {}
-
- const AppcImageManifest manifest;
-
- // 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;
-
- // Absolute path of the extracted image.
- const std::string path;
- };
-
static Try<process::Owned<Store>> create(const Flags& flags);
~Store();
process::Future<Nothing> recover();
- // Get all images matched by name.
- process::Future<std::vector<Image>> get(const std::string& name);
-
- // TODO(xujyan): Implement a put() method that fetches an image into
- // the store. i.e.,
- // process::Future<StoredImage> put(const std::string& uri);
+ // Get the specified image (and all its recursive dependencies) as a list
+ // of rootfs layers in the topological order (dependencies go before
+ // dependents in the list). The images required to build this list are
+ // either retrieved from the local cache or fetched remotely.
+ // NOTE: The returned list should not have duplicates. e.g., in the
+ // following scenario the result should be [C, B, D, A] (B before D in this
+ // example is decided by the order in which A specifies its dependencies).
+ //
+ // A --> B --> C
+ // | ^
+ // |---> D ----|
+ //
+ // The returned future fails if the requested image or any of its
+ // dependencies cannot be found or failed to be fetched.
+ // TODO(xujyan): Fetching remotely is not implemented for now and until
+ // then the future fails directly if the image is not in the local cache.
+ // TODO(xujyan): The store currently doesn't support images that have
+ // dependencies and we should add it later.
+ process::Future<std::vector<std::string>> get(const Image::Appc& image);
private:
Store(process::Owned<StoreProcess> process);
http://git-wip-us.apache.org/repos/asf/mesos/blob/eeb4227a/src/tests/containerizer/appc_provisioner_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/appc_provisioner_tests.cpp b/src/tests/containerizer/appc_provisioner_tests.cpp
index 47b66b9..a4526e9 100644
--- a/src/tests/containerizer/appc_provisioner_tests.cpp
+++ b/src/tests/containerizer/appc_provisioner_tests.cpp
@@ -171,14 +171,14 @@ TEST_F(AppcProvisionerTest, StoreRecover)
// Recover the image from disk.
AWAIT_READY(store.get()->recover());
- Future<vector<Store::Image>> _images = store.get()->get("foo.com/bar");
- AWAIT_READY(_images);
+ Image image;
+ image.mutable_appc()->set_name("foo.com/bar");
+ Future<vector<string>> layers = store.get()->get(image.appc());
+ AWAIT_READY(layers);
- vector<Store::Image> images = _images.get();
-
- EXPECT_EQ(1u, images.size());
+ EXPECT_EQ(1u, layers.get().size());
ASSERT_SOME(os::realpath(imagePath));
- EXPECT_EQ(os::realpath(imagePath).get(), images.front().path);
+ EXPECT_EQ(os::realpath(imagePath).get(), layers.get().front());
}
} // namespace tests {