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 78454b221 [cgroups2] Introduce a device manager.
78454b221 is described below

commit 78454b2217dadea5ec85fc79d240e1bdf6705b0c
Author: Jason Zhou <[email protected]>
AuthorDate: Mon Jul 22 23:19:00 2024 -0400

    [cgroups2] Introduce a device manager.
    
    This change introduces the DeviceManager to help facilitate device
    access management in cgroups2 via ebpf program file changes. This
    centralization is needed since we no longer have control files to
    leverage as persistence for agent recovery, so we a component that
    keeps track of allow/deny device access information and re-configures
    the ebpf program for the cgroup.
    
    Device requests can be made to the manager by calling `configure` or
    `reconfigure`. Note that `configure` should only be used when setting
    up a cgroup's device access, i.e. it has not requested any device to
    be allowed/denied before.
    
    In addition, `reconfigure` cannot be used to add deny entries
    containing wildcards.
    
    This manager will be made available to all controllers under the
    cgroups2 isolator, and the GPU isolator.
    
    Review: https://reviews.apache.org/r/75006/
---
 src/CMakeLists.txt                                 |   3 +-
 src/Makefile.am                                    |   7 +-
 .../device_manager/device_manager.cpp              | 304 ++++++++++++++++++
 .../device_manager/device_manager.hpp              | 123 ++++++++
 src/tests/containerizer/cgroups2_tests.cpp         |   1 -
 src/tests/device_manager_tests.cpp                 | 344 +++++++++++++++++++++
 6 files changed, 778 insertions(+), 4 deletions(-)

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index ea0fee1bb..9526ed17a 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -349,7 +349,8 @@ set(LINUX_SRC
   slave/containerizer/mesos/isolators/cgroups2/controllers/core.cpp
   slave/containerizer/mesos/isolators/cgroups2/controllers/cpu.cpp
   slave/containerizer/mesos/isolators/cgroups2/controllers/memory.cpp
-  slave/containerizer/mesos/isolators/cgroups2/controllers/perf_event.cpp)
+  slave/containerizer/mesos/isolators/cgroups2/controllers/perf_event.cpp
+  slave/containerizer/device_manager/device_manager.cpp)
 
 if (ENABLE_XFS_DISK_ISOLATOR)
   list(APPEND LINUX_SRC
diff --git a/src/Makefile.am b/src/Makefile.am
index 03eb0cc28..456942389 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1496,7 +1496,9 @@ MESOS_LINUX_FILES =                                       
                                \
   slave/containerizer/mesos/isolators/cgroups2/controllers/memory.cpp    \
   slave/containerizer/mesos/isolators/cgroups2/controllers/memory.hpp    \
   slave/containerizer/mesos/isolators/cgroups2/controllers/perf_event.cpp    \
-  slave/containerizer/mesos/isolators/cgroups2/controllers/perf_event.hpp
+  slave/containerizer/mesos/isolators/cgroups2/controllers/perf_event.hpp    \
+  slave/containerizer/device_manager/device_manager.cpp    \
+  slave/containerizer/device_manager/device_manager.hpp
 
 if ENABLE_XFS_DISK_ISOLATOR
 MESOS_LINUX_FILES +=                                                   \
@@ -2829,7 +2831,8 @@ mesos_tests_SOURCES =                                     
        \
   tests/containerizer/rootfs.hpp                               \
   tests/containerizer/setns_test_helper.hpp                    \
   tests/containerizer/volume_sandbox_path_isolator_tests.cpp    \
-  tests/containerizer/cgroups2_tests.cpp
+  tests/containerizer/cgroups2_tests.cpp    \
+  tests/device_manager_tests.cpp
 
 if ENABLE_XFS_DISK_ISOLATOR
 mesos_tests_SOURCES +=                                         \
diff --git a/src/slave/containerizer/device_manager/device_manager.cpp 
b/src/slave/containerizer/device_manager/device_manager.cpp
new file mode 100644
index 000000000..a392000ea
--- /dev/null
+++ b/src/slave/containerizer/device_manager/device_manager.cpp
@@ -0,0 +1,304 @@
+// 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 KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <algorithm>
+#include <string>
+
+#include <process/dispatch.hpp>
+#include <process/future.hpp>
+#include <process/id.hpp>
+#include <process/process.hpp>
+
+#include <stout/foreach.hpp>
+#include <stout/stringify.hpp>
+
+#include "slave/containerizer/device_manager/device_manager.hpp"
+#include "slave/paths.hpp"
+
+using std::string;
+using std::vector;
+
+using process::dispatch;
+using process::Failure;
+using process::Future;
+using process::Owned;
+
+using cgroups::devices::Entry;
+
+namespace mesos {
+namespace internal {
+namespace slave {
+
+
+vector<Entry> convert_to_entries(
+    const vector<DeviceManager::NonWildcardEntry>& non_wildcards_entries)
+{
+  vector<Entry> entries = {};
+  foreach (const DeviceManager::NonWildcardEntry& non_wildcards_entry,
+           non_wildcards_entries) {
+    Entry entry;
+    entry.access = non_wildcards_entry.access;
+    entry.selector.type = [&]() {
+      switch (non_wildcards_entry.selector.type) {
+        case DeviceManager::NonWildcardEntry::Selector::Type::BLOCK:
+          return Entry::Selector::Type::BLOCK;
+        case DeviceManager::NonWildcardEntry::Selector::Type::CHARACTER:
+          return Entry::Selector::Type::CHARACTER;
+      }
+    }();
+    entry.selector.major = non_wildcards_entry.selector.major;
+    entry.selector.minor = non_wildcards_entry.selector.minor;
+    entries.push_back(entry);
+  }
+  return entries;
+}
+
+
+class DeviceManagerProcess : public process::Process<DeviceManagerProcess>
+{
+public:
+  DeviceManagerProcess(const string& work_dir)
+    : ProcessBase(process::ID::generate("device-manager")),
+      meta_dir(paths::getMetaRootDir(work_dir)) {}
+
+  Future<Nothing> configure(
+      const string& cgroup,
+      const vector<Entry>& allow_list,
+      const vector<DeviceManager::NonWildcardEntry>& non_wildcard_deny_list)
+  {
+    vector<Entry> deny_list = convert_to_entries(non_wildcard_deny_list);
+    foreach (const Entry& allow_entry, allow_list) {
+      foreach (const Entry& deny_entry, deny_list) {
+        if (deny_entry.encompasses(allow_entry)) {
+          return Failure(
+              "Failed to configure allow and deny devices:"
+              " allow entry '" + stringify(allow_entry) + "' cannot be"
+              " encompassed by deny entry '" + stringify(deny_entry) + "'");
+        }
+      }
+    }
+
+    device_access_per_cgroup[cgroup].allow_list = allow_list;
+    device_access_per_cgroup[cgroup].deny_list = deny_list;
+
+    return Nothing();
+  }
+
+  Future<Nothing> reconfigure(
+      const string& cgroup,
+      const vector<DeviceManager::NonWildcardEntry>& non_wildcard_additions,
+      const vector<DeviceManager::NonWildcardEntry>& non_wildcard_removals)
+  {
+    vector<Entry> additions = convert_to_entries(non_wildcard_additions);
+    vector<Entry> removals = convert_to_entries(non_wildcard_removals);
+    foreach (const Entry& addition, additions) {
+      foreach (const Entry& removal, removals) {
+        if (removal.encompasses(addition)) {
+          return Failure(
+              "Failed to configure allow and deny devices:"
+              " addition '" + stringify(addition) + "' cannot be"
+              " encompassed by removal '" + stringify(removal) + "'");
+        }
+      }
+    }
+
+    device_access_per_cgroup[cgroup] = DeviceManager::apply_diff(
+        device_access_per_cgroup[cgroup],
+        non_wildcard_additions,
+        non_wildcard_removals);
+
+    return Nothing();
+  }
+
+  hashmap<string, DeviceManager::CgroupDeviceAccess> state() const
+  {
+    return device_access_per_cgroup;
+  }
+
+  DeviceManager::CgroupDeviceAccess state(const string& cgroup) const
+  {
+    return device_access_per_cgroup.contains(cgroup)
+      ? device_access_per_cgroup.at(cgroup)
+      : DeviceManager::CgroupDeviceAccess();
+  }
+
+private:
+  const string meta_dir;
+
+  hashmap<string, DeviceManager::CgroupDeviceAccess> device_access_per_cgroup;
+};
+
+
+Try<DeviceManager*> DeviceManager::create(const Flags& flags)
+{
+  return new DeviceManager(
+      Owned<DeviceManagerProcess>(new DeviceManagerProcess(flags.work_dir)));
+}
+
+
+DeviceManager::DeviceManager(
+    const Owned<DeviceManagerProcess>& _process)
+  : process(_process)
+{
+  spawn(*process);
+}
+
+
+DeviceManager::~DeviceManager()
+{
+  terminate(*process);
+  process::wait(*process);
+}
+
+
+Future<Nothing> DeviceManager::reconfigure(
+    const string& cgroup,
+    const vector<DeviceManager::NonWildcardEntry>& additions,
+    const vector<DeviceManager::NonWildcardEntry>& removals)
+{
+  return dispatch(
+      *process,
+      &DeviceManagerProcess::reconfigure,
+      cgroup,
+      additions,
+      removals);
+}
+
+
+Future<Nothing> DeviceManager::configure(
+    const string& cgroup,
+    const vector<Entry>& allow_list,
+    const vector<DeviceManager::NonWildcardEntry>& deny_list)
+{
+  return dispatch(
+      *process,
+      &DeviceManagerProcess::configure,
+      cgroup,
+      allow_list,
+      deny_list);
+}
+
+
+hashmap<string, DeviceManager::CgroupDeviceAccess> DeviceManager::state() const
+{
+  return process->state();
+}
+
+
+DeviceManager::CgroupDeviceAccess DeviceManager::state(
+    const string& cgroup) const
+{
+  return process->state(cgroup);
+}
+
+
+DeviceManager::CgroupDeviceAccess DeviceManager::apply_diff(
+    const DeviceManager::CgroupDeviceAccess& old_state,
+    const vector<DeviceManager::NonWildcardEntry>& non_wildcard_additions,
+    const vector<DeviceManager::NonWildcardEntry>& non_wildcard_removals)
+{
+  auto revoke_accesses = [](Entry* entry, const Entry& diff_entry) {
+    CHECK(!entry->selector.has_wildcard());
+    CHECK(!diff_entry.selector.has_wildcard());
+
+    if (entry->selector.major == diff_entry.selector.major
+        && entry->selector.minor == diff_entry.selector.minor
+        && entry->selector.type == diff_entry.selector.type) {
+      entry->access.mknod = entry->access.mknod && !diff_entry.access.mknod;
+      entry->access.read = entry->access.read && !diff_entry.access.read;
+      entry->access.write = entry->access.write && !diff_entry.access.write;
+    }
+  };
+
+  DeviceManager::CgroupDeviceAccess new_state = old_state;
+  vector<Entry> additions = convert_to_entries(non_wildcard_additions);
+  vector<Entry> removals = convert_to_entries(non_wildcard_removals);
+
+  foreach (const Entry& addition, additions) {
+    // Go over each entry in deny list, find any entries that match the new
+    // addition's major & minor numbers, remove any accesses they specify
+    // that the addition also specifies.
+    // Invariant: No device wildcards are allowed in the deny list.
+    foreach (Entry& deny_entry, new_state.deny_list) {
+      revoke_accesses(&deny_entry, addition);
+    }
+
+    new_state.allow_list.push_back(addition);
+  }
+
+  foreach (const Entry& removal, removals) {
+    Entry::Access accesses_by_matching_wildcards;
+    accesses_by_matching_wildcards.read = false;
+    accesses_by_matching_wildcards.write = false;
+    accesses_by_matching_wildcards.mknod = false;
+
+    foreach (Entry& allow_entry, new_state.allow_list) {
+      // Matching against wildcard - we cannot revoke wildcard privileges
+      // so we will insert a deny entry replicating whatever privileges we
+      // need to deny which the wildcard grants.
+      if (allow_entry.selector.has_wildcard()) {
+        // Does the allow wildcard match the removal device? Skip if not.
+        if (allow_entry.selector.type != Entry::Selector::Type::ALL
+            && allow_entry.selector.type != removal.selector.type) {
+          continue; // Type doesn't match.
+        }
+        if (allow_entry.selector.major.isSome()
+            && allow_entry.selector.major != removal.selector.major) {
+          continue; // Major doesn't match.
+        }
+        if (allow_entry.selector.minor.isSome()
+            && allow_entry.selector.minor != removal.selector.minor) {
+          continue; // Minor doesn't match.
+        }
+        accesses_by_matching_wildcards.mknod |= allow_entry.access.mknod;
+        accesses_by_matching_wildcards.read |= allow_entry.access.read;
+        accesses_by_matching_wildcards.write |= allow_entry.access.write;
+      } else {
+        revoke_accesses(&allow_entry, removal);
+      }
+    }
+
+    Entry::Access removal_access = removal.access;
+    removal_access.mknod &= accesses_by_matching_wildcards.mknod;
+    removal_access.read &= accesses_by_matching_wildcards.read;
+    removal_access.write &= accesses_by_matching_wildcards.write;
+
+    if (!removal_access.none()) {
+      Entry to_push = removal;
+      to_push.access = removal_access;
+      new_state.deny_list.push_back(to_push);
+    }
+  }
+
+  auto strip_empties = [](const vector<Entry>& entries) {
+    vector<Entry> res = {};
+    foreach (const Entry& entry, entries) {
+      if (!entry.access.none()) {
+        res.push_back(entry);
+      }
+    }
+    return res;
+  };
+
+  new_state.allow_list = strip_empties(new_state.allow_list);
+  new_state.deny_list = strip_empties(new_state.deny_list);
+
+  return new_state;
+}
+
+} // 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
new file mode 100644
index 000000000..a6e87dd54
--- /dev/null
+++ b/src/slave/containerizer/device_manager/device_manager.hpp
@@ -0,0 +1,123 @@
+// 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 KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef __DEVICE_MANAGER_HPP__
+#define __DEVICE_MANAGER_HPP__
+
+#include <process/future.hpp>
+#include <stout/hashmap.hpp>
+#include <stout/nothing.hpp>
+#include <stout/try.hpp>
+
+#include "linux/cgroups.hpp"
+#include "slave/flags.hpp"
+
+namespace mesos {
+namespace internal {
+namespace slave {
+
+class DeviceManagerProcess;
+
+// Manages cgroups V2 device access state.
+//
+// In cgroups V2, device control is managed via ebpf programs, as opposed
+// to the control files in cgroups V1. As such we lose the ability to
+// checkpoint a cgroup's device access state via the control files, and
+// we have to checkpoint this information ourselves via this centralized
+// device manager.
+//
+// In addition, we always construct the ebpf programs from the cgroup's entire
+// state, which we cannot keep track of via control files anymore. So we need
+// the centralized device manager to manage the state and provide an interface
+// to incrementally adjust the device access state for a cgroup and generate
+// the appropriate ebpf programs.
+//
+// TODO(jasonzhou): This initial implementation only adds in-memory tracking
+// of the access, add bpf program attachment and checkpointing in follow-on
+// patches.
+class DeviceManager
+{
+public:
+  // Used to enforce non-wildcard entry restrictions at compile time.
+  struct NonWildcardEntry
+  {
+    struct Selector
+    {
+      enum class Type { BLOCK, CHARACTER };
+      Type type;
+      unsigned int major;
+      unsigned int minor;
+    };
+
+    Selector selector;
+    cgroups::devices::Entry::Access access;
+  };
+
+  struct CgroupDeviceAccess
+  {
+    std::vector<cgroups::devices::Entry> allow_list;
+    std::vector<cgroups::devices::Entry> deny_list;
+  };
+
+  static Try<DeviceManager*> create(const Flags& flags);
+
+  ~DeviceManager();
+
+  // This is used for initial cgroup device access setup.
+  // Device type, major number, and minor number wildcards are allowed in
+  // allow_list entries, but not in deny_list entries due to implementation
+  // complexities.
+  process::Future<Nothing> configure(
+    const std::string& cgroup,
+    const std::vector<cgroups::devices::Entry>& allow,
+    const std::vector<NonWildcardEntry>& deny);
+
+  // Modifies the device access settings for a specified cgroup.
+  //
+  // Wildcards are not allowed in the param entry vectors.
+  // The additionals vector contains entries specifying the devices to which
+  // access should be granted.
+  // The removals vector contains entries specifying the devices to which
+  // access should be revoked.
+  process::Future<Nothing> reconfigure(
+    const std::string& cgroup,
+    const std::vector<NonWildcardEntry>& additionals,
+    const std::vector<NonWildcardEntry>& removals);
+
+  // Return the cgroup's device access state, which can be
+  // used to query if a device access would be granted.
+  CgroupDeviceAccess state(const std::string& cgroup) const;
+
+  // Return the cgroup's device access state for all cgroups tracked.
+  hashmap<std::string, CgroupDeviceAccess> state() const;
+
+  // Returns cgroup device state with additions and removals applied to it.
+  // Exposed for unit testing.
+  static DeviceManager::CgroupDeviceAccess apply_diff(
+      const DeviceManager::CgroupDeviceAccess& state,
+      const std::vector<DeviceManager::NonWildcardEntry>& additions,
+      const std::vector<DeviceManager::NonWildcardEntry>& removals);
+
+private:
+  explicit DeviceManager(const process::Owned<DeviceManagerProcess>& process);
+  process::Owned<DeviceManagerProcess> process;
+};
+
+} // namespace slave {
+} // namespace internal {
+} // namespace mesos {
+
+#endif // __DEVICE_MANAGER_HPP__
diff --git a/src/tests/containerizer/cgroups2_tests.cpp 
b/src/tests/containerizer/cgroups2_tests.cpp
index 3982e2598..c73045632 100644
--- a/src/tests/containerizer/cgroups2_tests.cpp
+++ b/src/tests/containerizer/cgroups2_tests.cpp
@@ -840,7 +840,6 @@ TEST_F(Cgroups2Test, ROOT_CGROUPS2_AtomicReplace)
 }
 
 
-
 TEST_F(Cgroups2Test, ROOT_CGROUPS2_GetBpfFdById)
 {
   const string& cgroup = TEST_CGROUP;
diff --git a/src/tests/device_manager_tests.cpp 
b/src/tests/device_manager_tests.cpp
new file mode 100644
index 000000000..398846bdd
--- /dev/null
+++ b/src/tests/device_manager_tests.cpp
@@ -0,0 +1,344 @@
+// 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 KIND, 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 <utility>
+#include <tuple>
+
+#include <process/gtest.hpp>
+
+#include <stout/unreachable.hpp>
+#include <stout/tests/utils.hpp>
+
+#include "linux/cgroups2.hpp"
+#include "slave/containerizer/device_manager/device_manager.hpp"
+
+using mesos::internal::slave::DeviceManager;
+using mesos::internal::slave::DeviceManagerProcess;
+
+using process::Future;
+using process::Owned;
+
+using std::string;
+using std::tuple;
+using std::vector;
+
+namespace devices = cgroups2::devices;
+
+namespace mesos {
+namespace internal {
+namespace tests {
+
+const string TEST_CGROUP = "test";
+
+
+Try<vector<DeviceManager::NonWildcardEntry>> convert_to_non_wildcards(
+    const vector<devices::Entry>& entries)
+{
+  vector<DeviceManager::NonWildcardEntry> non_wildcards = {};
+
+  foreach (const devices::Entry& entry, entries) {
+    if (entry.selector.has_wildcard()) {
+      return Error("Entry cannot have wildcard");
+    }
+    DeviceManager::NonWildcardEntry non_wildcard;
+    non_wildcard.access = entry.access;
+    non_wildcard.selector.major = *entry.selector.major;
+    non_wildcard.selector.minor = *entry.selector.minor;
+    non_wildcard.selector.type = [&]() {
+      switch (entry.selector.type) {
+        case cgroups::devices::Entry::Selector::Type::BLOCK:
+          return DeviceManager::NonWildcardEntry::Selector::Type::BLOCK;
+        case cgroups::devices::Entry::Selector::Type::CHARACTER:
+          return DeviceManager::NonWildcardEntry::Selector::Type::CHARACTER;
+        case cgroups::devices::Entry::Selector::Type::ALL:
+          UNREACHABLE();
+      }
+    }();
+    non_wildcards.push_back(non_wildcard);
+  }
+
+  return non_wildcards;
+}
+
+
+class DeviceManagerTest : public TemporaryDirectoryTest
+{
+  void SetUp() override
+  {
+    TemporaryDirectoryTest::SetUp();
+
+    // Cleanup the test cgroup, in case a previous test run didn't clean it
+    // up properly.
+    if (cgroups2::exists(TEST_CGROUP)) {
+      AWAIT_READY(cgroups2::destroy(TEST_CGROUP));
+    }
+  }
+
+  void TearDown() override
+  {
+    if (cgroups2::exists(TEST_CGROUP)) {
+      AWAIT_READY(cgroups2::destroy(TEST_CGROUP));
+    }
+
+    TemporaryDirectoryTest::TearDown();
+  }
+};
+
+
+TEST(NonWildcardEntry, NonWildcardFromWildcard)
+{
+  EXPECT_ERROR(convert_to_non_wildcards(
+      vector<devices::Entry>{*devices::Entry::parse("c *:1 w")}));
+}
+
+
+TEST_F(DeviceManagerTest, ROOT_DeviceManagerConfigure_Normal)
+{
+  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))));
+
+  DeviceManager::CgroupDeviceAccess cgroup_state = dm->state(TEST_CGROUP);
+
+  EXPECT_EQ(cgroup_state.allow_list, allow_list);
+  EXPECT_EQ(cgroup_state.deny_list, deny_list);
+}
+
+
+TEST_F(DeviceManagerTest, ROOT_DeviceManagerReconfigure_Normal)
+{
+  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))));
+  DeviceManager::CgroupDeviceAccess cgroup_state = dm->state(TEST_CGROUP);
+
+  EXPECT_EQ(cgroup_state.allow_list, allow_list);
+  EXPECT_EQ(cgroup_state.deny_list, deny_list);
+
+  vector<devices::Entry> additions = {*devices::Entry::parse("b 3:4 w")};
+  vector<devices::Entry> removals = allow_list;
+
+  AWAIT_ASSERT_READY(dm->reconfigure(
+      TEST_CGROUP,
+      CHECK_NOTERROR(convert_to_non_wildcards(additions)),
+      CHECK_NOTERROR(convert_to_non_wildcards(removals))));
+  cgroup_state = dm->state(TEST_CGROUP);
+
+  EXPECT_EQ(cgroup_state.allow_list, additions);
+  EXPECT_EQ(cgroup_state.deny_list, deny_list);
+}
+
+
+TEST_F(DeviceManagerTest, ROOT_DeviceManagerConfigure_AllowMatchesDeny)
+{
+  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 1:3 w"),
+    *devices::Entry::parse("c 21:1 w")
+  };
+
+  AWAIT_ASSERT_FAILED(dm->configure(
+      TEST_CGROUP,
+      allow_list,
+      CHECK_NOTERROR(convert_to_non_wildcards(deny_list))));
+}
+
+
+TEST_F(DeviceManagerTest, ROOT_DeviceManagerConfigure_AllowWildcard)
+{
+  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("a *:* m")};
+  vector<devices::Entry> deny_list = {*devices::Entry::parse("c 3:1 m")};
+
+  AWAIT_ASSERT_READY(dm->configure(
+      TEST_CGROUP,
+      allow_list,
+      CHECK_NOTERROR(convert_to_non_wildcards(deny_list))));
+
+  DeviceManager::CgroupDeviceAccess dm_state = dm->state(TEST_CGROUP);
+
+  EXPECT_EQ(dm_state.allow_list, allow_list);
+  EXPECT_EQ(dm_state.deny_list, deny_list);
+}
+
+
+TEST_F(DeviceManagerTest, ROOT_DeviceManagerGetDiffState_AllowMatchesDeny)
+{
+  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> additions = {*devices::Entry::parse("c 1:3 w")};
+  vector<devices::Entry> removals = {
+    *devices::Entry::parse("c 1:3 w"),
+    *devices::Entry::parse("c 21:1 w")
+  };
+
+  AWAIT_ASSERT_FAILED(dm->reconfigure(
+      TEST_CGROUP,
+      CHECK_NOTERROR(convert_to_non_wildcards(additions)),
+      CHECK_NOTERROR(convert_to_non_wildcards(removals))));
+}
+
+
+using DeviceManagerGetDiffStateTestParams = tuple<
+  vector<devices::Entry>, // Allow list for initial configure.
+  vector<devices::Entry>, // Deny list for initial configure.
+  vector<devices::Entry>, // Additions for reconfigure.
+  vector<devices::Entry>, // Removals for reconfigure.
+  vector<devices::Entry>, // Expected allow list after reconfigure.
+  vector<devices::Entry>  // Expected deny list after reconfigure.
+>;
+
+
+class DeviceManagerGetDiffStateTestFixture
+  : public DeviceManagerTest,
+    public ::testing::WithParamInterface<DeviceManagerGetDiffStateTestParams>
+{};
+
+
+TEST_P(DeviceManagerGetDiffStateTestFixture, ROOT_DeviceManagerGetDiffState)
+{
+  auto params = GetParam();
+  vector<devices::Entry> setup_allow = std::get<0>(params);
+  vector<devices::Entry> setup_deny = std::get<1>(params);
+  vector<devices::Entry> additions = std::get<2>(params);
+  vector<devices::Entry> removals = std::get<3>(params);
+  vector<devices::Entry> reconfigured_allow = std::get<4>(params);
+  vector<devices::Entry> reconfigured_deny = std::get<5>(params);
+
+  ASSERT_SOME(cgroups2::create(TEST_CGROUP));
+  slave::Flags flags;
+  flags.work_dir = *sandbox;
+  Owned<DeviceManager> dm =
+    Owned<DeviceManager>(CHECK_NOTERROR(DeviceManager::create(flags)));
+
+  AWAIT_ASSERT_READY(dm->configure(
+      TEST_CGROUP,
+      setup_allow,
+      CHECK_NOTERROR(convert_to_non_wildcards(setup_deny))));
+
+  DeviceManager::CgroupDeviceAccess dm_state = dm->state(TEST_CGROUP);
+
+  EXPECT_EQ(dm_state.allow_list, setup_allow);
+  EXPECT_EQ(dm_state.deny_list, setup_deny);
+
+  dm_state = dm->apply_diff(
+      dm->state(TEST_CGROUP),
+      CHECK_NOTERROR(convert_to_non_wildcards(additions)),
+      CHECK_NOTERROR(convert_to_non_wildcards(removals)));
+
+  EXPECT_EQ(dm_state.allow_list, reconfigured_allow);
+  EXPECT_EQ(dm_state.deny_list, reconfigured_deny);
+}
+
+
+INSTANTIATE_TEST_CASE_P(
+  DeviceManagerGetDiffStateTestParams,
+  DeviceManagerGetDiffStateTestFixture,
+  ::testing::Values<DeviceManagerGetDiffStateTestParams>(
+    // Remove existing allow entry accesses:
+    DeviceManagerGetDiffStateTestParams{
+      vector<devices::Entry>{*devices::Entry::parse("c 3:1 rwm")},
+      vector<devices::Entry>{},
+      vector<devices::Entry>{},
+      vector<devices::Entry>{*devices::Entry::parse("c 3:1 rm")},
+      vector<devices::Entry>{*devices::Entry::parse("c 3:1 w")},
+      vector<devices::Entry>{}},
+    // Remove existing deny entry accesses:
+    DeviceManagerGetDiffStateTestParams{
+      vector<devices::Entry>{*devices::Entry::parse("c 3:* rwm")},
+      vector<devices::Entry>{*devices::Entry::parse("c 3:1 rwm")},
+      vector<devices::Entry>{*devices::Entry::parse("c 3:1 rm")},
+      vector<devices::Entry>{},
+      vector<devices::Entry>{
+        *devices::Entry::parse("c 3:* rwm"),
+        *devices::Entry::parse("c 3:1 rm")},
+      vector<devices::Entry>{*devices::Entry::parse("c 3:1 w")}},
+    // Remove entire existing allow entry:
+    DeviceManagerGetDiffStateTestParams{
+      vector<devices::Entry>{*devices::Entry::parse("c 3:1 rm")},
+      vector<devices::Entry>{},
+      vector<devices::Entry>{},
+      vector<devices::Entry>{*devices::Entry::parse("c 3:1 rwm")},
+      vector<devices::Entry>{},
+      vector<devices::Entry>{}},
+    // Remove entire existing deny entry:
+    DeviceManagerGetDiffStateTestParams{
+      vector<devices::Entry>{*devices::Entry::parse("c 3:* rm")},
+      vector<devices::Entry>{*devices::Entry::parse("c 3:1 rm")},
+      vector<devices::Entry>{*devices::Entry::parse("c 3:1 rm")},
+      vector<devices::Entry>{},
+      vector<devices::Entry>{
+        *devices::Entry::parse("c 3:* rm"), *devices::Entry::parse("c 3:1 
rm")},
+      vector<devices::Entry>{}},
+    // Overlapping entries where none encompasses the other:
+    DeviceManagerGetDiffStateTestParams{
+      vector<devices::Entry>{*devices::Entry::parse("c 3:* rm")},
+      vector<devices::Entry>{*devices::Entry::parse("c 3:1 rm")},
+      vector<devices::Entry>{*devices::Entry::parse("c 3:1 rw")},
+      vector<devices::Entry>{},
+      vector<devices::Entry>{
+        *devices::Entry::parse("c 3:* rm"), *devices::Entry::parse("c 3:1 
rw")},
+      vector<devices::Entry>{*devices::Entry::parse("c 3:1 m")}},
+    // Overlapping with non-encompassing wildcard:
+    DeviceManagerGetDiffStateTestParams{
+      vector<devices::Entry>{*devices::Entry::parse("c 3:* rm")},
+      vector<devices::Entry>{},
+      vector<devices::Entry>{},
+      vector<devices::Entry>{*devices::Entry::parse("c 3:1 rw")},
+      vector<devices::Entry>{*devices::Entry::parse("c 3:* rm")},
+      vector<devices::Entry>{*devices::Entry::parse("c 3:1 r")}}));
+
+} // namespace tests {
+} // namespace internal {
+} // namespace mesos {

Reply via email to