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/10 04:58:25 UTC

[1/8] mesos git commit: Enhanced stout's Version to support prerelease and build labels.

Repository: mesos
Updated Branches:
  refs/heads/1.3.x 95ec15322 -> 2157f624e


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/39d89327
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/39d89327
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/39d89327

Branch: refs/heads/1.3.x
Commit: 39d89327675f465b7267b947734141712d00e528
Parents: 95ec153
Author: Neil Conway <ne...@gmail.com>
Authored: Fri Apr 21 09:03:10 2017 -0700
Committer: Neil Conway <ne...@gmail.com>
Committed: Tue May 9 21:28:16 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/39d89327/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/39d89327/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) {


[7/8] mesos git commit: Cleaned up comments in stout's Version.

Posted by ne...@apache.org.
Cleaned up comments in stout's Version.

Review: https://reviews.apache.org/r/59122


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/f63d1157
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/f63d1157
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/f63d1157

Branch: refs/heads/1.3.x
Commit: f63d1157b4f0ae786f1cd9ffab9a8a9274f3eced
Parents: d43d745
Author: Neil Conway <ne...@gmail.com>
Authored: Tue May 9 17:52:20 2017 -0700
Committer: Neil Conway <ne...@gmail.com>
Committed: Tue May 9 21:29:33 2017 -0700

----------------------------------------------------------------------
 3rdparty/stout/include/stout/version.hpp | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/f63d1157/3rdparty/stout/include/stout/version.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/version.hpp b/3rdparty/stout/include/stout/version.hpp
index bff6e4c..b396cad 100644
--- a/3rdparty/stout/include/stout/version.hpp
+++ b/3rdparty/stout/include/stout/version.hpp
@@ -38,11 +38,6 @@
 //
 // TODO(neilc): Consider providing a "strict" variant that does not
 // allow these extensions.
-//
-// 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:
@@ -55,6 +50,11 @@ struct Version
   // consist of one or more dot-separated identifiers. An identifier
   // is a non-empty string containing ASCII alphanumeric characters or
   // hyphens.
+  //
+  // 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.
   static Try<Version> parse(const std::string& input)
   {
     // The input string consists of the numeric components, optionally


[3/8] 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/2ccaf993
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/2ccaf993
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/2ccaf993

Branch: refs/heads/1.3.x
Commit: 2ccaf9936f725801a70e0ca8026de0f68f962c45
Parents: b550a8f
Author: Neil Conway <ne...@gmail.com>
Authored: Mon Mar 6 10:55:07 2017 -0800
Committer: Neil Conway <ne...@gmail.com>
Committed: Tue May 9 21:28:35 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/2ccaf993/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/2ccaf993/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;


[6/8] mesos git commit: Allowed leading zeros in input to stout's Version parser.

Posted by ne...@apache.org.
Allowed leading zeros in input to stout's Version parser.

The original behavior was to allow leading zeros. When Version was
enhanced with more complete support for SemVer [1], it was also changed
to reject leading zeros in numeric components (version numbers and
prerelease identifiers). Although this change was consistent with the
SemVer spec (which mandates that such version strings "MUST" be
considered invalid), it breaks compatibility with the versions used by
some common software packages (e.g., Docker).

This commit reverts the change in behavior, so that leading zeros are
once again allowed. In the future, we might consider adding a "strict"
mode to the Version parser, and/or supporting an "options" scheme that
would allow the user to customize the version parser's behavior.

[1] 287556284d76e03c11cff3fc076224fe966096e0

Review: https://reviews.apache.org/r/59105/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/d43d7453
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/d43d7453
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/d43d7453

Branch: refs/heads/1.3.x
Commit: d43d745353eaf9f27cd11956cf578b95fac2c3dd
Parents: 1af90df
Author: Neil Conway <ne...@gmail.com>
Authored: Tue May 9 17:50:11 2017 -0700
Committer: Neil Conway <ne...@gmail.com>
Committed: Tue May 9 21:29:26 2017 -0700

----------------------------------------------------------------------
 3rdparty/stout/include/stout/version.hpp | 63 ++++++++-------------------
 3rdparty/stout/tests/version_tests.cpp   | 14 +++---
 2 files changed, 26 insertions(+), 51 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/d43d7453/3rdparty/stout/include/stout/version.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/version.hpp b/3rdparty/stout/include/stout/version.hpp
index 4be7222..bff6e4c 100644
--- a/3rdparty/stout/include/stout/version.hpp
+++ b/3rdparty/stout/include/stout/version.hpp
@@ -30,7 +30,14 @@
 
 // This class provides convenience routines for working with version
 // numbers.  We support the SemVer 2.0.0 (http://semver.org) format,
-// with minor extensions.
+// with two differences:
+//
+//   (1) Numeric components with leading zeros are allowed.
+//
+//   (2) Missing version components are allowed and treated as zero.
+//
+// TODO(neilc): Consider providing a "strict" variant that does not
+// allow these extensions.
 //
 // Ideally, the version components would be called simply "major",
 // "minor", and "patch". However, GNU libstdc++ already defines these
@@ -42,11 +49,6 @@ struct Version
   //   <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`
@@ -69,9 +71,7 @@ struct Version
     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);
+      Try<std::vector<std::string>> parsed = parseLabel(buildString);
       if (parsed.isError()) {
         return Error("Invalid build label: " + parsed.error());
       }
@@ -92,8 +92,7 @@ struct Version
     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);
