-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48477/#review136893
-----------------------------------------------------------
Ship it!
Very nice!
I made some slight adjustments, let me know if you see any issues!
```
diff --git a/src/slave/containerizer/mesos/containerizer.cpp
b/src/slave/containerizer/mesos/containerizer.cpp
index 0e2a9c8..b151121 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -257,13 +257,17 @@ Try<MesosContainerizer*> MesosContainerizer::create(
}
// Create isolators for the MesosContainerizer.
- // The order of elements in this vector is important. It
- // defines the relative dependencies between each isolator.
- // Isolators that appear later in the vector depend on
- // isolators that appear earlier. Specifically, this means that
- // the `create()` and `prepare()` calls for each isolator are
- // serialized in the order in which they appear, while the
- // `cleanup()` call is serialized in reverse order.
+ //
+ // The order of elements in the creation vecot is used to express
+ // ordering dependencies between isolators. Currently, the `create`
+ // and `prepare` calls for each isolator are run serially in the
+ // order in which they appear in this list, while the `cleanup` call
+ // is serialized in reverse order. The current dependencies are:
+ //
+ // (1) The filesystem isolator must be the first isolator so
+ // that the runtime isolators have a consistent view of
+ // the prepared filesystem (e.g. volume mounts).
+
const vector<pair<string, lambda::function<Try<Isolator*>(const Flags&)>>>
creators = {
// Filesystem isolators.
@@ -315,33 +319,34 @@ Try<MesosContainerizer*> MesosContainerizer::create(
vector<Owned<Isolator>> isolators;
- vector<string> types_ = strings::tokenize(flags_.isolation, ",");
- set<string> types = set<string>(types_.begin(), types_.end());
+ const vector<string> tokens = strings::tokenize(flags_.isolation, ",");
+ const set<string> isolations(tokens.begin(), tokens.end());
- foreach (const auto creator, creators) {
- if (types.count(creator.first) > 0) {
+ if (isolations.size() != tokens.size()) {
+ return Error("Duplicate entries found in --isolation flag"
+ " '" + stringify(tokens) + "'");
+ }
+
+ foreach (const auto& creator, creators) {
+ if (isolations.count(creator.first) > 0) {
Try<Isolator*> isolator = creator.second(flags_);
if (isolator.isError()) {
- return Error("Could not create isolator '" +
- creator.first + "': " + isolator.error());
+ return Error("Failed to create isolator '" + creator.first + "': " +
+ isolator.error());
}
-
- isolators.push_back(Owned<Isolator>(isolator.get()));
- types.erase(creator.first);
+ isolators.emplace_back(isolator.get());
} else if (ModuleManager::contains<Isolator>(creator.first)) {
Try<Isolator*> isolator = ModuleManager::create<Isolator>(creator.first);
if (isolator.isError()) {
- return Error("Could not create isolator '" +
- creator.first + "': " + isolator.error());
+ return Error("Could not create isolator '" + creator.first + "': " +
+ isolator.error());
}
-
- isolators.push_back(Owned<Isolator>(isolator.get()));
- types.erase(creator.first);
+ isolators.emplace_back(isolator.get());
}
}
- if (types.size() != 0) {
- return Error("Unknown or unsupported isolators '" + stringify(types) +
"'");
+ if (isolations.empty()) {
+ return Error("Unknown --isolation entries '" + stringify(isolations) +
"'");
}
return new MesosContainerizer(
```
- Benjamin Mahler
On June 9, 2016, 7:31 a.m., Kevin Klues wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48477/
> -----------------------------------------------------------
>
> (Updated June 9, 2016, 7:31 a.m.)
>
>
> Review request for mesos, Benjamin Mahler and Jie Yu.
>
>
> Bugs: MESOS-5581
> https://issues.apache.org/jira/browse/MESOS-5581
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Some isolators depend on other isolators. However, we previously did
> not have a generic method of expressing these dependencies. We special
> cased the `filesystem/*` isolators to make sure that dependencies on
> them were satisfied, but no other dependencies could be expressed.
>
> Now we use a vector to represent the pairing of isolator name to
> isolator creator function, and the relative dependencies between each
> isolator is implicit in the ordering of this vector. Previously, a
> hashmap was used to hold this pairing, but this was inadequate because
> hashmaps are inherently unordered. The new implementation using a
> vector ensures everything is processed in the order it is listed.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.cpp
> c7b9744463cf8e1921dcb5e2b7dec7d4e2c0e45f
>
> Diff: https://reviews.apache.org/r/48477/diff/
>
>
> Testing
> -------
>
> make -j check
>
>
> Thanks,
>
> Kevin Klues
>
>
