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/05 23:54:43 UTC

[1/6] mesos git commit: Improved unit tests for Version in stout.

Repository: mesos
Updated Branches:
  refs/heads/1.2.x 854712fbd -> 7da4d5293


Improved unit tests for Version in stout.

Switch to table-based test cases, validate that version parsing produces
the expected results more precisely.

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


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

Branch: refs/heads/1.2.x
Commit: 15b8ee256f2d2d4a2a94f9b862cd2c63adeccd2b
Parents: 854712f
Author: Neil Conway <ne...@gmail.com>
Authored: Fri Apr 21 08:49:00 2017 -0700
Committer: Neil Conway <ne...@gmail.com>
Committed: Fri May 5 15:13:46 2017 -0700

----------------------------------------------------------------------
 3rdparty/stout/tests/version_tests.cpp | 83 +++++++++++++++++++++--------
 1 file changed, 60 insertions(+), 23 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/15b8ee25/3rdparty/stout/tests/version_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/tests/version_tests.cpp b/3rdparty/stout/tests/version_tests.cpp
index 724ed22..925f383 100644
--- a/3rdparty/stout/tests/version_tests.cpp
+++ b/3rdparty/stout/tests/version_tests.cpp
@@ -10,12 +10,22 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include <map>
+#include <string>
+#include <utility>
+#include <vector>
+
 #include <gtest/gtest.h>
 
+#include <stout/foreach.hpp>
 #include <stout/gtest.hpp>
 #include <stout/stringify.hpp>
 #include <stout/version.hpp>
 
+using std::map;
+using std::pair;
+using std::string;
+using std::vector;
 
 // Verify version comparison operations.
 TEST(VersionTest, Comparison)
@@ -37,28 +47,55 @@ TEST(VersionTest, Comparison)
 }
 
 
-// Verify version parser.
-TEST(VersionTest, Parse)
+// Verify that valid version strings are parsed successfully.
+TEST(VersionTest, ParseValid)
+{
+  // Each test case consists of an input value and a corresponding
+  // expected value: the `Version` that corresponds to the input, and
+  // the result of `operator<<` for that Version.
+
+  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"}}
+  };
+
+  foreachpair (const string& input, const ExpectedValue& expected, testCases) {
+    Try<Version> actual = Version::parse(input);
+
+    ASSERT_SOME(actual)
+      << "Error parsing input '" << input << "'";
+
+    EXPECT_EQ(std::get<0>(expected), actual.get())
+      << "Incorrect parse of input '" << input << "'";
+    EXPECT_EQ(std::get<1>(expected), stringify(actual.get()))
+      << "Unexpected stringify output for input '" << input << "'";
+  }
+}
+
+
+// Verify that invalid version strings result in a parse error.
+TEST(VersionTest, ParseInvalid)
 {
-  Try<Version> version1 = Version::parse("1.20.3");
-  Try<Version> version2 = Version::parse("1.20");
-  Try<Version> version3 = Version::parse("1");
-
-  EXPECT_GT(version1.get(), version2.get());
-  EXPECT_GT(version2.get(), version3.get());
-  EXPECT_EQ(stringify(version2.get()), "1.20.0");
-  EXPECT_EQ(stringify(version3.get()), "1.0.0");
-
-  // Verify that tagged/labeled versions work.
-  Try<Version> version4 = Version::parse("1.20.3-rc1");
-  EXPECT_SOME(version4);
-  EXPECT_EQ(version4.get(), version1.get());
-  EXPECT_EQ(stringify(version4.get()), "1.20.3");
-
-  EXPECT_ERROR(Version::parse("0.a.b"));
-  EXPECT_ERROR(Version::parse(""));
-  EXPECT_ERROR(Version::parse("a"));
-  EXPECT_ERROR(Version::parse("1."));
-  EXPECT_ERROR(Version::parse(".1.2"));
-  EXPECT_ERROR(Version::parse("0.1.2.3"));
+  const vector<string> inputs = {
+    "",
+    "0.a.b",
+    "a",
+    "1.",
+    ".1.2",
+    "0.1.2.3",
+    "-1.1.2"
+  };
+
+  foreach (const string& input, inputs) {
+    Try<Version> parse = Version::parse(input);
+
+    EXPECT_ERROR(parse)
+      << "Expected error on input '" << input
+      << "'; got " << parse.get();
+  }
 }


[6/6] mesos git commit: Added MESOS-1987, MESOS-6976, MESOS-7389 to 1.2.1 CHANGELOG.

Posted by ne...@apache.org.
Added MESOS-1987, MESOS-6976, MESOS-7389 to 1.2.1 CHANGELOG.


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

Branch: refs/heads/1.2.x
Commit: 7da4d52933ac339c15f241a85fcad6b35ffbc7fa
Parents: cf98293
Author: Neil Conway <ne...@gmail.com>
Authored: Fri May 5 16:53:11 2017 -0700
Committer: Neil Conway <ne...@gmail.com>
Committed: Fri May 5 16:53:11 2017 -0700