+      Try<std::vector<std::string>> parsed = parseLabel(prereleaseString);
       if (parsed.isError()) {
         return Error("Invalid prerelease label: " + parsed.error());
       }
@@ -121,11 +120,6 @@ struct Version
                      ": " + result.error());
       }
 
-      if (hasLeadingZero(numericComponents[i])) {
-        return Error("Invalid version component '" + numericComponents[i] + "'"
-                     ": cannot contain leading zero");
-      }
-
       versionNumbers[i] = result.get();
     }
 
@@ -153,11 +147,11 @@ struct Version
         // valid prerelease and build identifiers.
 
         foreach (const std::string& identifier, prerelease) {
-          CHECK_NONE(validateIdentifier(identifier, true));
+          CHECK_NONE(validateIdentifier(identifier));
         }
 
         foreach (const std::string& identifier, build) {
-          CHECK_NONE(validateIdentifier(identifier, false));
+          CHECK_NONE(validateIdentifier(identifier));
         }
       }
 
@@ -290,10 +284,9 @@ struct Version
 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)
+  // or hyphen. We allow leading zeros in numeric identifiers, which
+  // inconsistent with the SemVer spec.
+  static Option<Error> validateIdentifier(const std::string& identifier)
   {
     if (identifier.empty()) {
       return Error("Empty identifier");
@@ -311,23 +304,13 @@ private:
                    "'" + 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)
+  // single identifier.
+  static Try<std::vector<std::string>> parseLabel(const std::string& label)
   {
     if (label.empty()) {
       return Error("Empty label");
@@ -336,7 +319,7 @@ private:
     std::vector<std::string> identifiers = strings::split(label, ".");
 
     foreach (const std::string& identifier, identifiers) {
-      Option<Error> error = validateIdentifier(identifier, rejectLeadingZeros);
+      Option<Error> error = validateIdentifier(identifier);
       if (error.isSome()) {
         return error.get();
       }
@@ -345,16 +328,6 @@ private:
     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<uint32_t> numericIdentifier = parseNumericIdentifier(identifier);
-
-    return numericIdentifier.isSome() &&
-      numericIdentifier.get() != 0 &&
-      strings::startsWith(identifier, '0');
-  }
-
   // Try to parse the given string as a numeric identifier. According
   // to the SemVer spec, identifiers that begin with hyphens are
   // considered non-numeric.

http://git-wip-us.apache.org/repos/asf/mesos/blob/d43d7453/3rdparty/stout/tests/version_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/tests/version_tests.cpp b/3rdparty/stout/tests/version_tests.cpp
index bce185e..a532d0c 100644
--- a/3rdparty/stout/tests/version_tests.cpp
+++ b/3rdparty/stout/tests/version_tests.cpp
@@ -124,7 +124,13 @@ TEST(VersionTest, ParseValid)
     {"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"}}
+     {Version(1, 0, 0, {"2", "-3"}, {"4", "-5"}), "1.0.0-2.-3+4.-5"}},
+    // Allow leading zeros: in violation of the SemVer spec, but we
+    // tolerate it for compatibility with common practice (e.g., Docker).
+    {"01.2.3", {Version(1, 2, 3), "1.2.3"}},
+    {"1.02.3", {Version(1, 2, 3), "1.2.3"}},
+    {"1.2.03", {Version(1, 2, 3), "1.2.3"}},
+    {"1.2.3-alpha.001", {Version(1, 2, 3, {"alpha", "001"}), "1.2.3-alpha.001"}}
   };
 
   foreachpair (const string& input, const ExpectedValue& expected, testCases) {
@@ -152,11 +158,8 @@ TEST(VersionTest, ParseInvalid)
     ".1.2",
     "0.1.-2",
     "0.-1.2",
-    "0.1.2.3",
+    "1.2.3.4",
     "-1.1.2",
-    "01.2.3",
-    "1.02.3",
-    "1.2.03",
     "1.1.2-",
     "1.1.2+",
     "1.1.2-+",
@@ -169,7 +172,6 @@ TEST(VersionTest, ParseInvalid)
     "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"


[2/8] 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/b550a8fb
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/b550a8fb
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/b550a8fb

Branch: refs/heads/1.3.x
Commit: b550a8fbb7763fcab0845fd1503710f33167b5f6
Parents: 39d8932
Author: Neil Conway <ne...@gmail.com>
Authored: Wed May 3 11:10:39 2017 -0700
Committer: Neil Conway <ne...@gmail.com>
Committed: Tue May 9 21:28:26 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/b550a8fb/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);
   }
 };
 


