Repository: mesos
Updated Branches:
  refs/heads/master 7ff4920e1 -> 0966e6ec4


Always store validated and combined Resource objects in C++ Resources.

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


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

Branch: refs/heads/master
Commit: a8d0244b20313e3deb2d3b8483353555a8147ac3
Parents: 85728f8
Author: Jie Yu <[email protected]>
Authored: Fri Nov 14 14:45:05 2014 -0800
Committer: Jie Yu <[email protected]>
Committed: Wed Nov 19 00:14:25 2014 -0800

----------------------------------------------------------------------
 include/mesos/resources.hpp                     | 101 ++--
 src/common/resources.cpp                        | 536 ++++++++-----------
 src/examples/low_level_scheduler_libprocess.cpp |   2 +-
 src/examples/low_level_scheduler_pthread.cpp    |   2 +-
 src/examples/test_framework.cpp                 |   4 +-
 src/master/drf_sorter.cpp                       |  11 +-
 src/master/hierarchical_allocator_process.hpp   |   4 +-
 src/master/http.cpp                             |  10 +-
 src/master/master.cpp                           |  18 +-
 src/slave/containerizer/containerizer.cpp       |  16 +-
 src/tests/gc_tests.cpp                          |  20 +-
 src/tests/mesos.hpp                             |   5 +-
 src/tests/resource_offers_tests.cpp             |  10 +-
 src/tests/resources_tests.cpp                   | 157 +++---
 14 files changed, 414 insertions(+), 482 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/a8d0244b/include/mesos/resources.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/resources.hpp b/include/mesos/resources.hpp
index 86b70df..3009395 100644
--- a/include/mesos/resources.hpp
+++ b/include/mesos/resources.hpp
@@ -43,8 +43,13 @@
 
 namespace mesos {
 
-// TODO(bmahler): Ensure that the underlying resources are kept
-// in a flattened state: MESOS-1714.
+// NOTE: Resource objects stored in the class are always valid and
+// kept combined if possible. It is the caller's responsibility to
+// validate any Resource object or repeated Resource protobufs before
+// constructing a Resources object. Otherwise, invalid Resource
+// objects will be silently stripped. Invalid Resource objects will
+// also be silently ignored when used in arithmetic operations (e.g.,
+// +=, -=, etc.).
 class Resources
 {
 public:
@@ -62,49 +67,38 @@ public:
       const std::string& text,
       const std::string& defaultRole = "*");
 
-  // Returns true iff this resource has a name, a valid type, i.e.
-  // scalar, range, or set, and has the appropriate value set for its
-  // type.
-  static bool isValid(const Resource& resource);
+  // Validates the given Resource object. Returns Error if it is not
+  // valid. A Resource object is valid if it has a name, a valid type,
+  // i.e. scalar, range, or set, and has the appropriate value set.
+  static Option<Error> validate(const Resource& resource);
 
-  // Returns true iff this resource is valid and allocatable. In
-  // particular, a scalar is allocatable if it's value is greater than
-  // zero, a ranges is allocatable if there is at least one valid
-  // range in it, and a set is allocatable if it has at least one
-  // item.
-  static bool isAllocatable(const Resource& resource);
+  // Validates the given protobufs.
+  // TODO(jieyu): Right now, it's the same as checking each individual
+  // Resource object in the protobufs. In the future, we could add
+  // more checks that are not possible if checking each Resource
+  // object individually. For example, we could check multiple usage
+  // of an item in a set or a ranges, etc.
+  static Option<Error> validate(
+      const google::protobuf::RepeatedPtrField<Resource>& resources);
 
-  // Returns true iff this resource is zero valued, i.e. is zero for
-  // scalars, has a range size of zero for ranges, and has no items
-  // for sets.
-  static bool isZero(const Resource& resource);
+  // Tests if the given Resource object is empty.
+  static bool empty(const Resource& resource);
 
   Resources() {}
 
   // TODO(jieyu): Consider using C++11 initializer list.
-  /*implicit*/ Resources(const Resource& resource)
-  {
-    resources.Add()->CopyFrom(resource);
-  }
+  /*implicit*/ Resources(const Resource& resource);
 
   /*implicit*/
-  Resources(const google::protobuf::RepeatedPtrField<Resource>& _resources)
-  {
-    resources.MergeFrom(_resources);
-  }
+  Resources(const google::protobuf::RepeatedPtrField<Resource>& _resources);
 
-  Resources(const Resources& that)
-  {
-    resources.MergeFrom(that.resources);
-  }
+  Resources(const Resources& that) : resources(that.resources) {}
 
   Resources& operator = (const Resources& that)
   {
     if (this != &that) {
-      resources.Clear();
-      resources.MergeFrom(that.resources);
+      resources = that.resources;
     }
-
     return *this;
   }
 
@@ -118,31 +112,25 @@ public:
   Resources extract(const std::string& role) const;
 
   // Returns a Resources object with the same amount of each resource
-  // type as these Resources, but with only one Resource object per
-  // type and all Resource object marked as the specified role.
+  // type as these Resources, but with all Resource objects marked as
+  // the specified role.
   Resources flatten(const std::string& role = "*") const;
 
-  // Finds a number of resources equal to toFind in these Resources
-  // and returns them marked with appropriate roles. For each resource
-  // type, resources are first taken from the specified role, then
-  // from '*', then from any other role.
-  Option<Resources> find(
-      const Resources& toFind,
-      const std::string& role = "*") const;
-
-  // Returns the Resource from these Resources that matches the
-  // argument in name, type, and role, if it exists.
-  Option<Resource> get(const Resource& r) const;
-
-  // Returns all Resources from these Resources that match the
-  // argument in name and type, regardless of role.
-  Option<Resources> getAll(const Resource& r) const;
-
+  // Finds a Resources object with the same amount of each resource
+  // type as "targets" from these Resources. The roles specified in
+  // "targets" set the preference order. For each resource type,
+  // resources are first taken from the specified role, then from '*',
+  // then from any other role.
+  // TODO(jieyu): 'find' contains some allocation logic for scalars and
+  // fixed set / range elements. However, this is not sufficient for
+  // schedulers that want, say, any N available ports. We should
+  // consider moving this to an internal "allocation" library for our
+  // example frameworks to leverage.
+  Option<Resources> find(const Resources& targets) const;
+
+  // Helpers to get resource values. We consider all roles here.
   template <typename T>
-  T get(const std::string& name, const T& t) const;
-
-  // Returns a Resources object with only the allocatable resources.
-  Resources allocatable() const;
+  Option<T> get(const std::string& name) const;
 
   // Helpers to get known resource types.
   // TODO(vinod): Fix this when we make these types as first class
@@ -195,6 +183,13 @@ public:
   Resources& operator -= (const Resources& that);
 
 private:
+  bool contains(const Resource& that) const;
+
+  // Similar to the public 'find', but only for a single Resource
+  // object. The target resource may span multiple roles, so this
+  // returns Resources.
+  Option<Resources> find(const Resource& target) const;
+
   google::protobuf::RepeatedPtrField<Resource> resources;
 };
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/a8d0244b/src/common/resources.cpp
----------------------------------------------------------------------
diff --git a/src/common/resources.cpp b/src/common/resources.cpp
index 8474cdd..3a3c6a6 100644
--- a/src/common/resources.cpp
+++ b/src/common/resources.cpp
@@ -251,83 +251,92 @@ Try<Resources> Resources::parse(
 }
 
 
