You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by "andreaspeters (via GitHub)" <gi...@apache.org> on 2024/09/25 19:30:20 UTC
[PR] ADD: cni support for the docker executor. [mesos]
andreaspeters opened a new pull request, #586:
URL: https://github.com/apache/mesos/pull/586
With this PR I would like to add support for CNI (ContainerNetworkInterface) to the Docker Executor. As far as possible, I have followed the CNI implementation of the Mesos Containerizer.
The following parameters and their default values are added to the Docker Executor:
- network_cni_plugins_dir: /usr/lib/cni/
- network_cni_config_dir: /etc/mesos/cni/net.d
**How is CNI used with Docker?**
By setting a name on the mesosproto NetworkInfo object which matches the CNI name in the CNI configuration.
Here is an example of a CNI configuration:
```
cat /etc/mesos/cni/net.d/10-mesos-net.conf
{
"cniVersion": "0.2.0",
"name": "mesos-net",
"type": "ipvlan",
"bridge": "cni0",
"isGateway": true,
"ipMasq": true,
"vlanId": 5,
"ipam": {
"type": "host-local",
"ranges": [
[
{
"subnet": "10.10.0.0/16",
"rangeStart": "10.10.0.10",
"rangeEnd": "10.10.0.250"
}
]
],
"routes": [
{ "dst": "0.0.0.0/0" }
]
}
}
```
**How does the Docker Executor work?**
If the name of the mesosproto NetworkInfo object has been set, the executor checks whether a CNI configuration exists for it. If this is the case, it is used to create a network interface in the container. If no CNI configuration exists, an error message appears in the stderr file in the sandbox directory. As the name can also be a Docker Network Plugin, the container is started in any case. If there is no Docker Network Plugin with the name, the container runs without an additional network interface.
**How can the CNI support be tested?**
I have added a test that can be used via the Mesos Test Tool.
`mesos-tests --verbose --gtest_filter="DockerCniTest.*”`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@mesos.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ADD: cni support for the docker executor. [mesos]
Posted by "andreaspeters (via GitHub)" <gi...@apache.org>.
andreaspeters commented on PR #586:
URL: https://github.com/apache/mesos/pull/586#issuecomment-2455589746
Friendly ping :-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@mesos.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ADD: cni support for the docker executor. [mesos]
Posted by "andreaspeters (via GitHub)" <gi...@apache.org>.
andreaspeters commented on code in PR #586:
URL: https://github.com/apache/mesos/pull/586#discussion_r1943641141
##########
src/launcher/docker_executor.cpp:
##########
@@ -148,6 +148,14 @@ int main(int argc, char** argv)
<< flags.usage("Missing required option --mapped_directory");
}
+ if (flags.network_cni_plugins_dir.isNone()) {
+ flags.network_cni_plugins_dir = "/usr/lib/cni/";
+ }
+
+ if (flags.network_cni_config_dir.isNone()) {
+ flags.network_cni_config_dir = "/etc/mesos/cni/net.d";
+ }
Review Comment:
👍 your right. I think the behavior should be like with the mesos executor. I will change that to:
- It has to be set when we want to use CNI
- No default values
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@mesos.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ADD: cni support for the docker executor. [mesos]
Posted by "andreaspeters (via GitHub)" <gi...@apache.org>.
andreaspeters commented on code in PR #586:
URL: https://github.com/apache/mesos/pull/586#discussion_r1943694563
##########
src/docker/executor.cpp:
##########
@@ -785,6 +825,261 @@ class DockerExecutorProcess : public ProtobufProcess<DockerExecutorProcess>
driver.get()->stop();
}
+ // Return the content of the CNI plugin config file and return it as json object
+ Try<JSON::Object> getNetworkConfigJSON(const string& fname)
+ {
+ Try<string> read = os::read(fname);
+ if (read.isError()) {
+ return Error("Cannot read CNI plugin file: " + read.error());
+ }
+
+ Try<JSON::Object> parse = JSON::parse<JSON::Object>(read.get());
+ if (parse.isError()) {
+ return Error("Cannot parse CNI plugin file." + parse.error());
+ }
+
+ return parse;
+ }
+
+ // Search the config file of the network in the path
+ Try<std::string> getNetworkConfigFile(const string& network, const string& path)
+ {
+ struct dirent *ent;
+ DIR *dir;
+ JSON::Object parse;
+
+ // path can include mutiple path's seperated by ":".
+ char *sPath = strtok(const_cast<char*>(path.c_str()), ":");
+ while (sPath != NULL) {
+ if ((dir = opendir(sPath)) != NULL) {
+ while ((ent = readdir(dir)) != NULL) {
+ if (ent->d_type != DT_REG || !strcmp(ent->d_name, ".") || !strcmp(ent->d_name, "..")) {
+ continue;
+ }
+
+ std::string fname = string(sPath) + "/" + string(ent->d_name);
+ Try<string> read = os::read(fname);
+ if (read.isError()) {
+ LOG(ERROR) << "Cannot read CNI network config file: " << fname;
+ continue;
+ }
+
+ Try<JSON::Object> parse = JSON::parse<JSON::Object>(read.get());
+ if (parse.isError()) {
+ LOG(ERROR) << "Cannot parse CNI network config file.";
+ continue;
+ }
+
+ Result<JSON::String> name = parse->at<JSON::String>("name");
+ if (!name.isSome()) {
+ return Error(
+ "Cannot determine the 'name' of the CNI network for this "
+ "configuration " +
+ (name.isNone() ? "'" : ("': " + name.error())));
+ }
+
+ // Verify the configuration is for this network
+ if (network == name->value) {
+ return fname;
+ }
+ }
+ closedir(dir);
+ }
+ sPath = strtok(NULL, ":");
+ }
+ return Error("Cannot find a CNI plugin config for this network name: " + network);
+ }
+
+ // attach CNI at the container
+ Try<std::tuple<Subprocess, std::string>> attachCNI(const TaskInfo& task)
+ {
+ container = task.container();
+ return commandCNI(container, "ADD");
+ }
+
+ // detach CNI from the container
+ Try<std::tuple<Subprocess, std::string>> detachCNI()
+ {
+ return commandCNI(container, "DEL");
+ }
+
+ // function to attach or detach CNI from/to the container
+ Try<std::tuple<Subprocess, std::string>> commandCNI(
+ const mesos::ContainerInfo& container,
+ const string command)
Review Comment:
It returns the CNI name in line 1011. We need it in line 335. But maybe I also misunderstand you. 😅
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@mesos.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ADD: cni support for the docker executor. [mesos]
Posted by "andreaspeters (via GitHub)" <gi...@apache.org>.
andreaspeters commented on PR #586:
URL: https://github.com/apache/mesos/pull/586#issuecomment-2456553303
> This could use a more clear overview of the approach and its limitations, shouldn't we be using docker network plugins?
>
> https://docs.docker.com/engine/extend/plugins_network/#use-network-driver-plugins
>
> That seems like the right way to do this? Whereas the approach taken in this patch looks problematic and looks like a hack? We're doing CNI stuff **after** we've `docker run` the container?
There is no official "Docker Inc" way to support CNI. They prefer to use there own docker-network plugin. Problem with that is, the plugins are pretty old. Some of them are even not in development anymore but still on the official list. 🤷🏻♂️
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@mesos.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ADD: cni support for the docker executor. [mesos]
Posted by "andreaspeters (via GitHub)" <gi...@apache.org>.
andreaspeters commented on code in PR #586:
URL: https://github.com/apache/mesos/pull/586#discussion_r1943675117
##########
src/docker/executor.cpp:
##########
@@ -318,7 +329,26 @@ class DockerExecutorProcess : public ProtobufProcess<DockerExecutorProcess>
}
};
- if (container.ipAddress.isSome()) {
+ bool useCNI = false;
+#ifdef __linux__
+ if (network_cni_config_dir.size() > 0 && network_cni_plugins_dir.size() > 0) {
+ Try<std::tuple<Subprocess, std::string>> cni = attachCNI(task);
+ if (!cni.isError()) {
+ Subprocess cniSub = std::get<0>(cni.get());
+ std::string cniNetworkName = std::get<1>(cni.get());
+
+ WIFEXITED(cniSub.status().get().get());
+ Try<std::string> cniIP = attachCNISuccess(cniSub);
+ if (cniIP.isError()) {
+ LOG(ERROR) << cniIP.error();
+ } else {
+ useCNI = true;
+ setIPAddresses(cniIP.get(), false);
+ }
+ }
Review Comment:
We can only add CNI if the container is already running since there is no way for us to "infiltrate" the container during the startup.
There are up to one second where the workload could have connection issues. But the container will not fail (crash). That's a point we have to write as notice.
My personal opinion is: Docker inc. should add CNI support to there engine. But they will not and on the other hand they also do not care there network plugins. 🤷🏻♂️ At least with these CNI implementation we can use a wide range of new network interfaces with our docker executor and does not have to use unmaintained docker network plugins.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@mesos.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ADD: cni support for the docker executor. [mesos]
Posted by "andreaspeters (via GitHub)" <gi...@apache.org>.
andreaspeters commented on code in PR #586:
URL: https://github.com/apache/mesos/pull/586#discussion_r1943655611
##########
src/docker/executor.cpp:
##########
@@ -318,7 +329,26 @@ class DockerExecutorProcess : public ProtobufProcess<DockerExecutorProcess>
}
};
- if (container.ipAddress.isSome()) {
+ bool useCNI = false;
+#ifdef __linux__
+ if (network_cni_config_dir.size() > 0 && network_cni_plugins_dir.size() > 0) {
Review Comment:
> !x.empty()
Oh... much better, thanks. :-)
> Also, does it make sense for only one to be set?
I don't think so.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@mesos.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ADD: cni support for the docker executor. [mesos]
Posted by "bmahler (via GitHub)" <gi...@apache.org>.
bmahler commented on code in PR #586:
URL: https://github.com/apache/mesos/pull/586#discussion_r1828426706
##########
src/tests/containerizer/docker_cni_tests.cpp:
##########
@@ -0,0 +1,326 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY K:5IND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <gmock/gmock.h>
+
+#include <gtest/gtest.h>
+
+#include <mesos/v1/mesos.hpp>
+#include <mesos/slave/container_logger.hpp>
+
+#include <process/owned.hpp>
+
+#include "slave/containerizer/docker.hpp"
+#include "slave/containerizer/fetcher.hpp"
+#include "slave/containerizer/mesos/isolators/network/cni/cni.hpp"
+
+#include "slave/paths.hpp"
+#include "slave/slave.hpp"
+#include "slave/state.hpp"
+
+#include "tests/flags.hpp"
+#include "tests/mesos.hpp"
+#include "tests/environment.hpp"
+#include "tests/containerizer/docker_common.hpp"
+#include "tests/mock_docker.hpp"
+
+
+using namespace process;
+
+using namespace mesos::internal::slave::paths;
+using namespace mesos::internal::slave::state;
+
+using mesos::internal::master::Master;
+using mesos::internal::slave::DockerContainerizer;
+using mesos::internal::slave::DockerContainerizerProcess;
+using mesos::internal::slave::Fetcher;
+using mesos::internal::slave::Slave;
+
+using mesos::master::detector::MasterDetector;
+
+using mesos::slave::ContainerConfig;
+using mesos::slave::ContainerLogger;
+using mesos::slave::ContainerTermination;
+
+using std::list;
+using std::string;
+using std::vector;
+
+using testing::_;
+using testing::DoAll;
+using testing::DoDefault;
+using testing::Eq;
+using testing::Invoke;
+using testing::Return;
+
+namespace mesos {
+namespace internal {
+namespace tests {
+
+constexpr char MESOS_MOCK_CNI_CONFIG[] = "mockConfig";
+
+
+static ContainerInfo createDockerInfo(const string& imageName)
+{
+ ContainerInfo containerInfo;
+
+ containerInfo.set_type(ContainerInfo::DOCKER);
+ containerInfo.mutable_docker()->set_image(imageName);
+
+ return containerInfo;
+}
+
+class DockerCniTest : public MesosTest
+{
+public:
+ string cniPluginDir;
+ string cniConfigDir;
+
+ void SetUp() override
+ {
+ MesosTest::SetUp();
+
+ cniPluginDir = path::join(sandbox.get(), "plugins");
+ cniConfigDir = path::join(sandbox.get(), "configs");
+
+ // This is a veth CNI plugin that is written in bash. It creates a
+ // veth virtual network pair, one end of the pair will be moved to
+ // container network namespace.
+ //
+ // The veth CNI plugin uses 203.0.113.0/24 subnet, it is reserved
+ // for documentation and examples [rfc5737]. The plugin can
+ // allocate up to 128 veth pairs.
+ Try<Nothing> result = setupMockPlugin(
+ strings::format(R"~(
+ #!/bin/sh
+ set -e
+ IP_ADDR="203.0.113.10/32"
+
+ NETNS="mesos-test-veth-${PPID}"
+ mkdir -p /var/run/netns
+ cleanup() {
+ rm -f /var/run/netns/$NETNS
+ }
+ trap cleanup EXIT
+ ln -sf $CNI_NETNS /var/run/netns/$NETNS
+
+ VETH0="vmesos1"
+ VETH1="eth0"
+
+ case $CNI_COMMAND in
+ "ADD"*)
+ ip link delete $VETH0 2>/dev/null || true
+ ip netns exec $NETNS ip link delete $VETH1 2>/dev/null || true
+
+ ip link add name $VETH0 type veth peer name $VETH1 > /tmp/cni.log || continue
+ ip addr add "${IP_ADDR}" dev $VETH0 >> /tmp/cni.log
+ ip link set $VETH0 up
+ ip link set $VETH1 netns $NETNS
+ ip netns exec $NETNS ip addr add "${IP_ADDR}" dev $VETH1
+ ip netns exec $NETNS ip link set $VETH1 up
+
+ echo "{"
+ echo " \"ip4\": {"
+ echo " \"ip\": \"${IP_ADDR}\""
+ echo " },"
+ echo " \"dns\": {"
+ echo " \"nameservers\": [ \"8.8.8.8\" ]"
+ echo " }"
+ echo "}"
+ ;;
+ "DEL"*)
+ # $VETH0 on host network namespace will be removed automatically.
+ # If the plugin can't destroy the veth pair, it will be destroyed
+ # with the container network namespace.
+ ip link delete $VETH0 2>/dev/null || true
+ ip netns exec $NETNS ip link del $VETH1 || true
+ ;;
+ esac
+ )~").get());
+
+ ASSERT_SOME(result);
+
+ // Generate the mock CNI config for veth CNI plugin.
+ ASSERT_SOME(os::mkdir(cniConfigDir));
+
+ result = os::write(
+ path::join(cniConfigDir, MESOS_MOCK_CNI_CONFIG),
+ R"~(
+ {
+ "name": "__MESOS_TEST__",
+ "type": "mockPlugin"
+ })~");
+ }
+
+ // Generate the mock CNI plugin based on the given script.
+ Try<Nothing> setupMockPlugin(const string& pluginScript)
+ {
+ return setupPlugin(pluginScript, "mockPlugin");
+ }
+
+ // Generate the CNI plugin based on the given script.
+ Try<Nothing> setupPlugin(const string& pluginScript, const string& pluginName)
+ {
+ Try<Nothing> mkdir = os::mkdir(cniPluginDir);
+ if (mkdir.isError()) {
+ return Error("Failed to mkdir '" + cniPluginDir + "': " + mkdir.error());
+ }
+
+ string pluginPath = path::join(cniPluginDir, pluginName);
+
+ Try<Nothing> write = os::write(pluginPath, pluginScript);
+ if (write.isError()) {
+ return Error("Failed to write '" + pluginPath + "': " + write.error());
+ }
+
+ // Make sure the plugin has execution permission.
+ Try<Nothing> chmod = os::chmod(
+ pluginPath,
+ S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH);
+
+ if (chmod.isError()) {
+ return Error("Failed to chmod '" + pluginPath + "': " + chmod.error());
+ }
+
+ return Nothing();
+ }
+
+ static string containerName(const ContainerID& containerId)
+ {
+ return slave::DOCKER_NAME_PREFIX + containerId.value();
+ }
+
+protected:
+ slave::Flags CreateSlaveFlags() override
+ {
+ slave::Flags flags = MesosTest::CreateSlaveFlags();
+
+ flags.network_cni_plugins_dir = cniPluginDir;
+ flags.network_cni_config_dir = cniConfigDir;
+
+
+ return flags;
+ }
+};
+
+// This test tests the functionality of the docker's interfaces.
+TEST_F(DockerCniTest, ROOT_DOCKER_ip_interface)
+{
+ SetUp();
Review Comment:
nit: inconsistent indentation in your patch, can you sweep it to use a 2 space indent?
##########
src/docker/executor.cpp:
##########
@@ -318,7 +329,26 @@ class DockerExecutorProcess : public ProtobufProcess<DockerExecutorProcess>
}
};
- if (container.ipAddress.isSome()) {
+ bool useCNI = false;
+#ifdef __linux__
+ if (network_cni_config_dir.size() > 0 && network_cni_plugins_dir.size() > 0) {
Review Comment:
use `!x.empty()`? Or it seems like these should be `Option<string>` instead to explicitly type in the optionality and avoid empty string == unset
Also, does it make sense for only one to be set? If not, should there be some validation somewhere for this?
##########
src/docker/executor.cpp:
##########
@@ -318,7 +329,26 @@ class DockerExecutorProcess : public ProtobufProcess<DockerExecutorProcess>
}
};
- if (container.ipAddress.isSome()) {
+ bool useCNI = false;
+#ifdef __linux__
+ if (network_cni_config_dir.size() > 0 && network_cni_plugins_dir.size() > 0) {
+ Try<std::tuple<Subprocess, std::string>> cni = attachCNI(task);
+ if (!cni.isError()) {
+ Subprocess cniSub = std::get<0>(cni.get());
+ std::string cniNetworkName = std::get<1>(cni.get());
+
+ WIFEXITED(cniSub.status().get().get());
+ Try<std::string> cniIP = attachCNISuccess(cniSub);
+ if (cniIP.isError()) {
+ LOG(ERROR) << cniIP.error();
+ } else {
+ useCNI = true;
+ setIPAddresses(cniIP.get(), false);
+ }
+ }
Review Comment:
hm.. this looks like a bad design? docker run has already been called, *and then* we attach CNI? This means the workload could have started and failed to perform networking operations? It seems like the correct way to do this is to perform CNI work while setting up the container, whereas this looks like a hack where it's set up *after* the container has been started already?
##########
src/launcher/docker_executor.cpp:
##########
@@ -148,6 +148,14 @@ int main(int argc, char** argv)
<< flags.usage("Missing required option --mapped_directory");
}
+ if (flags.network_cni_plugins_dir.isNone()) {
+ flags.network_cni_plugins_dir = "/usr/lib/cni/";
+ }
+
+ if (flags.network_cni_config_dir.isNone()) {
+ flags.network_cni_config_dir = "/etc/mesos/cni/net.d";
+ }
Review Comment:
These defaults look a bit odd, as if the agent's CNI flags actually have default values only when used with the docker executor? It seems like the agent flags should be directly used and therefore the operator has to set these explicitly? Will that cause a problem for you?
Right now this is a bit odd:
1. Don't set the agent's CNI flags, this gets you these default values only for docker containerizer / executor, but no defaults for the mesos containerizer. That looks strange.
2. Set the agent's CNI flags, in which case both the docker containerizer / mesos containerizer would use the values?
##########
src/docker/executor.cpp:
##########
@@ -785,6 +825,261 @@ class DockerExecutorProcess : public ProtobufProcess<DockerExecutorProcess>
driver.get()->stop();
}
+ // Return the content of the CNI plugin config file and return it as json object
+ Try<JSON::Object> getNetworkConfigJSON(const string& fname)
+ {
+ Try<string> read = os::read(fname);
+ if (read.isError()) {
+ return Error("Cannot read CNI plugin file: " + read.error());
+ }
+
+ Try<JSON::Object> parse = JSON::parse<JSON::Object>(read.get());
+ if (parse.isError()) {
+ return Error("Cannot parse CNI plugin file." + parse.error());
+ }
+
+ return parse;
+ }
+
+ // Search the config file of the network in the path
+ Try<std::string> getNetworkConfigFile(const string& network, const string& path)
+ {
+ struct dirent *ent;
+ DIR *dir;
+ JSON::Object parse;
+
+ // path can include mutiple path's seperated by ":".
+ char *sPath = strtok(const_cast<char*>(path.c_str()), ":");
+ while (sPath != NULL) {
+ if ((dir = opendir(sPath)) != NULL) {
+ while ((ent = readdir(dir)) != NULL) {
+ if (ent->d_type != DT_REG || !strcmp(ent->d_name, ".") || !strcmp(ent->d_name, "..")) {
+ continue;
+ }
+
+ std::string fname = string(sPath) + "/" + string(ent->d_name);
+ Try<string> read = os::read(fname);
+ if (read.isError()) {
+ LOG(ERROR) << "Cannot read CNI network config file: " << fname;
+ continue;
+ }
+
+ Try<JSON::Object> parse = JSON::parse<JSON::Object>(read.get());
+ if (parse.isError()) {
+ LOG(ERROR) << "Cannot parse CNI network config file.";
+ continue;
+ }
+
+ Result<JSON::String> name = parse->at<JSON::String>("name");
+ if (!name.isSome()) {
+ return Error(
+ "Cannot determine the 'name' of the CNI network for this "
+ "configuration " +
+ (name.isNone() ? "'" : ("': " + name.error())));
+ }
+
+ // Verify the configuration is for this network
+ if (network == name->value) {
+ return fname;
+ }
+ }
+ closedir(dir);
+ }
+ sPath = strtok(NULL, ":");
+ }
+ return Error("Cannot find a CNI plugin config for this network name: " + network);
+ }
+
+ // attach CNI at the container
+ Try<std::tuple<Subprocess, std::string>> attachCNI(const TaskInfo& task)
+ {
+ container = task.container();
+ return commandCNI(container, "ADD");
+ }
+
+ // detach CNI from the container
+ Try<std::tuple<Subprocess, std::string>> detachCNI()
+ {
+ return commandCNI(container, "DEL");
+ }
+
+ // function to attach or detach CNI from/to the container
+ Try<std::tuple<Subprocess, std::string>> commandCNI(
+ const mesos::ContainerInfo& container,
+ const string command)
Review Comment:
feels like these should probably just return a Future of whatever information you want? not clear what the string here would represent
##########
src/docker/executor.hpp:
##########
@@ -90,6 +90,20 @@ struct Flags : public virtual mesos::internal::logging::Flags
"Cgroups feature flag to enable hard limits on CPU resources\n"
"via the CFS bandwidth limiting subfeature.\n",
false);
+
+ add(&Flags::network_cni_plugins_dir,
+ "network_cni_plugins_dir",
+ "A search path for CNI plugin binaries. The docker executer\n"
+ "will find CNI plugins under these set of directories so that\n"
+ "it can execute the plugins to add/delete container from the CNI\n"
+ "networks.");
+
+ add(&Flags::network_cni_config_dir,
+ "network_cni_config_dir",
+ "Directory path of the CNI network configuration files. For each\n"
+ "network that containers launched in Mesos agent can connect to,\n"
+ "the operator should install a network configuration file in JSON\n"
+ "format in the specified directory.");
Review Comment:
nit: inconsistent alignment
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@mesos.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org