(mesos) branch master updated: [cgroups2] Separate responsibility for creating cgroup and assigning pids.

[bmahler]

This is an automated email from the ASF dual-hosted git repository.

bmahler pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git


The following commit(s) were added to refs/heads/master by this push:
     new 721ce9060 [cgroups2] Separate responsibility for creating cgroup and 
assigning pids.
721ce9060 is described below

commit 721ce9060c9c7943799a034ae4428c2fa51ffea1
Author: Jason Zhou <jasonzhou...@gmail.com>
AuthorDate: Thu Aug 15 18:08:48 2024 -0400

    [cgroups2] Separate responsibility for creating cgroup and assigning pids.
    
    In cgroups2, the current linux launcher does not create cgroups nor does
    it move the pids into the container's leaf cgroup during fork().
    
    When we launch a container, we first prepare it via the isolators,
    then the launcher will call fork to, among other things, move the pid
    into its appropriate cgroup. Once the fork is over, isolate() is called
    on the isolators.
    
    As such, we will remove the cgroups2 isolator's current behavior of
    assigning pids into leaf cgroups as it is already done by the linux
    launcher.
    
    Review: https://reviews.apache.org/r/75170/
---
 .../containerizer/mesos/isolators/cgroups2/cgroups2.cpp    |  9 ++-------
 src/slave/containerizer/mesos/linux_launcher.cpp           | 14 ++++++++++++++
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.cpp 
b/src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.cpp
index d1507ecd9..2bf253874 100644
--- a/src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.cpp
+++ b/src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.cpp
@@ -672,13 +672,8 @@ Future<Nothing> Cgroups2IsolatorProcess::_isolate(
         "Failed to find cgroup for container '" + stringify(containerId) + 
"'");
   }
 
-  // Move the process into its leaf cgroup.
-  Try<Nothing> assign = cgroups2::assign(info->cgroup_leaf, pid);
-  if (assign.isError()) {
-    return Failure("Failed to assign container '" + stringify(containerId) + 
"'"
-                   " to cgroup '" + info->cgroup_leaf + "': " + 
assign.error());
-  }
-
+  // At this point, the pid should already be placed in the leaf by linux
+  // launcher, no need to assign it ourselves.
   return Nothing();
 }
 
diff --git a/src/slave/containerizer/mesos/linux_launcher.cpp 
b/src/slave/containerizer/mesos/linux_launcher.cpp
index f651afee0..7110e8d11 100644
--- a/src/slave/containerizer/mesos/linux_launcher.cpp
+++ b/src/slave/containerizer/mesos/linux_launcher.cpp
@@ -624,6 +624,20 @@ Try<pid_t> LinuxLauncherProcess::fork(
           child);
       }));
     }
+  } else {
+    parentHooks.emplace_back(
+      Subprocess::ParentHook([=](pid_t child) -> Try<Nothing> {
+        string leaf = containerizer::paths::cgroups2::container(
+          this->flags.cgroups_root, containerId, true);
+        CHECK(cgroups2::exists(leaf));
+
+        Try<Nothing> assign = cgroups2::assign(leaf, child);
+        if (assign.isError()) {
+          return Error("Failed to assign process " + stringify(child)
+                       + " to cgroup " + leaf + ": " + assign.error());
+        }
+        return Nothing();
+      }));
   }
 
   vector<Subprocess::ChildHook> childHooks;