-bool Resources::isValid(const Resource& resource)
+Option<Error> Resources::validate(const Resource& resource)
 {
-  if (!resource.has_name() ||
-      resource.name() == "" ||
-      !resource.has_type() ||
-      !Value::Type_IsValid(resource.type())) {
-    return false;
+  if (resource.name().empty()) {
+    return Error("Empty resource name");
   }
 
-  if (resource.type() == Value::SCALAR) {
-    return resource.has_scalar();
-  } else if (resource.type() == Value::RANGES) {
-    return resource.has_ranges();
-  } else if (resource.type() == Value::SET) {
-    return resource.has_set();
-  } else if (resource.type() == Value::TEXT) {
-    // Resources doesn't support text.
-    return false;
+  if (!Value::Type_IsValid(resource.type())) {
+    return Error("Invalid resource type");
   }
 
-  return false;
-}
+  if (resource.type() == Value::SCALAR) {
+    if (!resource.has_scalar() ||
+        resource.has_ranges() ||
+        resource.has_set()) {
+      return Error("Invalid scalar resource");
+    }
 
+    if (resource.scalar().value() < 0) {
+      return Error("Invalid scalar resource: value < 0");
+    }
+  } else if (resource.type() == Value::RANGES) {
+    if (resource.has_scalar() ||
+        !resource.has_ranges() ||
+        resource.has_set()) {
+      return Error("Invalid ranges resource");
+    }
 
-bool Resources::isAllocatable(const Resource& resource)
-{
-  if (isValid(resource)) {
-    if (resource.type() == Value::SCALAR) {
-      if (resource.scalar().value() <= 0) {
-        return false;
+    for (int i = 0; i < resource.ranges().range_size(); i++) {
+      const Value::Range& range = resource.ranges().range(i);
+
+      // Ensure the range make sense (isn't inverted).
+      if (range.begin() > range.end()) {
+        return Error("Invalid ranges resource: begin > end");
       }
-    } else if (resource.type() == Value::RANGES) {
-      if (resource.ranges().range_size() == 0) {
-        return false;
-      } else {
-        for (int i = 0; i < resource.ranges().range_size(); i++) {
-          const Value::Range& range = resource.ranges().range(i);
-
-          // Ensure the range make sense (isn't inverted).
-          if (range.begin() > range.end()) {
-            return false;
-          }
-
-          // Ensure ranges don't overlap (but not necessarily coalesced).
-          for (int j = i + 1; j < resource.ranges().range_size(); j++) {
-            if (range.begin() <= resource.ranges().range(j).begin() &&
-                resource.ranges().range(j).begin() <= range.end()) {
-              return false;
-            }
-          }
+
+      // Ensure ranges don't overlap (but not necessarily coalesced).
+      for (int j = i + 1; j < resource.ranges().range_size(); j++) {
+        if (range.begin() <= resource.ranges().range(j).begin() &&
+            resource.ranges().range(j).begin() <= range.end()) {
+          return Error("Invalid ranges resource: overlapping ranges");
         }
       }
-    } else if (resource.type() == Value::SET) {
-      if (resource.set().item_size() == 0) {
-        return false;
-      } else {
-        for (int i = 0; i < resource.set().item_size(); i++) {
-          const string& item = resource.set().item(i);
-
-          // Ensure no duplicates.
-          for (int j = i + 1; j < resource.set().item_size(); j++) {
-            if (item == resource.set().item(j)) {
-              return false;
-            }
-          }
+    }
+  } else if (resource.type() == Value::SET) {
+    if (resource.has_scalar() ||
+        resource.has_ranges() ||
+        !resource.has_set()) {
+      return Error("Invalid set resource");
+    }
+
+    for (int i = 0; i < resource.set().item_size(); i++) {
+      const string& item = resource.set().item(i);
+
+      // Ensure no duplicates.
+      for (int j = i + 1; j < resource.set().item_size(); j++) {
+        if (item == resource.set().item(j)) {
+          return Error("Invalid set resource: duplicated elements");
         }
       }
     }
+  } else {
+    // Resource doesn't support TEXT or other value types.
+    return Error("Unsupported resource type");
+  }
 
-    return true;
+  return None();
+}
+
+
+Option<Error> Resources::validate(
+    const google::protobuf::RepeatedPtrField<Resource>& resources)
+{
+  foreach (const Resource& resource, resources) {
+    Option<Error> error = validate(resource);
+    if (error.isSome()) {
+      return Error(
+          "Resource '" + stringify(resource) +
+          "' is invalid: " + error.get().message);
+    }
   }
 
-  return false;
+  return None();
 }
 
 
-bool Resources::isZero(const Resource& resource)
+bool Resources::empty(const Resource& resource)
 {
   if (resource.type() == Value::SCALAR) {
     return resource.scalar().value() == 0;
@@ -335,9 +344,9 @@ bool Resources::isZero(const Resource& resource)
     return resource.ranges().range_size() == 0;
   } else if (resource.type() == Value::SET) {
     return resource.set().item_size() == 0;
+  } else {
+    return false;
   }
-
-  return false;
 }
 
 
@@ -346,6 +355,23 @@ bool Resources::isZero(const Resource& resource)
 /////////////////////////////////////////////////
 
 
+Resources::Resources(const Resource& resource)
+{
+  // NOTE: Invalid and zero Resource object will be ignored.
+  *this += resource;
+}
+
+
+Resources::Resources(
+    const google::protobuf::RepeatedPtrField<Resource>& resources)
+{
+  foreach (const Resource& resource, resources) {
+    // NOTE: Invalid and zero Resource objects will be ignored.
+    *this += resource;
+  }
+}
+
+
 Resources Resources::extract(const string& role) const
 {
   Resources r;
@@ -364,85 +390,61 @@ Resources Resources::flatten(const string& role) const
 {
   Resources flattened;
 
-  foreach (const Resource& r, resources) {
-    Resource toRemove = r;
-    toRemove.set_role(role);
-
-    bool found = false;
-    for (int i = 0; i < flattened.resources.size(); i++) {
-      Resource removed = flattened.resources.Get(i);
-      if (toRemove.name() == removed.name() &&
-          toRemove.type() == removed.type()) {
-        flattened.resources.Mutable(i)->MergeFrom(toRemove + removed);
-        found = true;
-        break;
-      }
-    }
-
-    if (!found) {
-      flattened.resources.Add()->MergeFrom(toRemove);
-    }
+  foreach (Resource resource, resources) {
+    resource.set_role(role);
+    flattened += resource;
   }
 
   return flattened;
 }
 
 
-Option<Resources> Resources::find(
-    const Resources& toFind,
-    const string& role) const
-{
-  Resources foundResources;
-
-  foreach (const Resource& findResource, toFind) {
-    Resource remaining = findResource;
-    Option<Resources> all = getAll(findResource);
-    bool done = false;
-
-    if (isZero(findResource)) {
-      // Done, as no resources of this type have been requested.
-      done = true;
-    } else if (all.isSome()) {
-      for (int i = 0; i < 3 && !done; i++) {
-        foreach (const Resource& potential, all.get()) {
-          // Ensures that we take resources first from the specified role,
-          // then from the default role, and then from any other role.
-          if ((i == 0 && potential.role() == role) ||
-              (i == 1 && potential.role() == "*" && potential.role() != role) 
||
-              (i == 2 && potential.role() != "*" && potential.role() != role)) 
{
-            // The resources must have the same role for <= to work.
-            Resource potential_ = potential;
-            potential_.set_role(remaining.role());
-            if (remaining <= potential_) {
-              // We can satisfy the remaining requirements for this
-              // resource type.
-              Resource found = remaining;
-              found.set_role(potential.role());
-              foundResources += found;
-              done = true;
-            } else {
-              foundResources += potential;
-              remaining -= potential_;
-            }
-          }
-        }
-      }
-    }
+class RoleFilter
+{
+public:
+  static RoleFilter any() { return RoleFilter(); }
 
-    if (!done) {
-      return None();
-    }
+  RoleFilter() : type(ANY) {}
+  /*implicit*/ RoleFilter(const string& _role) : type(SOME), role(_role) {}
+
+  Resources apply(const Resources& resources) const
+  {
+    return type == ANY? resources : resources.extract(role);
   }
 
-  return foundResources;
-}
+private:
+  enum { ANY, SOME } type;
+  string role;
+};
 
 
-Option<Resource> Resources::get(const Resource& r) const
+Option<Resources> Resources::find(const Resource& target) const
 {
-  foreach (const Resource& resource, resources) {
-    if (addable(resource, r)) {
-      return resource;
+  Resources found;
+  Resources total = *this;
+  Resources remaining = Resources(target).flatten();
+
+  // First look in the target role, then "*", then any remaining role.
+  vector<RoleFilter> filters = {
+    RoleFilter(target.role()),
+    RoleFilter("*"),
+    RoleFilter::any()
+  };
+
+  foreach (const RoleFilter& filter, filters) {
+    foreach (const Resource& resource, filter.apply(total)) {
+      // Need to flatten to ignore the roles in contains().
+      Resources flattened = Resources(resource).flatten();
+
+      if (remaining <= flattened) {
+        // Done!
+        return found + remaining.flatten(resource.role());
+      } else if (flattened <= remaining) {
+        found += resource;
+        total -= resource;
+        remaining -= flattened;
+        break;
+      }
     }
   }
 
@@ -450,29 +452,27 @@ Option<Resource> Resources::get(const Resource& r) const
 }
 
 
-Option<Resources> Resources::getAll(const Resource& r) const
+Option<Resources> Resources::find(const Resources& targets) const
 {
   Resources total;
 
-  foreach (const Resource& resource, resources) {
-    if (r.name() == resource.name() &&
-        r.type() == resource.type()) {
-      total += resource;
+  foreach (const Resource& target, targets) {
+    Option<Resources> found = find(target);
+
+    // Each target needs to be found!
+    if (found.isNone()) {
+      return None();
     }
-  }
 
-  if (total.size() > 0) {
-    return total;
+    total += found.get();
   }
 
-  return None();
+  return total;
 }
 
 
 template <>
-Value::Scalar Resources::get(
-    const string& name,
-    const Value::Scalar& scalar) const
+Option<Value::Scalar> Resources::get(const string& name) const
 {
   Value::Scalar total;
   bool found = false;
@@ -489,22 +489,20 @@ Value::Scalar Resources::get(
     return total;
   }
 
-  return scalar;
+  return None();
 }
 
 
 template <>
-Value::Ranges Resources::get(
-    const string& name,
-    const Value::Ranges& ranges) const
+Option<Value::Set> Resources::get(const string& name) const
 {
-  Value::Ranges total;
+  Value::Set total;
   bool found = false;
 
   foreach (const Resource& resource, resources) {
     if (resource.name() == name &&
-        resource.type() == Value::RANGES) {
-      total += resource.ranges();
+        resource.type() == Value::SET) {
+      total += resource.set();
       found = true;
     }
   }
@@ -513,22 +511,20 @@ Value::Ranges Resources::get(
     return total;
   }
 
-  return ranges;
+  return None();
 }
 
 
 template <>
-Value::Set Resources::get(
-    const string& name,
-    const Value::Set& set) const
+Option<Value::Ranges> Resources::get(const string& name) const
 {
-  Value::Set total;
+  Value::Ranges total;
   bool found = false;
 
   foreach (const Resource& resource, resources) {
     if (resource.name() == name &&
-        resource.type() == Value::SET) {
-      total += resource.set();
+        resource.type() == Value::RANGES) {
+      total += resource.ranges();
       found = true;
     }
   }
@@ -537,125 +533,74 @@ Value::Set Resources::get(
     return total;
   }
 
-  return set;
-}
-
-
-Resources Resources::allocatable() const
-{
-  Resources result;
-
-  foreach (const Resource& resource, resources) {
-    if (isAllocatable(resource)) {
-      result.resources.Add()->MergeFrom(resource);
-    }
-  }
-
-  return result;
+  return None();
 }
 
 
 Option<double> Resources::cpus() const
 {
-  double total= 0;
-  bool found = false;
-
-  foreach (const Resource& resource, resources) {
-    if (resource.name() == "cpus" && resource.type() == Value::SCALAR) {
-      total += resource.scalar().value();
-      found = true;
-    }
-  }
-
-  if (found) {
-    return total;
+  Option<Value::Scalar> value = get<Value::Scalar>("cpus");
+  if (value.isSome()) {
+    return value.get().value();
+  } else {
+    return None();
   }
-
-  return None();
 }
 
 
 Option<Bytes> Resources::mem() const
 {
-  double total = 0;
-  bool found = false;
-
-  foreach (const Resource& resource, resources) {
-    if (resource.name() == "mem" &&
-        resource.type() == Value::SCALAR) {
-      total += resource.scalar().value();
-      found = true;
-    }
-  }
-
-  if (found) {
-    return Megabytes(static_cast<uint64_t>(total));
+  Option<Value::Scalar> value = get<Value::Scalar>("mem");
+  if (value.isSome()) {
+    return Megabytes(static_cast<uint64_t>(value.get().value()));
+  } else {
+    return None();
   }
-
-  return None();
 }
 
 
 Option<Bytes> Resources::disk() const
 {
-  double total = 0;
-  bool found = false;
-
-  foreach (const Resource& resource, resources) {
-    if (resource.name() == "disk" &&
-        resource.type() == Value::SCALAR) {
-      total += resource.scalar().value();
-      found = true;
-    }
-  }
-
-  if (found) {
-    return Megabytes(static_cast<uint64_t>(total));
+  Option<Value::Scalar> value = get<Value::Scalar>("disk");
+  if (value.isSome()) {
+    return Megabytes(static_cast<uint64_t>(value.get().value()));
+  } else {
+    return None();
   }
-
-  return None();
 }
 
 
 Option<Value::Ranges> Resources::ports() const
 {
-  Value::Ranges total;
-  bool found = false;
-
-  foreach (const Resource& resource, resources) {
-    if (resource.name() == "ports" &&
-        resource.type() == Value::RANGES) {
-      total += resource.ranges();
-      found = true;
-    }
-  }
-
-  if (found) {
-    return total;
+  Option<Value::Ranges> value = get<Value::Ranges>("ports");
+  if (value.isSome()) {
+    return value.get();
+  } else {
+    return None();
   }
-
-  return None();
 }
 
 
 Option<Value::Ranges> Resources::ephemeral_ports() const
 {
-  Value::Ranges total;
-  bool found = false;
+  Option<Value::Ranges> value = get<Value::Ranges>("ephemeral_ports");
+  if (value.isSome()) {
+    return value.get();
+  } else {
+    return None();
+  }
+}
+
 
+bool Resources::contains(const Resource& that) const
+{
   foreach (const Resource& resource, resources) {
-    if (resource.name() == "ephemeral_ports" &&
-        resource.type() == Value::RANGES) {
-      total += resource.ranges();
-      found = true;
+    if (mesos::contains(resource, that)) {
+      return true;
     }
   }
 
-  if (found) {
-    return total;
-  }
-
-  return None();
+  return false;
 }
 
 
@@ -672,22 +617,7 @@ Resources::operator const 
google::protobuf::RepeatedPtrField<Resource>& () const
 
 bool Resources::operator == (const Resources& that) const
 {
-  if (size() != that.size()) {
-    return false;
-  }
-
-  foreach (const Resource& resource, resources) {
-    Option<Resource> option = that.get(resource);
-    if (option.isNone()) {
-      return false;
-      } else {
-      if (!(resource == option.get())) {
-        return false;
-      }
-    }
-  }
-
-    return true;
+  return *this <= that && that <= *this;
 }
 
 
@@ -700,15 +630,8 @@ bool Resources::operator != (const Resources& that) const
 bool Resources::operator <= (const Resources& that) const
 {
   foreach (const Resource& resource, resources) {
-    Option<Resource> option = that.get(resource);
-    if (option.isNone()) {
-      if (!isZero(resource)) {
-        return false;
-      }
-    } else {
-      if (!(resource <= option.get())) {
-        return false;
-      }
+    if (!that.contains(resource)) {
+      return false;
     }
   }
 
@@ -718,42 +641,38 @@ bool Resources::operator <= (const Resources& that) const
 
 Resources Resources::operator + (const Resource& that) const
 {
-  Resources result;
-
-  bool added = false;
-
-  foreach (const Resource& resource, resources) {
-    if (addable(resource, that)) {
-      result.resources.Add()->MergeFrom(resource + that);
-      added = true;
-    } else {
-      result.resources.Add()->MergeFrom(resource);
-    }
-  }
-
-  if (!added) {
-    result.resources.Add()->MergeFrom(that);
-  }
-
+  Resources result = *this;
+  result += that;
   return result;
 }
 
 
 Resources Resources::operator + (const Resources& that) const
 {
-  Resources result(*this);
-
-  foreach (const Resource& resource, that.resources) {
-    result += resource;
-  }
-
+  Resources result = *this;
+  result += that;
   return result;
 }
 
 
 Resources& Resources::operator += (const Resource& that)
 {
-  *this = *this + that;
+  if (validate(that).isNone() && !empty(that)) {
+    bool found = false;
+    foreach (Resource& resource, resources) {
+      if (addable(resource, that)) {
+        resource += that;
+        found = true;
+        break;
+      }
+    }
+
+    // Cannot be combined with any existing Resource object.
+    if (!found) {
+      resources.Add()->CopyFrom(that);
+    }
+  }
+
   return *this;
 }
 
@@ -770,38 +689,41 @@ Resources& Resources::operator += (const Resources& that)
 
 Resources Resources::operator - (const Resource& that) const
 {
-  Resources result;
-
-  foreach (const Resource& resource, resources) {
-    if (subtractable(resource, that)) {
-      Resource r = resource - that;
-      if (!isZero(r)) {
-        result.resources.Add()->MergeFrom(r);
-      }
-    } else {
-      result.resources.Add()->MergeFrom(resource);
-    }
-  }
-
+  Resources result = *this;
+  result -= that;
   return result;
 }
 
 
 Resources Resources::operator - (const Resources& that) const
 {
-  Resources result(*this);
-
-  foreach (const Resource& resource, that.resources) {
-    result -= resource;
-  }
-
+  Resources result = *this;
+  result -= that;
   return result;
 }
 
 
 Resources& Resources::operator -= (const Resource& that)
 {
-  *this = *this - that;
+  if (validate(that).isNone() && !empty(that)) {
+    for (int i = 0; i < resources.size(); i++) {
+      Resource* resource = resources.Mutable(i);
+
+      if (subtractable(*resource, that)) {
+        *resource -= that;
+
+        // Remove the resource if it becomes invalid or zero. We need
+        // to do the validation because we want to strip negative
+        // scalar Resource object.
+        if (validate(*resource).isSome() || empty(*resource)) {
+          resources.DeleteSubrange(i, 1);
+        }
+
+        break;
+      }
+    }
+  }
+
   return *this;
 }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/a8d0244b/src/examples/low_level_scheduler_libprocess.cpp
----------------------------------------------------------------------
diff --git a/src/examples/low_level_scheduler_libprocess.cpp 
b/src/examples/low_level_scheduler_libprocess.cpp
index 89b4318..7229797 100644
--- a/src/examples/low_level_scheduler_libprocess.cpp
+++ b/src/examples/low_level_scheduler_libprocess.cpp
@@ -238,7 +238,7 @@ private:
         task.mutable_executor()->MergeFrom(executor);
 
         Option<Resources> resources =
-          remaining.find(TASK_RESOURCES, framework.role());
+          remaining.find(TASK_RESOURCES.flatten(framework.role()));
 
         CHECK_SOME(resources);
         task.mutable_resources()->MergeFrom(resources.get());

http://git-wip-us.apache.org/repos/asf/mesos/blob/a8d0244b/src/examples/low_level_scheduler_pthread.cpp
----------------------------------------------------------------------
diff --git a/src/examples/low_level_scheduler_pthread.cpp 
b/src/examples/low_level_scheduler_pthread.cpp
index e5cd48a..2b012a8 100644
--- a/src/examples/low_level_scheduler_pthread.cpp
+++ b/src/examples/low_level_scheduler_pthread.cpp
@@ -288,7 +288,7 @@ private:
         task.mutable_executor()->MergeFrom(executor);
 
         Option<Resources> resources =
-          remaining.find(TASK_RESOURCES, framework.role());
+          remaining.find(TASK_RESOURCES.flatten(framework.role()));
 
         CHECK_SOME(resources);
         task.mutable_resources()->MergeFrom(resources.get());

http://git-wip-us.apache.org/repos/asf/mesos/blob/a8d0244b/src/examples/test_framework.cpp
----------------------------------------------------------------------
diff --git a/src/examples/test_framework.cpp b/src/examples/test_framework.cpp
index e87198b..ce1616d 100644
--- a/src/examples/test_framework.cpp
+++ b/src/examples/test_framework.cpp
@@ -104,7 +104,9 @@ public:
         task.mutable_slave_id()->MergeFrom(offer.slave_id());
         task.mutable_executor()->MergeFrom(executor);
 
-        Option<Resources> resources = remaining.find(TASK_RESOURCES, role);
+        Option<Resources> resources =
+          remaining.find(TASK_RESOURCES.flatten(role));
+
         CHECK_SOME(resources);
         task.mutable_resources()->MergeFrom(resources.get());
         remaining -= resources.get();

http://git-wip-us.apache.org/repos/asf/mesos/blob/a8d0244b/src/master/drf_sorter.cpp
----------------------------------------------------------------------
diff --git a/src/master/drf_sorter.cpp b/src/master/drf_sorter.cpp
index 5464900..0ad6c52 100644
--- a/src/master/drf_sorter.cpp
+++ b/src/master/drf_sorter.cpp
@@ -226,11 +226,14 @@ double DRFSorter::calculateShare(const string& name)
       double total = resource.scalar().value();
 
       if (total > 0) {
-        Value::Scalar none;
-        const Value::Scalar& scalar =
-          allocations[name].get(resource.name(), none);
+        Option<Value::Scalar> scalar =
+          allocations[name].get<Value::Scalar>(resource.name());
 
-        share = std::max(share, scalar.value() / total);
+        if (scalar.isNone()) {
+          scalar = Value::Scalar();
+        }
+
+        share = std::max(share, scalar.get().value() / total);
       }
     }
   }

http://git-wip-us.apache.org/repos/asf/mesos/blob/a8d0244b/src/master/hierarchical_allocator_process.hpp
----------------------------------------------------------------------
diff --git a/src/master/hierarchical_allocator_process.hpp 
b/src/master/hierarchical_allocator_process.hpp
index 31dfb2c..77f9741 100644
--- a/src/master/hierarchical_allocator_process.hpp
+++ b/src/master/hierarchical_allocator_process.hpp
@@ -537,7 +537,7 @@ HierarchicalAllocatorProcess<RoleSorter, 
FrameworkSorter>::resourcesRecovered(
 {
   CHECK(initialized);
 
-  if (resources.allocatable().size() == 0) {
+  if (resources.size() == 0) {
     return;
   }
 
@@ -560,7 +560,7 @@ HierarchicalAllocatorProcess<RoleSorter, 
FrameworkSorter>::resourcesRecovered(
   if (slaves.contains(slaveId)) {
     slaves[slaveId].available += resources;
 
-    LOG(INFO) << "Recovered " << resources.allocatable()
+    LOG(INFO) << "Recovered " << resources
               << " (total allocatable: " << slaves[slaveId].available
               << ") on slave " << slaveId
               << " from framework " << frameworkId;

http://git-wip-us.apache.org/repos/asf/mesos/blob/a8d0244b/src/master/http.cpp
----------------------------------------------------------------------
diff --git a/src/master/http.cpp b/src/master/http.cpp
index 3189933..9157a41 100644
--- a/src/master/http.cpp
+++ b/src/master/http.cpp
@@ -426,12 +426,16 @@ Future<Response> Master::Http::stats(const Request& 
request)
 
   foreach (const Resource& resource, totalResources) {
     CHECK(resource.has_scalar());
+
     double total = resource.scalar().value();
     object.values[resource.name() + "_total"] = total;
-    Option<Resource> option = usedResources.get(resource);
-    CHECK(!option.isSome() || option.get().has_scalar());
-    double used = option.isSome() ? option.get().scalar().value() : 0.0;
+
+    Option<Value::Scalar> _used =
+      usedResources.get<Value::Scalar>(resource.name());
+
+    double used = _used.isSome() ? _used.get().value() : 0.0;
     object.values[resource.name() + "_used"] = used;
+
     double percent = used / total;
     object.values[resource.name() + "_percent"] = percent;
   }

http://git-wip-us.apache.org/repos/asf/mesos/blob/a8d0244b/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 9e2d768..4cd9a4d 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -1961,9 +1961,15 @@ struct ResourceUsageChecker : TaskInfoVisitor
       return Error("Task uses no resources");
     }
 
+    Option<Error> error = Resources::validate(task.resources());
+    if (error.isSome()) {
+      return Error("Task uses invalid resources: " + error.get().message);
+    }
+
+    // Ensure no empty resource is used.
     foreach (const Resource& resource, task.resources()) {
-      if (!Resources::isAllocatable(resource)) {
-        return Error("Task uses invalid resources: " + stringify(resource));
+      if (Resources::empty(resource)) {
+        return Error("Task uses empty resources: " + stringify(resource));
       }
     }
 
@@ -1972,7 +1978,8 @@ struct ResourceUsageChecker : TaskInfoVisitor
 
     if (task.has_executor()) {
       foreach (const Resource& resource, task.executor().resources()) {
-        if (!Resources::isAllocatable(resource)) {
+        Option<Error> error = Resources::validate(resource);
+        if (error.isSome() || Resources::empty(resource)) {
           // TODO(benh): Send back the invalid resources?
           return Error(
               "Executor for task " + stringify(task.task_id()) +
@@ -2664,13 +2671,10 @@ void Master::_launchTasks(
     }
   }
 
-  // All used resources should be allocatable, enforced by our validators.
-  CHECK_EQ(usedResources, usedResources.allocatable());
-
   // Calculate unused resources.
   Resources unusedResources = totalResources - usedResources;
 
-  if (unusedResources.allocatable().size() > 0) {
+  if (unusedResources.size() > 0) {
     // Tell the allocator about the unused (e.g., refused) resources.
     allocator->resourcesRecovered(
         frameworkId, slaveId, unusedResources, filters);

http://git-wip-us.apache.org/repos/asf/mesos/blob/a8d0244b/src/slave/containerizer/containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/containerizer.cpp 
b/src/slave/containerizer/containerizer.cpp
index f234835..1448bea 100644
--- a/src/slave/containerizer/containerizer.cpp
+++ b/src/slave/containerizer/containerizer.cpp
@@ -26,6 +26,7 @@
 #include <stout/hashmap.hpp>
 #include <stout/net.hpp>
 #include <stout/stringify.hpp>
+#include <stout/strings.hpp>
 #include <stout/uuid.hpp>
 
 #include "slave/flags.hpp"
@@ -67,8 +68,13 @@ Try<Resources> Containerizer::resources(const Flags& flags)
 
   Resources resources = parsed.get();
 
-  // CPU resource.
-  if (!resources.cpus().isSome()) {
+  // NOTE: We need to check for the "cpus" string within the flag
+  // because once Resources are parsed, we cannot distinguish between
+  //  (1) "cpus:0", and
+  //  (2) no cpus specified.
+  // We only auto-detect cpus in case (2).
+  // The same logic applies for the other resources!
+  if (!strings::contains(flags.resources.get(""), "cpus")) {
     // No CPU specified so probe OS or resort to DEFAULT_CPUS.
     double cpus;
     Try<long> cpus_ = os::cpus();
@@ -88,7 +94,7 @@ Try<Resources> Containerizer::resources(const Flags& flags)
   }
 
   // Memory resource.
-  if (!resources.mem().isSome()) {
+  if (!strings::contains(flags.resources.get(""), "mem")) {
     // No memory specified so probe OS or resort to DEFAULT_MEM.
     Bytes mem;
     Try<os::Memory> mem_ = os::memory();
@@ -113,7 +119,7 @@ Try<Resources> Containerizer::resources(const Flags& flags)
   }
 
   // Disk resource.
-  if (!resources.disk().isSome()) {
+  if (!strings::contains(flags.resources.get(""), "disk")) {
     // No disk specified so probe OS or resort to DEFAULT_DISK.
     Bytes disk;
 
@@ -141,7 +147,7 @@ Try<Resources> Containerizer::resources(const Flags& flags)
   }
 
   // Network resource.
-  if (!resources.ports().isSome()) {
+  if (!strings::contains(flags.resources.get(""), "ports")) {
     // No ports specified so resort to DEFAULT_PORTS.
     resources += Resources::parse(
         "ports",

http://git-wip-us.apache.org/repos/asf/mesos/blob/a8d0244b/src/tests/gc_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/gc_tests.cpp b/src/tests/gc_tests.cpp
index 8618ae1..cb5dc5f 100644
--- a/src/tests/gc_tests.cpp
+++ b/src/tests/gc_tests.cpp
@@ -276,8 +276,8 @@ TEST_F(GarbageCollectorIntegrationTest, Restart)
     .Times(1);
 
   Resources resources = Resources::parse(flags.resources.get()).get();
-  double cpus = resources.get("cpus", Value::Scalar()).value();
-  double mem = resources.get("mem", Value::Scalar()).value();
+  double cpus = resources.get<Value::Scalar>("cpus").get().value();
+  double mem = resources.get<Value::Scalar>("mem").get().value();
 
   EXPECT_CALL(sched, resourceOffers(_, _))
     .WillOnce(LaunchTasks(DEFAULT_EXECUTOR_INFO, 1, cpus, mem, "*"))
@@ -383,8 +383,8 @@ TEST_F(GarbageCollectorIntegrationTest, ExitedFramework)
     .WillOnce(SaveArg<1>(&frameworkId));
 
   Resources resources = Resources::parse(flags.resources.get()).get();
-  double cpus = resources.get("cpus", Value::Scalar()).value();
-  double mem = resources.get("mem", Value::Scalar()).value();
+  double cpus = resources.get<Value::Scalar>("cpus").get().value();
+  double mem = resources.get<Value::Scalar>("mem").get().value();
 
   EXPECT_CALL(sched, resourceOffers(_, _))
     .WillOnce(LaunchTasks(DEFAULT_EXECUTOR_INFO, 1, cpus, mem, "*"))
@@ -498,8 +498,8 @@ TEST_F(GarbageCollectorIntegrationTest, ExitedExecutor)
     .WillOnce(FutureArg<1>(&frameworkId));
 
   Resources resources = Resources::parse(flags.resources.get()).get();
-  double cpus = resources.get("cpus", Value::Scalar()).value();
-  double mem = resources.get("mem", Value::Scalar()).value();
+  double cpus = resources.get<Value::Scalar>("cpus").get().value();
+  double mem = resources.get<Value::Scalar>("mem").get().value();
 
   EXPECT_CALL(sched, resourceOffers(_, _))
     .WillOnce(LaunchTasks(DEFAULT_EXECUTOR_INFO, 1, cpus, mem, "*"))
@@ -602,8 +602,8 @@ TEST_F(GarbageCollectorIntegrationTest, DiskUsage)
     .WillOnce(FutureArg<1>(&frameworkId));
 
   Resources resources = Resources::parse(flags.resources.get()).get();
-  double cpus = resources.get("cpus", Value::Scalar()).value();
-  double mem = resources.get("mem", Value::Scalar()).value();
+  double cpus = resources.get<Value::Scalar>("cpus").get().value();
+  double mem = resources.get<Value::Scalar>("mem").get().value();
 
   EXPECT_CALL(sched, resourceOffers(_, _))
     .WillOnce(LaunchTasks(DEFAULT_EXECUTOR_INFO, 1, cpus, mem, "*"))
@@ -730,8 +730,8 @@ TEST_F(GarbageCollectorIntegrationTest, Unschedule)
     .WillOnce(FutureArg<1>(&frameworkId));
 
   Resources resources = Resources::parse(flags.resources.get()).get();
-  double cpus = resources.get("cpus", Value::Scalar()).value();
-  double mem = resources.get("mem", Value::Scalar()).value();
+  double cpus = resources.get<Value::Scalar>("cpus").get().value();
+  double mem = resources.get<Value::Scalar>("mem").get().value();
 
   EXPECT_CALL(sched, resourceOffers(_, _))
     .WillOnce(LaunchTasks(executor1, 1, cpus, mem, "*"));

http://git-wip-us.apache.org/repos/asf/mesos/blob/a8d0244b/src/tests/mesos.hpp
----------------------------------------------------------------------
diff --git a/src/tests/mesos.hpp b/src/tests/mesos.hpp
index c1d64a7..0a5e8ec 100644
--- a/src/tests/mesos.hpp
+++ b/src/tests/mesos.hpp
@@ -413,8 +413,11 @@ ACTION_P5(LaunchTasks, executor, tasks, cpus, mem, role)
       task.mutable_slave_id()->MergeFrom(offer.slave_id());
       task.mutable_executor()->MergeFrom(executor);
 
-      Option<Resources> resources = remaining.find(TASK_RESOURCES, role);
+      Option<Resources> resources =
+        remaining.find(TASK_RESOURCES.flatten(role));
+
       CHECK_SOME(resources);
+
       task.mutable_resources()->MergeFrom(resources.get());
       remaining -= resources.get();
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/a8d0244b/src/tests/resource_offers_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/resource_offers_tests.cpp 
b/src/tests/resource_offers_tests.cpp
index 43820b0..7e09319 100644
--- a/src/tests/resource_offers_tests.cpp
+++ b/src/tests/resource_offers_tests.cpp
@@ -200,7 +200,7 @@ TEST_F(TaskValidationTest, TaskUsesNoResources)
 }
 
 
-TEST_F(TaskValidationTest, TaskUsesInvalidResources)
+TEST_F(TaskValidationTest, TaskUsesEmptyResources)
 {
   Try<PID<Master>> master = StartMaster();
   ASSERT_SOME(master);
@@ -250,7 +250,7 @@ TEST_F(TaskValidationTest, TaskUsesInvalidResources)
   EXPECT_EQ(TASK_ERROR, status.get().state());
   EXPECT_EQ(TaskStatus::REASON_TASK_INVALID, status.get().reason());
   EXPECT_TRUE(status.get().has_message());
-  EXPECT_EQ("Task uses invalid resources: cpus(*):0", status.get().message());
+  EXPECT_EQ("Task uses empty resources: cpus(*):0", status.get().message());
 
   driver.stop();
   driver.join();
@@ -653,8 +653,8 @@ TEST_F(ResourceOffersTest, ResourceOfferWithMultipleSlaves)
   EXPECT_GE(10u, offers.get().size());
 
   Resources resources(offers.get()[0].resources());
-  EXPECT_EQ(2, resources.get("cpus", Value::Scalar()).value());
-  EXPECT_EQ(1024, resources.get("mem", Value::Scalar()).value());
+  EXPECT_EQ(2, resources.get<Value::Scalar>("cpus").get().value());
+  EXPECT_EQ(1024, resources.get<Value::Scalar>("mem").get().value());
 
   driver.stop();
   driver.join();
@@ -818,7 +818,7 @@ TEST_F(ResourceOffersTest, 
ResourcesGetReofferedAfterTaskInfoError)
   EXPECT_EQ(TASK_ERROR, status.get().state());
   EXPECT_EQ(TaskStatus::REASON_TASK_INVALID, status.get().reason());
   EXPECT_TRUE(status.get().has_message());
-  EXPECT_EQ("Task uses invalid resources: cpus(*):0", status.get().message());
+  EXPECT_EQ("Task uses empty resources: cpus(*):0", status.get().message());
 
   MockScheduler sched2;
   MesosSchedulerDriver driver2(

http://git-wip-us.apache.org/repos/asf/mesos/blob/a8d0244b/src/tests/resources_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/resources_tests.cpp b/src/tests/resources_tests.cpp
index 8d1e86a..9387a29 100644
--- a/src/tests/resources_tests.cpp
+++ b/src/tests/resources_tests.cpp
@@ -192,14 +192,14 @@ TEST(ResourcesTest, BadResourcesNotAllocatable)
   Resources r;
   r += cpus;
 
-  EXPECT_EQ(0u, r.allocatable().size());
+  EXPECT_EQ(0u, r.size());
 
   cpus.set_name("cpus");
   cpus.mutable_scalar()->set_value(0);
 
   r += cpus;
 
-  EXPECT_EQ(0u, r.allocatable().size());
+  EXPECT_EQ(0u, r.size());
 }
 
 
@@ -293,15 +293,15 @@ TEST(ResourcesTest, ScalarAddition)
   Resources sum = r1 + r2;
 
   EXPECT_EQ(2u, sum.size());
-  EXPECT_EQ(3, sum.get("cpus", Value::Scalar()).value());
-  EXPECT_EQ(15, sum.get("mem", Value::Scalar()).value());
+  EXPECT_EQ(3, sum.get<Value::Scalar>("cpus").get().value());
+  EXPECT_EQ(15, sum.get<Value::Scalar>("mem").get().value());
 
   Resources r = r1;
   r += r2;
 
   EXPECT_EQ(2u, r.size());
-  EXPECT_EQ(3, r.get("cpus", Value::Scalar()).value());
-  EXPECT_EQ(15, r.get("mem", Value::Scalar()).value());
+  EXPECT_EQ(3, r.get<Value::Scalar>("cpus").get().value());
+  EXPECT_EQ(15, r.get<Value::Scalar>("mem").get().value());
 }
 
 
@@ -345,18 +345,19 @@ TEST(ResourcesTest, ScalarSubtraction)
   Resources diff = r1 - r2;
 
   EXPECT_EQ(2u, diff.size());
-  EXPECT_EQ(49.5, diff.get("cpus", Value::Scalar()).value());
-  EXPECT_EQ(3072, diff.get("mem", Value::Scalar()).value());
+  EXPECT_EQ(49.5, diff.get<Value::Scalar>("cpus").get().value());
+  EXPECT_EQ(3072, diff.get<Value::Scalar>("mem").get().value());
 
   Resources r = r1;
   r -= r2;
 
-  EXPECT_EQ(49.5, diff.get("cpus", Value::Scalar()).value());
-  EXPECT_EQ(3072, diff.get("mem", Value::Scalar()).value());
+  EXPECT_EQ(49.5, diff.get<Value::Scalar>("cpus").get().value());
+  EXPECT_EQ(3072, diff.get<Value::Scalar>("mem").get().value());
 
   r = r1;
   r -= r1;
-  EXPECT_EQ(0u, r.allocatable().size());
+
+  EXPECT_EQ(0u, r.size());
 }
 
 
@@ -444,7 +445,7 @@ TEST(ResourcesTest, RangesSubset)
 TEST(ResourcesTest, RangesAddition)
 {
   Resource ports1 = Resources::parse(
-      "ports", "[20000-40000, 21000-38000]", "*").get();
+      "ports", "[20000-40000]", "*").get();
 
   Resource ports2 = Resources::parse(
       "ports", "[30000-50000, 10000-20000]", "*").get();
@@ -455,9 +456,9 @@ TEST(ResourcesTest, RangesAddition)
 
   EXPECT_EQ(1u, r.size());
 
-  const Value::Ranges& ranges = r.get("ports", Value::Ranges());
-
-  EXPECT_EQ(values::parse("[10000-50000]").get().ranges(), ranges);
+  EXPECT_SOME_EQ(
+      values::parse("[10000-50000]").get().ranges(),
+      r.get<Value::Ranges>("ports"));
 }
 
 
@@ -472,9 +473,9 @@ TEST(ResourcesTest, RangesAddition2)
 
   EXPECT_EQ(1u, r.size());
 
-  const Value::Ranges& ranges = r.get("ports", Value::Ranges());
-
-  EXPECT_EQ(values::parse("[1-65, 70-80]").get().ranges(), ranges);
+  EXPECT_SOME_EQ(
+      values::parse("[1-65, 70-80]").get().ranges(),
+      r.get<Value::Ranges>("ports"));
 }
 
 
@@ -491,9 +492,9 @@ TEST(ResourcesTest, RangesAdditon3)
 
   EXPECT_EQ(1u, r1.size());
 
-  const Value::Ranges& ranges = r1.get("ports", Value::Ranges());
-
-  EXPECT_EQ(values::parse("[1-4]").get().ranges(), ranges);
+  EXPECT_SOME_EQ(
+      values::parse("[1-4]").get().ranges(),
+      r1.get<Value::Ranges>("ports"));
 
   Resources r2;
   r2 += ports3;
@@ -501,17 +502,17 @@ TEST(ResourcesTest, RangesAdditon3)
 
   EXPECT_EQ(1u, r2.size());
 
-  const Value::Ranges& ranges2 = r2.get("ports", Value::Ranges());
-
-  EXPECT_EQ(values::parse("[5-8]").get().ranges(), ranges2);
+  EXPECT_SOME_EQ(
+      values::parse("[5-8]").get().ranges(),
+      r2.get<Value::Ranges>("ports"));
 
   r2 += r1;
 
   EXPECT_EQ(1u, r2.size());
 
-  const Value::Ranges& ranges3 = r2.get("ports", Value::Ranges());
-
-  EXPECT_EQ(values::parse("[1-8]").get().ranges(), ranges3);
+  EXPECT_SOME_EQ(
+      values::parse("[1-8]").get().ranges(),
+      r2.get<Value::Ranges>("ports"));
 }
 
 
@@ -529,9 +530,9 @@ TEST(ResourcesTest, RangesAddition4)
 
   EXPECT_EQ(1u, r.size());
 
-  const Value::Ranges& ranges = r.get("ports", Value::Ranges());
-
-  EXPECT_EQ(values::parse("[1-10, 20-30]").get().ranges(), ranges);
+  EXPECT_SOME_EQ(
+      values::parse("[1-10, 20-30]").get().ranges(),
+      r.get<Value::Ranges>("ports"));
 }
 
 
@@ -549,9 +550,9 @@ TEST(ResourcesTest, RangesSubtraction)
 
   EXPECT_EQ(1u, r.size());
 
-  const Value::Ranges& ranges = r.get("ports", Value::Ranges());
-
-  EXPECT_EQ(values::parse("[20001-29999]").get().ranges(), ranges);
+  EXPECT_SOME_EQ(
+      values::parse("[20001-29999]").get().ranges(),
+      r.get<Value::Ranges>("ports"));
 }
 
 
@@ -566,9 +567,9 @@ TEST(ResourcesTest, RangesSubtraction1)
 
   EXPECT_EQ(1u, r.size());
 
-  const Value::Ranges& ranges = r.get("ports", Value::Ranges());
-
-  EXPECT_EQ(values::parse("[50002-60000]").get().ranges(), ranges);
+  EXPECT_SOME_EQ(
+      values::parse("[50002-60000]").get().ranges(),
+      r.get<Value::Ranges>("ports"));
 }
 
 
@@ -583,9 +584,9 @@ TEST(ResourcesTest, RangesSubtraction2)
 
   EXPECT_EQ(1u, r.size());
 
-  const Value::Ranges& ranges = r.get("ports", Value::Ranges());
-
-  EXPECT_EQ(values::parse("[50001-60000]").get().ranges(), ranges);
+  EXPECT_SOME_EQ(
+      values::parse("[50001-60000]").get().ranges(),
+      r.get<Value::Ranges>("ports"));
 }
 
 
@@ -597,13 +598,12 @@ TEST(ResourcesTest, RangesSubtraction3)
   Resources resourcesInUse = Resources::parse("ports:[50000-50001]").get();
 
   Resources resourcesFree = resources - (resourcesOffered + resourcesInUse);
-  resourcesFree = resourcesFree.allocatable();
 
   EXPECT_EQ(1u, resourcesFree.size());
 
-  const Value::Ranges& ranges = resourcesFree.get("ports", Value::Ranges());
-
-  EXPECT_EQ(values::parse("[50002-60000]").get().ranges(), ranges);
+  EXPECT_SOME_EQ(
+      values::parse("[50002-60000]").get().ranges(),
+      resourcesFree.get<Value::Ranges>("ports"));
 }
 
 
@@ -616,10 +616,7 @@ TEST(ResourcesTest, RangesSubtraction4)
   resourcesOffered -= resources;
 
   EXPECT_EQ(0u, resourcesOffered.size());
-
-  const Value::Ranges& ranges = resourcesOffered.get("ports", Value::Ranges());
-
-  EXPECT_EQ(0, ranges.range_size());
+  EXPECT_NONE(resourcesOffered.get<Value::Ranges>("ports"));
 }
 
 
@@ -637,9 +634,9 @@ TEST(ResourcesTest, RangesSubtraction5)
 
   EXPECT_EQ(1u, r.size());
 
-  const Value::Ranges& ranges = r.get("ports", Value::Ranges());
-
-  EXPECT_EQ(values::parse("[1-1, 10-10, 46-47]").get().ranges(), ranges);
+  EXPECT_SOME_EQ(
+      values::parse("[1-1, 10-10, 46-47]").get().ranges(),
+      r.get<Value::Ranges>("ports"));
 }
 
 
@@ -654,9 +651,9 @@ TEST(ResourcesTest, RangesSubtraction6)
 
   EXPECT_EQ(1u, r.size());
 
-  const Value::Ranges& ranges = r.get("ports", Value::Ranges());
-
-  EXPECT_EQ(values::parse("[1-10]").get().ranges(), ranges);
+  EXPECT_SOME_EQ(
+      values::parse("[1-10]").get().ranges(),
+      r.get<Value::Ranges>("ports"));
 }
 
 
@@ -709,9 +706,10 @@ TEST(ResourcesTest, SetAddition)
 
   EXPECT_EQ(1u, r.size());
 
-  const Value::Set& set = r.get("disks", Value::Set());
+  Option<Value::Set> set = r.get<Value::Set>("disks");
 
-  EXPECT_EQ(4, set.item_size());
+  ASSERT_SOME(set);
+  EXPECT_EQ(4, set.get().item_size());
 }
 
 
@@ -729,10 +727,11 @@ TEST(ResourcesTest, SetSubtraction)
 
   EXPECT_EQ(1u, r.size());
 
-  const Value::Set& set = r.get("disks", Value::Set());
+  Option<Value::Set> set = r.get<Value::Set>("disks");
 
-  EXPECT_EQ(1, set.item_size());
-  EXPECT_EQ("sda1", set.item(0));
+  ASSERT_SOME(set);
+  EXPECT_EQ(1, set.get().item_size());
+  EXPECT_EQ("sda1", set.get().item(0));
 }
 
 
@@ -765,43 +764,37 @@ TEST(ResourcesTest, Find)
   Resources resources1 = Resources::parse(
       "cpus(role1):2;mem(role1):10;cpus:4;mem:20").get();
 
-  Resources toFind1 = Resources::parse("cpus:3;mem:15").get();
+  Resources targets1 = Resources::parse(
+      "cpus(role1):3;mem(role1):15").get();
 
-  Resources found1 = resources1.find(toFind1, "role1").get();
-
-  Resources expected1 = Resources::parse(
-      "cpus(role1):2;mem(role1):10;cpus:1;mem:5").get();
-
-  EXPECT_EQ(found1, expected1);
+  EXPECT_SOME_EQ(
+      Resources::parse("cpus(role1):2;mem(role1):10;cpus:1;mem:5").get(),
+      resources1.find(targets1));
 
   Resources resources2 = Resources::parse(
       "cpus(role1):1;mem(role1):5;cpus(role2):2;"
       "mem(role2):8;cpus:1;mem:7").get();
 
-  Resources toFind2 = Resources::parse("cpus:3;mem:15").get();
-
-  Resources found2 = resources2.find(toFind2, "role1").get();
+  Resources targets2 = Resources::parse(
+      "cpus(role1):3;mem(role1):15").get();
 
-  Resources expected2 = Resources::parse(
-      "cpus(role1):1;mem(role1):5;cpus:1;mem:7;"
-      "cpus(role2):1;mem(role2):3").get();
-
-  EXPECT_EQ(found2, expected2);
+  EXPECT_SOME_EQ(
+      Resources::parse(
+        "cpus(role1):1;mem(role1):5;cpus:1;mem:7;"
+        "cpus(role2):1;mem(role2):3").get(),
+      resources2.find(targets2));
 
   Resources resources3 = Resources::parse(
       "cpus(role1):5;mem(role1):5;cpus:5;mem:5").get();
 
-  Resources toFind3 = Resources::parse("cpus:6;mem:6").get();
-
-  Resources found3 = resources3.find(toFind3).get();
-
-  Resources expected3 = Resources::parse(
-      "cpus:5;mem:5;cpus(role1):1;mem(role1):1").get();
+  Resources targets3 = Resources::parse("cpus:6;mem:6").get();
 
-  EXPECT_EQ(found3, expected3);
+  EXPECT_SOME_EQ(
+      Resources::parse("cpus:5;mem:5;cpus(role1):1;mem(role1):1").get(),
+      resources3.find(targets3));
 
   Resources resources4 = Resources::parse("cpus(role1):1;mem(role1):1").get();
-  Resources toFind4 = Resources::parse("cpus:2;mem:2").get();
+  Resources targets4 = Resources::parse("cpus(role1):2;mem(role1):2").get();
 
-  EXPECT_NONE(resources4.find(toFind1, "role1"));
+  EXPECT_NONE(resources4.find(targets4));
 }

Reply via email to