----------------------------------------------------------------------
 CHANGELOG | 3 +++
 1 file changed, 3 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/7da4d529/CHANGELOG
----------------------------------------------------------------------
diff --git a/CHANGELOG b/CHANGELOG
index 1b53ee0..12dec29 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -4,8 +4,10 @@ Release Notes - Mesos - Version 1.2.1 (WIP)
 
 All Issues:
 ** Bug
+  * [MESOS-1987] - Add support for SemVer build and prerelease labels to stout.
   * [MESOS-5028] - Copy provisioner cannot replace directory with symlink.
   * [MESOS-5172] - Registry puller cannot fetch blobs correctly from http Redirect 3xx urls.
+  * [MESOS-6976] - Disallow (re-)registration attempts by old agents.
   * [MESOS-6951] - Docker containerizer: mangled environment when env value contains LF byte.
   * [MESOS-7197] - Requesting tiny amount of CPU crashes master.
   * [MESOS-7208] - Persistent volume ownership is set to root when task is running with non-root user
@@ -20,6 +22,7 @@ All Issues:
   * [MESOS-7383] - Docker executor logs possibly sensitive parameters.
   * [MESOS-7350] - Failed to pull image from Nexus Registry due to signature missing.
   * [MESOS-7346] - Agent crashes if the task name is too long.
+  * [MESOS-7389] - Mesos 1.2.0 crashes with pre-1.0 Mesos agents.
   * [MESOS-7400] - The mesos crashes due to an incorrect invariant check in the decoder.
   * [MESOS-7427] - Registry puller cannot fetch manifests from Amazon ECR: 405 Unsupported.
   * [MESOS-7453] - glyphicons-halflings-regular.woff2 is missing in WebUI.


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

Posted by ne...@apache.org.
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/405891d2
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/405891d2
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/405891d2

Branch: refs/heads/1.2.x
Commit: 405891d207482c860ff020d2cceba136c91aaf6c
Parents: 15b8ee2
Author: Neil Conway <ne...@gmail.com>
Authored: Fri Apr 21 09:03:10 2017 -0700
Committer: Neil Conway <ne...@gmail.com>
Committed: Fri May 5 15:14:02 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/405891d2/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/405891d2/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/6] 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/c56851ef
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/c56851ef
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/c56851ef

Branch: refs/heads/1.2.x
Commit: c56851ef10ea35c77a34106cd85fe2748bf8bdf4
Parents: 405891d
Author: Neil Conway <ne...@gmail.com>
Authored: Wed May 3 11:10:39 2017 -0700
Committer: Neil Conway <ne...@gmail.com>
Committed: Fri May 5 15:14:10 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/c56851ef/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/6] 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/cf98293f
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/cf98293f
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/cf98293f

