You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ne...@apache.org on 2017/05/03 22:38:56 UTC
[1/4] mesos git commit: Enhanced stout's Version to support
prerelease and build labels.
Repository: mesos
Updated Branches:
refs/heads/master 310c0d62d -> 088087397
Enhanced stout's Version to support prerelease and build labels.
Previously, Stout's `Version` abstraction only supported a subset of
Semver: version numbers with three numeric components (an optional
trailing "label" with a leading hyphen was supported but ignored).
This commit adds support for SemVer 2.0.0, which defines two additional
optional fields: a "prerelease label" and a "build metadata label",
e.g., "1.2.3-alpha.1+foo". Both labels consist of a series of
dot-separated identifiers.
Review: https://reviews.apache.org/r/58707
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/28755628
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/28755628
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/28755628
Branch: refs/heads/master
Commit: 287556284d76e03c11cff3fc076224fe966096e0
Parents: 310c0d6
Author: Neil Conway <ne...@gmail.com>
Authored: Fri Apr 21 09:03:10 2017 -0700
Committer: Neil Conway <ne...@gmail.com>
Committed: Wed May 3 15:37:31 2017 -0700
----------------------------------------------------------------------
3rdparty/stout/include/stout/version.hpp | 346 ++++++++++++++++++++++----
3rdparty/stout/tests/version_tests.cpp | 120 +++++++--
2 files changed, 399 insertions(+), 67 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/28755628/3rdparty/stout/include/stout/version.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/version.hpp b/3rdparty/stout/include/stout/version.hpp
index 7717c85..5e2bd2e 100644
--- a/3rdparty/stout/include/stout/version.hpp
+++ b/3rdparty/stout/include/stout/version.hpp
@@ -13,73 +13,161 @@
#ifndef __STOUT_VERSION_HPP__
#define __STOUT_VERSION_HPP__
+#include <algorithm>
+#include <cctype>
#include <ostream>
#include <string>
#include <vector>
+#include <stout/check.hpp>
#include <stout/error.hpp>
#include <stout/numify.hpp>
+#include <stout/option.hpp>
#include <stout/stringify.hpp>
#include <stout/strings.hpp>
#include <stout/try.hpp>
-// This class provides convenience routines for version checks.
+// This class provides convenience routines for working with version
+// numbers. We support the SemVer 2.0.0 (http://semver.org) format,
+// with minor extensions.
//
-// Ideally, the components would be called simply major, minor and
-// patch. However, GNU libstdc++ already defines these as macros for
-// compatibility reasons (man 3 makedev for more information) implicitly
-// included in every compilation.
-//
-// TODO(karya): Consider adding support for more than 3 components, and
-// compatibility operators.
-// TODO(karya): Add support for labels and build metadata. Consider
-// semantic versioning (http://semvar.org/) for specs.
+// Ideally, the version components would be called simply "major",
+// "minor", and "patch". However, GNU libstdc++ already defines these
+// as macros for compatibility reasons (man 3 makedev for more
+// information) implicitly included in every compilation.
struct Version
{
// Expect the string in the following format:
- // <major>[.<minor>[.<patch>]]
- // Missing components are treated as zero.
- static Try<Version> parse(const std::string& s)
+ // <major>[.<minor>[.<patch>]][-prerelease][+build]
+ //
+ // Missing `minor` or `patch` components are treated as zero.
+ // Allowing some version numbers to be omitted is an extension to
+ // SemVer, albeit one that several other SemVer libraries implement.
+ //
+ // TODO(neilc): Consider providing a "strict" variant that does not
+ // allow version numbers to be omitted.
+ //
+ // `prerelease` is a prerelease label (e.g., "beta", "rc.1");
+ // `build` is a build metadata label. Both `prerelease` and `build`
+ // consist of one or more dot-separated identifiers. An identifier
+ // is a non-empty string containing ASCII alphanumeric characters or
+ // hyphens.
+ static Try<Version> parse(const std::string& input)
{
- const size_t maxComponents = 3;
-
- // Use only the part before '-', i.e. strip and discard the tags
- // and labels.
- // TODO(karya): Once we have support for labels and tags, we
- // should not discard the remaining string.
- std::vector<std::string> split =
- strings::split(strings::split(s, "-")[0], ".");
-
- if (split.size() > maxComponents) {
- return Error("Version string has " + stringify(split.size()) +
- " components; maximum " + stringify(maxComponents) +
+ // The input string consists of the numeric components, optionally
+ // followed by the prerelease label (prefixed with '-') and/or the
+ // build label (prefixed with '+'). We parse the string from right
+ // to left: build label (if any), prerelease label (if any), and
+ // finally numeric components.
+
+ std::vector<std::string> buildLabel;
+
+ std::vector<std::string> buildParts = strings::split(input, "+", 2);
+ CHECK(buildParts.size() == 1 || buildParts.size() == 2);
+
+ if (buildParts.size() == 2) {
+ const std::string& buildString = buildParts.back();
+
+ // NOTE: Build metadata identifiers can contain leading zeros
+ // (unlike numeric prerelease identifiers; see below).
+ Try<std::vector<std::string>> parsed = parseLabel(buildString, false);
+ if (parsed.isError()) {
+ return Error("Invalid build label: " + parsed.error());
+ }
+
+ buildLabel = parsed.get();
+ }
+
+ std::string remainder = buildParts.front();
+
+ // Parse the prerelease label, if any. Note that the prerelease
+ // label might itself contain hyphens.
+ std::vector<std::string> prereleaseLabel;
+
+ std::vector<std::string> prereleaseParts =
+ strings::split(remainder, "-", 2);
+ CHECK(prereleaseParts.size() == 1 || prereleaseParts.size() == 2);
+
+ if (prereleaseParts.size() == 2) {
+ const std::string& prereleaseString = prereleaseParts.back();
+
+ // Prerelease identifiers cannot contain leading zeros.
+ Try<std::vector<std::string>> parsed = parseLabel(prereleaseString, true);
+ if (parsed.isError()) {
+ return Error("Invalid prerelease label: " + parsed.error());
+ }
+
+ prereleaseLabel = parsed.get();
+ }
+
+ remainder = prereleaseParts.front();
+
+ constexpr size_t maxNumericComponents = 3;
+ std::vector<std::string> numericComponents = strings::split(remainder, ".");
+
+ if (numericComponents.size() > maxNumericComponents) {
+ return Error("Version has " + stringify(numericComponents.size()) +
+ " components; maximum " + stringify(maxNumericComponents) +
" components allowed");
}
- int components[maxComponents] = {0};
+ int versionNumbers[maxNumericComponents] = {0};
- for (size_t i = 0; i < split.size(); i++) {
- Try<int> result = numify<int>(split[i]);
+ for (size_t i = 0; i < numericComponents.size(); i++) {
+ Try<int> result = parseNumericIdentifier(numericComponents[i]);
if (result.isError()) {
- return Error("Invalid version component '" + split[i] + "': " +
- result.error());
+ return Error("Invalid version component '" + numericComponents[i] + "'"
+ ": " + result.error());
+ }
+
+ if (hasLeadingZero(numericComponents[i])) {
+ return Error("Invalid version component '" + numericComponents[i] + "'"
+ ": cannot contain leading zero");
}
- components[i] = result.get();
+
+ versionNumbers[i] = result.get();
}
- return Version(components[0], components[1], components[2]);
+ return Version(versionNumbers[0],
+ versionNumbers[1],
+ versionNumbers[2],
+ prereleaseLabel,
+ buildLabel);
}
- Version(int _majorVersion, int _minorVersion, int _patchVersion)
+ // Construct a new Version. The `_prerelease` and `_build` arguments
+ // contain lists of prerelease and build identifiers, respectively.
+ Version(int _majorVersion,
+ int _minorVersion,
+ int _patchVersion,
+ const std::vector<std::string>& _prerelease = {},
+ const std::vector<std::string>& _build = {})
: majorVersion(_majorVersion),
minorVersion(_minorVersion),
- patchVersion(_patchVersion) {}
+ patchVersion(_patchVersion),
+ prerelease(_prerelease),
+ build(_build)
+ {
+ // As a sanity check, ensure that the caller has provided
+ // valid prerelease and build identifiers.
+
+ foreach (const std::string& identifier, prerelease) {
+ CHECK_NONE(validateIdentifier(identifier, true));
+ }
+
+ foreach (const std::string& identifier, build) {
+ CHECK_NONE(validateIdentifier(identifier, false));
+ }
+ }
bool operator==(const Version& other) const
{
+ // NOTE: The `build` field is ignored when comparing two versions
+ // for equality, per SemVer spec.
return majorVersion == other.majorVersion &&
minorVersion == other.minorVersion &&
- patchVersion == other.patchVersion;
+ patchVersion == other.patchVersion &&
+ prerelease == other.prerelease;
}
bool operator!=(const Version& other) const
@@ -87,28 +175,94 @@ struct Version
return !(*this == other);
}
+ // SemVer 2.0.0 defines version precedence (ordering) like so:
+ //
+ // Precedence MUST be calculated by separating the version into
+ // major, minor, patch and pre-release identifiers in that order
+ // (Build metadata does not figure into precedence). Precedence is
+ // determined by the first difference when comparing each of these
+ // identifiers from left to right as follows: Major, minor, and
+ // patch versions are always compared numerically. Example: 1.0.0
+ // < 2.0.0 < 2.1.0 < 2.1.1. When major, minor, and patch are
+ // equal, a pre-release version has lower precedence than a normal
+ // version. Example: 1.0.0-alpha < 1.0.0. Precedence for two
+ // pre-release versions with the same major, minor, and patch
+ // version MUST be determined by comparing each dot separated
+ // identifier from left to right until a difference is found as
+ // follows: identifiers consisting of only digits are compared
+ // numerically and identifiers with letters or hyphens are
+ // compared lexically in ASCII sort order. Numeric identifiers
+ // always have lower precedence than non-numeric identifiers. A
+ // larger set of pre-release fields has a higher precedence than a
+ // smaller set, if all of the preceding identifiers are equal.
+ // Example: 1.0.0-alpha < 1.0.0-alpha.1 < 1.0.0-alpha.beta <
+ // 1.0.0-beta < 1.0.0-beta.2 < 1.0.0-beta.11 < 1.0.0-rc.1 < 1.0.0.
+ //
+ // NOTE: The `build` field is ignored when comparing two versions
+ // for precedence, per the SemVer spec text above.
bool operator<(const Version& other) const
{
- // Lexicographic ordering.
+ // Compare version numbers numerically.
if (majorVersion != other.majorVersion) {
return majorVersion < other.majorVersion;
- } else if (minorVersion != other.minorVersion) {
+ }
+ if (minorVersion != other.minorVersion) {
return minorVersion < other.minorVersion;
- } else {
+ }
+ if (patchVersion != other.patchVersion) {
return patchVersion < other.patchVersion;
}
+
+ // If one version has a prerelease label and the other does not,
+ // the prerelease version has lower precedence.
+ if (!prerelease.empty() && other.prerelease.empty()) {
+ return true;
+ }
+ if (prerelease.empty() && !other.prerelease.empty()) {
+ return false;
+ }
+
+ // Compare two versions with prerelease labels by proceeding from
+ // left to right.
+ size_t minPrereleaseSize = std::min(
+ prerelease.size(), other.prerelease.size());
+
+ for (size_t i = 0; i < minPrereleaseSize; i++) {
+ // Check whether the two prerelease identifiers can be converted
+ // to numbers.
+ Try<int> identifier = parseNumericIdentifier(prerelease.at(i));
+ Try<int> otherIdentifier = parseNumericIdentifier(other.prerelease.at(i));
+
+ if (identifier.isSome() && otherIdentifier.isSome()) {
+ // Both identifiers are numeric.
+ if (identifier.get() != otherIdentifier.get()) {
+ return identifier.get() < otherIdentifier.get();
+ }
+ } else if (identifier.isSome()) {
+ // `identifier` is numeric but `otherIdentifier` is not, so
+ // `identifier` comes first.
+ return true;
+ } else if (otherIdentifier.isSome()) {
+ // `otherIdentifier` is numeric but `identifier` is not, so
+ // `otherIdentifier` comes first.
+ return false;
+ } else {
+ // Neither identifier is numeric, so compare via ASCII sort.
+ if (prerelease.at(i) != other.prerelease.at(i)) {
+ return prerelease.at(i) < other.prerelease.at(i);
+ }
+ }
+ }
+
+ // If two versions have different numbers of prerelease labels but
+ // they match on the common prefix, the version with the smaller
+ // set of labels comes first.
+ return prerelease.size() < other.prerelease.size();
}
bool operator>(const Version& other) const
{
- // Lexicographic ordering.
- if (majorVersion != other.majorVersion) {
- return majorVersion > other.majorVersion;
- } else if (minorVersion != other.minorVersion) {
- return minorVersion > other.minorVersion;
- } else {
- return patchVersion > other.patchVersion;
- }
+ return other < *this;
}
bool operator<=(const Version& other) const
@@ -128,6 +282,90 @@ struct Version
const int majorVersion;
const int minorVersion;
const int patchVersion;
+ const std::vector<std::string> prerelease;
+ const std::vector<std::string> build;
+
+private:
+ // Check that a string contains a valid identifier. An identifier is
+ // a non-empty string; each character must be an ASCII alphanumeric
+ // or hyphen.
+ static Option<Error> validateIdentifier(
+ const std::string& identifier,
+ bool rejectLeadingZero)
+ {
+ if (identifier.empty()) {
+ return Error("Empty identifier");
+ }
+
+ auto alphaNumericOrHyphen = [](char c) -> bool {
+ return std::isalnum(c) || c == '-';
+ };
+
+ auto firstInvalid = std::find_if_not(
+ identifier.begin(), identifier.end(), alphaNumericOrHyphen);
+
+ if (firstInvalid != identifier.end()) {
+ return Error("Identifier contains illegal character: "
+ "'" + stringify(*firstInvalid) + "'");
+ }
+
+ // If requested, disallow identifiers that contain a leading
+ // zero. Note that this only applies to numeric identifiers, and
+ // that zero-valued identifiers are allowed.
+ if (rejectLeadingZero && hasLeadingZero(identifier)) {
+ return Error("Identifier contains leading zero");
+ }
+
+ return None();
+ }
+
+ // Parse a string containing a series of dot-separated identifiers
+ // into a vector of strings; each element of the vector contains a
+ // single identifier. If `rejectLeadingZeros` is true, we reject any
+ // numeric identifier in the label that contains a leading zero.
+ static Try<std::vector<std::string>> parseLabel(
+ const std::string& label,
+ bool rejectLeadingZeros)
+ {
+ if (label.empty()) {
+ return Error("Empty label");
+ }
+
+ std::vector<std::string> identifiers = strings::split(label, ".");
+
+ foreach (const std::string& identifier, identifiers) {
+ Option<Error> error = validateIdentifier(identifier, rejectLeadingZeros);
+ if (error.isSome()) {
+ return error.get();
+ }
+ }
+
+ return identifiers;
+ }
+
+ // Checks whether the input string is numeric and contains a leading
+ // zero. Note that "0" by itself is not considered a "leading zero".
+ static bool hasLeadingZero(const std::string& identifier) {
+ Try<int> numericIdentifier = parseNumericIdentifier(identifier);
+
+ return numericIdentifier.isSome() &&
+ numericIdentifier.get() != 0 &&
+ strings::startsWith(identifier, '0');
+ }
+
+ // Attempt to parse the given string as a numeric identifier.
+ // According to the SemVer spec, identifiers that begin with hyphens
+ // are considered non-numeric.
+ //
+ // TODO(neilc): Consider adding a variant of `numify<T>` that only
+ // supports non-negative inputs.
+ static Try<int> parseNumericIdentifier(const std::string& identifier) {
+ if (strings::startsWith(identifier, '-')) {
+ return Error("Contains leading hyphen");
+ }
+
+ return numify<int>(identifier);
+ }
};
@@ -135,9 +373,19 @@ inline std::ostream& operator<<(
std::ostream& stream,
const Version& version)
{
- return stream << version.majorVersion << "."
- << version.minorVersion << "."
- << version.patchVersion;
+ stream << version.majorVersion << "."
+ << version.minorVersion << "."
+ << version.patchVersion;
+
+ if (!version.prerelease.empty()) {
+ stream << "-" << strings::join(".", version.prerelease);
+ }
+
+ if (!version.build.empty()) {
+ stream << "+" << strings::join(".", version.build);
+ }
+
+ return stream;
}
#endif // __STOUT_VERSION_HPP__
http://git-wip-us.apache.org/repos/asf/mesos/blob/28755628/3rdparty/stout/tests/version_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/tests/version_tests.cpp b/3rdparty/stout/tests/version_tests.cpp
index 925f383..bce185e 100644
--- a/3rdparty/stout/tests/version_tests.cpp
+++ b/3rdparty/stout/tests/version_tests.cpp
@@ -30,20 +30,64 @@ using std::vector;
// Verify version comparison operations.
TEST(VersionTest, Comparison)
{
- Version version1(0, 10, 4);
- Version version2(0, 20, 3);
- Try<Version> version3 = Version::parse("0.20.3");
-
- EXPECT_EQ(version2, version3.get());
- EXPECT_NE(version1, version2);
- EXPECT_LT(version1, version2);
- EXPECT_LE(version1, version2);
- EXPECT_LE(version2, version3.get());
- EXPECT_GT(version2, version1);
- EXPECT_GE(version2, version1);
- EXPECT_GE(version3.get(), version1);
-
- EXPECT_EQ(stringify(version2), "0.20.3");
+ const vector<string> inputs = {
+ "0.0.0",
+ "0.2.3",
+ "0.9.9",
+ "0.10.4",
+ "0.20.3",
+ "1.0.0-alpha",
+ "1.0.0-alpha.1",
+ "1.0.0-alpha.-02",
+ "1.0.0-alpha.-1",
+ "1.0.0-alpha.1-1",
+ "1.0.0-alpha.beta",
+ "1.0.0-beta",
+ "1.0.0-beta.2",
+ "1.0.0-beta.11",
+ "1.0.0-rc.1",
+ "1.0.0-rc.1.2",
+ "1.0.0",
+ "1.0.1",
+ "2.0.0"
+ };
+
+ vector<Version> versions;
+
+ foreach (const string& input, inputs) {
+ Try<Version> version = Version::parse(input);
+ ASSERT_SOME(version);
+
+ versions.push_back(version.get());
+ }
+
+ // Check that `versions` is in ascending order.
+ for (size_t i = 0; i < versions.size(); i++) {
+ EXPECT_FALSE(versions[i] < versions[i])
+ << "Expected " << versions[i] << " < " << versions[i] << " to be false";
+
+ for (size_t j = i + 1; j < versions.size(); j++) {
+ EXPECT_TRUE(versions[i] < versions[j])
+ << "Expected " << versions[i] << " < " << versions[j];
+
+ EXPECT_FALSE(versions[j] < versions[i])
+ << "Expected " << versions[i] << " < " << versions[j] << " to be false";
+ }
+ }
+}
+
+
+// Verify that build metadata labels are ignored when determining
+// equality and ordering between versions.
+TEST(VersionTest, BuildMetadataComparison)
+{
+ Version plain = Version(1, 2, 3);
+ Version buildMetadata = Version(1, 2, 3, {}, {"abc"});
+
+ EXPECT_TRUE(plain == buildMetadata);
+ EXPECT_FALSE(plain != buildMetadata);
+ EXPECT_FALSE(plain < buildMetadata);
+ EXPECT_FALSE(plain > buildMetadata);
}
@@ -57,11 +101,30 @@ TEST(VersionTest, ParseValid)
typedef pair<Version, string> ExpectedValue;
const map<string, ExpectedValue> testCases = {
- // Prerelease labels are currently accepted but ignored.
- {"1.20.3-rc1", {Version(1, 20, 3), "1.20.3"}},
{"1.20.3", {Version(1, 20, 3), "1.20.3"}},
{"1.20", {Version(1, 20, 0), "1.20.0"}},
- {"1", {Version(1, 0, 0), "1.0.0"}}
+ {"1", {Version(1, 0, 0), "1.0.0"}},
+ {"1.20.3-rc1", {Version(1, 20, 3, {"rc1"}), "1.20.3-rc1"}},
+ {"1.20.3--", {Version(1, 20, 3, {"-"}), "1.20.3--"}},
+ {"1.20.3+-.-", {Version(1, 20, 3, {}, {"-", "-"}), "1.20.3+-.-"}},
+ {"1.0.0-alpha.1", {Version(1, 0, 0, {"alpha", "1"}), "1.0.0-alpha.1"}},
+ {"1.0.0-alpha+001",
+ {Version(1, 0, 0, {"alpha"}, {"001"}), "1.0.0-alpha+001"}},
+ {"1.0.0-alpha.-123",
+ {Version(1, 0, 0, {"alpha", "-123"}), "1.0.0-alpha.-123"}},
+ {"1+20130313144700",
+ {Version(1, 0, 0, {}, {"20130313144700"}), "1.0.0+20130313144700"}},
+ {"1.0.0-beta+exp.sha.5114f8",
+ {Version(1, 0, 0, {"beta"}, {"exp", "sha", "5114f8"}),
+ "1.0.0-beta+exp.sha.5114f8"}},
+ {"1.0.0--1", {Version(1, 0, 0, {"-1"}), "1.0.0--1"}},
+ {"1.0.0-----1", {Version(1, 0, 0, {"----1"}), "1.0.0-----1"}},
+ {"1-2-3+4-5",
+ {Version(1, 0, 0, {"2-3"}, {"4-5"}), "1.0.0-2-3+4-5"}},
+ {"1-2-3.4+5.6-7",
+ {Version(1, 0, 0, {"2-3", "4"}, {"5", "6-7"}), "1.0.0-2-3.4+5.6-7"}},
+ {"1-2.-3+4.-5",
+ {Version(1, 0, 0, {"2", "-3"}, {"4", "-5"}), "1.0.0-2.-3+4.-5"}}
};
foreachpair (const string& input, const ExpectedValue& expected, testCases) {
@@ -87,8 +150,29 @@ TEST(VersionTest, ParseInvalid)
"a",
"1.",
".1.2",
+ "0.1.-2",
+ "0.-1.2",
"0.1.2.3",
- "-1.1.2"
+ "-1.1.2",
+ "01.2.3",
+ "1.02.3",
+ "1.2.03",
+ "1.1.2-",
+ "1.1.2+",
+ "1.1.2-+",
+ "1.1.2-.",
+ "1.1.2+.",
+ "1.1.2-foo..",
+ "1.1.2-.foo",
+ "1.1.2+",
+ "1.1.2+foo..",
+ "1.1.2+.foo",
+ "1.1.2-al^pha",
+ "1.1.2+exp;",
+ "1.1.2-alpha.001",
+ "-foo",
+ "+foo",
+ u8"1.0.0-b\u00e9ta"
};
foreach (const string& input, inputs) {
[3/4] mesos git commit: Checked validity of master and agent version
numbers on startup.
Posted by ne...@apache.org.
Checked validity of master and agent version numbers on startup.
During startup, we now check that the compiled-in version number of the
master and agent can be parsed by stout's `Version::parse` (i.e., that
`MESOS_VERSION` is valid according to stout's extended variant of the
Semver 2.0.0 format).
Review: https://reviews.apache.org/r/58708
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/ecc73219
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/ecc73219
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/ecc73219
Branch: refs/heads/master
Commit: ecc73219d3ae29993b1b6d7429c51ce7713b801b
Parents: 59180f9
Author: Neil Conway <ne...@gmail.com>
Authored: Mon Mar 6 10:55:07 2017 -0800
Committer: Neil Conway <ne...@gmail.com>
Committed: Wed May 3 15:37:48 2017 -0700
----------------------------------------------------------------------
src/master/main.cpp | 11 +++++++++++
src/slave/main.cpp | 11 +++++++++++
2 files changed, 22 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/ecc73219/src/master/main.cpp
----------------------------------------------------------------------
diff --git a/src/master/main.cpp b/src/master/main.cpp
index 462ed32..06c735e 100644
--- a/src/master/main.cpp
+++ b/src/master/main.cpp
@@ -57,6 +57,7 @@
#include <stout/stringify.hpp>
#include <stout/strings.hpp>
#include <stout/try.hpp>
+#include <stout/version.hpp>
#include "common/build.hpp"
#include "common/http.hpp"
@@ -175,6 +176,16 @@ int main(int argc, char** argv)
return EXIT_FAILURE;
}
+ // Check that master's version has the expected format (SemVer).
+ {
+ Try<Version> version = Version::parse(MESOS_VERSION);
+ if (version.isError()) {
+ EXIT(EXIT_FAILURE)
+ << "Failed to parse Mesos version '" << MESOS_VERSION << "': "
+ << version.error();
+ }
+ }
+
if (flags.ip_discovery_command.isSome() && flags.ip.isSome()) {
EXIT(EXIT_FAILURE) << flags.usage(
"Only one of `--ip` or `--ip_discovery_command` should be specified");
http://git-wip-us.apache.org/repos/asf/mesos/blob/ecc73219/src/slave/main.cpp
----------------------------------------------------------------------
diff --git a/src/slave/main.cpp b/src/slave/main.cpp
index 72b141c..507d599 100644
--- a/src/slave/main.cpp
+++ b/src/slave/main.cpp
@@ -45,6 +45,7 @@
#include <stout/stringify.hpp>
#include <stout/try.hpp>
+#include <stout/version.hpp>
#include "common/build.hpp"
#include "common/http.hpp"
@@ -267,6 +268,16 @@ int main(int argc, char** argv)
return EXIT_FAILURE;
}
+ // Check that agent's version has the expected format (SemVer).
+ {
+ Try<Version> version = Version::parse(MESOS_VERSION);
+ if (version.isError()) {
+ EXIT(EXIT_FAILURE)
+ << "Failed to parse Mesos version '" << MESOS_VERSION << "': "
+ << version.error();
+ }
+ }
+
if (flags.master.isNone() && flags.master_detector.isNone()) {
cerr << flags.usage("Missing required option `--master` or "
"`--master_detector`.") << endl;
[2/4] mesos git commit: Switched to using unsigned types to represent
versions in stout.
Posted by ne...@apache.org.
Switched to using unsigned types to represent versions in stout.
Review: https://reviews.apache.org/r/58971
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/59180f9f
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/59180f9f
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/59180f9f
Branch: refs/heads/master
Commit: 59180f9fc0aa8bbb6cb9ec73a52e529e2c091bfa
Parents: 2875562
Author: Neil Conway <ne...@gmail.com>
Authored: Wed May 3 11:10:39 2017 -0700
Committer: Neil Conway <ne...@gmail.com>
Committed: Wed May 3 15:37:41 2017 -0700
----------------------------------------------------------------------
3rdparty/stout/include/stout/version.hpp | 34 ++++++++++++++-------------
1 file changed, 18 insertions(+), 16 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/59180f9f/3rdparty/stout/include/stout/version.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/version.hpp b/3rdparty/stout/include/stout/version.hpp
index 5e2bd2e..9120e42 100644
--- a/3rdparty/stout/include/stout/version.hpp
+++ b/3rdparty/stout/include/stout/version.hpp
@@ -15,6 +15,7 @@
#include <algorithm>
#include <cctype>
+#include <cstdint>
#include <ostream>
#include <string>
#include <vector>
@@ -111,10 +112,10 @@ struct Version
" components allowed");
}
- int versionNumbers[maxNumericComponents] = {0};
+ uint32_t versionNumbers[maxNumericComponents] = {0};
for (size_t i = 0; i < numericComponents.size(); i++) {
- Try<int> result = parseNumericIdentifier(numericComponents[i]);
+ Try<uint32_t> result = parseNumericIdentifier(numericComponents[i]);
if (result.isError()) {
return Error("Invalid version component '" + numericComponents[i] + "'"
": " + result.error());
@@ -137,9 +138,9 @@ struct Version
// Construct a new Version. The `_prerelease` and `_build` arguments
// contain lists of prerelease and build identifiers, respectively.
- Version(int _majorVersion,
- int _minorVersion,
- int _patchVersion,
+ Version(uint32_t _majorVersion,
+ uint32_t _minorVersion,
+ uint32_t _patchVersion,
const std::vector<std::string>& _prerelease = {},
const std::vector<std::string>& _build = {})
: majorVersion(_majorVersion),
@@ -230,8 +231,9 @@ struct Version
for (size_t i = 0; i < minPrereleaseSize; i++) {
// Check whether the two prerelease identifiers can be converted
// to numbers.
- Try<int> identifier = parseNumericIdentifier(prerelease.at(i));
- Try<int> otherIdentifier = parseNumericIdentifier(other.prerelease.at(i));
+ Try<uint32_t> identifier = parseNumericIdentifier(prerelease.at(i));
+ Try<uint32_t> otherIdentifier =
+ parseNumericIdentifier(other.prerelease.at(i));
if (identifier.isSome() && otherIdentifier.isSome()) {
// Both identifiers are numeric.
@@ -279,9 +281,9 @@ struct Version
std::ostream& stream,
const Version& version);
- const int majorVersion;
- const int minorVersion;
- const int patchVersion;
+ const uint32_t majorVersion;
+ const uint32_t minorVersion;
+ const uint32_t patchVersion;
const std::vector<std::string> prerelease;
const std::vector<std::string> build;
@@ -346,25 +348,25 @@ private:
// Checks whether the input string is numeric and contains a leading
// zero. Note that "0" by itself is not considered a "leading zero".
static bool hasLeadingZero(const std::string& identifier) {
- Try<int> numericIdentifier = parseNumericIdentifier(identifier);
+ Try<uint32_t> numericIdentifier = parseNumericIdentifier(identifier);
return numericIdentifier.isSome() &&
numericIdentifier.get() != 0 &&
strings::startsWith(identifier, '0');
}
- // Attempt to parse the given string as a numeric identifier.
- // According to the SemVer spec, identifiers that begin with hyphens
- // are considered non-numeric.
+ // Try to parse the given string as a numeric identifier. According
+ // to the SemVer spec, identifiers that begin with hyphens are
+ // considered non-numeric.
//
// TODO(neilc): Consider adding a variant of `numify<T>` that only
// supports non-negative inputs.
- static Try<int> parseNumericIdentifier(const std::string& identifier) {
+ static Try<uint32_t> parseNumericIdentifier(const std::string& identifier) {
if (strings::startsWith(identifier, '-')) {
return Error("Contains leading hyphen");
}
- return numify<int>(identifier);
+ return numify<uint32_t>(identifier);
}
};
[4/4] mesos git commit: Prevent old Mesos agents from registering or
re-registering.
Posted by ne...@apache.org.
Prevent old Mesos agents from registering or re-registering.
Officially, Mesos 1.0.0 (and newer) masters do not support pre-1.0.0
Mesos agents. However, we previously allowed old agents to register,
which resulted in several master crashes. As a short-term solution, we
fixed those crashes by adding backward compatibility mechanisms into the
master, but that backward compatibility code has made the master logic
more complicated and difficult to understand.
This commit changes the master to ignore registration attempts by Mesos
agents that precede Mesos 1.0.0. Now that this safety check is in place,
master logic can safely assume that all agents are running at least
Mesos 1.0.0, which will allow several simplifications to be made.
Review: https://reviews.apache.org/r/58709
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/08808739
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/08808739
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/08808739
Branch: refs/heads/master
Commit: 08808739753447059c1585379dc51637d50fea35
Parents: ecc7321
Author: Neil Conway <ne...@gmail.com>
Authored: Mon Mar 6 10:10:45 2017 -0800
Committer: Neil Conway <ne...@gmail.com>
Committed: Wed May 3 15:38:14 2017 -0700
----------------------------------------------------------------------
src/master/constants.hpp | 4 ++
src/master/master.cpp | 36 +++++++++++++++++
src/tests/master_tests.cpp | 86 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 126 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/08808739/src/master/constants.hpp
----------------------------------------------------------------------
diff --git a/src/master/constants.hpp b/src/master/constants.hpp
index 7edf9f6..725680b 100644
--- a/src/master/constants.hpp
+++ b/src/master/constants.hpp
@@ -21,6 +21,7 @@
#include <stout/bytes.hpp>
#include <stout/duration.hpp>
+#include <stout/version.hpp>
namespace mesos {
namespace internal {
@@ -145,6 +146,9 @@ constexpr char READWRITE_HTTP_AUTHENTICATION_REALM[] =
constexpr char DEFAULT_HTTP_FRAMEWORK_AUTHENTICATION_REALM[] =
"mesos-master-scheduler";
+// Agents older than this version are not allowed to register.
+const Version MINIMUM_AGENT_VERSION = Version(1, 0, 0);
+
} // namespace master {
} // namespace internal {
} // namespace mesos {
http://git-wip-us.apache.org/repos/asf/mesos/blob/08808739/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 87c647b..4e7a161 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -5505,6 +5505,24 @@ void Master::_registerSlave(
return;
}
+ // Ignore registration attempts by agents running old Mesos versions.
+ // We expect that the agent's version is in SemVer format; if the
+ // version cannot be parsed, the registration attempt is ignored.
+ Try<Version> parsedVersion = Version::parse(version);
+
+ if (parsedVersion.isError()) {
+ LOG(WARNING) << "Failed to parse version '" << version << "'"
+ << " of agent at " << pid << ": " << parsedVersion.error()
+ << "; ignoring agent registration attempt";
+ return;
+ } else if (parsedVersion.get() < MINIMUM_AGENT_VERSION) {
+ LOG(WARNING) << "Ignoring registration attempt from old agent at "
+ << pid << ": agent version is " << parsedVersion.get()
+ << ", minimum supported agent version is "
+ << MINIMUM_AGENT_VERSION;
+ return;
+ }
+
// Check if this slave is already registered (because it retries).
if (Slave* slave = slaves.registered.get(pid)) {
if (!slave->connected) {
@@ -5775,6 +5793,24 @@ void Master::_reregisterSlave(
return;
}
+ // Ignore re-registration attempts by agents running old Mesos versions.
+ // We expect that the agent's version is in SemVer format; if the
+ // version cannot be parsed, the re-registration attempt is ignored.
+ Try<Version> parsedVersion = Version::parse(version);
+
+ if (parsedVersion.isError()) {
+ LOG(WARNING) << "Failed to parse version '" << version << "'"
+ << " of agent at " << pid << ": " << parsedVersion.error()
+ << "; ignoring agent re-registration attempt";
+ return;
+ } else if (parsedVersion.get() < MINIMUM_AGENT_VERSION) {
+ LOG(WARNING) << "Ignoring re-registration attempt from old agent at "
+ << pid << ": agent version is " << parsedVersion.get()
+ << ", minimum supported agent version is "
+ << MINIMUM_AGENT_VERSION;
+ return;
+ }
+
if (Slave* slave = slaves.registered.get(slaveInfo.id())) {
CHECK(!slaves.recovered.contains(slaveInfo.id()));
http://git-wip-us.apache.org/repos/asf/mesos/blob/08808739/src/tests/master_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_tests.cpp b/src/tests/master_tests.cpp
index 7cb4774..ceee2f4 100644
--- a/src/tests/master_tests.cpp
+++ b/src/tests/master_tests.cpp
@@ -7012,6 +7012,92 @@ TEST_F(MasterTest, MultiRoleSchedulerUnsubscribeFromRole)
driver2.join();
}
+
+// Check that the master does not allow old Mesos agents to register.
+// We do this by intercepting the agent's `RegisterSlaveMessage` and
+// then re-sending it with a tweaked version number.
+TEST_F(MasterTest, IgnoreOldAgentRegistration)
+{
+ Try<Owned<cluster::Master>> master = StartMaster();
+ ASSERT_SOME(master);
+
+ Future<RegisterSlaveMessage> registerSlaveMessage =
+ DROP_PROTOBUF(RegisterSlaveMessage(), _, _);
+
+ Clock::pause();
+
+ slave::Flags slaveFlags = CreateSlaveFlags();
+ Owned<MasterDetector> detector = master.get()->createDetector();
+ Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(), slaveFlags);
+ ASSERT_SOME(slave);
+
+ Clock::advance(slaveFlags.authentication_backoff_factor);
+ Clock::advance(slaveFlags.registration_backoff_factor);
+
+ AWAIT_READY(registerSlaveMessage);
+
+ RegisterSlaveMessage message = registerSlaveMessage.get();
+ message.set_version("0.28.1-rc1");
+
+ // The master should ignore the agent's registration attempt. Hence,
+ // we do not expect the master to try to update the registry.
+ EXPECT_CALL(*master.get()->registrar.get(), apply(_))
+ .Times(0);
+
+ process::post(slave.get()->pid, master.get()->pid, message);
+
+ // Settle the clock to retire in-flight messages.
+ Clock::settle();
+}
+
+
+// Check that the master does not allow old Mesos agents to re-register.
+// We do this by intercepting the agent's `ReregisterSlaveMessage` and
+// then re-sending it with a tweaked version number.
+TEST_F(MasterTest, IgnoreOldAgentReregistration)
+{
+ Try<Owned<cluster::Master>> master = StartMaster();
+ ASSERT_SOME(master);
+
+ Future<SlaveRegisteredMessage> slaveRegisteredMessage =
+ FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, _);
+
+ StandaloneMasterDetector detector(master.get()->pid);
+ slave::Flags slaveFlags = CreateSlaveFlags();
+ Try<Owned<cluster::Slave>> slave = StartSlave(&detector, slaveFlags);
+ ASSERT_SOME(slave);
+
+ AWAIT_READY(slaveRegisteredMessage);
+
+ // Intercept the agent's `ReregisterSlaveMessage`.
+ Future<ReregisterSlaveMessage> reregisterSlaveMessage =
+ DROP_PROTOBUF(ReregisterSlaveMessage(), _, _);
+
+ Clock::pause();
+
+ // Simulate a new master detected event on the slave,
+ // so that the slave will attempt to re-register.
+ detector.appoint(master.get()->pid);
+
+ Clock::advance(slaveFlags.authentication_backoff_factor);
+ Clock::advance(slaveFlags.registration_backoff_factor);
+
+ AWAIT_READY(reregisterSlaveMessage);
+
+ ReregisterSlaveMessage message = reregisterSlaveMessage.get();
+ message.set_version("0.28.1-rc1");
+
+ // The master should ignore the agent's re-registration attempt, so
+ // we do not expect the master to try to update the registry.
+ EXPECT_CALL(*master.get()->registrar.get(), apply(_))
+ .Times(0);
+
+ process::post(slave.get()->pid, master.get()->pid, message);
+
+ // Settle the clock to retire in-flight messages.
+ Clock::settle();
+}
+
} // namespace tests {
} // namespace internal {
} // namespace mesos {