You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by jp...@apache.org on 2018/01/11 22:38:48 UTC
[1/2] mesos git commit: Made the `os::read` error less redundant.
Repository: mesos
Updated Branches:
refs/heads/master 06195ea0d -> 090f112fa
Made the `os::read` error less redundant.
The error from `os::read` would typically format as "Failed to open
file: File not found", which is unnecessarily redundant. It should
be sufficient to just report the `ErrnoError()`.
Review: https://reviews.apache.org/r/64993/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/2fd4fe06
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/2fd4fe06
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/2fd4fe06
Branch: refs/heads/master
Commit: 2fd4fe0656c1ee73ddf3a2277a1e932706e29b07
Parents: 06195ea
Author: James Peach <jp...@apache.org>
Authored: Thu Jan 11 13:55:49 2018 -0800
Committer: James Peach <jp...@apache.org>
Committed: Thu Jan 11 13:55:49 2018 -0800
----------------------------------------------------------------------
3rdparty/stout/include/stout/os/read.hpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/2fd4fe06/3rdparty/stout/include/stout/os/read.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/os/read.hpp b/3rdparty/stout/include/stout/os/read.hpp
index 2eb7421..49878e4 100644
--- a/3rdparty/stout/include/stout/os/read.hpp
+++ b/3rdparty/stout/include/stout/os/read.hpp
@@ -108,7 +108,7 @@ inline Try<std::string> read(const std::string& path)
{
FILE* file = ::fopen(path.c_str(), "r");
if (file == nullptr) {
- return ErrnoError("Failed to open file");
+ return ErrnoError();
}
// Use a buffer to read the file in BUFSIZ
[2/2] mesos git commit: Improved image store manifest parsing errors.
Posted by jp...@apache.org.
Improved image store manifest parsing errors.
When one of the image store backends fails to parse an image manifest,
it is most probable that the the on-disk state is incorrect. Emitting
the path in the error makes it easier for an operator to figure out
what went wrong.
Review: https://reviews.apache.org/r/64995/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/090f112f
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/090f112f
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/090f112f
Branch: refs/heads/master
Commit: 090f112fa5a3a6bd1f415842c4d5c7ffc5fa5614
Parents: 2fd4fe0
Author: James Peach <jp...@apache.org>
Authored: Thu Jan 11 13:55:52 2018 -0800
Committer: James Peach <jp...@apache.org>
Committed: Thu Jan 11 13:55:52 2018 -0800
----------------------------------------------------------------------
src/appc/spec.cpp | 12 +++++++++---
.../mesos/provisioner/appc/cache.cpp | 11 +++++++----
.../mesos/provisioner/docker/local_puller.cpp | 17 +++++++++++------
.../mesos/provisioner/docker/store.cpp | 18 +++++++++++-------
4 files changed, 38 insertions(+), 20 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/090f112f/src/appc/spec.cpp
----------------------------------------------------------------------
diff --git a/src/appc/spec.cpp b/src/appc/spec.cpp
index 8471ccb..fc5e24e 100644
--- a/src/appc/spec.cpp
+++ b/src/appc/spec.cpp
@@ -64,14 +64,20 @@ string getImageManifestPath(const string& imagePath)
Try<ImageManifest> getManifest(const string& imagePath)
{
- Try<string> read = os::read(getImageManifestPath(imagePath));
+ const string path = getImageManifestPath(imagePath);
+
+ Try<string> read = os::read(path);
if (read.isError()) {
- return Error("Failed to read manifest file: " + read.error());
+ return Error(
+ "Failed to read manifest from '" + path + "': " +
+ read.error());
}
Try<ImageManifest> parseManifest = parse(read.get());
if (parseManifest.isError()) {
- return Error("Failed to parse manifest: " + parseManifest.error());
+ return Error(
+ "Failed to parse manifest from '" + path + "': " +
+ parseManifest.error());
}
return parseManifest.get();
http://git-wip-us.apache.org/repos/asf/mesos/blob/090f112f/src/slave/containerizer/mesos/provisioner/appc/cache.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/provisioner/appc/cache.cpp b/src/slave/containerizer/mesos/provisioner/appc/cache.cpp
index 09190e2..0cd444b 100644
--- a/src/slave/containerizer/mesos/provisioner/appc/cache.cpp
+++ b/src/slave/containerizer/mesos/provisioner/appc/cache.cpp
@@ -80,16 +80,19 @@ Try<Nothing> Cache::recover()
Try<Nothing> Cache::add(const string& imageId)
{
- const Path imageDir(paths::getImagePath(storeDir, imageId));
+ const string path = spec::getImageManifestPath(
+ paths::getImagePath(storeDir, imageId));
- Try<string> read = os::read(spec::getImageManifestPath(imageDir));
+ Try<string> read = os::read(path);
if (read.isError()) {
- return Error("Failed to read manifest: " + read.error());
+ return Error(
+ "Failed to read manifest from '" + path + "': " + read.error());
}
Try<spec::ImageManifest> manifest = spec::parse(read.get());
if (manifest.isError()) {
- return Error("Failed to parse manifest: " + manifest.error());
+ return Error(
+ "Failed to parse manifest from '" + path + "': " + manifest.error());
}
map<string, string> labels;
http://git-wip-us.apache.org/repos/asf/mesos/blob/090f112f/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 c4879ec..5ce49ac 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp
@@ -246,27 +246,32 @@ Result<string> LocalPullerProcess::getParentLayerId(
const string& directory,
const string& layerId)
{
- const string layerPath = path::join(directory, layerId);
+ const string path =
+ paths::getImageLayerManifestPath(path::join(directory, layerId));
- Try<string> _manifest = os::read(paths::getImageLayerManifestPath(layerPath));
+ Try<string> _manifest = os::read(path);
if (_manifest.isError()) {
- return Error("Failed to read manifest: " + _manifest.error());
+ return Error(
+ "Failed to read manifest from '" + path + "': " + _manifest.error());
}
Try<JSON::Object> manifest = JSON::parse<JSON::Object>(_manifest.get());
if (manifest.isError()) {
- return Error("Failed to parse manifest: " + manifest.error());
+ return Error(
+ "Failed to parse manifest from '" + path + "': " + manifest.error());
}
Result<JSON::Value> parentLayerId = manifest->find<JSON::Value>("parent");
if (parentLayerId.isError()) {
- return Error("Failed to parse 'parent': " + parentLayerId.error());
+ return Error(
+ "Failed to parse 'parent' key in manifest from '" + path + "': " +
+ parentLayerId.error());
} else if (parentLayerId.isNone()) {
return None();
} else if (parentLayerId->is<JSON::Null>()) {
return None();
} else if (!parentLayerId->is<JSON::String>()) {
- return Error("Unexpected 'parent' type");
+ return Error("Unexpected 'parent' type in manifest from '" + path + "'");
}
const string id = parentLayerId->as<JSON::String>().value;
http://git-wip-us.apache.org/repos/asf/mesos/blob/090f112f/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 d64e6eb..7b6ddb1 100644
--- a/src/slave/containerizer/mesos/provisioner/docker/store.cpp
+++ b/src/slave/containerizer/mesos/provisioner/docker/store.cpp
@@ -367,22 +367,26 @@ Future<ImageInfo> StoreProcess::__get(
backend));
}
+ const string path = paths::getImageLayerManifestPath(
+ flags.docker_store_dir,
+ image.layer_ids(image.layer_ids_size() - 1));
+
// Read the manifest from the last layer because all runtime config
// are merged at the leaf already.
- Try<string> manifest = os::read(
- paths::getImageLayerManifestPath(
- flags.docker_store_dir,
- image.layer_ids(image.layer_ids_size() - 1)));
-
+ Try<string> manifest = os::read(path);
if (manifest.isError()) {
- return Failure("Failed to read manifest: " + manifest.error());
+ return Failure(
+ "Failed to read manifest from '" + path + "': " +
+ manifest.error());
}
Try<::docker::spec::v1::ImageManifest> v1 =
::docker::spec::v1::parse(manifest.get());
if (v1.isError()) {
- return Failure("Failed to parse docker v1 manifest: " + v1.error());
+ return Failure(
+ "Failed to parse docker v1 manifest from '" + path + "': " +
+ v1.error());
}
return ImageInfo{layerPaths, v1.get()};