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 21:55:12 UTC
[1/2] mesos git commit: Replaced docker Image::Name proto with
ImageReference.
Repository: mesos
Updated Branches:
refs/heads/master 7c04395a3 -> df503d3f6
Replaced docker Image::Name proto with ImageReference.
Review: https://reviews.apache.org/r/43371/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/c3c2c22a
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/c3c2c22a
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/c3c2c22a
Branch: refs/heads/master
Commit: c3c2c22a3c2577332e1dec4a6cf028f5f67aed97
Parents: 7c04395
Author: Gilbert Song <so...@gmail.com>
Authored: Fri Feb 12 12:12:01 2016 -0800
Committer: Jie Yu <yu...@gmail.com>
Committed: Fri Feb 12 12:26:39 2016 -0800
----------------------------------------------------------------------
include/mesos/docker/spec.hpp | 4 +
src/docker/spec.cpp | 17 ++++
.../mesos/provisioner/docker/local_puller.cpp | 33 ++++---
.../mesos/provisioner/docker/local_puller.hpp | 5 +-
.../mesos/provisioner/docker/message.hpp | 93 --------------------
.../mesos/provisioner/docker/message.proto | 19 ++--
.../provisioner/docker/metadata_manager.cpp | 43 ++++-----
.../provisioner/docker/metadata_manager.hpp | 15 ++--
.../mesos/provisioner/docker/puller.hpp | 4 +-
.../provisioner/docker/registry_client.cpp | 32 +++----
.../provisioner/docker/registry_client.hpp | 10 +--
.../provisioner/docker/registry_puller.cpp | 34 +++----
.../provisioner/docker/registry_puller.hpp | 7 +-
.../mesos/provisioner/docker/store.cpp | 44 +++++----
.../containerizer/provisioner_docker_tests.cpp | 93 +++++---------------
15 files changed, 172 insertions(+), 281 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/c3c2c22a/include/mesos/docker/spec.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/docker/spec.hpp b/include/mesos/docker/spec.hpp
index 6033e82..5fb6647 100644
--- a/include/mesos/docker/spec.hpp
+++ b/include/mesos/docker/spec.hpp
@@ -17,6 +17,7 @@
#ifndef __MESOS_DOCKER_SPEC_HPP__
#define __MESOS_DOCKER_SPEC_HPP__
+#include <iostream>
#include <string>
#include <stout/error.hpp>
@@ -50,6 +51,9 @@ namespace spec {
Try<ImageReference> parseImageReference(const std::string& s);
+std::ostream& operator<<(std::ostream& stream, const ImageReference& reference);
+
+
namespace v1 {
/**
http://git-wip-us.apache.org/repos/asf/mesos/blob/c3c2c22a/src/docker/spec.cpp
----------------------------------------------------------------------
diff --git a/src/docker/spec.cpp b/src/docker/spec.cpp
index 1f7adf7..07c0b87 100644
--- a/src/docker/spec.cpp
+++ b/src/docker/spec.cpp
@@ -22,6 +22,7 @@
#include <mesos/docker/spec.hpp>
+using std::ostream;
using std::string;
using std::vector;
@@ -81,6 +82,22 @@ Try<ImageReference> parseImageReference(const string& _s)
}
+ostream& operator<<(ostream& stream, const ImageReference& reference)
+{
+ if (reference.has_registry()) {
+ stream << reference.registry() << "/" << reference.repository();
+ } else {
+ stream << reference.repository();
+ }
+
+ if (reference.has_tag()) {
+ stream << ":" << reference.tag();
+ }
+
+ return stream;
+}
+
+
namespace v1 {
Option<Error> validate(const ImageManifest& manifest)
http://git-wip-us.apache.org/repos/asf/mesos/blob/c3c2c22a/src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp b/src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp
index abf6ddf..c980e44 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp
@@ -38,6 +38,8 @@
using namespace process;
+namespace spec = docker::spec;
+
using std::list;
using std::map;
using std::pair;
@@ -57,12 +59,12 @@ public:
~LocalPullerProcess() {}
Future<list<pair<string, string>>> pull(
- const Image::Name& name,
+ const spec::ImageReference& reference,
const string& directory);
private:
Future<list<pair<string, string>>> putImage(
- const Image::Name& name,
+ const spec::ImageReference& reference,
const string& directory);
Future<list<pair<string, string>>> putLayers(
@@ -102,23 +104,24 @@ LocalPuller::~LocalPuller()
Future<list<pair<string, string>>> LocalPuller::pull(
- const Image::Name& name,
+ const spec::ImageReference& reference,
const Path& directory)
{
- return dispatch(process.get(), &LocalPullerProcess::pull, name, directory);
+ return dispatch(
+ process.get(), &LocalPullerProcess::pull, reference, directory);
}
Future<list<pair<string, string>>> LocalPullerProcess::pull(
- const Image::Name& name,
+ const spec::ImageReference& reference,
const string& directory)
{
const string tarPath = paths::getImageArchiveTarPath(
archivesDir,
- stringify(name));
+ stringify(reference));
if (!os::exists(tarPath)) {
- return Failure("Failed to find archive for image '" + stringify(name) +
+ return Failure("Failed to find archive for image '" + stringify(reference) +
"' at '" + tarPath + "'");
}
@@ -126,7 +129,7 @@ Future<list<pair<string, string>>> LocalPullerProcess::pull(
<< "' to '" << directory << "'";
return untar(tarPath, directory)
- .then(defer(self(), &Self::putImage, name, directory));
+ .then(defer(self(), &Self::putImage, reference, directory));
}
@@ -158,7 +161,7 @@ static Result<string> getParentId(
Future<list<pair<string, string>>> LocalPullerProcess::putImage(
- const Image::Name& name,
+ const spec::ImageReference& reference,
const string& directory)
{
Try<string> value =
@@ -174,22 +177,26 @@ Future<list<pair<string, string>>> LocalPullerProcess::putImage(
}
Result<JSON::Object> repositoryValue =
- json.get().find<JSON::Object>(name.repository());
+ json.get().find<JSON::Object>(reference.repository());
if (repositoryValue.isError()) {
return Failure("Failed to find repository: " + repositoryValue.error());
} else if (repositoryValue.isNone()) {
- return Failure("Repository '" + name.repository() + "' is not found");
+ return Failure("Repository '" + reference.repository() + "' is not found");
}
const JSON::Object repositoryJson = repositoryValue.get();
+ const string tag = reference.has_tag()
+ ? reference.tag()
+ : "latest";
+
// We don't use JSON find here because a tag might contain a '.'.
map<string, JSON::Value>::const_iterator entry =
- repositoryJson.values.find(name.tag());
+ repositoryJson.values.find(tag);
if (entry == repositoryJson.values.end()) {
- return Failure("Tag '" + name.tag() + "' is not found");
+ return Failure("Tag '" + tag + "' is not found");
} else if (!entry->second.is<JSON::String>()) {
return Failure("Tag JSON value expected to be JSON::String");
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/c3c2c22a/src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp b/src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp
index 7f441de..811c24b 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp
@@ -17,9 +17,10 @@
#ifndef __PROVISIONER_DOCKER_LOCAL_PULLER_HPP__
#define __PROVISIONER_DOCKER_LOCAL_PULLER_HPP__
+#include <mesos/docker/spec.hpp>
+
#include "slave/containerizer/mesos/provisioner/store.hpp"
-#include "slave/containerizer/mesos/provisioner/docker/message.hpp"
#include "slave/containerizer/mesos/provisioner/docker/puller.hpp"
#include "slave/flags.hpp"
@@ -46,7 +47,7 @@ public:
~LocalPuller();
process::Future<std::list<std::pair<std::string, std::string>>> pull(
- const Image::Name& name,
+ const ::docker::spec::ImageReference& reference,
const Path& directory);
private:
http://git-wip-us.apache.org/repos/asf/mesos/blob/c3c2c22a/src/slave/containerizer/mesos/provisioner/docker/message.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/provisioner/docker/message.hpp b/src/slave/containerizer/mesos/provisioner/docker/message.hpp
index 43c3608..f57f42f 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/message.hpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/message.hpp
@@ -17,100 +17,7 @@
#ifndef __MESSAGES_DOCKER_PROVISIONER_HPP__
#define __MESSAGES_DOCKER_PROVISIONER_HPP__
-#include <stout/strings.hpp>
-
// ONLY USEFUL AFTER RUNNING PROTOC.
#include "slave/containerizer/mesos/provisioner/docker/message.pb.h"
-namespace mesos {
-namespace internal {
-namespace slave {
-namespace docker {
-
-// Docker expects the image to be specified on the command line as:
-// [REGISTRY_HOST[:REGISTRY_PORT]/]REPOSITORY[:TAG|@TYPE:DIGEST]
-//
-// This format is inherently ambiguous when dealing with repository
-// names that include forward slashes. To disambiguate, the docker
-// code looks for '.', or ':', or 'localhost' to decide if the
-// first component is a registry or a respository name. For more
-// detail, drill into the implementation of docker pull.
-//
-// TODO(bmahler): We currently store the digest as a tag, does
-// that makes sense?
-//
-// TODO(bmahler): Validate based on docker's validation logic
-// and return a Try here.
-//
-// TODO(jieyu): Remove this in favor of using 'spec::parseImageName'.
-inline Image::Name parseImageName(std::string s)
-{
- Image::Name name;
-
- // Extract the digest.
- if (strings::contains(s, "@")) {
- std::vector<std::string> split = strings::split(s, "@");
-
- s = split[0];
- name.set_tag(split[1]);
- }
-
- // Remove the tag. We need to watch out for a
- // host:port registry, which also contains ':'.
- if (strings::contains(s, ":")) {
- std::vector<std::string> split = strings::split(s, ":");
-
- // The tag must be the last component. If a slash is
- // present there is a registry port and no tag.
- if (!strings::contains(split.back(), "/")) {
- name.set_tag(split.back());
- split.pop_back();
-
- s = strings::join(":", split);
- }
- }
-
- // Default to the 'latest' tag when omitted.
- if (name.tag().empty()) {
- name.set_tag("latest");
- }
-
- // Extract the registry and repository. The first component can
- // either be the registry, or the first part of the repository!
- // We resolve this ambiguity using the same hacks used in the
- // docker code ('.', ':', 'localhost' indicate a registry).
- std::vector<std::string> split = strings::split(s, "/", 2);
-
- if (split.size() == 1) {
- name.set_repository(s);
- } else if (strings::contains(split[0], ".") ||
- strings::contains(split[0], ":") ||
- split[0] == "localhost") {
- name.set_registry(split[0]);
- name.set_repository(split[1]);
- } else {
- name.set_repository(s);
- }
-
- return name;
-}
-
-
-inline std::ostream& operator<<(
- std::ostream& stream,
- const Image::Name& name)
-{
- if (name.has_registry()) {
- return stream << name.registry() << "/" << name.repository() << ":"
- << name.tag();
- }
-
- return stream << name.repository() << ":" << name.tag();
-}
-
-} // namespace docker {
-} // namespace slave {
-} // namespace internal {
-} // namespace mesos {
-
#endif // __MESSAGES_DOCKER_PROVISIONER_HPP__
http://git-wip-us.apache.org/repos/asf/mesos/blob/c3c2c22a/src/slave/containerizer/mesos/provisioner/docker/message.proto
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/provisioner/docker/message.proto b/src/slave/containerizer/mesos/provisioner/docker/message.proto
index 003c666..c93c7a9 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/message.proto
+++ b/src/slave/containerizer/mesos/provisioner/docker/message.proto
@@ -14,26 +14,17 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-import "mesos/mesos.proto";
+import "mesos/docker/spec.proto";
package mesos.internal.slave.docker;
/**
- * A Docker Image name and the layer ids of the layers that comprise the image.
- * The layerIds are ordered, with the root layer id (no parent layer id) first
- * and the leaf layer id last.
+ * A Docker Image reference and the layer ids of the layers that comprise the
+ * image. The layerIds are ordered, with the root layer id (no parent layer id)
+ * first and the leaf layer id last.
*/
message Image {
- // TODO(jieyu): Remove this in favor of using spec::ImageReference.
- message Name {
- optional string registry = 1;
- required string repository = 2;
-
- // TODO(bmahler): This may hold a tag or a digest, split these?
- required string tag = 3;
- }
-
- required Name name = 1;
+ required .docker.spec.ImageReference reference = 1;
// The order of the layers represents the dependency between layers.
repeated string layer_ids = 2;
http://git-wip-us.apache.org/repos/asf/mesos/blob/c3c2c22a/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp b/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp
index bf85038..f391a94 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp
@@ -38,6 +38,8 @@
using namespace process;
+namespace spec = docker::spec;
+
using std::list;
using std::string;
using std::vector;
@@ -57,10 +59,10 @@ public:
Future<Nothing> recover();
Future<Image> put(
- const Image::Name& name,
+ const spec::ImageReference& reference,
const vector<string>& layerIds);
- Future<Option<Image>> get(const Image::Name& name);
+ Future<Option<Image>> get(const spec::ImageReference& reference);
// TODO(chenlily): Implement removal of unreferenced images.
@@ -106,36 +108,37 @@ Future<Nothing> MetadataManager::recover()
Future<Image> MetadataManager::put(
- const Image::Name& name,
+ const spec::ImageReference& reference,
const vector<string>& layerIds)
{
return dispatch(
process.get(),
&MetadataManagerProcess::put,
- name,
+ reference,
layerIds);
}
-Future<Option<Image>> MetadataManager::get(const Image::Name& name)
+Future<Option<Image>> MetadataManager::get(
+ const spec::ImageReference& reference)
{
- return dispatch(process.get(), &MetadataManagerProcess::get, name);
+ return dispatch(process.get(), &MetadataManagerProcess::get, reference);
}
Future<Image> MetadataManagerProcess::put(
- const Image::Name& name,
+ const spec::ImageReference& reference,
const vector<string>& layerIds)
{
- const string imageName = stringify(name);
+ const string imageReference = stringify(reference);
Image dockerImage;
- dockerImage.mutable_name()->CopyFrom(name);
+ dockerImage.mutable_reference()->CopyFrom(reference);
foreach (const string& layerId, layerIds) {
dockerImage.add_layer_ids(layerId);
}
- storedImages[imageName] = dockerImage;
+ storedImages[imageReference] = dockerImage;
Try<Nothing> status = persist();
if (status.isError()) {
@@ -147,15 +150,15 @@ Future<Image> MetadataManagerProcess::put(
Future<Option<Image>> MetadataManagerProcess::get(
- const Image::Name& name)
+ const spec::ImageReference& reference)
{
- const string imageName = stringify(name);
+ const string imageReference = stringify(reference);
- if (!storedImages.contains(imageName)) {
+ if (!storedImages.contains(imageReference)) {
return None();
}
- return storedImages[imageName];
+ return storedImages[imageReference];
}
@@ -211,18 +214,18 @@ Future<Nothing> MetadataManagerProcess::recover()
}
}
- const string imageName = stringify(image.name());
+ const string imageReference = stringify(image.reference());
if (!missingLayerIds.empty()) {
- LOG(WARNING) << "Skipped loading image '" << imageName << "'";
+ LOG(WARNING) << "Skipped loading image '" << imageReference << "'";
continue;
}
- if (storedImages.contains(imageName)) {
- LOG(WARNING) << "Found duplicate image in recovery for image name '"
- << imageName << "'";
+ if (storedImages.contains(imageReference)) {
+ LOG(WARNING) << "Found duplicate image in recovery for image reference '"
+ << imageReference << "'";
} else {
- storedImages[imageName] = image;
+ storedImages[imageReference] = image;
}
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/c3c2c22a/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp b/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp
index cb827bf..137af50 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp
@@ -30,6 +30,8 @@
#include <process/owned.hpp>
#include <process/process.hpp>
+#include <mesos/docker/spec.hpp>
+
#include "slave/containerizer/mesos/provisioner/provisioner.hpp"
#include "slave/containerizer/mesos/provisioner/docker/message.hpp"
@@ -68,23 +70,24 @@ public:
* Create an Image, put it in metadata manager and persist the reference
* store state to disk.
*
- * @param name the name of the Docker image to place in the reference
- * store.
+ * @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 Image::Name& name,
+ const ::docker::spec::ImageReference& reference,
const std::vector<std::string>& layerIds);
/**
- * Retrieve Image based on image name if it is among the Images
+ * Retrieve Image based on image reference if it is among the Images
* stored in memory.
*
- * @param name the name of the Docker image to retrieve
+ * @param reference the reference of the Docker image to retrieve
*/
- process::Future<Option<Image>> get(const Image::Name& name);
+ process::Future<Option<Image>> get(
+ const ::docker::spec::ImageReference& reference);
private:
explicit MetadataManager(process::Owned<MetadataManagerProcess> process);
http://git-wip-us.apache.org/repos/asf/mesos/blob/c3c2c22a/src/slave/containerizer/mesos/provisioner/docker/puller.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/provisioner/docker/puller.hpp b/src/slave/containerizer/mesos/provisioner/docker/puller.hpp
index d12b3d0..5b2d72c 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/puller.hpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/puller.hpp
@@ -28,7 +28,7 @@
#include <process/future.hpp>
#include <process/owned.hpp>
-#include "slave/containerizer/mesos/provisioner/docker/message.hpp"
+#include <mesos/docker/spec.hpp>
#include "slave/flags.hpp"
@@ -56,7 +56,7 @@ public:
* dependency.
*/
virtual process::Future<std::list<std::pair<std::string, std::string>>> pull(
- const docker::Image::Name& name,
+ const ::docker::spec::ImageReference& reference,
const Path& directory) = 0;
};
http://git-wip-us.apache.org/repos/asf/mesos/blob/c3c2c22a/src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp b/src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp
index 10c28f5..2ba6b10 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp
@@ -39,6 +39,9 @@
#include "slave/containerizer/mesos/provisioner/docker/registry_client.hpp"
#include "slave/containerizer/mesos/provisioner/docker/token_manager.hpp"
+namespace http = process::http;
+namespace spec = docker::spec;
+
using std::string;
using std::vector;
@@ -47,9 +50,6 @@ using process::Future;
using process::Owned;
using process::Process;
-namespace http = process::http;
-namespace spec = docker::spec;
-
using http::Pipe;
namespace mesos {
@@ -68,10 +68,10 @@ public:
const Option<Credentials>& credentials);
Future<spec::v2::ImageManifest> getManifest(
- const Image::Name& imageName);
+ const spec::ImageReference& reference);
Future<size_t> getBlob(
- const Image::Name& imageName,
+ const spec::ImageReference& reference,
const Option<string>& digest,
const Path& filePath);
@@ -109,7 +109,7 @@ private:
int fd,
Pipe::Reader reader);
- string getRepositoryPath(const Image::Name& imageName) const;
+ string getRepositoryPath(const spec::ImageReference& reference) const;
string getAPIVersion() const;
@@ -168,24 +168,24 @@ RegistryClient::~RegistryClient()
Future<spec::v2::ImageManifest> RegistryClient::getManifest(
- const Image::Name& imageName)
+ const spec::ImageReference& reference)
{
return dispatch(
process_.get(),
&RegistryClientProcess::getManifest,
- imageName);
+ reference);
}
Future<size_t> RegistryClient::getBlob(
- const Image::Name& imageName,
+ const spec::ImageReference& reference,
const Option<string>& digest,
const Path& filePath)
{
return dispatch(
process_.get(),
&RegistryClientProcess::getBlob,
- imageName,
+ reference,
digest,
filePath);
}
@@ -528,18 +528,18 @@ string RegistryClientProcess::getAPIVersion() const
// TODO(tnachen): Support unoffical repositories and loading in repository
// information.
string RegistryClientProcess::getRepositoryPath(
- const Image::Name& imageName) const
+ const spec::ImageReference& reference) const
{
- return getAPIVersion() + "/library/" + imageName.repository();
+ return getAPIVersion() + "/library/" + reference.repository();
}
Future<spec::v2::ImageManifest> RegistryClientProcess::getManifest(
- const Image::Name& imageName)
+ const spec::ImageReference& reference)
{
http::URL manifestURL(registryServer_);
manifestURL.path =
- getRepositoryPath(imageName) + "/manifests/" + imageName.tag();
+ getRepositoryPath(reference) + "/manifests/" + reference.tag();
return doHttpGet(manifestURL, None(), false, true, None())
.then(defer(self(), [this] (
@@ -604,7 +604,7 @@ Future<size_t> RegistryClientProcess::saveBlob(
Future<size_t> RegistryClientProcess::getBlob(
- const Image::Name& imageName,
+ const spec::ImageReference& reference,
const Option<string>& digest,
const Path& filePath)
{
@@ -614,7 +614,7 @@ Future<size_t> RegistryClientProcess::getBlob(
"Failed to create directory to download blob: " + mkdir.error());
}
- const string blobURLPath = getRepositoryPath(imageName) + "/blobs/" +
+ const string blobURLPath = getRepositoryPath(reference) + "/blobs/" +
digest.getOrElse("");
http::URL blobURL(registryServer_);
http://git-wip-us.apache.org/repos/asf/mesos/blob/c3c2c22a/src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp b/src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp
index bfe3086..377da0c 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp
@@ -31,8 +31,6 @@
#include <mesos/docker/spec.hpp>
-#include "slave/containerizer/mesos/provisioner/docker/message.hpp"
-
namespace mesos {
namespace internal {
namespace slave {
@@ -77,25 +75,25 @@ public:
/**
* Fetches manifest for a repository from the client's remote registry server.
*
- * @param imageName Image information(Name, tag).
+ * @param reference Image information(Name, tag).
* @return Manifest on success.
* Failure on process failure.
*/
process::Future<::docker::spec::v2::ImageManifest> getManifest(
- const Image::Name& imageName);
+ const ::docker::spec::ImageReference& reference);
/**
* Fetches blob for a repository from the client's remote registry server.
*
- * @param imageName the Docker image to download.
+ * @param reference the Docker image to download.
* @param digest digest of the blob (from manifest).
* @param filePath file path to store the fetched blob.
* @return size of downloaded blob on success.
* Failure in case of any errors.
*/
process::Future<size_t> getBlob(
- const Image::Name& imageName,
+ const ::docker::spec::ImageReference& reference,
const Option<std::string>& digest,
const Path& filePath);
http://git-wip-us.apache.org/repos/asf/mesos/blob/c3c2c22a/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp b/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
index 623c54e..3fcf147 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
@@ -57,7 +57,7 @@ public:
static Try<Owned<RegistryPullerProcess>> create(const Flags& flags);
process::Future<list<pair<string, string>>> pull(
- const Image::Name& imageName,
+ const spec::ImageReference& reference,
const Path& directory);
private:
@@ -66,18 +66,18 @@ private:
const Duration& timeout);
Future<pair<string, string>> downloadLayer(
- const Image::Name& imageName,
+ const spec::ImageReference& reference,
const Path& directory,
const string& blobSum,
const string& id);
Future<list<pair<string, string>>> downloadLayers(
const spec::v2::ImageManifest& manifest,
- const Image::Name& imageName,
+ const spec::ImageReference& reference,
const Path& downloadDir);
process::Future<list<pair<string, string>>> _pull(
- const Image::Name& imageName,
+ const spec::ImageReference& reference,
const Path& downloadDir);
Future<list<pair<string, string>>> untarLayers(
@@ -121,13 +121,13 @@ RegistryPuller::~RegistryPuller()
Future<list<pair<string, string>>> RegistryPuller::pull(
- const Image::Name& imageName,
+ const spec::ImageReference& reference,
const Path& downloadDir)
{
return dispatch(
process_.get(),
&RegistryPullerProcess::pull,
- imageName,
+ reference,
downloadDir);
}
@@ -174,17 +174,17 @@ RegistryPullerProcess::RegistryPullerProcess(
Future<pair<string, string>> RegistryPullerProcess::downloadLayer(
- const Image::Name& imageName,
+ const spec::ImageReference& reference,
const Path& directory,
const string& blobSum,
const string& layerId)
{
VLOG(1) << "Downloading layer '" << layerId
- << "' for image '" << stringify(imageName) << "'";
+ << "' for image '" << stringify(reference) << "'";
if (downloadTracker_.contains(layerId)) {
VLOG(1) << "Download already in progress for image '"
- << stringify(imageName) << "', layer '" << layerId << "'";
+ << stringify(reference) << "', layer '" << layerId << "'";
return downloadTracker_.at(layerId)->future();
}
@@ -197,7 +197,7 @@ Future<pair<string, string>> RegistryPullerProcess::downloadLayer(
const Path downloadFile(path::join(directory, layerId + ".tar"));
registryClient_->getBlob(
- imageName,
+ reference,
blobSum,
downloadFile)
.onAny(process::defer(
@@ -225,21 +225,21 @@ Future<pair<string, string>> RegistryPullerProcess::downloadLayer(
Future<list<pair<string, string>>> RegistryPullerProcess::pull(
- const Image::Name& imageName,
+ const spec::ImageReference& reference,
const Path& directory)
{
// TODO(jojy): Have one outgoing manifest request per image.
- return registryClient_->getManifest(imageName)
- .then(process::defer(self(), [this, directory, imageName](
+ return registryClient_->getManifest(reference)
+ .then(process::defer(self(), [this, directory, reference](
const spec::v2::ImageManifest& manifest) {
- return downloadLayers(manifest, imageName, directory);
+ return downloadLayers(manifest, reference, directory);
}))
.then(process::defer(self(), [this, directory](
const Future<list<pair<string, string>>>& layerFutures)
-> Future<list<pair<string, string>>> {
return untarLayers(layerFutures, directory);
}))
- .after(pullTimeout_, [imageName](
+ .after(pullTimeout_, [reference](
Future<list<pair<string, string>>> future) {
future.discard();
@@ -250,7 +250,7 @@ Future<list<pair<string, string>>> RegistryPullerProcess::pull(
Future<list<pair<string, string>>> RegistryPullerProcess::downloadLayers(
const spec::v2::ImageManifest& manifest,
- const Image::Name& imageName,
+ const spec::ImageReference& reference,
const Path& directory)
{
list<Future<pair<string, string>>> downloadFutures;
@@ -261,7 +261,7 @@ Future<list<pair<string, string>>> RegistryPullerProcess::downloadLayers(
CHECK(manifest.history(i).has_v1());
downloadFutures.push_back(
- downloadLayer(imageName,
+ downloadLayer(reference,
directory,
manifest.fslayers(i).blobsum(),
manifest.history(i).v1().id()));
http://git-wip-us.apache.org/repos/asf/mesos/blob/c3c2c22a/src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp b/src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp
index 0c37e1f..bccbac1 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp
@@ -28,7 +28,8 @@
#include <process/http.hpp>
#include <process/process.hpp>
-#include "slave/containerizer/mesos/provisioner/docker/message.hpp"
+#include <mesos/docker/spec.hpp>
+
#include "slave/containerizer/mesos/provisioner/docker/puller.hpp"
namespace mesos {
@@ -56,11 +57,11 @@ public:
* Pulls an image into a download directory. This image could consist
* multiple layers of blobs.
*
- * @param imageName local name of the image.
+ * @param reference local name of the image.
* @param downloadDir path to which the layers should be downloaded.
*/
process::Future<std::list<std::pair<std::string, std::string>>> pull(
- const Image::Name& imageName,
+ const ::docker::spec::ImageReference& reference,
const Path& downloadDir);
private:
http://git-wip-us.apache.org/repos/asf/mesos/blob/c3c2c22a/src/slave/containerizer/mesos/provisioner/docker/store.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/provisioner/docker/store.cpp b/src/slave/containerizer/mesos/provisioner/docker/store.cpp
index 251973c..12a3906 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/store.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/store.cpp
@@ -39,6 +39,8 @@
using namespace process;
+namespace spec = docker::spec;
+
using std::list;
using std::pair;
using std::string;
@@ -67,14 +69,17 @@ public:
Future<ImageInfo> get(const mesos::Image& image);
private:
- Future<Image> _get(const Image::Name& name, const Option<Image>& image);
+ Future<Image> _get(
+ const spec::ImageReference& reference,
+ const Option<Image>& image);
+
Future<ImageInfo> __get(const Image& image);
Future<vector<string>> moveLayers(
const list<pair<string, string>>& layerPaths);
Future<Image> storeImage(
- const Image::Name& name,
+ const spec::ImageReference& reference,
const vector<string>& layerIds);
Future<Nothing> moveLayer(
@@ -162,16 +167,21 @@ Future<ImageInfo> StoreProcess::get(const mesos::Image& image)
return Failure("Docker provisioner store only supports Docker images");
}
- Image::Name imageName = parseImageName(image.docker().name());
+ Try<spec::ImageReference> reference =
+ spec::parseImageReference(image.docker().name());
+ if (reference.isError()) {
+ return Failure("Failed to parse docker image '" + image.docker().name() +
+ "' as 'reference': " + reference.error());
+ }
- return metadataManager->get(imageName)
- .then(defer(self(), &Self::_get, imageName, lambda::_1))
+ return metadataManager->get(reference.get())
+ .then(defer(self(), &Self::_get, reference.get(), lambda::_1))
.then(defer(self(), &Self::__get, lambda::_1));
}
Future<Image> StoreProcess::_get(
- const Image::Name& name,
+ const spec::ImageReference& reference,
const Option<Image>& image)
{
if (image.isSome()) {
@@ -185,18 +195,18 @@ Future<Image> StoreProcess::_get(
return Failure("Failed to create a staging directory");
}
- const string imageName = stringify(name);
+ const string imageReference = stringify(reference);
- if (!pulling.contains(imageName)) {
+ if (!pulling.contains(imageReference)) {
Owned<Promise<Image>> promise(new Promise<Image>());
- Future<Image> future = puller->pull(name, Path(staging.get()))
+ Future<Image> future = puller->pull(reference, Path(staging.get()))
.then(defer(self(), &Self::moveLayers, lambda::_1))
- .then(defer(self(), &Self::storeImage, name, lambda::_1))
- .onAny(defer(self(), [this, imageName](const Future<Image>&) {
- pulling.erase(imageName);
+ .then(defer(self(), &Self::storeImage, reference, lambda::_1))
+ .onAny(defer(self(), [this, imageReference](const Future<Image>&) {
+ pulling.erase(imageReference);
}))
- .onAny([staging, imageName]() {
+ .onAny([staging, imageReference]() {
Try<Nothing> rmdir = os::rmdir(staging.get());
if (rmdir.isError()) {
LOG(WARNING) << "Failed to remove staging directory: "
@@ -205,12 +215,12 @@ Future<Image> StoreProcess::_get(
});
promise->associate(future);
- pulling[imageName] = promise;
+ pulling[imageReference] = promise;
return promise->future();
}
- return pulling[imageName]->future();
+ return pulling[imageReference]->future();
}
@@ -274,10 +284,10 @@ Future<vector<string>> StoreProcess::moveLayers(
Future<Image> StoreProcess::storeImage(
- const Image::Name& name,
+ const spec::ImageReference& reference,
const vector<string>& layerIds)
{
- return metadataManager->put(name, layerIds);
+ return metadataManager->put(reference, layerIds);
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/c3c2c22a/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 4f77dcb..4db6793 100644
--- a/src/tests/containerizer/provisioner_docker_tests.cpp
+++ b/src/tests/containerizer/provisioner_docker_tests.cpp
@@ -70,7 +70,6 @@ using process::Subprocess;
using process::network::Socket;
-using slave::docker::parseImageName;
using slave::docker::Puller;
using slave::docker::RegistryPuller;
@@ -84,66 +83,6 @@ namespace mesos {
namespace internal {
namespace tests {
-// TODO(jieyu): Remove this test in favor of using
-// DockerSpecTest.ParseImageReference.
-TEST(DockerUtilsTest, ParseImageName)
-{
- slave::docker::Image::Name name;
-
- name = parseImageName("library/busybox");
- EXPECT_FALSE(name.has_registry());
- EXPECT_EQ("library/busybox", name.repository());
- EXPECT_EQ("latest", name.tag());
-
- name = parseImageName("busybox");
- EXPECT_FALSE(name.has_registry());
- EXPECT_EQ("busybox", name.repository());
- EXPECT_EQ("latest", name.tag());
-
- name = parseImageName("library/busybox:tag");
- EXPECT_FALSE(name.has_registry());
- EXPECT_EQ("library/busybox", name.repository());
- EXPECT_EQ("tag", name.tag());
-
- // Note that the digest is stored as a tag.
- name = parseImageName(
- "library/busybox"
- "@sha256:bc8813ea7b3603864987522f02a7"
- "6101c17ad122e1c46d790efc0fca78ca7bfb");
- EXPECT_FALSE(name.has_registry());
- EXPECT_EQ("library/busybox", name.repository());
- EXPECT_EQ("sha256:bc8813ea7b3603864987522f02a7"
- "6101c17ad122e1c46d790efc0fca78ca7bfb",
- name.tag());
-
- name = parseImageName("registry.io/library/busybox");
- EXPECT_EQ("registry.io", name.registry());
- EXPECT_EQ("library/busybox", name.repository());
- EXPECT_EQ("latest", name.tag());
-
- name = parseImageName("registry.io/library/busybox:tag");
- EXPECT_EQ("registry.io", name.registry());
- EXPECT_EQ("library/busybox", name.repository());
- EXPECT_EQ("tag", name.tag());
-
- name = parseImageName("registry.io:80/library/busybox:tag");
- EXPECT_EQ("registry.io:80", name.registry());
- EXPECT_EQ("library/busybox", name.repository());
- EXPECT_EQ("tag", name.tag());
-
- // Note that the digest is stored as a tag.
- name = parseImageName(
- "registry.io:80/library/busybox"
- "@sha256:bc8813ea7b3603864987522f02a7"
- "6101c17ad122e1c46d790efc0fca78ca7bfb");
- EXPECT_EQ("registry.io:80", name.registry());
- EXPECT_EQ("library/busybox", name.repository());
- EXPECT_EQ("sha256:bc8813ea7b3603864987522f02a7"
- "6101c17ad122e1c46d790efc0fca78ca7bfb",
- name.tag());
-}
-
-
/**
* Provides token operations and defaults.
*/
@@ -477,8 +416,12 @@ TEST_F(RegistryClientTest, SimpleGetManifest)
ASSERT_SOME(registryClient);
+ Try<spec::ImageReference> reference =
+ spec::parseImageReference("library/busybox");
+ ASSERT_SOME(reference);
+
Future<spec::v2::ImageManifest> manifestResponse =
- registryClient.get()->getManifest(parseImageName("library/busybox"));
+ registryClient.get()->getManifest(reference.get());
const string unauthResponseHeaders = "Www-Authenticate: Bearer"
" realm=\"https://auth.docker.io/token\","
@@ -652,9 +595,12 @@ TEST_F(RegistryClientTest, SimpleGetBlob)
const Path blobPath(path::join(os::getcwd(), "blob"));
+ Try<spec::ImageReference> reference = spec::parseImageReference("blob");
+ ASSERT_SOME(reference);
+
Future<size_t> result =
registryClient.get()->getBlob(
- parseImageName("blob"),
+ reference.get(),
"digest",
blobPath);
@@ -758,9 +704,12 @@ TEST_F(RegistryClientTest, BadRequest)
const Path blobPath(path::join(os::getcwd(), "blob"));
+ Try<spec::ImageReference> reference = spec::parseImageReference("blob");
+ ASSERT_SOME(reference);
+
Future<size_t> result =
registryClient.get()->getBlob(
- parseImageName("blob"),
+ reference.get(),
"digest",
blobPath);
@@ -819,11 +768,11 @@ TEST_F(RegistryClientTest, SimpleRegistryPuller)
const Path registryPullerPath(os::getcwd());
- Try<slave::docker::Image::Name> imageName = parseImageName("busybox");
- ASSERT_SOME(imageName);
+ Try<spec::ImageReference> reference = spec::parseImageReference("busybox");
+ ASSERT_SOME(reference);
Future<list<pair<string, string>>> registryPullerFuture =
- registryPuller.get()->pull(imageName.get(), registryPullerPath);
+ registryPuller.get()->pull(reference.get(), registryPullerPath);
const string unauthResponseHeaders = "WWW-Authenticate: Bearer"
" realm=\"https://auth.docker.io/token\","
@@ -1077,7 +1026,7 @@ protected:
TemporaryDirectoryTest::SetUp();
const string archivesDir = path::join(os::getcwd(), "images");
- const string image = path::join(archivesDir, "abc:latest");
+ const string image = path::join(archivesDir, "abc");
ASSERT_SOME(os::mkdir(archivesDir));
ASSERT_SOME(os::mkdir(image));
@@ -1125,7 +1074,7 @@ protected:
ASSERT_SOME(os::rmdir(path::join(image, "456", "layer")));
ASSERT_SOME(os::chdir(image));
- ASSERT_SOME(os::tar(".", "../abc:latest.tar"));
+ ASSERT_SOME(os::tar(".", "../abc.tar"));
ASSERT_SOME(os::chdir(cwd));
ASSERT_SOME(os::rmdir(image));
}
@@ -1138,7 +1087,7 @@ protected:
TEST_F(ProvisionerDockerLocalStoreTest, LocalStoreTestWithTar)
{
const string archivesDir = path::join(os::getcwd(), "images");
- const string image = path::join(archivesDir, "abc:latest");
+ const string image = path::join(archivesDir, "abc");
ASSERT_SOME(os::mkdir(archivesDir));
ASSERT_SOME(os::mkdir(image));
@@ -1206,11 +1155,11 @@ public:
MOCK_METHOD2(
pull,
Future<list<pair<string, string>>>(
- const slave::docker::Image::Name&,
+ const spec::ImageReference&,
const Path&));
Future<list<pair<string, string>>> unmocked_pull(
- const slave::docker::Image::Name& name,
+ const spec::ImageReference& name,
const Path& directory)
{
// TODO(gilbert): Allow return list to be overridden.
[2/2] mesos git commit: Improved docs for volume/reservation HTTP
endpoints.
Posted by ji...@apache.org.
Improved docs for volume/reservation HTTP endpoints.
Review: https://reviews.apache.org/r/43118/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/df503d3f
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/df503d3f
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/df503d3f
Branch: refs/heads/master
Commit: df503d3f600b390a2511f5e95a1db10214be4425
Parents: c3c2c22
Author: Neil Conway <ne...@gmail.com>
Authored: Fri Feb 12 12:50:54 2016 -0800
Committer: Jie Yu <yu...@gmail.com>
Committed: Fri Feb 12 12:50:54 2016 -0800
----------------------------------------------------------------------
docs/endpoints/master/create-volumes.md | 5 +++-
docs/endpoints/master/destroy-volumes.md | 5 +++-
docs/endpoints/master/reserve.md | 5 +++-
docs/endpoints/master/unreserve.md | 5 +++-
docs/persistent-volume.md | 35 ++++++++++++++++++-----
docs/reservation.md | 40 +++++++++++++++++++++------
src/master/http.cpp | 20 +++++++++++---
7 files changed, 92 insertions(+), 23 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/df503d3f/docs/endpoints/master/create-volumes.md
----------------------------------------------------------------------
diff --git a/docs/endpoints/master/create-volumes.md b/docs/endpoints/master/create-volumes.md
index f167452..1e8fd20 100644
--- a/docs/endpoints/master/create-volumes.md
+++ b/docs/endpoints/master/create-volumes.md
@@ -7,6 +7,9 @@
Create persistent volumes on reserved resources.
### DESCRIPTION ###
-Returns 200 OK if volume creation was successful.
+Returns 200 OK if the request was accepted. This does not
+imply that the volume was created successfully: volume
+creation is done asynchronously and may fail.
+
Please provide "slaveId" and "volumes" values designating
the volumes to be created.
\ No newline at end of file
http://git-wip-us.apache.org/repos/asf/mesos/blob/df503d3f/docs/endpoints/master/destroy-volumes.md
----------------------------------------------------------------------
diff --git a/docs/endpoints/master/destroy-volumes.md b/docs/endpoints/master/destroy-volumes.md
index a9a12ad..7209a7c 100644
--- a/docs/endpoints/master/destroy-volumes.md
+++ b/docs/endpoints/master/destroy-volumes.md
@@ -7,6 +7,9 @@
Destroy persistent volumes.
### DESCRIPTION ###
-Returns 200 OK if volume deletion was successful.
+Returns 200 OK if the request was accepted. This does not
+imply that the volume was destroyed successfully: volume
+destruction is done asynchronously and may fail.
+
Please provide "slaveId" and "volumes" values designating
the volumes to be destroyed.
\ No newline at end of file
http://git-wip-us.apache.org/repos/asf/mesos/blob/df503d3f/docs/endpoints/master/reserve.md
----------------------------------------------------------------------
diff --git a/docs/endpoints/master/reserve.md b/docs/endpoints/master/reserve.md
index 6d4dee4..a71eb8e 100644
--- a/docs/endpoints/master/reserve.md
+++ b/docs/endpoints/master/reserve.md
@@ -7,6 +7,9 @@
Reserve resources dynamically on a specific slave.
### DESCRIPTION ###
-Returns 200 OK if resource reservation was successful.
+Returns 200 OK if the request was accepted. This does not
+imply that the requested resources have been reserved successfully:
+resource reservation is done asynchronously and may fail.
+
Please provide "slaveId" and "resources" values designating
the resources to be reserved.
\ No newline at end of file
http://git-wip-us.apache.org/repos/asf/mesos/blob/df503d3f/docs/endpoints/master/unreserve.md
----------------------------------------------------------------------
diff --git a/docs/endpoints/master/unreserve.md b/docs/endpoints/master/unreserve.md
index 81c72d3..5de7734 100644
--- a/docs/endpoints/master/unreserve.md
+++ b/docs/endpoints/master/unreserve.md
@@ -7,6 +7,9 @@
Unreserve resources dynamically on a specific slave.
### DESCRIPTION ###
-Returns 200 OK if resource unreservation was successful.
+Returns 200 OK if the request was accepted. This does not
+imply that the requested resources have been unreserved successfully:
+resource unreservation is done asynchronously and may fail.
+
Please provide "slaveId" and "resources" values designating
the resources to be unreserved.
\ No newline at end of file
http://git-wip-us.apache.org/repos/asf/mesos/blob/df503d3f/docs/persistent-volume.md
----------------------------------------------------------------------
diff --git a/docs/persistent-volume.md b/docs/persistent-volume.md
index 9657c3f..4d7821f 100644
--- a/docs/persistent-volume.md
+++ b/docs/persistent-volume.md
@@ -286,14 +286,24 @@ curl -i \
The user receives one of the following HTTP responses:
-* `200 OK`: Success (the persistent volumes have been created).
+* `200 OK`: Request accepted (see below).
* `400 BadRequest`: Invalid arguments (e.g., missing parameters).
* `401 Unauthorized`: Unauthenticated request.
* `403 Forbidden`: Unauthorized request.
* `409 Conflict`: Insufficient resources to create the volumes.
-Note that a single `/create-volumes` request can create multiple persistent
-volumes, but all of the volumes must be on the same slave.
+A single `/create-volumes` request can create multiple persistent volumes, but
+all of the volumes must be on the same slave.
+
+Note that when `200 OK` is returned by this endpoint, it does __not__ mean that
+the persistent volumes have been created successfully. Instead, this return code
+indicates that the create operation has been validated successfully by the
+master. The request is then forwarded asynchronously to the Mesos slave where
+the reserved resources are located. That asynchronous message may not be
+delivered or creating the volumes at the slave might fail, in which case no
+volumes will be created. To determine if a create operation has succeeded, the
+user can examine the state of the appropriate Mesos slave (e.g., via the slave's
+`/state` HTTP endpoint).
#### `/destroy-volumes`
@@ -328,14 +338,24 @@ curl -i \
The user receives one of the following HTTP responses:
-* `200 OK`: Success (the volumes have been destroyed).
+* `200 OK`: Request accepted (see below).
* `400 BadRequest`: Invalid arguments (e.g., missing parameters).
* `401 Unauthorized`: Unauthenticated request.
* `403 Forbidden`: Unauthorized request.
* `409 Conflict`: Insufficient resources to destroy the volumes.
-Note that a single `/destroy-volumes` request can destroy multiple persistent
-volumes, but all of the volumes must be on the same slave.
+A single `/destroy-volumes` request can destroy multiple persistent volumes, but
+all of the volumes must be on the same slave.
+
+Note that when `200 OK` is returned by this endpoint, it does __not__ mean that
+the persistent volumes have been destroyed successfully. Instead, this return
+code indicates that the destroy operation has been validated successfully by the
+master. The request is then forwarded asynchronously to the Mesos slave where
+the persistent volumes are located. That asynchronous message may not be
+delivered or destroying the volumes at the slave might fail, in which case no
+volumes will be destroyed. To determine if a destroy operation has succeeded,
+the user can examine the state of the appropriate Mesos slave (e.g., via the
+slave's `/state` HTTP endpoint).
### Programming with Persistent Volumes
@@ -355,7 +375,8 @@ volumes:
* When using HTTP endpoints to reserve resources or create persistent volumes,
_some_ failures can be detected by examining the HTTP response code returned
to the client. However, it is still possible for a `200` response code to be
- returned to the client but for the associated operation to fail.
+ returned to the client but for the associated operation to fail---see
+ discussion above.
* When using the scheduler API, detecting that a dynamic reservation has failed
is a little tricky: reservations do not have unique identifiers, and the Mesos
http://git-wip-us.apache.org/repos/asf/mesos/blob/df503d3f/docs/reservation.md
----------------------------------------------------------------------
diff --git a/docs/reservation.md b/docs/reservation.md
index d3ae445..41321d4 100644
--- a/docs/reservation.md
+++ b/docs/reservation.md
@@ -13,7 +13,7 @@ and authorized __frameworks__ to dynamically reserve resources in the cluster.
In both types of reservations, resources are reserved for a [__role__](roles.md).
-## Static Reservation (since 0.14.0)
+## Static Reservation
An operator can configure a slave with resources reserved for a role.
The reserved resources are specified via the `--resources` flag.
@@ -37,13 +37,13 @@ __NOTE:__ This feature is supported for backwards compatibility.
reservations dynamically via the master HTTP endpoints.
-## Dynamic Reservation (since 0.23.0)
+## Dynamic Reservation
As mentioned in [Static Reservation](#static-reservation-since-0140), specifying
the reserved resources via the `--resources` flag makes the reservation static.
That is, statically reserved resources cannot be reserved for another role nor
-be unreserved. Dynamic Reservation enables operators and authorized frameworks
-to reserve and unreserve resources post slave-startup.
+be unreserved. Dynamic reservation enables operators and authorized frameworks
+to reserve and unreserve resources after slave-startup.
We require a `principal` from the operator or framework in order to
authenticate/authorize the operations. Permissions are specified via the
@@ -289,12 +289,21 @@ HTTP POST request to the `/reserve` HTTP endpoint like so:
The user receives one of the following HTTP responses:
-* `200 OK`: Success (the requested resources have been reserved).
+* `200 OK`: Request accepted (see below).
* `400 BadRequest`: Invalid arguments (e.g., missing parameters).
* `401 Unauthorized`: Unauthenticated request.
* `403 Forbidden`: Unauthorized request.
* `409 Conflict`: Insufficient resources to satisfy the reserve operation.
+Note that when `200 OK` is returned by this endpoint, it does __not__ mean that
+the requested resources have been reserved. Instead, this return code indicates
+that the reservation request has been validated successfully by the master. The
+reservation request is then forwarded asynchronously to the Mesos slave where
+the resources are located. That asynchronous message may not be delivered, in
+which case no resources will be reserved. To determine if a reserve operation
+has succeeded, the user can examine the state of the appropriate Mesos slave
+(e.g., via the slave's `/state` HTTP endpoint).
+
#### `/unreserve` (since 0.25.0)
Suppose we want to unreserve the resources that we dynamically reserved above.
@@ -310,7 +319,7 @@ We can send an HTTP POST request to the `/unreserve` HTTP endpoint like so:
"scalar": { "value": 8 },
"role": "ads",
"reservation": {
- "principal": <operator_principal>
+ "principal": <reserver_principal>
}
},
{
@@ -319,16 +328,31 @@ We can send an HTTP POST request to the `/unreserve` HTTP endpoint like so:
"scalar": { "value": 4096 },
"role": "ads",
"reservation": {
- "principal": <operator_principal>
+ "principal": <reserver_principal>
}
}
]' \
-X POST http://<ip>:<port>/master/unreserve
+Note that `reserver_principal` is the principal that was used to make the
+reservation, while `operator_principal` is the principal that is attempting to
+perform the unreserve operation---in some cases, these principals might be the
+same. The `operator_principal` must be [authorized](authorization.md) to
+unreserve reservations made by `reserver_principal`.
+
The user receives one of the following HTTP responses:
-* `200 OK`: Success (the requested resources have been unreserved).
+* `200 OK`: Request accepted (see below).
* `400 BadRequest`: Invalid arguments (e.g., missing parameters).
* `401 Unauthorized`: Unauthenticated request.
* `403 Forbidden`: Unauthorized request.
* `409 Conflict`: Insufficient resources to satisfy the unreserve operation.
+
+Note that when `200 OK` is returned by this endpoint, it does __not__ mean that
+the requested resources have been unreserved. Instead, this return code
+indicates that the unreserve request has been validated successfully by the
+master. The request is then forwarded asynchronously to the Mesos slave where
+the resources are located. That asynchronous message may not be delivered, in
+which case no resources will be unreserved. To determine if an unreserve
+operation has succeeded, the user can examine the state of the appropriate Mesos
+slave (e.g., via the slave's `/state` HTTP endpoint).
http://git-wip-us.apache.org/repos/asf/mesos/blob/df503d3f/src/master/http.cpp
----------------------------------------------------------------------
diff --git a/src/master/http.cpp b/src/master/http.cpp
index 3d7a624..f92212b 100644
--- a/src/master/http.cpp
+++ b/src/master/http.cpp
@@ -699,7 +699,10 @@ string Master::Http::CREATE_VOLUMES_HELP()
TLDR(
"Create persistent volumes on reserved resources."),
DESCRIPTION(
- "Returns 200 OK if volume creation was successful.",
+ "Returns 200 OK if the request was accepted. This does not",
+ "imply that the volume was created successfully: volume",
+ "creation is done asynchronously and may fail.",
+ "",
"Please provide \"slaveId\" and \"volumes\" values designating",
"the volumes to be created."));
}
@@ -803,7 +806,10 @@ string Master::Http::DESTROY_VOLUMES_HELP()
TLDR(
"Destroy persistent volumes."),
DESCRIPTION(
- "Returns 200 OK if volume deletion was successful.",
+ "Returns 200 OK if the request was accepted. This does not",
+ "imply that the volume was destroyed successfully: volume",
+ "destruction is done asynchronously and may fail.",
+ "",
"Please provide \"slaveId\" and \"volumes\" values designating",
"the volumes to be destroyed."));
}
@@ -1132,7 +1138,10 @@ string Master::Http::RESERVE_HELP()
TLDR(
"Reserve resources dynamically on a specific slave."),
DESCRIPTION(
- "Returns 200 OK if resource reservation was successful.",
+ "Returns 200 OK if the request was accepted. This does not",
+ "imply that the requested resources have been reserved successfully:",
+ "resource reservation is done asynchronously and may fail.",
+ "",
"Please provide \"slaveId\" and \"resources\" values designating",
"the resources to be reserved."));
}
@@ -2400,7 +2409,10 @@ string Master::Http::UNRESERVE_HELP()
TLDR(
"Unreserve resources dynamically on a specific slave."),
DESCRIPTION(
- "Returns 200 OK if resource unreservation was successful.",
+ "Returns 200 OK if the request was accepted. This does not",
+ "imply that the requested resources have been unreserved successfully:",
+ "resource unreservation is done asynchronously and may fail.",
+ "",
"Please provide \"slaveId\" and \"resources\" values designating",
"the resources to be unreserved."));
}