You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by gi...@apache.org on 2019/04/05 05:55:59 UTC
[mesos] 02/08: Refactored the UCR docker store to construct 'Image'
proto at pullers.
This is an automated email from the ASF dual-hosted git repository.
gilbert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit ed1587bc0ee4a94329b379c6c45430062bfd8e37
Author: Gilbert Song <so...@gmail.com>
AuthorDate: Mon Apr 1 16:11:43 2019 -0700
Refactored the UCR docker store to construct 'Image' proto at pullers.
This refactoring is needed for supporting docker manifest v2s2 because
the puller has to let the docker store knows the v1 config, and this
is also needed for garbage collecting the v1 config in the image layer
store.
Review: https://reviews.apache.org/r/70354
---
.../mesos/provisioner/docker/image_tar_puller.cpp | 22 +++++++++++------
.../mesos/provisioner/docker/image_tar_puller.hpp | 2 +-
.../mesos/provisioner/docker/message.proto | 4 +++-
.../mesos/provisioner/docker/metadata_manager.cpp | 28 ++++++----------------
.../mesos/provisioner/docker/metadata_manager.hpp | 12 ++--------
.../mesos/provisioner/docker/puller.hpp | 4 +++-
.../mesos/provisioner/docker/registry_puller.cpp | 28 +++++++++++++---------
.../mesos/provisioner/docker/registry_puller.hpp | 2 +-
.../mesos/provisioner/docker/store.cpp | 16 ++++++-------
.../containerizer/provisioner_docker_tests.cpp | 20 +++++++++++-----
10 files changed, 71 insertions(+), 67 deletions(-)
diff --git a/src/slave/containerizer/mesos/provisioner/docker/image_tar_puller.cpp b/src/slave/containerizer/mesos/provisioner/docker/image_tar_puller.cpp
index 3a33388..3e6741d 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/image_tar_puller.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/image_tar_puller.cpp
@@ -66,13 +66,13 @@ public:
~ImageTarPullerProcess() override {}
- Future<vector<string>> pull(
+ Future<Image> pull(
const spec::ImageReference& reference,
const string& directory,
const string& backend);
private:
- Future<vector<string>> _pull(
+ Future<Image> _pull(
const spec::ImageReference& reference,
const string& directory,
const string& backend);
@@ -152,7 +152,7 @@ ImageTarPuller::~ImageTarPuller()
}
-Future<vector<string>> ImageTarPuller::pull(
+Future<Image> ImageTarPuller::pull(
const spec::ImageReference& reference,
const string& directory,
const string& backend,
@@ -167,7 +167,7 @@ Future<vector<string>> ImageTarPuller::pull(
}
-Future<vector<string>> ImageTarPullerProcess::pull(
+Future<Image> ImageTarPullerProcess::pull(
const spec::ImageReference& reference,
const string& directory,
const string& backend)
@@ -186,7 +186,7 @@ Future<vector<string>> ImageTarPullerProcess::pull(
<< "' to '" << directory << "' using HDFS uri fetcher";
return fetcher->fetch(uri, directory)
- .then(defer(self(), [=]() -> Future<vector<string>> {
+ .then(defer(self(), [=]() -> Future<Image> {
const string source = paths::getImageArchiveTarPath(directory, image);
VLOG(1) << "Untarring image '" << reference
@@ -216,7 +216,7 @@ Future<vector<string>> ImageTarPullerProcess::pull(
}
-Future<vector<string>> ImageTarPullerProcess::_pull(
+Future<Image> ImageTarPullerProcess::_pull(
const spec::ImageReference& reference,
const string& directory,
const string& backend)
@@ -294,7 +294,15 @@ Future<vector<string>> ImageTarPullerProcess::_pull(
}
return extractLayers(directory, layerIds, backend)
- .then([layerIds]() -> vector<string> { return layerIds; });
+ .then([reference, layerIds]() -> Image {
+ Image image;
+ image.mutable_reference()->CopyFrom(reference);
+ foreach (const string& layerId, layerIds) {
+ image.add_layer_ids(layerId);
+ }
+
+ return image;
+ });
}
diff --git a/src/slave/containerizer/mesos/provisioner/docker/image_tar_puller.hpp b/src/slave/containerizer/mesos/provisioner/docker/image_tar_puller.hpp
index b5ec924..6c0fa83 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/image_tar_puller.hpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/image_tar_puller.hpp
@@ -50,7 +50,7 @@ public:
~ImageTarPuller() override;
- process::Future<std::vector<std::string>> pull(
+ process::Future<Image> pull(
const ::docker::spec::ImageReference& reference,
const std::string& directory,
const std::string& backend,
diff --git a/src/slave/containerizer/mesos/provisioner/docker/message.proto b/src/slave/containerizer/mesos/provisioner/docker/message.proto
index a55ac90..612c591 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/message.proto
+++ b/src/slave/containerizer/mesos/provisioner/docker/message.proto
@@ -28,7 +28,9 @@ package mesos.internal.slave.docker;
message Image {
required .docker.spec.ImageReference reference = 1;
- // The order of the layers represents the dependency between layers.
+ // The order of the layers represents the dependency between layers,
+ // where the root layer's id (no parent layer) is first and the leaf
+ // layer's id is last.
repeated string layer_ids = 2;
}
diff --git a/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp b/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp
index 847eb9a..53de625 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp
@@ -59,9 +59,7 @@ public:
Future<Nothing> recover();
- Future<Image> put(
- const spec::ImageReference& reference,
- const vector<string>& layerIds);
+ Future<Image> put(const Image& image);
Future<Option<Image>> get(
const spec::ImageReference& reference,
@@ -111,15 +109,12 @@ Future<Nothing> MetadataManager::recover()
}
-Future<Image> MetadataManager::put(
- const spec::ImageReference& reference,
- const vector<string>& layerIds)
+Future<Image> MetadataManager::put(const Image& image)
{
return dispatch(
process.get(),
&MetadataManagerProcess::put,
- reference,
- layerIds);
+ image);
}
@@ -145,19 +140,10 @@ Future<hashset<string>> MetadataManager::prune(
}
-Future<Image> MetadataManagerProcess::put(
- const spec::ImageReference& reference,
- const vector<string>& layerIds)
+Future<Image> MetadataManagerProcess::put(const Image& image)
{
- const string imageReference = stringify(reference);
-
- Image dockerImage;
- dockerImage.mutable_reference()->CopyFrom(reference);
- foreach (const string& layerId, layerIds) {
- dockerImage.add_layer_ids(layerId);
- }
-
- storedImages[imageReference] = dockerImage;
+ const string imageReference = stringify(image.reference());
+ storedImages[imageReference] = image;
Try<Nothing> status = persist();
if (status.isError()) {
@@ -166,7 +152,7 @@ Future<Image> MetadataManagerProcess::put(
VLOG(1) << "Successfully cached image '" << imageReference << "'";
- return dockerImage;
+ return image;
}
diff --git a/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp b/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp
index cfafd44..445363f 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp
@@ -68,18 +68,10 @@ public:
process::Future<Nothing> recover();
/**
- * Create an Image, put it in metadata manager and persist the reference
+ * Put the image in metadata manager and persist the reference
* store state to disk.
- *
- * @param reference the reference of the Docker image to place in the
- * reference store.
- * @param layerIds the list of layer ids that comprise the Docker image in
- * order where the root layer's id (no parent layer) is first
- * and the leaf layer's id is last.
*/
- process::Future<Image> put(
- const ::docker::spec::ImageReference& reference,
- const std::vector<std::string>& layerIds);
+ process::Future<Image> put(const Image& image);
/**
* Retrieve Image based on image reference if it is among the Images
diff --git a/src/slave/containerizer/mesos/provisioner/docker/puller.hpp b/src/slave/containerizer/mesos/provisioner/docker/puller.hpp
index a79f2ad..c793fd1 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/puller.hpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/puller.hpp
@@ -32,6 +32,8 @@
#include <mesos/secret/resolver.hpp>
+#include "slave/containerizer/mesos/provisioner/docker/message.hpp"
+
#include "slave/flags.hpp"
namespace mesos {
@@ -58,7 +60,7 @@ public:
* @param directory The target directory to store the layers.
* @return an ordered list of layer ids.
*/
- virtual process::Future<std::vector<std::string>> pull(
+ virtual process::Future<Image> pull(
const ::docker::spec::ImageReference& reference,
const std::string& directory,
const std::string& backend,
diff --git a/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp b/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
index a5683e3..7778976 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
@@ -66,26 +66,26 @@ public:
const Shared<uri::Fetcher>& _fetcher,
SecretResolver* _secretResolver);
- Future<vector<string>> pull(
+ Future<Image> pull(
const spec::ImageReference& reference,
const string& directory,
const string& backend,
const Option<Secret>& config);
private:
- Future<vector<string>> _pull(
+ Future<Image> _pull(
const spec::ImageReference& reference,
const string& directory,
const string& backend,
const Option<Secret::Value>& config = None());
- Future<vector<string>> __pull(
+ Future<Image> __pull(
const spec::ImageReference& reference,
const string& directory,
const string& backend,
const Option<Secret::Value>& config);
- Future<vector<string>> ___pull(
+ Future<Image> ___pull(
const spec::ImageReference& reference,
const string& directory,
const spec::v2::ImageManifest& manifest,
@@ -153,7 +153,7 @@ RegistryPuller::~RegistryPuller()
}
-Future<vector<string>> RegistryPuller::pull(
+Future<Image> RegistryPuller::pull(
const spec::ImageReference& reference,
const string& directory,
const string& backend,
@@ -216,7 +216,7 @@ static spec::ImageReference normalize(
}
-Future<vector<string>> RegistryPullerProcess::pull(
+Future<Image> RegistryPullerProcess::pull(
const spec::ImageReference& reference,
const string& directory,
const string& backend,
@@ -236,7 +236,7 @@ Future<vector<string>> RegistryPullerProcess::pull(
}
-Future<vector<string>> RegistryPullerProcess::_pull(
+Future<Image> RegistryPullerProcess::_pull(
const spec::ImageReference& _reference,
const string& directory,
const string& backend,
@@ -295,7 +295,7 @@ Future<vector<string>> RegistryPullerProcess::_pull(
}
-Future<vector<string>> RegistryPullerProcess::__pull(
+Future<Image> RegistryPullerProcess::__pull(
const spec::ImageReference& reference,
const string& directory,
const string& backend,
@@ -331,7 +331,7 @@ Future<vector<string>> RegistryPullerProcess::__pull(
}
-Future<vector<string>> RegistryPullerProcess::___pull(
+Future<Image> RegistryPullerProcess::___pull(
const spec::ImageReference& reference,
const string& directory,
const spec::v2::ImageManifest& manifest,
@@ -413,7 +413,7 @@ Future<vector<string>> RegistryPullerProcess::___pull(
}
return collect(futures)
- .then([=]() -> Future<vector<string>> {
+ .then([=]() -> Future<Image> {
// Remove the tarballs after the extraction.
foreach (const string& blobSum, blobSums) {
const string tar = path::join(directory, blobSum);
@@ -426,7 +426,13 @@ Future<vector<string>> RegistryPullerProcess::___pull(
}
}
- return layerIds;
+ Image image;
+ image.mutable_reference()->CopyFrom(reference);
+ foreach (const string& layerId, layerIds) {
+ image.add_layer_ids(layerId);
+ }
+
+ return image;
});
}
diff --git a/src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp b/src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp
index 926fab1..c2ceda2 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp
@@ -58,7 +58,7 @@ public:
* @param reference local name of the image.
* @param directory path to which the layers will be downloaded.
*/
- process::Future<std::vector<std::string>> pull(
+ process::Future<Image> pull(
const ::docker::spec::ImageReference& reference,
const std::string& directory,
const std::string& backend,
diff --git a/src/slave/containerizer/mesos/provisioner/docker/store.cpp b/src/slave/containerizer/mesos/provisioner/docker/store.cpp
index 60507aa..e0f2371 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/store.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/store.cpp
@@ -124,9 +124,9 @@ private:
const Image& image,
const string& backend);
- Future<vector<string>> moveLayers(
+ Future<Image> moveLayers(
const string& staging,
- const vector<string>& layerIds,
+ const Image& image,
const string& backend);
Future<Nothing> moveLayer(
@@ -356,8 +356,8 @@ Future<Image> StoreProcess::_get(
staging.get(),
lambda::_1,
backend))
- .then(defer(self(), [=](const vector<string>& layerIds) {
- return metadataManager->put(reference, layerIds);
+ .then(defer(self(), [=](const Image& image) {
+ return metadataManager->put(image);
}))
.onAny(defer(self(), [=](const Future<Image>&) {
pulling.erase(name);
@@ -419,18 +419,18 @@ Future<ImageInfo> StoreProcess::__get(
}
-Future<vector<string>> StoreProcess::moveLayers(
+Future<Image> StoreProcess::moveLayers(
const string& staging,
- const vector<string>& layerIds,
+ const Image& image,
const string& backend)
{
vector<Future<Nothing>> futures;
- foreach (const string& layerId, layerIds) {
+ foreach (const string& layerId, image.layer_ids()) {
futures.push_back(moveLayer(staging, layerId, backend));
}
return collect(futures)
- .then([layerIds]() -> vector<string> { return layerIds; });
+ .then([image]() -> Image { return image; });
}
diff --git a/src/tests/containerizer/provisioner_docker_tests.cpp b/src/tests/containerizer/provisioner_docker_tests.cpp
index a2a7f11..3da6628 100644
--- a/src/tests/containerizer/provisioner_docker_tests.cpp
+++ b/src/tests/containerizer/provisioner_docker_tests.cpp
@@ -36,6 +36,7 @@
#include "slave/containerizer/mesos/provisioner/constants.hpp"
#include "slave/containerizer/mesos/provisioner/paths.hpp"
+#include "slave/containerizer/mesos/provisioner/docker/message.hpp"
#include "slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp"
#include "slave/containerizer/mesos/provisioner/docker/paths.hpp"
#include "slave/containerizer/mesos/provisioner/docker/puller.hpp"
@@ -313,20 +314,20 @@ public:
MOCK_METHOD4(
pull,
- Future<vector<string>>(
+ Future<slave::docker::Image>(
const spec::ImageReference&,
const string&,
const string&,
const Option<Secret>&));
- Future<vector<string>> unmocked_pull(
+ Future<slave::docker::Image> unmocked_pull(
const spec::ImageReference& reference,
const string& directory,
const string& backend,
const Option<Secret>& config)
{
- // TODO(gilbert): Allow return list to be overridden.
- return vector<string>();
+ // TODO(gilbert): Allow return Image to be overridden.
+ return slave::docker::Image();
}
};
@@ -343,7 +344,7 @@ TEST_F(ProvisionerDockerLocalStoreTest, PullingSameImageSimutanuously)
MockPuller* puller = new MockPuller();
Future<Nothing> pull;
Future<string> directory;
- Promise<vector<string>> promise;
+ Promise<slave::docker::Image> promise;
EXPECT_CALL(*puller, pull(_, _, _, _))
.WillOnce(testing::DoAll(FutureSatisfy(&pull),
@@ -384,7 +385,14 @@ TEST_F(ProvisionerDockerLocalStoreTest, PullingSameImageSimutanuously)
Future<slave::ImageInfo> imageInfo2 =
store.get()->get(mesosImage, COPY_BACKEND);
- const vector<string> result = {"456"};
+ Try<spec::ImageReference> reference =
+ spec::parseImageReference(mesosImage.docker().name());
+
+ ASSERT_SOME(reference);
+
+ slave::docker::Image result;
+ result.mutable_reference()->CopyFrom(reference.get());
+ result.add_layer_ids("456");
ASSERT_TRUE(imageInfo2.isPending());
promise.set(result);