This is an automated email from the ASF dual-hosted git repository. qianzhang pushed a commit to branch 1.4.x in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 0161d5decf2df4f1fa12f63dfb31cf8e69a4aac9 Author: Gilbert Song <[email protected]> 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/local_puller.cpp | 20 +++++++++++----- .../mesos/provisioner/docker/local_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, 70 insertions(+), 66 deletions(-) diff --git a/src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp b/src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp index c4879ec..4fe3b46 100644 --- a/src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp +++ b/src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp @@ -58,13 +58,13 @@ public: ~LocalPullerProcess() {} - 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); @@ -119,7 +119,7 @@ LocalPuller::~LocalPuller() } -Future<vector<string>> LocalPuller::pull( +Future<Image> LocalPuller::pull( const spec::ImageReference& reference, const string& directory, const string& backend, @@ -134,7 +134,7 @@ Future<vector<string>> LocalPuller::pull( } -Future<vector<string>> LocalPullerProcess::pull( +Future<Image> LocalPullerProcess::pull( const spec::ImageReference& reference, const string& directory, const string& backend) @@ -160,7 +160,7 @@ Future<vector<string>> LocalPullerProcess::pull( } -Future<vector<string>> LocalPullerProcess::_pull( +Future<Image> LocalPullerProcess::_pull( const spec::ImageReference& reference, const string& directory, const string& backend) @@ -238,7 +238,15 @@ Future<vector<string>> LocalPullerProcess::_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/local_puller.hpp b/src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp index 4d2e497..fb4cdbc 100644 --- a/src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp +++ b/src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp @@ -46,7 +46,7 @@ public: ~LocalPuller(); - 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 7dfeb65..1078109 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, @@ -110,15 +108,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); } @@ -134,19 +129,10 @@ Future<Option<Image>> MetadataManager::get( } -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()) { @@ -155,7 +141,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 954da16..a5e321a 100644 --- a/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp +++ b/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp @@ -67,18 +67,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 693e841..e5abb68 100644 --- a/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp +++ b/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp @@ -67,26 +67,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, @@ -154,7 +154,7 @@ RegistryPuller::~RegistryPuller() } -Future<vector<string>> RegistryPuller::pull( +Future<Image> RegistryPuller::pull( const spec::ImageReference& reference, const string& directory, const string& backend, @@ -217,7 +217,7 @@ static spec::ImageReference normalize( } -Future<vector<string>> RegistryPullerProcess::pull( +Future<Image> RegistryPullerProcess::pull( const spec::ImageReference& reference, const string& directory, const string& backend, @@ -237,7 +237,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, @@ -296,7 +296,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, @@ -332,7 +332,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, @@ -414,7 +414,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); @@ -427,7 +427,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 4df679b..7489d7d 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 34d36a7..8d12104 100644 --- a/src/slave/containerizer/mesos/provisioner/docker/store.cpp +++ b/src/slave/containerizer/mesos/provisioner/docker/store.cpp @@ -96,9 +96,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( @@ -300,8 +300,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); @@ -359,18 +359,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) { list<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 44e13a5..970a33d 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" @@ -308,20 +309,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(); } }; @@ -338,7 +339,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), @@ -379,7 +380,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);
