This is an automated email from the ASF dual-hosted git repository.
jieyu pushed a commit to branch 1.6.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 7d979c79470c4adbb41d55af1953a398f91ad2cc
Author: Jie Yu <yujie....@gmail.com>
AuthorDate: Thu Jan 10 12:38:04 2019 -0800
Kept `CNI_NETNS` unset in detach if network namespace is gone.
We introduced a new agent flag in MESOS-9492 so that CNI configs can be
persisted across reboot. This is for some CNI plugins to be able to
cleanup IP allocated to the containers after a sudden reboot of the host
(not all CNI plugins need this).
It's important to unset `CNI_NETNS` environment variable after reboot
when invoking CNI plugin "DEL" command so that it conforms to the spec.
Review: https://reviews.apache.org/r/69706
(cherry picked from commit 9863daca0baaa52984dc42c27c6e4e33c3b169b5)
---
.../mesos/isolators/network/cni/cni.cpp | 103 +++++++++++++++------
1 file changed, 75 insertions(+), 28 deletions(-)
diff --git a/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
b/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
index 1ad3e2a..ecadfe4 100644
--- a/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
+++ b/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
@@ -1472,6 +1472,30 @@ Future<ContainerStatus>
NetworkCniIsolatorProcess::status(
}
+static Try<bool> isNetworkNamespaceHandle(const string& netNsHandle)
+{
+ // Determine if a path is a network namespace handle or not by
+ // checking its device number against that of '/proc'. If they are
+ // not the same, the given path shouldn't be a network namespac
+ // handle.
+ Try<dev_t> netNsHandleDev = os::stat::dev(netNsHandle);
+ if (netNsHandleDev.isError()) {
+ return Error(
+ "Failed to get the device number of '" + netNsHandle +
+ "': " + netNsHandleDev.error());
+ }
+
+ Try<dev_t> procDev = os::stat::dev("/proc");
+ if (procDev.isError()) {
+ return Error(
+ "Failed to get the device number of '/proc'"
+ ": " + procDev.error());
+ }
+
+ return netNsHandleDev.get() == procDev.get();
+}
+
+
Future<Nothing> NetworkCniIsolatorProcess::cleanup(
const ContainerID& containerId)
{
@@ -1543,15 +1567,22 @@ Future<Nothing> NetworkCniIsolatorProcess::_cleanup(
paths::getNamespacePath(rootDir.get(), containerId);
if (os::exists(target)) {
- Try<Nothing> unmount = fs::unmount(target);
- if (unmount.isError()) {
- return Failure(
- "Failed to unmount the network namespace handle '" +
- target + "': " + unmount.error());
+ Try<bool> isNetNsHandle = isNetworkNamespaceHandle(target);
+ if (isNetNsHandle.isError()) {
+ return Failure(isNetNsHandle.error());
}
- LOG(INFO) << "Unmounted the network namespace handle '"
- << target << "' for container " << containerId;
+ if (isNetNsHandle.get()) {
+ Try<Nothing> unmount = fs::unmount(target);
+ if (unmount.isError()) {
+ return Failure(
+ "Failed to unmount the network namespace handle '" +
+ target + "': " + unmount.error());
+ }
+
+ LOG(INFO) << "Unmounted the network namespace handle '"
+ << target << "' for container " << containerId;
+ }
}
if (os::exists(containerDir)) {
@@ -1581,27 +1612,6 @@ Future<Nothing> NetworkCniIsolatorProcess::detach(
const ContainerNetwork& containerNetwork =
infos[containerId]->containerNetworks[networkName];
- // Prepare environment variables for CNI plugin.
- CHECK_SOME(flags.network_cni_plugins_dir);
-
- map<string, string> environment;
- environment["CNI_COMMAND"] = "DEL";
- environment["CNI_CONTAINERID"] = stringify(containerId);
- environment["CNI_PATH"] = flags.network_cni_plugins_dir.get();
- environment["CNI_IFNAME"] = containerNetwork.ifName;
- environment["CNI_NETNS"] =
- paths::getNamespacePath(rootDir.get(), containerId);
-
- // Some CNI plugins need to run "iptables" to set up IP Masquerade, so we
- // need to set the "PATH" environment variable so that the plugin can locate
- // the "iptables" executable file.
- Option<string> value = os::getenv("PATH");
- if (value.isSome()) {
- environment["PATH"] = value.get();
- } else {
- environment["PATH"] = os::host_default_path();
- }
-
// Use the checkpointed CNI network configuration to call the
// CNI plugin to detach the container from the CNI network.
const string networkConfigPath = paths::getNetworkConfigPath(
@@ -1625,6 +1635,43 @@ Future<Nothing> NetworkCniIsolatorProcess::detach(
return Nothing();
}
+ // Prepare environment variables for CNI plugin.
+ CHECK_SOME(flags.network_cni_plugins_dir);
+
+ map<string, string> environment;
+ environment["CNI_COMMAND"] = "DEL";
+ environment["CNI_CONTAINERID"] = stringify(containerId);
+ environment["CNI_PATH"] = flags.network_cni_plugins_dir.get();
+ environment["CNI_IFNAME"] = containerNetwork.ifName;
+
+ // If the file is not a network namespace handle, do not set
+ // `CNI_NETNS`. This is possible after a reboot where all bind
+ // mounts of the network namespace handles are gone. According to
+ // the CNI spec, we should not set `CNI_NETNS` in such a case, but
+ // still call `DEL` command so that CNI plugins can do best effort
+ // cleanup (e.g., deallocating IP allocated for the container).
+ const string netNsHandle =
+ paths::getNamespacePath(rootDir.get(), containerId);
+
+ Try<bool> isNetNsHandle = isNetworkNamespaceHandle(netNsHandle);
+ if (isNetNsHandle.isError()) {
+ return Failure(isNetNsHandle.error());
+ }
+
+ if (isNetNsHandle.get()) {
+ environment["CNI_NETNS"] = netNsHandle;
+ }
+
+ // Some CNI plugins need to run "iptables" to set up IP Masquerade, so we
+ // need to set the "PATH" environment variable so that the plugin can locate
+ // the "iptables" executable file.
+ Option<string> value = os::getenv("PATH");
+ if (value.isSome()) {
+ environment["PATH"] = value.get();
+ } else {
+ environment["PATH"] = os::host_default_path();
+ }
+
Try<JSON::Object> networkConfigJSON = getNetworkConfigJSON(
networkName,
networkConfigPath);