[5/8] mesos git commit: Fixed `Version::validateIdentifier()` for Unicode edge case.

Posted by ne...@apache.org.
Fixed `Version::validateIdentifier()` for Unicode edge case.

The test case `u8"1.0.0-b\u00e9ta"` in `VersionTest.ParseInvalid` caused
an abort in `isctype.cpp` with VS 2017. Changing the `char` to an
`unsigned char` prevents this.

Review: https://reviews.apache.org/r/59110/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/1af90dfc
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/1af90dfc
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/1af90dfc

Branch: refs/heads/1.3.x
Commit: 1af90dfc954eaaea6bc927d60bf336315ec29e66
Parents: 347ad21
Author: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Authored: Tue May 9 14:06:01 2017 -0700
Committer: Neil Conway <ne...@gmail.com>
Committed: Tue May 9 21:29:16 2017 -0700

----------------------------------------------------------------------
 3rdparty/stout/include/stout/version.hpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/1af90dfc/3rdparty/stout/include/stout/version.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/version.hpp b/3rdparty/stout/include/stout/version.hpp
index 9120e42..4be7222 100644
--- a/3rdparty/stout/include/stout/version.hpp
+++ b/3rdparty/stout/include/stout/version.hpp
@@ -299,7 +299,7 @@ private:
       return Error("Empty identifier");
     }
 
