(mesos) branch master updated: [devices] Add ability to remove cgroup from DeviceManager state.

[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 d5ab9a20b [devices] Add ability to remove cgroup from DeviceManager 
state.
d5ab9a20b is described below

commit d5ab9a20b1b39ef74bed0468ee54ea2cd20ee24c
Author: Jason Zhou <jasonzhou...@gmail.com>
AuthorDate: Wed Jul 31 15:15:34 2024 -0700

    [devices] Add ability to remove cgroup from DeviceManager state.
    
    When destroy() is called on a container, its cgroup and its children
    will be cleaned up. We need to remove the cgroup from the device manager
    state when this happens to ensure that the state is accurate.
    
    Review: https://reviews.apache.org/r/75120/
---
 .../device_manager/device_manager.cpp              | 17 ++++++++++
 .../device_manager/device_manager.hpp              |  3 ++
 src/tests/device_manager_tests.cpp                 | 38 ++++++++++++++++++++++
 3 files changed, 58 insertions(+)

diff --git a/src/slave/containerizer/device_manager/device_manager.cpp 
b/src/slave/containerizer/device_manager/device_manager.cpp
index 5c7c88b98..c255880aa 100644
--- a/src/slave/containerizer/device_manager/device_manager.cpp
+++ b/src/slave/containerizer/device_manager/device_manager.cpp
@@ -175,6 +175,14 @@ public:
       : CHECK_NOTERROR(DeviceManager::CgroupDeviceAccess::create({}, {}));
   }
 
+  Future<Nothing> remove(const std::string& cgroup)
+  {
+    if (device_access_per_cgroup.contains(cgroup)) {
+      device_access_per_cgroup.erase(cgroup);
+    }
+    return Nothing();
+  }
+
 private:
   const string meta_dir;
 
@@ -402,6 +410,15 @@ DeviceManager::CgroupDeviceAccess::create(
   return CgroupDeviceAccess(allow_list, deny_list);
 }
 
+
+Future<Nothing> DeviceManager::remove(const std::string& cgroup)
+{
+  return dispatch(
+      *process,
+      &DeviceManagerProcess::remove,
+      cgroup);
+}
+
 } // namespace slave {
 } // namespace internal {
 } // namespace mesos {
diff --git a/src/slave/containerizer/device_manager/device_manager.hpp 
b/src/slave/containerizer/device_manager/device_manager.hpp
index 6d5a7693b..c1bf3c35d 100644
--- a/src/slave/containerizer/device_manager/device_manager.hpp
+++ b/src/slave/containerizer/device_manager/device_manager.hpp
@@ -123,6 +123,9 @@ public:
       const std::vector<DeviceManager::NonWildcardEntry>& additions,
       const std::vector<DeviceManager::NonWildcardEntry>& removals);
 
+  // Remove the cgroup from the DeviceManager state if the state contains it.
+  process::Future<Nothing> remove(const std::string& cgroup);
+
 private:
   explicit DeviceManager(const process::Owned<DeviceManagerProcess>& process);
   process::Owned<DeviceManagerProcess> process;
diff --git a/src/tests/device_manager_tests.cpp 
b/src/tests/device_manager_tests.cpp
index 4f925f3ff..7b06e2864 100644
--- a/src/tests/device_manager_tests.cpp
+++ b/src/tests/device_manager_tests.cpp
@@ -287,6 +287,44 @@ TEST_F(DeviceManagerTest, 
ROOT_DeviceManagerGetDiffState_AllowMatchesDeny)
 }
 
 
+TEST_F(DeviceManagerTest, ROOT_DeviceManagerRemove)
+{
+  ASSERT_SOME(cgroups2::create(TEST_CGROUP));
+  slave::Flags flags;
+  flags.work_dir = *sandbox;
+  Owned<DeviceManager> dm =
+    Owned<DeviceManager>(CHECK_NOTERROR(DeviceManager::create(flags)));
+
+  vector<devices::Entry> allow_list = {*devices::Entry::parse("c 1:3 w")};
+  vector<devices::Entry> deny_list = {*devices::Entry::parse("c 3:1 w")};
+
+  AWAIT_ASSERT_READY(dm->configure(
+      TEST_CGROUP,
+      allow_list,
+      CHECK_NOTERROR(convert_to_non_wildcards(deny_list))));
+
+  Future<DeviceManager::CgroupDeviceAccess> cgroup_state =
+    dm->state(TEST_CGROUP);
+
+  AWAIT_ASSERT_READY(cgroup_state);
+  EXPECT_EQ(allow_list, cgroup_state->allow_list);
+  EXPECT_EQ(deny_list, cgroup_state->deny_list);
+
+  Future<Nothing> removal = dm->remove(TEST_CGROUP);
+  AWAIT_ASSERT_READY(removal);
+
+  Future<hashmap<std::string, DeviceManager::CgroupDeviceAccess>> dm_state =
+    dm->state();
+  EXPECT_FALSE(dm_state->contains(TEST_CGROUP));
+
+  cgroup_state = dm->state(TEST_CGROUP);
+
+  AWAIT_ASSERT_READY(cgroup_state);
+  EXPECT_EQ(vector<devices::Entry>{}, cgroup_state->allow_list);
+  EXPECT_EQ(vector<devices::Entry>{}, cgroup_state->deny_list);
+}
+
+
 using DeviceManagerGetDiffStateTestParams = tuple<
   vector<devices::Entry>, // Allow list for initial configure.
   vector<devices::Entry>, // Deny list for initial configure.