You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ji...@apache.org on 2016/10/12 18:01:56 UTC
[5/5] mesos git commit: Added `PluginError` to simplify error
reporting for CNI plugins.
Added `PluginError` to simplify error reporting for CNI plugins.
https://reviews.apache.org/r/51737/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/539d67f6
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/539d67f6
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/539d67f6
Branch: refs/heads/master
Commit: 539d67f6cd4dd15c28ae0e6ff4f90f927e801329
Parents: fba4c1e
Author: Avinash sridharan <av...@mesosphere.io>
Authored: Wed Oct 12 09:48:46 2016 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Wed Oct 12 10:59:45 2016 -0700
----------------------------------------------------------------------
.../network/cni/plugins/port_mapper/main.cpp | 11 +--
.../cni/plugins/port_mapper/port_mapper.cpp | 74 +++++++++++---------
.../cni/plugins/port_mapper/port_mapper.hpp | 5 +-
.../mesos/isolators/network/cni/spec.hpp | 17 +++++
4 files changed, 64 insertions(+), 43 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/539d67f6/src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/main.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/main.cpp b/src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/main.cpp
index 11c03fd..0bb58c4 100644
--- a/src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/main.cpp
+++ b/src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/main.cpp
@@ -34,6 +34,7 @@ using std::string;
using process::Owned;
using mesos::internal::slave::cni::PortMapper;
+using mesos::internal::slave::cni::spec::PluginError;
constexpr int STDIN_READ_LENGTH = 1000;
@@ -60,19 +61,13 @@ int main(int argc, char** argv)
return EXIT_FAILURE;
}
- // If the `PortMapper` returns an error it will already be a JSON
- // formatted string of type `spec::Error` so we don't need to format
- // it again. Reason we rely on the `PortMapper` to return a JSON
- // formatted `spec::Error` is that the error codes for `spec::Error`
- // might vary, depending on the cause of error, and the context of
- // the error is only visible to the `PortMapper` object.
- Try<Owned<PortMapper>> portMapper = PortMapper::create(config);
+ Try<Owned<PortMapper>, PluginError> portMapper = PortMapper::create(config);
if (portMapper.isError()) {
cout << portMapper.error() << endl;
return EXIT_FAILURE;
}
- Try<string> result = portMapper.get()->execute();
+ Try<string, PluginError> result = portMapper.get()->execute();
if (result.isError()) {
cout << result.error() << endl;
return EXIT_FAILURE;
http://git-wip-us.apache.org/repos/asf/mesos/blob/539d67f6/src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp b/src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
index 836fed5..5caa6d7 100644
--- a/src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
+++ b/src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
@@ -17,6 +17,12 @@
#include <stout/os.hpp>
#include <stout/protobuf.hpp>
+#include <process/collect.hpp>
+#include <process/dispatch.hpp>
+#include <process/io.hpp>
+#include <process/subprocess.hpp>
+
+#include "slave/containerizer/mesos/isolators/network/cni/spec.hpp"
#include "slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.hpp"
using std::string;
@@ -27,18 +33,20 @@ using process::Owned;
using mesos::NetworkInfo;
+using mesos::internal::slave::cni::spec::PluginError;
+
namespace mesos {
namespace internal {
namespace slave {
namespace cni {
-Try<Owned<PortMapper>> PortMapper::create(const string& _cniConfig)
+Try<Owned<PortMapper>, PluginError> PortMapper::create(const string& _cniConfig)
{
Option<string> cniCommand = os::getenv("CNI_COMMAND");
if (cniCommand.isNone()) {
- return Error(spec::error(
+ return PluginError(
"Unable to find environment variable 'CNI_COMMAND'",
- ERROR_BAD_ARGS));
+ ERROR_BAD_ARGS);
}
// 'CNI_CONTAINERID' is optional.
@@ -46,16 +54,16 @@ Try<Owned<PortMapper>> PortMapper::create(const string& _cniConfig)
Option<string> cniNetNs = os::getenv("CNI_NETNS");
if (cniNetNs.isNone()) {
- return Error(spec::error(
+ return PluginError(
"Unable to find environment variable 'CNI_NETNS'",
- ERROR_BAD_ARGS));
+ ERROR_BAD_ARGS);
}
Option<string> cniIfName = os::getenv("CNI_IFNAME");
if (cniIfName.isNone()) {
- return Error(spec::error(
+ return PluginError(
"Unable to find environment variable 'CNI_IFNAME'",
- ERROR_BAD_ARGS));
+ ERROR_BAD_ARGS);
}
// 'CNI_ARGS' is optional.
@@ -63,33 +71,33 @@ Try<Owned<PortMapper>> PortMapper::create(const string& _cniConfig)
Option<string> cniPath = os::getenv("CNI_PATH");
if (cniPath.isNone()) {
- return Error(spec::error(
+ return PluginError(
"Unable to find environment variable 'CNI_PATH'",
- ERROR_BAD_ARGS));
+ ERROR_BAD_ARGS);
}
// Verify the CNI config for this plugin.
Try<JSON::Object> cniConfig = JSON::parse<JSON::Object>(_cniConfig);
if (cniConfig.isError()) {
- return Error(spec::error(cniConfig.error(), ERROR_BAD_ARGS));
+ return PluginError(cniConfig.error(), ERROR_BAD_ARGS);
}
// TODO(jieyu): Validate 'cniVersion' and 'type'.
Result<JSON::String> name = cniConfig->find<JSON::String>("name");
if (!name.isSome()) {
- return Error(spec::error(
+ return PluginError(
"Failed to get the required field 'name': " +
(name.isError() ? name.error() : "Not found"),
- ERROR_BAD_ARGS));
+ ERROR_BAD_ARGS);
}
Result<JSON::String> chain = cniConfig->find<JSON::String>("chain");
if (!chain.isSome()) {
- return Error(spec::error(
+ return PluginError(
"Failed to get the required field 'chain': " +
(chain.isError() ? chain.error() : "Not found"),
- ERROR_BAD_ARGS));
+ ERROR_BAD_ARGS);
}
vector<string> excludeDevices;
@@ -98,17 +106,17 @@ Try<Owned<PortMapper>> PortMapper::create(const string& _cniConfig)
cniConfig->find<JSON::Array>("excludeDevices");
if (_excludeDevices.isError()) {
- return Error(spec::error(
+ return PluginError(
"Failed to parse field 'excludeDevices': " +
_excludeDevices.error(),
- ERROR_BAD_ARGS));
+ ERROR_BAD_ARGS);
} else if (_excludeDevices.isSome()) {
foreach (const JSON::Value& value, _excludeDevices->values) {
if (!value.is<JSON::String>()) {
- return Error(spec::error(
+ return PluginError(
"Failed to parse 'excludeDevices' list. "
"The excluded device needs to be a string",
- ERROR_BAD_ARGS));
+ ERROR_BAD_ARGS);
}
excludeDevices.push_back(value.as<JSON::String>().value);
@@ -120,10 +128,10 @@ Try<Owned<PortMapper>> PortMapper::create(const string& _cniConfig)
// framework might have requested for this container.
Result<JSON::Object> args = cniConfig->find<JSON::Object>("args");
if (!args.isSome()) {
- return Error(spec::error(
+ return PluginError(
"Failed to get the required field 'args': " +
(args.isError() ? args.error() : "Not found"),
- ERROR_BAD_ARGS));
+ ERROR_BAD_ARGS);
}
// NOTE: We can't directly use `find` to check for 'network_info'
@@ -133,27 +141,27 @@ Try<Owned<PortMapper>> PortMapper::create(const string& _cniConfig)
// 'network_info'.
Result<JSON::Object> mesos = args->at<JSON::Object>("org.apache.mesos");
if (!mesos.isSome()) {
- return Error(spec::error(
+ return PluginError(
"Failed to get the field 'args{org.apache.mesos}': " +
(mesos.isError() ? mesos.error() : "Not found"),
- ERROR_BAD_ARGS));
+ ERROR_BAD_ARGS);
}
Result<JSON::Object> _networkInfo = mesos->find<JSON::Object>("network_info");
if (!_networkInfo.isSome()) {
- return Error(spec::error(
+ return PluginError(
"Failed to get the field 'args{org.apache.mesos}{network_info}': " +
(_networkInfo.isError() ? _networkInfo.error() : "Not found"),
- ERROR_BAD_ARGS));
+ ERROR_BAD_ARGS);
}
Try<NetworkInfo> networkInfo =
::protobuf::parse<NetworkInfo>(_networkInfo.get());
if (networkInfo.isError()) {
- return Error(spec::error(
+ return PluginError(
"Unable to parse `NetworkInfo`: " + networkInfo.error(),
- ERROR_BAD_ARGS));
+ ERROR_BAD_ARGS);
}
// The port-mapper should always be used in conjunction with another
@@ -162,10 +170,10 @@ Try<Owned<PortMapper>> PortMapper::create(const string& _cniConfig)
cniConfig->find<JSON::Object>("delegate");
if (!delegateConfig.isSome()) {
- return Error(spec::error(
+ return PluginError(
"Failed to get the required field 'delegate'" +
(delegateConfig.isError() ? delegateConfig.error() : "Not found"),
- ERROR_BAD_ARGS));
+ ERROR_BAD_ARGS);
}
// TODO(jieyu): Validate that 'cniVersion' and 'name' exist in
@@ -176,17 +184,17 @@ Try<Owned<PortMapper>> PortMapper::create(const string& _cniConfig)
delegateConfig->find<JSON::String>("type");
if (!delegatePlugin.isSome()) {
- return Error(spec::error(
+ return PluginError(
"Failed to get the delegate plugin 'type'" +
(delegatePlugin.isError() ? delegatePlugin.error() : "Not found"),
- ERROR_BAD_ARGS));
+ ERROR_BAD_ARGS);
}
if (os::which(delegatePlugin->value, cniPath.get()).isNone()) {
- return Error(spec::error(
+ return PluginError(
"Could not find the delegate plugin '" + delegatePlugin->value +
"' in '" + cniPath.get() + "'",
- ERROR_BAD_ARGS));
+ ERROR_BAD_ARGS);
}
return Owned<PortMapper>(
@@ -205,7 +213,7 @@ Try<Owned<PortMapper>> PortMapper::create(const string& _cniConfig)
}
-Try<string> PortMapper::execute()
+Try<string, PluginError> PortMapper::execute()
{
return "OK";
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/539d67f6/src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.hpp b/src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.hpp
index b943254..c0bbef6 100644
--- a/src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.hpp
+++ b/src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.hpp
@@ -65,7 +65,8 @@ public:
//
// In case of an error returns a JSON formatted string of type
// `spec::Error` as the error message for the `Try`.
- static Try<process::Owned<PortMapper>> create(const std::string& cniConfig);
+ static Try<process::Owned<PortMapper>, spec::PluginError> create(
+ const std::string& cniConfig);
// Executes the CNI plugin specified in 'delegate'. On successful
// execution of the 'delegate' plugin will install port-forwarding
@@ -74,7 +75,7 @@ public:
// 'delegate' plugin. In case of an error will return a JSON string
// representation of `spec::Error` as the error message for the
// `Try`.
- Try<std::string> execute();
+ Try<std::string, spec::PluginError> execute();
virtual ~PortMapper() {};
http://git-wip-us.apache.org/repos/asf/mesos/blob/539d67f6/src/slave/containerizer/mesos/isolators/network/cni/spec.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/network/cni/spec.hpp b/src/slave/containerizer/mesos/isolators/network/cni/spec.hpp
index 2d0187b..9f0542a 100644
--- a/src/slave/containerizer/mesos/isolators/network/cni/spec.hpp
+++ b/src/slave/containerizer/mesos/isolators/network/cni/spec.hpp
@@ -48,6 +48,23 @@ Try<NetworkInfo> parseNetworkInfo(const std::string& s);
// https://github.com/containernetworking/cni/blob/master/SPEC.md#result
std::string error(const std::string& msg, uint32_t code);
+
+// This class encapsulates a JSON formatted string of type
+// `spec::Error`. It can be used by CNI plugins to return a CNI error
+// in a `Try` object when a failure occurs.
+class PluginError : public ::Error
+{
+public:
+ PluginError(const std::string& msg, uint32_t code)
+ : ::Error(error(msg, code)) {}
+};
+
+
+inline std::ostream& operator<<(std::ostream& stream, const PluginError& _error)
+{
+ return stream << _error.message;
+}
+
} // namespace spec {
} // namespace cni {
} // namespace slave {