-    auto alphaNumericOrHyphen = [](char c) -> bool {
+    auto alphaNumericOrHyphen = [](unsigned char c) -> bool {
       return std::isalnum(c) || c == '-';
     };
 


[4/8] 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/347ad21a
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/347ad21a
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/347ad21a

Branch: refs/heads/1.3.x
Commit: 347ad21aa07a9d1a40befca163689de36f4a0084
Parents: 2ccaf99
Author: Neil Conway <ne...@gmail.com>
Authored: Mon Mar 6 10:10:45 2017 -0800
Committer: Neil Conway <ne...@gmail.com>
Committed: Tue May 9 21:28:44 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/347ad21a/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/347ad21a/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 31a7a2f..e5005c6 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/347ad21a/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 {


[8/8] mesos git commit: Added MESOS-{1987, 6976, 7389, 7464, 7484} to 1.3.0 CHANGELOG.

Posted by ne...@apache.org.
Added MESOS-{1987,6976,7389,7464,7484} to 1.3.0 CHANGELOG.


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/2157f624
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/2157f624
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/2157f624

Branch: refs/heads/1.3.x
Commit: 2157f624ee0d01edab2fb3fe7e1081011cc31b96
Parents: f63d115
Author: Neil Conway <ne...@gmail.com>
Authored: Tue May 9 21:39:04 2017 -0700
Committer: Neil Conway <ne...@gmail.com>
Committed: Tue May 9 21:39:04 2017 -0700

----------------------------------------------------------------------
 CHANGELOG | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/2157f624/CHANGELOG
----------------------------------------------------------------------
diff --git a/CHANGELOG b/CHANGELOG
index 2656d01..f039c6c 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -97,7 +97,6 @@ Unresolved Critical Issues:
   * [MESOS-7378] - Build failure with missing gnu_dev_major and gnu_dev_minor symbols.
   * [MESOS-7381] - Flaky tests in NestedMesosContainerizerTest.
   * [MESOS-7386] - Executor not cleaning up existing running docker containers if external logrotate/logger processes die/killed.
-  * [MESOS-7389] - Mesos 1.2.0 crashes with pre-1.0 Mesos agents.
 
 Feature Graduations:
   * [MESOS-2449] - Support group of tasks (Pod) constructs and API in Mesos.
@@ -128,6 +127,7 @@ All Experimental Features:
 All Resolved Issues:
 
 ** Bug
+  * [MESOS-1987] - Add support for SemVer build and prerelease labels to stout.
   * [MESOS-4245] - Add `dist` target to CMake solution.
   * [MESOS-4263] - Report volume usage through ResourceStatistics.
   * [MESOS-5028] - Copy provisioner cannot replace directory with symlink.
@@ -151,6 +151,7 @@ All Resolved Issues:
   * [MESOS-6907] - FutureTest.After3 is flaky.
   * [MESOS-6951] - Docker containerizer: mangled environment when env value contains LF byte.
   * [MESOS-6953] - A compromised mesos-master node can execute code as root on agents.
+  * [MESOS-6976] - Disallow (re-)registration attempts by old agents.
   * [MESOS-6982] - PerfTest.Version fails on recent Arch Linux.
   * [MESOS-7022] - Update framework authorization to support multiple roles.
   * [MESOS-7029] - FaultToleranceTest.FrameworkReregister is flaky.
@@ -193,12 +194,15 @@ All Resolved Issues:
   * [MESOS-7363] - Improver master robustness against duplicate UPIDs.
   * [MESOS-7365] - Compile error with recent glibc.
   * [MESOS-7372] - Improve agent re-registration robustness.
+  * [MESOS-7389] - Mesos 1.2.0 crashes with pre-1.0 Mesos agents.
   * [MESOS-7400] - The mesos master crashes due to an incorrect invariant check in the decoder.
   * [MESOS-7427] - Registry puller cannot fetch manifests from Amazon ECR: 405 Unsupported.
   * [MESOS-7430] - Per-role Suppress call implementation is broken.
   * [MESOS-7453] - glyphicons-halflings-regular.woff2 is missing in WebUI.
   * [MESOS-7456] - Compilation error on recent glibc in cgroups device subsystem.
+  * [MESOS-7464] - Recent Docker versions cannot be parsed by stout.
   * [MESOS-7478] - Pre-1.2.x master does not work with 1.2.x agent.
+  * [MESOS-7484] - VersionTest.ParseInvalid aborts on Windows.
 
 ** Documentation
   * [MESOS-7005] - Add executor authentication documentation.