Repository: mesos Updated Branches: refs/heads/master a9c1c775b -> 0a610695e
Captured the `stderr` during execution of CNI plugin. Till now we were capturing only the `stdout` of the CNI plugin. However, during errors it makes sense to add the output from `stderr` for the CNI plugin as well, since as per the CNI spec the CNI plugin is supposed to output all unstructured output (debugs for example) to `stderr`. This greatly helps in debugging the exact reason for CNI plugin failures. Review: https://reviews.apache.org/r/52771/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/0a610695 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/0a610695 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/0a610695 Branch: refs/heads/master Commit: 0a610695e42e7b221d87c9c9ce0c29a3bd56ac26 Parents: a9c1c77 Author: Avinash sridharan <avin...@mesosphere.io> Authored: Mon Oct 17 17:37:04 2016 -0700 Committer: Jie Yu <yujie....@gmail.com> Committed: Mon Oct 17 17:57:19 2016 -0700 ---------------------------------------------------------------------- .../mesos/isolators/network/cni/cni.cpp | 46 ++++++++++++++------ .../mesos/isolators/network/cni/cni.hpp | 2 + 2 files changed, 35 insertions(+), 13 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/0a610695/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp b/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp index 7c35c30..ae78c65 100644 --- a/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp +++ b/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp @@ -1150,7 +1150,7 @@ Future<Nothing> NetworkCniIsolatorProcess::attach( {networkConfig.config.type()}, Subprocess::PATH(networkConfigPath), Subprocess::PIPE(), - Subprocess::PATH("/dev/null"), + Subprocess::PIPE(), nullptr, environment); @@ -1160,7 +1160,7 @@ Future<Nothing> NetworkCniIsolatorProcess::attach( plugin.get() + "': " + s.error()); } - return await(s->status(), io::read(s->out().get())) + return await(s->status(), io::read(s->out().get()), io::read(s->err().get())) .then(defer( PID<NetworkCniIsolatorProcess>(this), &NetworkCniIsolatorProcess::_attach, @@ -1175,7 +1175,7 @@ Future<Nothing> NetworkCniIsolatorProcess::_attach( const ContainerID& containerId, const string& networkName, const string& plugin, - const tuple<Future<Option<int>>, Future<string>>& t) + const tuple<Future<Option<int>>, Future<string>, Future<string>>& t) { CHECK(infos.contains(containerId)); CHECK(infos[containerId]->containerNetworks.contains(networkName)); @@ -1204,10 +1204,18 @@ Future<Nothing> NetworkCniIsolatorProcess::_attach( } if (status.get() != 0) { + Future<string> error = std::get<2>(t); + if (!error.isReady()) { + return Failure( + "Failed to read stderr from the CNI plugin '" + + plugin + "' subprocess: " + + (error.isFailed() ? error.failure() : "discarded")); + } + return Failure( "The CNI plugin '" + plugin + "' failed to attach container " + - containerId.value() + " to CNI network '" + networkName + - "': " + output.get()); + stringify(containerId) + " to CNI network '" + networkName + + "': stdout='" + output.get() + "', stderr='" + error.get() + "'"); } // Parse the output of CNI plugin. @@ -1215,7 +1223,7 @@ Future<Nothing> NetworkCniIsolatorProcess::_attach( if (parse.isError()) { return Failure( "Failed to parse the output of the CNI plugin '" + - plugin + "': " + parse.error()); + plugin + "' :" + parse.error()); } if (parse.get().has_ip4()) { @@ -1472,12 +1480,14 @@ Future<Nothing> NetworkCniIsolatorProcess::detach( << "' to detach container " << containerId << " from network '" << networkName << "'"; + // NOTE: For 'DEL' the CNI plugin is not expected to return any + // result, hence setting the STDOUT to /dev/null. Try<Subprocess> s = subprocess( plugin.get(), {networkConfig.config.type()}, Subprocess::PATH(networkConfigPath), Subprocess::PIPE(), - Subprocess::PATH("/dev/null"), + Subprocess::PIPE(), nullptr, environment); @@ -1487,7 +1497,10 @@ Future<Nothing> NetworkCniIsolatorProcess::detach( "': " + s.error()); } - return await(s->status(), io::read(s->out().get())) + return await( + s->status(), + io::read(s->out().get()), + io::read(s->err().get())) .then(defer( PID<NetworkCniIsolatorProcess>(this), &NetworkCniIsolatorProcess::_detach, @@ -1502,7 +1515,7 @@ Future<Nothing> NetworkCniIsolatorProcess::_detach( const ContainerID& containerId, const string& networkName, const string& plugin, - const tuple<Future<Option<int>>, Future<string>>& t) + const tuple<Future<Option<int>>, Future<string>, Future<string>>& t) { CHECK(infos.contains(containerId)); CHECK(infos[containerId]->containerNetworks.contains(networkName)); @@ -1537,8 +1550,6 @@ Future<Nothing> NetworkCniIsolatorProcess::_detach( return Nothing(); } - // CNI plugin will print result (in case of success) or error (in - // case of failure) to stdout. Future<string> output = std::get<1>(t); if (!output.isReady()) { return Failure( @@ -1547,9 +1558,18 @@ Future<Nothing> NetworkCniIsolatorProcess::_detach( (output.isFailed() ? output.failure() : "discarded")); } + Future<string> error = std::get<2>(t); + if (!error.isReady()) { + return Failure( + "Failed to read stderr from the CNI plugin '" + + plugin + "' subprocess: " + + (error.isFailed() ? error.failure() : "discarded")); + } + return Failure( - "The CNI plugin '" + plugin + "' failed to detach container " - "from network '" + networkName + "': " + output.get()); + "The CNI plugin '" + plugin + "' failed to detach container " + + stringify(containerId) + " from CNI network '" + networkName + + "': stdout='" + output.get() + "', stderr='" + error.get() + "'"); } http://git-wip-us.apache.org/repos/asf/mesos/blob/0a610695/src/slave/containerizer/mesos/isolators/network/cni/cni.hpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/mesos/isolators/network/cni/cni.hpp b/src/slave/containerizer/mesos/isolators/network/cni/cni.hpp index a6861fc..b8fc755 100644 --- a/src/slave/containerizer/mesos/isolators/network/cni/cni.hpp +++ b/src/slave/containerizer/mesos/isolators/network/cni/cni.hpp @@ -151,6 +151,7 @@ private: const std::string& plugin, const std::tuple< process::Future<Option<int>>, + process::Future<std::string>, process::Future<std::string>>& t); process::Future<Nothing> detach( @@ -163,6 +164,7 @@ private: const std::string& plugin, const std::tuple< process::Future<Option<int>>, + process::Future<std::string>, process::Future<std::string>>& t); process::Future<Nothing> _cleanup(
