You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by be...@apache.org on 2015/02/11 08:32:29 UTC
[1/2] mesos git commit: Remove per-flag parsing of file:// arguments.
Repository: mesos
Updated Branches:
refs/heads/master 54545657c -> 1ee35b164
Remove per-flag parsing of file:// arguments.
Mostly simplifies things. There are two places where there things get
complicated:
1\. --whitelist: This code wants a file to watch for changes and update
the whitelist from periodically. Introduces Path to work around this.
2\. --credential, --credentials: Custom parsing logic since the
credentials can be given in two different formats. Once the old
format is removed, either teaching <stout/flags\> how to parse json
to protobuf generally or taking the flag as JSON then converting it
to protobuf later will make this clean.
Review: https://reviews.apache.org/r/30195
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/1ee35b16
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/1ee35b16
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/1ee35b16
Branch: refs/heads/master
Commit: 1ee35b16480d8e5c7e705e796192c4b0cc0c09a6
Parents: 1fa5d5b
Author: Cody Maloney <co...@mesosphere.io>
Authored: Tue Feb 10 23:22:09 2015 -0800
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Tue Feb 10 23:32:13 2015 -0800
----------------------------------------------------------------------
CHANGELOG | 4 ++
docs/configuration.md | 52 +++++++++++++++-----------
docs/upgrades.md | 1 +
src/credentials/credentials.hpp | 17 +++++----
src/examples/load_generator_framework.cpp | 15 --------
src/master/contender.cpp | 33 ++++++++++++----
src/master/contender.hpp | 7 ++--
src/master/detector.cpp | 39 ++++++++++++++-----
src/master/detector.hpp | 7 ++--
src/master/flags.hpp | 8 ++--
src/master/main.cpp | 15 +-------
src/master/master.cpp | 6 +--
src/slave/flags.hpp | 2 +-
src/slave/slave.cpp | 8 ++--
src/tests/credentials_tests.cpp | 10 +++--
src/tests/master_allocator_tests.cpp | 2 +-
src/tests/master_tests.cpp | 2 +-
src/tests/mesos.cpp | 4 +-
src/watcher/whitelist_watcher.cpp | 26 ++++++++++---
src/watcher/whitelist_watcher.hpp | 5 ++-
src/zookeeper/url.hpp | 17 +++++----
21 files changed, 160 insertions(+), 120 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ee35b16/CHANGELOG
----------------------------------------------------------------------
diff --git a/CHANGELOG b/CHANGELOG
index f515bc9..966661a 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -4,9 +4,13 @@
* API Changes:
* [MESOS-1143] - TASK_ERROR is now sent instead of TASK_LOST when rescheduling a task should not be attempted.
* [MESOS-2086] - Update messages.proto to use a raw bytestream instead of a string for AuthenticationStartMessage.
+ * [MESOS-2322] - All arguments can now read their values from a file, just specify
+ --name=file://path/to/file.
* Deprecations:
* [MESOS-2058] - Deprecate stats.json endpoints for Master and Slave.
+ * [MESOS-2322] - Deprecated specifying JSON blobs to parse using an absolute
+ path to point at the filename.
Release Notes - Mesos - Version 0.21.1
--------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ee35b16/docs/configuration.md
----------------------------------------------------------------------
diff --git a/docs/configuration.md b/docs/configuration.md
index 22f9e3d..47d8ccb 100644
--- a/docs/configuration.md
+++ b/docs/configuration.md
@@ -7,7 +7,7 @@ layout: documentation
The Mesos master and slave can take a variety of configuration options through command-line arguments, or environment variables. A list of the available options can be seen by running `mesos-master --help` or `mesos-slave --help`. Each option can be set in two ways:
-* By passing it to the binary using `--option_name=value`.
+* By passing it to the binary using `--option_name=value`, either specifying the value directly, or specifying a file in which the value resides (`--option_name=file://path/to/file`). The path can be absolute or relative to the current working directory.
* By setting the environment variable `MESOS_OPTION_NAME` (the option name with a `MESOS_` prefix added to it).
Configuration values are searched for first in the environment, then on the command-line.
@@ -158,8 +158,7 @@ If you have special compilation requirements, please refer to `./configure --hel
ZooKeeper URL (used for leader election amongst masters)
May be one of:
<pre><code>zk://host1:port1,host2:port2,.../path
-zk://username:password@host1:port1,host2:port2,.../path
-file://path/to/file (where file contains one of the above)</code></pre>
+zk://username:password@host1:port1,host2:port2,.../path</code></pre>
</td>
</tr>
</table>
@@ -181,10 +180,9 @@ file://path/to/file (where file contains one of the above)</code></pre>
--acls=VALUE
</td>
<td>
- The value could be a JSON formatted string of ACLs
- or a file path containing the JSON formatted ACLs used
- for authorization. Path could be of the form <code>file:///path/to/file</code>
- or <code>/path/to/file</code>.
+ The value is a JSON formatted string of ACLs. Remember you can also use
+ the <code>file:///path/to/file</code> argument value format to write the
+ JSON in a file.
<p/>
See the ACLs protobuf in mesos.proto for the expected format.
<p/>
@@ -267,7 +265,7 @@ file://path/to/file (where file contains one of the above)</code></pre>
Either a path to a text file with a list of credentials,
each line containing 'principal' and 'secret' separated by whitespace,
or, a path to a JSON-formatted file containing credentials.
- Path could be of the form <code>file:///path/to/file</code> or <code>/path/to/file</code>.
+ Path should be of the form <code>file:///path/to/file</code>
<p/>
JSON file Example:
<pre><code>{
@@ -324,8 +322,9 @@ file://path/to/file (where file contains one of the above)</code></pre>
subsystems.
<p/>
Use <code>--modules=filepath</code> to specify the list of modules via a
- file containing a JSON formatted string. 'filepath' can be
- of the form <code>file:///path/to/file</code> or <code>/path/to/file</code>.
+ file containing a JSON formatted string. Remember you can also use
+ the <code>file:///path/to/file</code> argument value format to write the
+ JSON in a file.
<p/>
Use <code>--modules="{...}"</code> to specify the list of modules inline.
<p/>
@@ -382,8 +381,9 @@ file://path/to/file (where file contains one of the above)</code></pre>
or a file path containing the JSON formatted rate limits used
for framework rate limiting.
<p/>
- Path could be of the form <code>file:///path/to/file</code>
- or <code>/path/to/file</code>.
+ Remember you can also use
+ the <code>file:///path/to/file</code> argument value format to write the
+ JSON in a file.
<p/>
See the RateLimits protobuf in mesos.proto for the expected format.
@@ -534,10 +534,15 @@ file://path/to/file (where file contains one of the above)</code></pre>
--whitelist=VALUE
</td>
<td>
- Path to a file with a list of slaves
- (one per line) to advertise offers for.
+ A filename which contains a list of slaves (one per line) to advertise
+ offers for. The file is watched, and periodically re-read to refresh
+ the slave whitelist. By default there is no whitelist / all machines are
+ accepted. (default: None)
+ <p/>
+
+ Example:
+ <pre><code>file:///etc/mesos/slave_whitelist</code></pre>
<p/>
- Path could be of the form <code>file:///path/to/file</code> or <code>/path/to/file</code>. (default: *)
</td>
</tr>
<tr>
@@ -613,7 +618,8 @@ file://path/to/file (where file contains one of the above)</code></pre>
</li>
<li> a path to a file containing either one of the above options </li>
-<pre><code> --master=file://path/to/file (where file contains one of the above)</code></pre>
+ <pre><code>You can also use the <code>file:///path/to/file</code> syntax
+ to read the argument from a file which contains one of the above</code></pre>
</li>
</ol>
Examples:
@@ -724,13 +730,18 @@ file://path/to/file (where file contains one of the above)</code></pre>
--credential=VALUE
</td>
<td>
- Either a path to a text with a single line
+ A path to a text file with a single line
containing 'principal' and 'secret' separated by whitespace.
+
<p/>
Or a path containing the JSON formatted information used for one credential.
<p/>
- Path could be of the form <code>file:///path/to/file< code> or <code>/path/to/file</code>.
+ Path should be of the form <code>file://path/to/file</code>.
<p/>
+ Remember you can also use
+ the <code>file:///path/to/file</code> argument value format to read the
+ value from a file.
+ </p>
JSON file example:
<pre><code>{
"principal": "username",
@@ -930,9 +941,8 @@ file://path/to/file (where file contains one of the above)</code></pre>
List of modules to be loaded and be available to the internal
subsystems.
<p/>
- Use <code>--modules=filepath</code> to specify the list of modules via a
- file containing a JSON formatted string. 'filepath' can be
- of the form <code>file:///path/to/file</code> or <code>/path/to/file</code>.
+ Remember you can also use the <code>file:///path/to/file</code>
+ argument value format to have the value read from a file.
<p/>
Use <code>--modules="{...}"</code> to specify the list of modules inline.
<p/>
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ee35b16/docs/upgrades.md
----------------------------------------------------------------------
diff --git a/docs/upgrades.md b/docs/upgrades.md
index 51c7e70..168e761 100644
--- a/docs/upgrades.md
+++ b/docs/upgrades.md
@@ -17,6 +17,7 @@ message AuthenticationStartMessage {
}
```
+All Mesos arguments can now be passed using file:// to read them out of a file (either an absolute or relative path). The --credentials, --whitelist, and any flags that expect JSON backed arguments (such as --modules) behave as before, although support for just passing a absolute path for any JSON flags rather than file:// has been deprecated and will produce a warning (and the absolute path behavior will be removed in a future release).
## Upgrading from 0.20.x to 0.21.x
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ee35b16/src/credentials/credentials.hpp
----------------------------------------------------------------------
diff --git a/src/credentials/credentials.hpp b/src/credentials/credentials.hpp
index 9965858..130beed 100644
--- a/src/credentials/credentials.hpp
+++ b/src/credentials/credentials.hpp
@@ -24,25 +24,26 @@
#include <stout/option.hpp>
#include <stout/os.hpp>
+#include <stout/path.hpp>
#include <stout/protobuf.hpp>
#include <stout/try.hpp>
namespace mesos {
namespace credentials {
-inline Result<Credentials> read(const std::string& path)
+inline Result<Credentials> read(const Path& path)
{
LOG(INFO) << "Loading credentials for authentication from '" << path << "'";
- Try<std::string> read = os::read(path);
+ Try<std::string> read = os::read(path.value);
if (read.isError()) {
- return Error("Failed to read credentials file '" + path +
+ return Error("Failed to read credentials file '" + path.value +
"': " + read.error());
} else if (read.get().empty()) {
return None();
}
- Try<os::Permissions> permissions = os::permissions(path);
+ Try<os::Permissions> permissions = os::permissions(path.value);
if (permissions.isError()) {
LOG(WARNING) << "Failed to stat credentials file '" << path
<< "': " << permissions.error();
@@ -78,19 +79,19 @@ inline Result<Credentials> read(const std::string& path)
}
-inline Result<Credential> readCredential(const std::string& path)
+inline Result<Credential> readCredential(const Path& path)
{
LOG(INFO) << "Loading credential for authentication from '" << path << "'";
- Try<std::string> read = os::read(path);
+ Try<std::string> read = os::read(path.value);
if (read.isError()) {
- return Error("Failed to read credential file '" + path +
+ return Error("Failed to read credential file '" + path.value +
"': " + read.error());
} else if (read.get().empty()) {
return None();
}
- Try<os::Permissions> permissions = os::permissions(path);
+ Try<os::Permissions> permissions = os::permissions(path.value);
if (permissions.isError()) {
LOG(WARNING) << "Failed to stat credential file '" << path
<< "': " << permissions.error();
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ee35b16/src/examples/load_generator_framework.cpp
----------------------------------------------------------------------
diff --git a/src/examples/load_generator_framework.cpp b/src/examples/load_generator_framework.cpp
index f803d92..16d2782 100644
--- a/src/examples/load_generator_framework.cpp
+++ b/src/examples/load_generator_framework.cpp
@@ -349,21 +349,6 @@ int main(int argc, char** argv)
string secret = flags.secret.get();
- // If --secret is specified as a file containing the secret,
- // replace 'secret' with the content.
- if (strings::startsWith(flags.secret.get(), "/") ||
- strings::startsWith(flags.secret.get(), "file://")) {
- const std::string& path =
- strings::remove(flags.secret.get(), "file://", strings::PREFIX);
-
- Try<std::string> read = os::read(path);
- if (read.isError()) {
- EXIT(1) << "Error reading secret file '" + path + "': " + read.error();
- }
-
- secret = read.get();
- }
-
Credential credential;
credential.set_principal(flags.principal);
credential.set_secret(strings::trim(secret));
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ee35b16/src/master/contender.cpp
----------------------------------------------------------------------
diff --git a/src/master/contender.cpp b/src/master/contender.cpp
index 0a8c099..594a764 100644
--- a/src/master/contender.cpp
+++ b/src/master/contender.cpp
@@ -70,12 +70,12 @@ private:
};
-Try<MasterContender*> MasterContender::create(const string& zk)
+Try<MasterContender*> MasterContender::create(const string& mechanism)
{
- if (zk == "") {
+ if (mechanism == "") {
return new StandaloneMasterContender();
- } else if (strings::startsWith(zk, "zk://")) {
- Try<zookeeper::URL> url = zookeeper::URL::parse(zk);
+ } else if (strings::startsWith(mechanism, "zk://")) {
+ Try<zookeeper::URL> url = zookeeper::URL::parse(mechanism);
if (url.isError()) {
return Error(url.error());
}
@@ -84,8 +84,25 @@ Try<MasterContender*> MasterContender::create(const string& zk)
"Expecting a (chroot) path for ZooKeeper ('/' is not supported)");
}
return new ZooKeeperMasterContender(url.get());
- } else if (strings::startsWith(zk, "file://")) {
- const string& path = zk.substr(7);
+ } else if (strings::startsWith(mechanism, "file://")) {
+ // Load the configuration out of a file. While Mesos and related
+ // programs always use <stout/flags> to process the command line
+ // arguments (and therefore file://) this entrypoint is exposed by
+ // libmesos, with frameworks currently calling it and expecting it
+ // to do the argument parsing for them which roughly matches the
+ // argument parsing Mesos will do.
+ // TODO(cmaloney): Rework the libmesos exposed APIs to expose
+ // A "flags" endpoint where the framework can pass the command
+ // line arguments and they will be parsed by <stout/flags> and the
+ // needed flags extracted, and then change this interface to
+ // require final values from teh flags. This means that a
+ // framework doesn't need to know how the flags are passed to
+ // match mesos' command line arguments if it wants, but if it
+ // needs to inspect/manipulate arguments, it can.
+ LOG(WARNING) << "Specifying master election mechanism / ZooKeeper URL to "
+ "be read out of a file via 'file://' is deprecated inside "
+ "Mesos and will be removed in a future release.";
+ const string& path = mechanism.substr(7);
const Try<string> read = os::read(path);
if (read.isError()) {
return Error("Failed to read from file at '" + path + "'");
@@ -94,7 +111,9 @@ Try<MasterContender*> MasterContender::create(const string& zk)
return create(strings::trim(read.get()));
}
- return Error("Failed to parse '" + zk + "'");
+ CHECK(!strings::startsWith(mechanism, "file://"));
+
+ return Error("Failed to parse '" + mechanism + "'");
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ee35b16/src/master/contender.hpp
----------------------------------------------------------------------
diff --git a/src/master/contender.hpp b/src/master/contender.hpp
index 8e3e25a..c009de3 100644
--- a/src/master/contender.hpp
+++ b/src/master/contender.hpp
@@ -52,13 +52,12 @@ class MasterContender
{
public:
// Attempts to create a master contender using the specified
- // Zookeeper.
- // The Zookeeper address should be one of:
+ // mechanism.
+ // The mechanism address should be one of:
// - zk://host1:port1,host2:port2,.../path
// - zk://username:password@host1:port1,host2:port2,.../path
- // - file:///path/to/file (where file contains one of the above)
// Note that the returned contender still needs to be 'initialize()'d.
- static Try<MasterContender*> create(const std::string& zk);
+ static Try<MasterContender*> create(const std::string& mechanism);
// Note that the contender's membership, if obtained, is scheduled
// to be cancelled during destruction.
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ee35b16/src/master/detector.cpp
----------------------------------------------------------------------
diff --git a/src/master/detector.cpp b/src/master/detector.cpp
index 367d1e1..053b0a7 100644
--- a/src/master/detector.cpp
+++ b/src/master/detector.cpp
@@ -195,12 +195,12 @@ private:
};
-Try<MasterDetector*> MasterDetector::create(const string& master)
+Try<MasterDetector*> MasterDetector::create(const string& mechanism)
{
- if (master == "") {
+ if (mechanism == "") {
return new StandaloneMasterDetector();
- } else if (master.find("zk://") == 0) {
- Try<zookeeper::URL> url = zookeeper::URL::parse(master);
+ } else if (strings::startsWith(mechanism, "zk://")) {
+ Try<zookeeper::URL> url = zookeeper::URL::parse(mechanism);
if (url.isError()) {
return Error(url.error());
}
@@ -209,8 +209,25 @@ Try<MasterDetector*> MasterDetector::create(const string& master)
"Expecting a (chroot) path for ZooKeeper ('/' is not supported)");
}
return new ZooKeeperMasterDetector(url.get());
- } else if (master.find("file://") == 0) {
- const string& path = master.substr(7);
+ } else if (strings::startsWith(mechanism, "file://")) {
+ // Load the configuration out of a file. While Mesos and related
+ // programs always use <stout/flags> to process the command line
+ // arguments (and therefore file://) this entrypoint is exposed by
+ // libmesos, with frameworks currently calling it and expecting it
+ // to do the argument parsing for them which roughly matches the
+ // argument parsing Mesos will do.
+ // TODO(cmaloney): Rework the libmesos exposed APIs to expose
+ // A "flags" endpoint where the framework can pass the command
+ // line arguments and they will be parsed by <stout/flags> and the
+ // needed flags extracted, and then change this interface to
+ // require final values from teh flags. This means that a
+ // framework doesn't need to know how the flags are passed to
+ // match mesos' command line arguments if it wants, but if it
+ // needs to inspect/manipulate arguments, it can.
+ LOG(WARNING) << "Specifying master detection mechanism / ZooKeeper URL to "
+ "be read out of a file via 'file://' is deprecated inside "
+ "Mesos and will be removed in a future release.";
+ const string& path = mechanism.substr(7);
const Try<string> read = os::read(path);
if (read.isError()) {
return Error("Failed to read from file at '" + path + "'");
@@ -219,13 +236,15 @@ Try<MasterDetector*> MasterDetector::create(const string& master)
return create(strings::trim(read.get()));
}
+ CHECK(!strings::startsWith(mechanism, "file://"));
+
// Okay, try and parse what we got as a PID.
- UPID pid = master.find("master@") == 0
- ? UPID(master)
- : UPID("master@" + master);
+ UPID pid = mechanism.find("master@") == 0
+ ? UPID(mechanism)
+ : UPID("master@" + mechanism);
if (!pid) {
- return Error("Failed to parse '" + master + "'");
+ return Error("Failed to parse '" + mechanism + "'");
}
return new StandaloneMasterDetector(protobuf::createMasterInfo(pid));
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ee35b16/src/master/detector.hpp
----------------------------------------------------------------------
diff --git a/src/master/detector.hpp b/src/master/detector.hpp
index 4810748..1373845 100644
--- a/src/master/detector.hpp
+++ b/src/master/detector.hpp
@@ -49,13 +49,12 @@ class ZooKeeperMasterDetectorProcess;
class MasterDetector
{
public:
- // Attempts to create a master detector for the specified master.
- // The master should be one of:
+ // Attempts to create a master detector for the specified mechanism.
+ // The mechanism should be one of:
// - host:port
// - zk://host1:port1,host2:port2,.../path
// - zk://username:password@host1:port1,host2:port2,.../path
- // - file:///path/to/file (where file contains one of the above)
- static Try<MasterDetector*> create(const std::string& master);
+ static Try<MasterDetector*> create(const std::string& mechanism);
virtual ~MasterDetector() = 0;
// Returns MasterInfo after an election has occurred and the elected
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ee35b16/src/master/flags.hpp
----------------------------------------------------------------------
diff --git a/src/master/flags.hpp b/src/master/flags.hpp
index 6cc0db6..51a6059 100644
--- a/src/master/flags.hpp
+++ b/src/master/flags.hpp
@@ -29,6 +29,7 @@
#include <stout/flags.hpp>
#include <stout/json.hpp>
#include <stout/option.hpp>
+#include <stout/path.hpp>
#include <stout/protobuf.hpp>
#include "common/parse.hpp"
@@ -168,8 +169,7 @@ public:
"whitelist",
"Path to a file with a list of slaves\n"
"(one per line) to advertise offers for.\n"
- "Path could be of the form 'file:///path/to/file' or '/path/to/file'.",
- "*");
+ "Path could be of the form 'file:///path/to/file' or '/path/to/file'.");
add(&Flags::user_sorter,
"user_sorter",
@@ -391,7 +391,7 @@ public:
std::string recovery_slave_removal_limit;
Option<std::string> slave_removal_rate_limit;
std::string webui_dir;
- std::string whitelist;
+ Option<Path> whitelist;
std::string user_sorter;
std::string framework_sorter;
Duration allocation_interval;
@@ -400,7 +400,7 @@ public:
Option<std::string> weights;
bool authenticate_frameworks;
bool authenticate_slaves;
- Option<std::string> credentials;
+ Option<Path> credentials;
Option<ACLs> acls;
Option<RateLimits> rate_limits;
Option<Duration> offer_timeout;
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ee35b16/src/master/main.cpp
----------------------------------------------------------------------
diff --git a/src/master/main.cpp b/src/master/main.cpp
index d4adae5..1dce7fb 100644
--- a/src/master/main.cpp
+++ b/src/master/main.cpp
@@ -222,20 +222,7 @@ int main(int argc, char** argv)
<< " registry when using ZooKeeper";
}
- string zk_;
- if (strings::startsWith(zk.get(), "file://")) {
- const string& path = zk.get().substr(7);
- const Try<string> read = os::read(path);
- if (read.isError()) {
- EXIT(1) << "Failed to read from file at '" + path + "': "
- << read.error();
- }
- zk_ = read.get();
- } else {
- zk_ = zk.get();
- }
-
- Try<zookeeper::URL> url = zookeeper::URL::parse(zk_);
+ Try<zookeeper::URL> url = zookeeper::URL::parse(zk.get());
if (url.isError()) {
EXIT(1) << "Error parsing ZooKeeper URL: " << url.error();
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ee35b16/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index dd594e8..9e75b6c 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -418,10 +418,8 @@ void Master::initialize()
// Load credentials.
if (flags.credentials.isSome()) {
- const string& path =
- strings::remove(flags.credentials.get(), "file://", strings::PREFIX);
-
- Result<Credentials> _credentials = credentials::read(path);
+ Result<Credentials> _credentials =
+ credentials::read(flags.credentials.get());
if (_credentials.isError()) {
EXIT(1) << _credentials.error() << " (see --credentials flag)";
} else if (_credentials.isNone()) {
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ee35b16/src/slave/flags.hpp
----------------------------------------------------------------------
diff --git a/src/slave/flags.hpp b/src/slave/flags.hpp
index b446159..6ef64ed 100644
--- a/src/slave/flags.hpp
+++ b/src/slave/flags.hpp
@@ -492,7 +492,7 @@ public:
Duration perf_interval;
Duration perf_duration;
#endif
- Option<std::string> credential;
+ Option<Path> credential;
Option<std::string> containerizer_path;
std::string containerizers;
Option<std::string> default_container_image;
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ee35b16/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 8a1b12e..f39a876 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -269,14 +269,12 @@ void Slave::initialize()
authenticateeName = flags.authenticatee;
if (flags.credential.isSome()) {
- const string& path =
- strings::remove(flags.credential.get(), "file://", strings::PREFIX);
-
- Result<Credential> _credential = credentials::readCredential(path);
+ Result<Credential> _credential =
+ credentials::readCredential(flags.credential.get());
if (_credential.isError()) {
EXIT(1) << _credential.error() << " (see --credential flag)";
} else if (_credential.isNone()) {
- EXIT(1) << "Empty credential file '" << path
+ EXIT(1) << "Empty credential file '" << flags.credential.get()
<< "' (see --credential flag)";
} else {
credential = _credential.get();
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ee35b16/src/tests/credentials_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/credentials_tests.cpp b/src/tests/credentials_tests.cpp
index e39db9e..4210d31 100644
--- a/src/tests/credentials_tests.cpp
+++ b/src/tests/credentials_tests.cpp
@@ -88,9 +88,11 @@ TEST_F(CredentialsTest, authenticatedSlaveText)
<< "Failed to write credentials to '" << path << "'";
CHECK_SOME(os::close(fd.get()));
- flags.credentials = "file://" + path;
+ map<string, Option<string>> values{{"credentials", Some("file://" + path)}};
- Try<PID<Master> > master = StartMaster(flags);
+ flags.load(values, true);
+
+ Try<PID<Master>> master = StartMaster(flags);
ASSERT_SOME(master);
Future<SlaveRegisteredMessage> slaveRegisteredMessage =
@@ -98,9 +100,9 @@ TEST_F(CredentialsTest, authenticatedSlaveText)
slave::Flags slaveFlags = CreateSlaveFlags();
- slaveFlags.credential = "file://" + path;
+ slaveFlags.load(values, true);
- Try<PID<Slave> > slave = StartSlave(slaveFlags);
+ Try<PID<Slave>> slave = StartSlave(slaveFlags);
ASSERT_SOME(slave);
AWAIT_READY(slaveRegisteredMessage);
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ee35b16/src/tests/master_allocator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_allocator_tests.cpp b/src/tests/master_allocator_tests.cpp
index 23a3ed3..b164625 100644
--- a/src/tests/master_allocator_tests.cpp
+++ b/src/tests/master_allocator_tests.cpp
@@ -1106,7 +1106,7 @@ TYPED_TEST(MasterAllocatorTest, Whitelist)
ASSERT_SOME(os::write(path, strings::join("\n", hosts)));
master::Flags masterFlags = this->CreateMasterFlags();
- masterFlags.whitelist = "file://" + path;
+ masterFlags.whitelist = path;
EXPECT_CALL(this->allocator, initialize(_, _, _));
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ee35b16/src/tests/master_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_tests.cpp b/src/tests/master_tests.cpp
index 5b6c485..c678527 100644
--- a/src/tests/master_tests.cpp
+++ b/src/tests/master_tests.cpp
@@ -1061,7 +1061,7 @@ TEST_F(WhitelistTest, WhitelistSlave)
ASSERT_SOME(os::write(path, hosts)) << "Error writing whitelist";
master::Flags flags = CreateMasterFlags();
- flags.whitelist = "file://" + path;
+ flags.whitelist = path;
Try<PID<Master> > master = StartMaster(flags);
ASSERT_SOME(master);
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ee35b16/src/tests/mesos.cpp
----------------------------------------------------------------------
diff --git a/src/tests/mesos.cpp b/src/tests/mesos.cpp
index 0b4637d..3483559 100644
--- a/src/tests/mesos.cpp
+++ b/src/tests/mesos.cpp
@@ -110,7 +110,7 @@ master::Flags MesosTest::CreateMasterFlags()
<< "Failed to write credentials to '" << path << "'";
CHECK_SOME(os::close(fd.get()));
- flags.credentials = "file://" + path;
+ flags.credentials = path;
// Set default ACLs.
flags.acls = ACLs();
@@ -159,7 +159,7 @@ slave::Flags MesosTest::CreateSlaveFlags()
CHECK_SOME(os::close(fd.get()));
- flags.credential = "file://" + path;
+ flags.credential = path;
// TODO(vinod): Consider making this true and fixing the tests.
flags.checkpoint = false;
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ee35b16/src/watcher/whitelist_watcher.cpp
----------------------------------------------------------------------
diff --git a/src/watcher/whitelist_watcher.cpp b/src/watcher/whitelist_watcher.cpp
index 2a1586e..e00b5c8 100644
--- a/src/watcher/whitelist_watcher.cpp
+++ b/src/watcher/whitelist_watcher.cpp
@@ -26,6 +26,7 @@
#include <stout/foreach.hpp>
#include <stout/os.hpp>
+#include <stout/path.hpp>
#include <stout/strings.hpp>
#include <stout/try.hpp>
@@ -42,7 +43,7 @@ using lambda::function;
WhitelistWatcher::WhitelistWatcher(
- const string& path,
+ const Option<Path>& path,
const Duration& watchInterval,
const function<void(const Option<hashset<string>>& whitelist)>& subscriber,
const Option<hashset<std::string>>& initialWhitelist)
@@ -60,7 +61,20 @@ void WhitelistWatcher::initialize()
// subscriber's initial policy was not permissive (initial
// whitelist is not in (1) absent), notify the subscriber that
// there is no whitelist any more.
- if (path == "*") { // Accept all nodes.
+ //
+ // TODO(cmaloney): In older versions of Mesos the default value for
+ // 'whitelist' which represented "accept all nodes" was the string
+ // value '*' rather than 'None'. For backwards compatibility we
+ // still check for '*'. Can be removed after 0.23.
+ if (path.isSome() && path.get().value == "*") {
+ LOG(WARNING)
+ << "Explicitly specifying '*' for the whitelist in order to "
+ << "\"accept all\" is deprecated and will be removed in a future "
+ << "release; simply don't specify the whitelist flag in order to "
+ << "\"accept all\" slaves";
+ }
+
+ if (path.isNone() || path.get().value == "*") { // Accept all nodes.
VLOG(1) << "No whitelist given";
if (lastWhitelist.isSome()) {
subscriber(None());
@@ -78,14 +92,16 @@ void WhitelistWatcher::watch()
// TODO(vinod): Ensure this read is atomic w.r.t external
// writes/updates to this file.
Option<hashset<string>> whitelist;
- Try<string> read = os::read(
- strings::remove(path, "file://", strings::PREFIX));
+
+ CHECK_SOME(path);
+ Try<string> read = os::read(path.get().value);
+
if (read.isError()) {
LOG(ERROR) << "Error reading whitelist file: " << read.error() << ". "
<< "Retrying";
whitelist = lastWhitelist;
} else if (read.get().empty()) {
- VLOG(1) << "Empty whitelist file " << path;
+ VLOG(1) << "Empty whitelist file " << path.get().value;
whitelist = hashset<string>();
} else {
hashset<string> hostnames;
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ee35b16/src/watcher/whitelist_watcher.hpp
----------------------------------------------------------------------
diff --git a/src/watcher/whitelist_watcher.hpp b/src/watcher/whitelist_watcher.hpp
index 16ea839..654a27d 100644
--- a/src/watcher/whitelist_watcher.hpp
+++ b/src/watcher/whitelist_watcher.hpp
@@ -27,6 +27,7 @@
#include <stout/hashset.hpp>
#include <stout/lambda.hpp>
#include <stout/option.hpp>
+#include <stout/path.hpp>
namespace mesos {
@@ -51,7 +52,7 @@ public:
// NOTE: The caller should ensure a callback exists throughout
// WhitelistWatcher's lifetime.
WhitelistWatcher(
- const std::string& path,
+ const Option<Path>& path,
const Duration& watchInterval,
const lambda::function<
void(const Option<hashset<std::string>>& whitelist)>& subscriber,
@@ -62,7 +63,7 @@ protected:
void watch();
private:
- const std::string path;
+ const Option<Path> path;
const Duration watchInterval;
lambda::function<void(const Option<hashset<std::string>>& whitelist)>
subscriber;
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ee35b16/src/zookeeper/url.hpp
----------------------------------------------------------------------
diff --git a/src/zookeeper/url.hpp b/src/zookeeper/url.hpp
index 16e711c..b4253e8 100644
--- a/src/zookeeper/url.hpp
+++ b/src/zookeeper/url.hpp
@@ -48,9 +48,13 @@ namespace zookeeper {
class URL
{
public:
- // TODO(kensipe): Add support for "file://" based urls.
static Try<URL> parse(const std::string& url);
+ static const char* scheme()
+ {
+ return "zk://";
+ }
+
const Option<Authentication> authentication;
const std::string servers;
const std::string path;
@@ -74,18 +78,15 @@ inline Try<URL> URL::parse(const std::string& url)
{
std::string s = strings::trim(url);
- size_t index = s.find_first_of("zk://");
-
- if (index != 0) {
+ if (!strings::startsWith(s, URL::scheme())) {
return Error("Expecting 'zk://' at the beginning of the URL");
}
-
s = s.substr(5);
// Look for the trailing '/' (if any), that's where the path starts.
std::string path;
do {
- index = s.find_last_of('/');
+ size_t index = s.find_last_of('/');
if (index == std::string::npos) {
break;
@@ -100,7 +101,7 @@ inline Try<URL> URL::parse(const std::string& url)
}
// Look for the trailing '@' (if any), that's where servers starts.
- index = s.find_last_of('@');
+ size_t index = s.find_last_of('@');
if (index != std::string::npos) {
return URL(s.substr(0, index), s.substr(index + 1), path);
@@ -111,7 +112,7 @@ inline Try<URL> URL::parse(const std::string& url)
inline std::ostream& operator << (std::ostream& stream, const URL& url)
{
- stream << "zk://";
+ stream << URL::scheme();
if (url.authentication.isSome()) {
stream << url.authentication.get() << "@";
}
[2/2] mesos git commit: Parse file:// arguments in Stout's flags.
Posted by be...@apache.org.
Parse file:// arguments in Stout's flags.
More cases kept being added which dealt with 'file://' urls, parsing
them, reading from the file, and then handling the argument as though
the file:// wasn't there.
We now always read an argument from a file in the argument parser if
it begins with 'file://'. If you want to pass an argument which is a
filename, then just use the path directly.
Adds a new fetch<> step to parsing an argument which occurs before
parse<>. This fetch can perform actions such as reading a file from
the filesystem, or fetching a resource from a URL.
This is added to alleviate the need to keep adding more code to deal
with 'file://' URLs in each new case where someone needs one.
The patchset introduces no outside-visible argument changes, but it
does require developers who want to specify 'give me a path' to ask
for that path using the new 'Path' type rather than a string, since
for 'file://' URLs, we want to keep the filename.
All existing instances have been modified.
Specifying reading JSON from a file using an absolute path rather than
a file:// URI has been deprecated, and will now log a deprecation
message when used.
>From a quick pass on JIRA (first page of results in JIRA searching for
files): This would have prevented: MESOS-1368, MESOS-1635, MESOS-1590
And simplifies supporting: MESOS-18
Review: https://reviews.apache.org/r/30194
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/1fa5d5b1
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/1fa5d5b1
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/1fa5d5b1
Branch: refs/heads/master
Commit: 1fa5d5b1de304bcb90638a889677e53756152c6b
Parents: 5454565
Author: Cody Maloney <co...@mesosphere.io>
Authored: Tue Feb 10 23:13:43 2015 -0800
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Tue Feb 10 23:32:13 2015 -0800
----------------------------------------------------------------------
.../3rdparty/stout/include/Makefile.am | 1 +
.../stout/include/stout/flags/fetch.hpp | 70 ++++++++++++++++++++
.../stout/include/stout/flags/flags.hpp | 10 +--
.../stout/include/stout/flags/parse.hpp | 37 ++++++-----
.../3rdparty/stout/include/stout/path.hpp | 23 ++++++-
.../3rdparty/stout/tests/flags_tests.cpp | 24 +++++++
6 files changed, 144 insertions(+), 21 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/1fa5d5b1/3rdparty/libprocess/3rdparty/stout/include/Makefile.am
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/Makefile.am b/3rdparty/libprocess/3rdparty/stout/include/Makefile.am
index dc3ef53..8f6ac15 100644
--- a/3rdparty/libprocess/3rdparty/stout/include/Makefile.am
+++ b/3rdparty/libprocess/3rdparty/stout/include/Makefile.am
@@ -11,6 +11,7 @@ nobase_include_HEADERS = \
stout/exit.hpp \
stout/fatal.hpp \
stout/flags.hpp \
+ stout/flags/fetch.hpp \
stout/flags/flag.hpp \
stout/flags/flags.hpp \
stout/flags/loader.hpp \
http://git-wip-us.apache.org/repos/asf/mesos/blob/1fa5d5b1/3rdparty/libprocess/3rdparty/stout/include/stout/flags/fetch.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/flags/fetch.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/flags/fetch.hpp
new file mode 100644
index 0000000..1df982c
--- /dev/null
+++ b/3rdparty/libprocess/3rdparty/stout/include/stout/flags/fetch.hpp
@@ -0,0 +1,70 @@
+/**
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#ifndef __STOUT_FLAGS_FETCH_HPP__
+#define __STOUT_FLAGS_FETCH_HPP__
+
+#include <sstream> // For istringstream.
+#include <string>
+
+#include <stout/duration.hpp>
+#include <stout/error.hpp>
+#include <stout/json.hpp>
+#include <stout/path.hpp>
+#include <stout/strings.hpp>
+#include <stout/try.hpp>
+
+#include <stout/flags/parse.hpp>
+
+#include <stout/os/read.hpp>
+
+namespace flags {
+
+// Allow the value for a flag to be fetched/resolved from a URL such
+// as file://foo/bar/baz. Use template specialization to add a custom
+// fetcher for a given type, such as Path from <stout/path.hpp> Path
+// has a custom fetcher so that it returns the path given rather than
+// the value read from that path.
+template <typename T>
+Try<T> fetch(const std::string& value)
+{
+ // If the flag value corresponds to a file indicated by file://
+ // fetch and then parse the contents of that file.
+ //
+ // TODO(cmaloney): Introduce fetching for things beyond just file://
+ // such as http:// as well!
+ if (strings::startsWith(value, "file://")) {
+ const std::string& path = value.substr(7);
+
+ Try<std::string> read = os::read(path);
+ if (read.isError()) {
+ return Error("Error reading file '" + path + "': " + read.error());
+ }
+
+ return parse<T>(read.get());
+ }
+
+ return parse<T>(value);
+}
+
+
+template <>
+inline Try<Path> fetch(const std::string& value)
+{
+ // Explicitly skip fetching the file if this is for a Path flag!
+ return parse<Path>(value);
+}
+
+} // namespace flags {
+
+#endif // __STOUT_FLAGS_FETCH_HPP__
http://git-wip-us.apache.org/repos/asf/mesos/blob/1fa5d5b1/3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp
index f4b7a95..aedb6ab 100644
--- a/3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp
+++ b/3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp
@@ -31,10 +31,10 @@
#include <stout/strings.hpp>
#include <stout/try.hpp>
+#include <stout/flags/fetch.hpp>
#include <stout/flags/flag.hpp>
#include <stout/flags/loader.hpp>
#include <stout/flags/stringifier.hpp>
-#include <stout/flags/parse.hpp>
namespace flags {
@@ -171,7 +171,7 @@ void FlagsBase::add(
&Loader<T1>::load,
t1,
lambda::function<Try<T1>(const std::string&)>(
- lambda::bind(&parse<T1>, lambda::_1)),
+ lambda::bind(&fetch<T1>, lambda::_1)),
name,
lambda::_2); // Use _2 because ignore FlagsBase*.
flag.stringify = lambda::bind(&Stringifier<T1>, t1);
@@ -201,7 +201,7 @@ void FlagsBase::add(
&OptionLoader<T>::load,
option,
lambda::function<Try<T>(const std::string&)>(
- lambda::bind(&parse<T>, lambda::_1)),
+ lambda::bind(&fetch<T>, lambda::_1)),
name,
lambda::_2); // Use _2 because ignore FlagsBase*.
flag.stringify = lambda::bind(&OptionStringifier<T>, option);
@@ -233,7 +233,7 @@ void FlagsBase::add(
lambda::_1,
t1,
lambda::function<Try<T1>(const std::string&)>(
- lambda::bind(&parse<T1>, lambda::_1)),
+ lambda::bind(&fetch<T1>, lambda::_1)),
name,
lambda::_2);
flag.stringify = lambda::bind(
@@ -272,7 +272,7 @@ void FlagsBase::add(
lambda::_1,
option,
lambda::function<Try<T>(const std::string&)>(
- lambda::bind(&parse<T>, lambda::_1)),
+ lambda::bind(&fetch<T>, lambda::_1)),
name,
lambda::_2);
flag.stringify = lambda::bind(
http://git-wip-us.apache.org/repos/asf/mesos/blob/1fa5d5b1/3rdparty/libprocess/3rdparty/stout/include/stout/flags/parse.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/flags/parse.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/flags/parse.hpp
index 1209469..e34ed78 100644
--- a/3rdparty/libprocess/3rdparty/stout/include/stout/flags/parse.hpp
+++ b/3rdparty/libprocess/3rdparty/stout/include/stout/flags/parse.hpp
@@ -76,28 +76,35 @@ inline Try<Bytes> parse(const std::string& value)
template <>
inline Try<JSON::Object> parse(const std::string& value)
{
- // If the flag value corresponds to a file parse the contents of the
- // file as JSON.
- // TODO(vinod): We do not support relative paths because it is
- // tricky to figure out if a flag value corresponds to a relative
- // path or a JSON string. For example, "{", " {" and " \n {" are
- // all valid prefixes of a JSON string.
- if (strings::startsWith(value, "/") ||
- strings::startsWith(value, "file://")) {
- const std::string& path =
- strings::remove(value, "file://", strings::PREFIX);
-
- Try<std::string> read = os::read(path);
+ // A value that already starts with 'file://' will properly be
+ // loaded from the file and put into 'value' but if it starts with
+ // '/' we need to explicitly handle it for backwards compatibility
+ // reasons (because we used to handle it before we introduced the
+ // 'fetch' mechanism for flags that first fetches the data from URIs
+ // such as 'file://').
+ if (strings::startsWith(value, "/")) {
+ LOG(WARNING) << "Specifying a absolute filename to read a command line "
+ "option out of without using 'file:// is deprecated and "
+ "will be removed in a future release. Simply adding "
+ "'file://' to the beginning of the path should eliminate "
+ "this warning.";
+
+ Try<std::string> read = os::read(value);
if (read.isError()) {
- return Error("Error reading file '" + path + "': " + read.error());
+ return Error("Error reading file '" + value + "': " + read.error());
}
-
return JSON::parse<JSON::Object>(read.get());
}
-
return JSON::parse<JSON::Object>(value);
}
+
+template <>
+inline Try<Path> parse(const std::string& value)
+{
+ return Path(value);
+}
+
} // namespace flags {
#endif // __STOUT_FLAGS_PARSE_HPP__
http://git-wip-us.apache.org/repos/asf/mesos/blob/1fa5d5b1/3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp
index 9f8f520..d4df650 100644
--- a/3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp
+++ b/3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp
@@ -15,10 +15,31 @@
#define __STOUT_PATH_HPP__
#include <string>
-#include <vector>
#include <stout/strings.hpp>
+// Basic abstraction for representing a path in a filesystem.
+class Path
+{
+public:
+ explicit Path(const std::string& path)
+ : value(strings::remove(path, "file://", strings::PREFIX)) {}
+
+ // TODO(cmaloney): Add more useful operations such as 'absolute()'
+ // and 'basename', and 'dirname', etc.
+
+ const std::string value;
+};
+
+
+inline std::ostream& operator << (
+ std::ostream& stream,
+ const Path& path)
+{
+ return stream << path.value;
+}
+
+
namespace path {
inline std::string join(const std::string& path1, const std::string& path2)
http://git-wip-us.apache.org/repos/asf/mesos/blob/1fa5d5b1/3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp b/3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp
index 3b60ff8..0028119 100644
--- a/3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp
+++ b/3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp
@@ -588,3 +588,27 @@ TEST_F(FlagsFileTest, JSONFile)
ASSERT_SOME_EQ(object, json);
}
+
+
+TEST_F(FlagsFileTest, FilePrefix)
+{
+ Flags<TestFlags> flags;
+
+ Option<std::string> something;
+
+ flags.add(&something,
+ "something",
+ "arg to be loaded from file");
+
+ // Write the JSON to a file.
+ const string& file = path::join(os::getcwd(), "file");
+ ASSERT_SOME(os::write(file, "testing"));
+
+ // Read the JSON from the file.
+ map<string, Option<string> > values;
+ values["something"] = Some("file://" + file);
+
+ ASSERT_SOME(flags.load(values));
+
+ ASSERT_SOME_EQ("testing", something);
+}