You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by vi...@apache.org on 2016/03/21 19:31:23 UTC
mesos git commit: Created URI.filename to name fetched files in
sandbox where appropriate.
Repository: mesos
Updated Branches:
refs/heads/master d0e745dc7 -> f4bfd4bd6
Created URI.filename to name fetched files in sandbox where appropriate.
Review: https://reviews.apache.org/r/45046/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/f4bfd4bd
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/f4bfd4bd
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/f4bfd4bd
Branch: refs/heads/master
Commit: f4bfd4bd68d4962f2b3a10ec6d440dbcddbaf61d
Parents: d0e745d
Author: Michael Browning <mr...@uber.com>
Authored: Mon Mar 21 11:30:40 2016 -0700
Committer: Vinod Kone <vi...@gmail.com>
Committed: Mon Mar 21 11:30:40 2016 -0700
----------------------------------------------------------------------
CHANGELOG | 8 ++++
docs/fetcher.md | 8 ++++
include/mesos/mesos.proto | 5 ++
include/mesos/v1/mesos.proto | 5 ++
src/launcher/fetcher.cpp | 10 +++-
src/slave/containerizer/fetcher.cpp | 25 ++++++++--
src/slave/containerizer/fetcher.hpp | 3 ++
src/tests/fetcher_cache_tests.cpp | 42 ++++++++++++++++
src/tests/fetcher_tests.cpp | 82 ++++++++++++++++++++++++++++++++
9 files changed, 183 insertions(+), 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/f4bfd4bd/CHANGELOG
----------------------------------------------------------------------
diff --git a/CHANGELOG b/CHANGELOG
index 761238c..eab924e 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,3 +1,11 @@
+Release Notes - Mesos - Version 0.29.0 (WIP)
+--------------------------------------------
+
+Additional API Changes:
+ * [MESOS-4735] - Add 'filename' field to CommandInfo.URI in Scheduler API
+ and v1 Scheduler HTTP API.
+
+
Release Notes - Mesos - Version 0.28.0
--------------------------------------------
This release contains the following new features:
http://git-wip-us.apache.org/repos/asf/mesos/blob/f4bfd4bd/docs/fetcher.md
----------------------------------------------------------------------
diff --git a/docs/fetcher.md b/docs/fetcher.md
index f70939d..15e56ec 100644
--- a/docs/fetcher.md
+++ b/docs/fetcher.md
@@ -73,6 +73,7 @@ each URI are determined based on the following protobuf structure. (See
optional bool executable = 2;
optional bool extract = 3 [default = true];
optional bool cache = 4;
+ optional string filename = 5;
}
...
optional string user = 5;
@@ -85,6 +86,9 @@ has no effect.
If the "cache" field is true, the fetcher cache is to be used for the URI.
+If the "filename" field is set, the fetcher will use that name for the copy
+stored in the sandbox directory.
+
### Specifying a user name
The framework may pass along a user name that becomes a fetch parameter. This
@@ -127,6 +131,10 @@ In case the cache is bypassed, both the archive and the unpacked results will be
found together in the sandbox. In case a cache file is unpacked, only the
extraction result will be found in the sandbox.
+The "filename" field is useful here for cases where the URI ends with query
+parameters, since these will otherwise end up in the file copied to the sandbox
+and will subsequently fail to be recognized as archives.
+
### Bypassing the cache
By default, the URI field "cache" is not present. If this is the case or its
http://git-wip-us.apache.org/repos/asf/mesos/blob/f4bfd4bd/include/mesos/mesos.proto
----------------------------------------------------------------------
diff --git a/include/mesos/mesos.proto b/include/mesos/mesos.proto
index deb9c09..b965f5a 100644
--- a/include/mesos/mesos.proto
+++ b/include/mesos/mesos.proto
@@ -375,6 +375,11 @@ message CommandInfo {
// downloading. See also "docs/fetcher.md" and
// "docs/fetcher-cache-internals.md".
optional bool cache = 4;
+
+ // The fetcher's default behavior is to use the URI string's basename to
+ // name the local copy. If this field is provided, the local copy will be
+ // named with its value instead.
+ optional string filename = 5;
}
repeated URI uris = 1;
http://git-wip-us.apache.org/repos/asf/mesos/blob/f4bfd4bd/include/mesos/v1/mesos.proto
----------------------------------------------------------------------
diff --git a/include/mesos/v1/mesos.proto b/include/mesos/v1/mesos.proto
index a981e75..d2ab6ed 100644
--- a/include/mesos/v1/mesos.proto
+++ b/include/mesos/v1/mesos.proto
@@ -375,6 +375,11 @@ message CommandInfo {
// downloading. See also "docs/fetcher.md" and
// "docs/fetcher-cache-internals.md".
optional bool cache = 4;
+
+ // The fetcher's default behavior is to use the URI string's basename to
+ // name the local copy. If this field is provided, the local copy will be
+ // named with its value instead.
+ optional string filename = 5;
}
repeated URI uris = 1;
http://git-wip-us.apache.org/repos/asf/mesos/blob/f4bfd4bd/src/launcher/fetcher.cpp
----------------------------------------------------------------------
diff --git a/src/launcher/fetcher.cpp b/src/launcher/fetcher.cpp
index 6eeafab..28dbb98 100644
--- a/src/launcher/fetcher.cpp
+++ b/src/launcher/fetcher.cpp
@@ -249,7 +249,10 @@ static Try<string> fetchBypassingCache(
{
LOG(INFO) << "Fetching directly into the sandbox directory";
- Try<string> basename = Fetcher::basename(uri.value());
+ Try<string> basename = uri.has_filename()
+ ? uri.filename()
+ : Fetcher::basename(uri.value());
+
if (basename.isError()) {
return Error("Failed to determine the basename of the URI '" +
uri.value() + "' with error: " + basename.error());
@@ -288,7 +291,10 @@ static Try<string> fetchFromCache(
{
LOG(INFO) << "Fetching from cache";
- Try<string> basename = Fetcher::basename(item.uri().value());
+ Try<string> basename = item.uri().has_filename()
+ ? item.uri().filename()
+ : Fetcher::basename(item.uri().value());
+
if (basename.isError()) {
return Error(basename.error());
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/f4bfd4bd/src/slave/containerizer/fetcher.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/fetcher.cpp b/src/slave/containerizer/fetcher.cpp
index 33dfcad..2b1e022 100644
--- a/src/slave/containerizer/fetcher.cpp
+++ b/src/slave/containerizer/fetcher.cpp
@@ -145,12 +145,31 @@ Try<Nothing> Fetcher::validateUri(const string& uri)
}
+Try<Nothing> Fetcher::validateFilename(const string& filename)
+{
+ Try<string> result = Path(filename).basename();
+ if (result.isError()) {
+ return Error(result.error());
+ }
+
+ return Nothing();
+}
+
+
static Try<Nothing> validateUris(const CommandInfo& commandInfo)
{
foreach (const CommandInfo::URI& uri, commandInfo.uris()) {
- Try<Nothing> validation = Fetcher::validateUri(uri.value());
- if (validation.isError()) {
- return Error(validation.error());
+ Try<Nothing> uriValidation = Fetcher::validateUri(uri.value());
+ if (uriValidation.isError()) {
+ return Error(uriValidation.error());
+ }
+
+ if (uri.has_filename()) {
+ Try<Nothing> filenameValidation =
+ Fetcher::validateFilename(uri.filename());
+ if (filenameValidation.isError()) {
+ return Error(filenameValidation.error());
+ }
}
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/f4bfd4bd/src/slave/containerizer/fetcher.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/fetcher.hpp b/src/slave/containerizer/fetcher.hpp
index bbdce88..eeb663e 100644
--- a/src/slave/containerizer/fetcher.hpp
+++ b/src/slave/containerizer/fetcher.hpp
@@ -67,6 +67,9 @@ public:
// reported to the user.
static Try<Nothing> validateUri(const std::string& uri);
+ // Checks to make sure the URI filename is valid.
+ static Try<Nothing> validateFilename(const std::string& filename);
+
// Determines if the given URI refers to a local file system path
// and prepends frameworksHome if it is a relative path. Fails if
// frameworksHome is empty and a local path is indicated.
http://git-wip-us.apache.org/repos/asf/mesos/blob/f4bfd4bd/src/tests/fetcher_cache_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/fetcher_cache_tests.cpp b/src/tests/fetcher_cache_tests.cpp
index 645dae2..798c0cf 100644
--- a/src/tests/fetcher_cache_tests.cpp
+++ b/src/tests/fetcher_cache_tests.cpp
@@ -660,6 +660,48 @@ TEST_F(FetcherCacheTest, LocalCached)
}
+TEST_F(FetcherCacheTest, CachedCustomFilename)
+{
+ startSlave();
+ driver->start();
+
+ const int index = 0;
+ const string customFilename = "my-command";
+ CommandInfo::URI uri;
+ uri.set_value(commandPath);
+ uri.set_executable(true);
+ uri.set_cache(true);
+ uri.set_filename(customFilename);
+
+ CommandInfo commandInfo;
+ commandInfo.set_value("./" + customFilename + " " + taskName(index));
+ commandInfo.add_uris()->CopyFrom(uri);
+
+ const Try<Task> task = launchTask(commandInfo, index);
+ ASSERT_SOME(task);
+
+ AWAIT_READY(awaitFinished(task.get()));
+
+ EXPECT_EQ(1u, fetcherProcess->cacheSize());
+ ASSERT_SOME(fetcherProcess->cacheFiles(slaveId, flags));
+ EXPECT_EQ(1u, fetcherProcess->cacheFiles(slaveId, flags).get().size());
+
+ // Verify that the downloaded executable lives at our custom filename path.
+ const string executablePath = path::join(
+ task.get().runDirectory.value, customFilename);
+
+ EXPECT_TRUE(isExecutable(executablePath));
+
+ // The script specified by COMMAND_SCRIPT just statically touches a file
+ // named $COMMAND_NAME + $1, so if we want to verify that it ran here we have
+ // to check this path in addition to the custom-named executable we saved.
+ const string outputPath = path::join(
+ task.get().runDirectory.value, COMMAND_NAME);
+
+ EXPECT_TRUE(os::exists(outputPath + taskName(index)));
+}
+
+
// Tests falling back on bypassing the cache when fetching the download
// size of a URI that is supposed to be cached fails.
TEST_F(FetcherCacheTest, CachedFallback)
http://git-wip-us.apache.org/repos/asf/mesos/blob/f4bfd4bd/src/tests/fetcher_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/fetcher_tests.cpp b/src/tests/fetcher_tests.cpp
index 2aef5f1..375e690 100644
--- a/src/tests/fetcher_tests.cpp
+++ b/src/tests/fetcher_tests.cpp
@@ -429,6 +429,7 @@ TEST_F(FetcherTest, FileLocalhostURI)
TEST_F(FetcherTest, NoExtractNotExecutable)
{
// First construct a temporary file that can be fetched.
+ // TODO(mrbrowning): Fix other tests to use this tempfile pattern.
Try<string> path = os::mktemp();
ASSERT_SOME(path);
@@ -804,6 +805,87 @@ TEST_F(FetcherTest, ExtractZipFileWithDuplicatedEntries)
}
+TEST_F(FetcherTest, UseCustomFilename)
+{
+ // First construct a temporary file that can be fetched.
+ Try<string> dir =
+ os::mkdtemp(path::join(os::getcwd(), "XXXXXX"));
+ ASSERT_SOME(dir);
+
+ Try<string> path = os::mktemp(path::join(dir.get(), "XXXXXX"));
+ ASSERT_SOME(path);
+
+ ASSERT_SOME(os::write(path.get(), "hello renamed file"));
+
+ ContainerID containerId;
+ containerId.set_value(UUID::random().toString());
+
+ const string customFilename = "custom.txt";
+ CommandInfo commandInfo;
+ CommandInfo::URI* uri = commandInfo.add_uris();
+ uri->set_value(path.get());
+ uri->set_extract(true);
+ uri->set_filename(customFilename);
+
+ slave::Flags flags;
+ flags.launcher_dir = getLauncherDir();
+
+ Fetcher fetcher;
+ SlaveID slaveId;
+
+ Future<Nothing> fetch = fetcher.fetch(
+ containerId, commandInfo, os::getcwd(), None(), slaveId, flags);
+
+ AWAIT_READY(fetch);
+
+ ASSERT_TRUE(os::exists(path::join(".", customFilename)));
+
+ ASSERT_SOME_EQ(
+ "hello renamed file", os::read(path::join(".", customFilename)));
+}
+
+
+TEST_F(FetcherTest, CustomGzipFilename)
+{
+ // First construct a temporary file that can be fetched.
+ Try<string> dir =
+ os::mkdtemp(path::join(os::getcwd(), "XXXXXX"));
+ ASSERT_SOME(dir);
+
+ Try<string> path = os::mktemp(path::join(dir.get(), "XXXXXX"));
+ ASSERT_SOME(path);
+
+ ASSERT_SOME(os::write(path.get(), "hello renamed gzip file"));
+ ASSERT_SOME(os::shell("gzip " + path.get()));
+
+ ContainerID containerId;
+ containerId.set_value(UUID::random().toString());
+
+ const string customFilename = "custom";
+ CommandInfo commandInfo;
+ CommandInfo::URI* uri = commandInfo.add_uris();
+ uri->set_value(path.get() + ".gz");
+ uri->set_extract(true);
+ uri->set_filename(customFilename + ".gz");
+
+ slave::Flags flags;
+ flags.launcher_dir = getLauncherDir();
+
+ Fetcher fetcher;
+ SlaveID slaveId;
+
+ Future<Nothing> fetch = fetcher.fetch(
+ containerId, commandInfo, os::getcwd(), None(), slaveId, flags);
+
+ AWAIT_READY(fetch);
+
+ string extractFile = path::join(".", customFilename);
+ ASSERT_TRUE(os::exists(extractFile));
+
+ ASSERT_SOME_EQ("hello renamed gzip file", os::read(extractFile));
+}
+
+
// Tests fetching via the local HDFS client. Since we cannot rely on
// Hadoop being installed, we use our own mock version that works on
// the local file system only, but this lets us exercise the exact