Branch: refs/heads/1.2.x
Commit: cf98293f6cf27ead01ebcc353f1c31fc673a149d
Parents: 5a5dd8a
Author: Neil Conway <ne...@gmail.com>
Authored: Mon Mar 6 10:10:45 2017 -0800
Committer: Neil Conway <ne...@gmail.com>
Committed: Fri May 5 16:34: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/cf98293f/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/cf98293f/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 205e52a..4742d21 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -5326,6 +5326,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 " << from << ": " << parsedVersion.error()
+                 << "; ignoring agent registration attempt";
+    return;
+  } else if (parsedVersion.get() < MINIMUM_AGENT_VERSION) {
+    LOG(WARNING) << "Ignoring registration attempt from old agent at "
+                 << from << ": 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 (slaves.registered.contains(from)) {
     Slave* slave = slaves.registered.get(from);
@@ -5516,6 +5534,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 " << from << ": " << 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 "
+                 << from << ": agent version is " << parsedVersion.get()
+                 << ", minimum supported agent version is "
+                 << MINIMUM_AGENT_VERSION;
+    return;
+  }
+
   Slave* slave = slaves.registered.get(slaveInfo.id());
 
   if (slave != nullptr) {

http://git-wip-us.apache.org/repos/asf/mesos/blob/cf98293f/src/tests/master_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_tests.cpp b/src/tests/master_tests.cpp
index 23d81e6..d812bb3 100644
--- a/src/tests/master_tests.cpp
+++ b/src/tests/master_tests.cpp
@@ -6372,6 +6372,92 @@ TEST_F(MasterTest, TaskWithTinyResources)
   driver.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 {


Re: [4/6] mesos git commit: Checked validity of master and agent version numbers on startup.

Posted by Neil Conway <ne...@gmail.com>.
There was an earlier thread on whether to ignore registration attempts
from pre-1.0 Mesos agents:

https://lists.apache.org/thread.html/f5e15f6d7a3f3b08d29e27455e2e1c801775418a148dded953c568e7@%3Cdev.mesos.apache.org%3E

We didn't explicitly discuss what to do when the compiled-in version
of Mesos is not parseable as SemVer. I'll start a separate thread to
let users know about that change.

Neil


On Mon, May 8, 2017 at 10:25 AM, Vinod Kone <vi...@apache.org> wrote:
> @Neil: Have we sent an email about this change to dev list? This might
> break people who were directly building off source and using a custom
> version number.
>
> On Fri, May 5, 2017 at 4:54 PM, <ne...@apache.org> wrote:
>
>> 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/5a5dd8a4
>> Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/5a5dd8a4
>> Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/5a5dd8a4
>>
>> Branch: refs/heads/1.2.x
>> Commit: 5a5dd8a4edcb52e2227e2a4607b95a7dcc6aa321
>> Parents: c56851e
>> Author: Neil Conway <ne...@gmail.com>
>> Authored: Mon Mar 6 10:55:07 2017 -0800
>> Committer: Neil Conway <ne...@gmail.com>
>> Committed: Fri May 5 15:16:38 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/5a5dd8a4/
>> src/master/main.cpp
>> ----------------------------------------------------------------------
>> diff --git a/src/master/main.cpp b/src/master/main.cpp
>> index da75fe9..d485a06 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/5a5dd8a4/
>> src/slave/main.cpp
>> ----------------------------------------------------------------------
>> diff --git a/src/slave/main.cpp b/src/slave/main.cpp
>> index 31f2b4f..f90aa2f 100644
>> --- a/src/slave/main.cpp
>> +++ b/src/slave/main.cpp
>> @@ -39,6 +39,7 @@
>>  #include <stout/os.hpp>
>>  #include <stout/stringify.hpp>
>>  #include <stout/try.hpp>
>> +#include <stout/version.hpp>
>>
>>  #include "common/build.hpp"
>>  #include "common/http.hpp"
>> @@ -142,6 +143,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;
>>
>>

Re: [4/6] mesos git commit: Checked validity of master and agent version numbers on startup.

Posted by Vinod Kone <vi...@apache.org>.
@Neil: Have we sent an email about this change to dev list? This might
break people who were directly building off source and using a custom
version number.

On Fri, May 5, 2017 at 4:54 PM, <ne...@apache.org> wrote:

> 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/5a5dd8a4
> Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/5a5dd8a4
> Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/5a5dd8a4
>
> Branch: refs/heads/1.2.x
> Commit: 5a5dd8a4edcb52e2227e2a4607b95a7dcc6aa321
> Parents: c56851e
> Author: Neil Conway <ne...@gmail.com>
> Authored: Mon Mar 6 10:55:07 2017 -0800
> Committer: Neil Conway <ne...@gmail.com>
> Committed: Fri May 5 15:16:38 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/5a5dd8a4/
> src/master/main.cpp
> ----------------------------------------------------------------------
> diff --git a/src/master/main.cpp b/src/master/main.cpp
> index da75fe9..d485a06 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/5a5dd8a4/
> src/slave/main.cpp
> ----------------------------------------------------------------------
> diff --git a/src/slave/main.cpp b/src/slave/main.cpp
> index 31f2b4f..f90aa2f 100644
> --- a/src/slave/main.cpp
> +++ b/src/slave/main.cpp
> @@ -39,6 +39,7 @@
>  #include <stout/os.hpp>
>  #include <stout/stringify.hpp>
>  #include <stout/try.hpp>
> +#include <stout/version.hpp>
>
>  #include "common/build.hpp"
>  #include "common/http.hpp"
> @@ -142,6 +143,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;
>
>

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

Branch: refs/heads/1.2.x
Commit: 5a5dd8a4edcb52e2227e2a4607b95a7dcc6aa321
Parents: c56851e
Author: Neil Conway <ne...@gmail.com>
Authored: Mon Mar 6 10:55:07 2017 -0800
Committer: Neil Conway <ne...@gmail.com>
Committed: Fri May 5 15:16:38 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/5a5dd8a4/src/master/main.cpp
----------------------------------------------------------------------
diff --git a/src/master/main.cpp b/src/master/main.cpp
index da75fe9..d485a06 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/5a5dd8a4/src/slave/main.cpp
----------------------------------------------------------------------
diff --git a/src/slave/main.cpp b/src/slave/main.cpp
index 31f2b4f..f90aa2f 100644
--- a/src/slave/main.cpp
+++ b/src/slave/main.cpp
@@ -39,6 +39,7 @@
 #include <stout/os.hpp>
 #include <stout/stringify.hpp>
 #include <stout/try.hpp>
+#include <stout/version.hpp>
 
 #include "common/build.hpp"
 #include "common/http.hpp"
@@ -142,6 +143,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;