(mesos) branch master updated: [device manager] Add wildcard conversion helper.
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 2c0f1bc91 [device manager] Add wildcard conversion helper. 2c0f1bc91 is described below commit 2c0f1bc917b47316266ef81e77c9ceb0f0539280 Author: Jason Zhou AuthorDate: Fri Aug 2 10:09:44 2024 -0700 [device manager] Add wildcard conversion helper. We currently have a wildcard conversion helper but it was only available for use inside the device manager test file. This change pulls out the helper and makes it available as a static function for use outside just tests. Review: https://reviews.apache.org/r/75137/ --- .../device_manager/device_manager.cpp | 32 .../device_manager/device_manager.hpp | 3 ++ src/tests/device_manager_tests.cpp | 57 +- 3 files changed, 48 insertions(+), 44 deletions(-) diff --git a/src/slave/containerizer/device_manager/device_manager.cpp b/src/slave/containerizer/device_manager/device_manager.cpp index fbc532d6e..e613323dc 100644 --- a/src/slave/containerizer/device_manager/device_manager.cpp +++ b/src/slave/containerizer/device_manager/device_manager.cpp @@ -77,6 +77,38 @@ vector convert_to_entries( } +Try> + DeviceManager::NonWildcardEntry::create( + const vector& entries) +{ + vector non_wildcards = {}; + + foreach (const cgroups::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(); + } + UNREACHABLE(); +}(); +non_wildcards.push_back(non_wildcard); + } + + return non_wildcards; +} + + class DeviceManagerProcess : public process::Process { public: diff --git a/src/slave/containerizer/device_manager/device_manager.hpp b/src/slave/containerizer/device_manager/device_manager.hpp index a987895d5..853350f70 100644 --- a/src/slave/containerizer/device_manager/device_manager.hpp +++ b/src/slave/containerizer/device_manager/device_manager.hpp @@ -52,6 +52,9 @@ public: // Used to enforce non-wildcard entry restrictions at compile time. struct NonWildcardEntry { +static Try> create( +const std::vector& entries); + struct Selector { enum class Type { BLOCK, CHARACTER }; diff --git a/src/tests/device_manager_tests.cpp b/src/tests/device_manager_tests.cpp index b818a5bd9..c4e9b8c58 100644 --- a/src/tests/device_manager_tests.cpp +++ b/src/tests/device_manager_tests.cpp @@ -48,37 +48,6 @@ namespace tests { const string TEST_CGROUP = "test"; -Try> convert_to_non_wildcards( -const vector& entries) -{ - vector 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(); - } - UNREACHABLE(); -}(); -non_wildcards.push_back(non_wildcard); - } - - return non_wildcards; -} - - class DeviceManagerTest : public TemporaryDirectoryTest { void SetUp() override @@ -105,7 +74,7 @@ class DeviceManagerTest : public TemporaryDirectoryTest TEST(NonWildcardEntry, NonWildcardFromWildcard) { - EXPECT_ERROR(convert_to_non_wildcards( + EXPECT_ERROR(DeviceManager::NonWildcardEntry::create( vector{*devices::Entry::parse("c *:1 w")})); } @@ -125,7 +94,7 @@ TEST_F(DeviceManagerTest, ROOT_DeviceManagerConfigure_Normal) AWAIT_ASSERT_READY(dm->configure( TEST_CGR
(mesos) branch master updated: [device manager] Let non-wildcards entries check device access.
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 067e14e82 [device manager] Let non-wildcards entries check device access. 067e14e82 is described below commit 067e14e8218acc1b95652b0452663e0c36c11043 Author: Jason Zhou AuthorDate: Thu Aug 1 14:11:45 2024 -0700 [device manager] Let non-wildcards entries check device access. Currently, we only allow normal Entry instances for checking whether a device access would be allowed for a cgroup. We want to also allow NonWildcardEntry instances to do this as well. Review: https://reviews.apache.org/r/75135/ --- .../device_manager/device_manager.cpp | 47 ++ .../device_manager/device_manager.hpp | 1 + 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/src/slave/containerizer/device_manager/device_manager.cpp b/src/slave/containerizer/device_manager/device_manager.cpp index c255880aa..fbc532d6e 100644 --- a/src/slave/containerizer/device_manager/device_manager.cpp +++ b/src/slave/containerizer/device_manager/device_manager.cpp @@ -45,26 +45,33 @@ namespace internal { namespace slave { +Entry convert_to_entry( +const DeviceManager::NonWildcardEntry& non_wildcard_entry) +{ + Entry entry; + entry.access = non_wildcard_entry.access; + entry.selector.type = [&]() { +switch (non_wildcard_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; +} +UNREACHABLE(); + }(); + entry.selector.major = non_wildcard_entry.selector.major; + entry.selector.minor = non_wildcard_entry.selector.minor; + return entry; +} + + vector convert_to_entries( -const vector& non_wildcards_entries) +const vector& non_wildcard_entries) { vector 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; - } - UNREACHABLE(); -}(); -entry.selector.major = non_wildcards_entry.selector.major; -entry.selector.minor = non_wildcards_entry.selector.minor; -entries.push_back(entry); + foreach (const DeviceManager::NonWildcardEntry& non_wildcard, + non_wildcard_entries) { +entries.push_back(convert_to_entry(non_wildcard)); } return entries; } @@ -390,6 +397,12 @@ bool DeviceManager::CgroupDeviceAccess::is_access_granted( return allowed() && !denied(); } +bool DeviceManager::CgroupDeviceAccess::is_access_granted( +const DeviceManager::NonWildcardEntry& query) const +{ + return is_access_granted(convert_to_entry(query)); +} + DeviceManager::CgroupDeviceAccess::CgroupDeviceAccess( const std::vector& _allow_list, diff --git a/src/slave/containerizer/device_manager/device_manager.hpp b/src/slave/containerizer/device_manager/device_manager.hpp index c1bf3c35d..a987895d5 100644 --- a/src/slave/containerizer/device_manager/device_manager.hpp +++ b/src/slave/containerizer/device_manager/device_manager.hpp @@ -72,6 +72,7 @@ public: // A device access is granted if it is encompassed by an allow entry // and does not have access overlaps with any deny entry. bool is_access_granted(const cgroups::devices::Entry& entry) const; +bool is_access_granted(const NonWildcardEntry& entry) const; // Returns an error if it the allow or deny lists are not normalized. static Try create(
(mesos) branch master updated: [cgroups2] Silence incorrect compiler error in the tests.
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 e16fcd937 [cgroups2] Silence incorrect compiler error in the tests. e16fcd937 is described below commit e16fcd93734906bde8bc0847b271f3bd70165668 Author: Benjamin Mahler AuthorDate: Wed Jul 31 16:59:50 2024 -0700 [cgroups2] Silence incorrect compiler error in the tests. --- src/tests/device_manager_tests.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tests/device_manager_tests.cpp b/src/tests/device_manager_tests.cpp index 7b06e2864..b818a5bd9 100644 --- a/src/tests/device_manager_tests.cpp +++ b/src/tests/device_manager_tests.cpp @@ -70,6 +70,7 @@ Try> convert_to_non_wildcards( case cgroups::devices::Entry::Selector::Type::ALL: UNREACHABLE(); } + UNREACHABLE(); }(); non_wildcards.push_back(non_wildcard); }
(mesos) 02/03: [cgroups2] create device controller in Cgroups2Isolator.
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 commit 2f545b11253800c33fa4d1fde073227deb772f89 Author: Jason Zhou AuthorDate: Wed Jul 31 15:51:32 2024 -0700 [cgroups2] create device controller in Cgroups2Isolator. DeviceController needs to be created in Cgroups2Isolator with the DeviceManager so that the default whitelist can be properly configured. Review: https://reviews.apache.org/r/75121/ --- src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.cpp | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.cpp b/src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.cpp index 3cf3645c4..5e2856437 100644 --- a/src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.cpp +++ b/src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.cpp @@ -22,6 +22,7 @@ #include "slave/containerizer/mesos/isolators/cgroups2/controllers/cpu.hpp" #include "slave/containerizer/mesos/isolators/cgroups2/controllers/memory.hpp" #include "slave/containerizer/mesos/isolators/cgroups2/controllers/perf_event.hpp" +#include "slave/containerizer/mesos/isolators/cgroups2/controllers/devices.hpp" #include #include @@ -84,9 +85,12 @@ Try Cgroups2IsolatorProcess::create( {"perf_event", ::create} }; - hashmap>(*) - (const Flags&, const Owned)> -creatorsWithDeviceManager = {}; + hashmap>(*)( + const Flags&, + const Owned)> +creatorsWithDeviceManager = { + {"devices", ::create}, +}; hashmap> controllers;
(mesos) 03/03: [cgroups2] Skip enabling of devices controller.
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 commit 5bed66c009a5b3c5670e3b7a5c0adae87b58417a Author: Jason Zhou AuthorDate: Wed Jul 31 15:53:15 2024 -0700 [cgroups2] Skip enabling of devices controller. Similar to the `perf_event` controller, the `devices` controller cannot be written into cgroup.subtree_control file, so we skip the call to cgroups2::controllers::enable for the device controller. Otherwise we will run into an "Invalid argument" error from cgroups. Review: https://reviews.apache.org/r/75130/ --- .../mesos/isolators/cgroups2/cgroups2.cpp | 24 -- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.cpp b/src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.cpp index 5e2856437..a027ee632 100644 --- a/src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.cpp +++ b/src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.cpp @@ -225,21 +225,23 @@ Future> Cgroups2IsolatorProcess::prepare( // The "core" controller is always enabled because the "cgroup.*" control // files exist for all cgroups. // - // Additionally, since "core" and "perf_event" aren't valid controller - // names (i.e. they don't exist in "cgroup.controllers"), calling - // `cgroups2::controllers::enable` with these cgroups will fail with - // "Invalid argument". + // Additionally, since "core", "perf_event", and "devices" aren't valid + // controller names (i.e. they don't exist in "cgroup.controllers"), + // calling `cgroups2::controllers::enable` with these cgroups will fail + // with "Invalid argument". // - // Therefore, we skip enabling the "core" and "perf_event" controller here. + // Therefore, we skip enabling the "core", "perf_event", and "devices" + // controller here. continue; } -// Similar to "core", "perf_event" does not exist in cgroup.controllers, -// and therefore we cannot call cgroups2::controllers::enable with it -// as it cannot be written into cgroup.subtree_control, but we still -// need to push it into the controllers of the containers, so we will -// only skip the call for cgroups2::controllers::enable -if (controller->name() != "perf_event") { +// Similar to "core", "perf_event" and "devices" do not exist in +// cgroup.controllers, and therefore we cannot call +// cgroups2::controllers::enable with it as it cannot be written into +// cgroup.subtree_control, but we still need to push it into the controllers +// of the containers, so we will only skip the call for +// cgroups2::controllers::enable. +if (controller->name() != "perf_event" && controller->name() != "devices") { Try enable = cgroups2::controllers::enable(nonLeafCgroup, {controller->name()}); if (enable.isError()) {
(mesos) 01/03: [cgroups2] Introduces the DeviceControllerProcess.
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 commit 8038681524f9b0efaacf77e09e7933ea7ca999ee Author: Jason Zhou AuthorDate: Wed Jul 31 15:27:47 2024 -0700 [cgroups2] Introduces the DeviceControllerProcess. Introduces a device controller that supports cgroups v2 and is available in the Cgroups2IsolatorProcess. Device access control is made through the DeviceManager. Review: https://reviews.apache.org/r/75098/ --- src/CMakeLists.txt | 1 + src/Makefile.am| 2 + .../mesos/isolators/cgroups2/constants.hpp | 1 + .../isolators/cgroups2/controllers/devices.cpp | 204 + .../isolators/cgroups2/controllers/devices.hpp | 82 + 5 files changed, 290 insertions(+) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 9526ed17a..50684b615 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -349,6 +349,7 @@ 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/devices.cpp slave/containerizer/mesos/isolators/cgroups2/controllers/perf_event.cpp slave/containerizer/device_manager/device_manager.cpp) diff --git a/src/Makefile.am b/src/Makefile.am index 456942389..5b91319d6 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1497,6 +1497,8 @@ MESOS_LINUX_FILES = \ 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/devices.cpp\ + slave/containerizer/mesos/isolators/cgroups2/controllers/devices.hpp\ slave/containerizer/device_manager/device_manager.cpp\ slave/containerizer/device_manager/device_manager.hpp diff --git a/src/slave/containerizer/mesos/isolators/cgroups2/constants.hpp b/src/slave/containerizer/mesos/isolators/cgroups2/constants.hpp index bad79ad0c..5e6426694 100644 --- a/src/slave/containerizer/mesos/isolators/cgroups2/constants.hpp +++ b/src/slave/containerizer/mesos/isolators/cgroups2/constants.hpp @@ -39,6 +39,7 @@ const std::string CGROUPS2_CONTROLLER_CORE_NAME = "core"; const std::string CGROUPS2_CONTROLLER_CPU_NAME = "cpu"; const std::string CGROUPS2_CONTROLLER_MEMORY_NAME = "memory"; const std::string CGROUPS2_CONTROLLER_PERF_EVENT_NAME = "perf_event"; +const std::string CGROUPS2_CONTROLLER_DEVICES_NAME = "devices"; } // namespace slave { } // namespace internal { diff --git a/src/slave/containerizer/mesos/isolators/cgroups2/controllers/devices.cpp b/src/slave/containerizer/mesos/isolators/cgroups2/controllers/devices.cpp new file mode 100644 index 0..702ed50b0 --- /dev/null +++ b/src/slave/containerizer/mesos/isolators/cgroups2/controllers/devices.cpp @@ -0,0 +1,204 @@ +// 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 + +// This header include must be enclosed in an `extern "C"` block to +// workaround a bug in glibc <= 2.12 (see MESOS-7378). +// +// TODO(neilc): Remove this when we no longer support glibc <= 2.12. +extern "C" { +#include +} + +#include +#include + +#include +#include +#include + +#include "slave/containerizer/mesos/isolators/cgroups2/constants.hpp" +#include "slave/containerizer/mesos/isolators/cgroups2/controllers/devices.hpp" + +using cgroups::devices::Entry; + +using process::Failure; +using process::Future; +using process::Owned; + +using std::string; +using std::vector; + +namespace mesos { +namespace internal { +namespace slave { + +// The default list of devices to whitelist when device isolation is +// turned on. The full lis
(mesos) branch master updated (b8936677f -> 5bed66c00)
This is an automated email from the ASF dual-hosted git repository. bmahler pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/mesos.git from b8936677f [cgroups2] Pass device manager to controllers & cgroups2 isolator. new 803868152 [cgroups2] Introduces the DeviceControllerProcess. new 2f545b112 [cgroups2] create device controller in Cgroups2Isolator. new 5bed66c00 [cgroups2] Skip enabling of devices controller. The 3 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. Summary of changes: src/CMakeLists.txt | 1 + src/Makefile.am| 2 + .../mesos/isolators/cgroups2/cgroups2.cpp | 34 ++--- .../mesos/isolators/cgroups2/constants.hpp | 1 + .../controllers}/devices.cpp | 137 - .../controllers}/devices.hpp | 35 +++--- 6 files changed, 92 insertions(+), 118 deletions(-) copy src/slave/containerizer/mesos/isolators/{cgroups/subsystems => cgroups2/controllers}/devices.cpp (51%) copy src/slave/containerizer/mesos/isolators/{cgroups/subsystems => cgroups2/controllers}/devices.hpp (68%)
(mesos) branch master updated: [cgroups2] Pass device manager to controllers & cgroups2 isolator.
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 b8936677f [cgroups2] Pass device manager to controllers & cgroups2 isolator. b8936677f is described below commit b8936677f99ab10e4210f9b72c8a6be228aa43c5 Author: Jason Zhou AuthorDate: Wed Jul 31 15:19:06 2024 -0700 [cgroups2] Pass device manager to controllers & cgroups2 isolator. Passes the device manager to the cgroups2 isolator on containerizer startup, and sets up the ability for the manager to be passed to the device controller and GPU isolator. Review: https://reviews.apache.org/r/75016/ --- src/slave/containerizer/containerizer.hpp | 2 ++ src/slave/containerizer/mesos/containerizer.cpp| 9 +++-- .../mesos/isolators/cgroups2/cgroups2.cpp | 22 -- .../mesos/isolators/cgroups2/cgroups2.hpp | 5 - 4 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/slave/containerizer/containerizer.hpp b/src/slave/containerizer/containerizer.hpp index 691fdfe29..dab99f262 100644 --- a/src/slave/containerizer/containerizer.hpp +++ b/src/slave/containerizer/containerizer.hpp @@ -45,6 +45,8 @@ #include "slave/containerizer/fetcher.hpp" +#include "slave/containerizer/device_manager/device_manager.hpp" + namespace mesos { namespace internal { namespace slave { diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp index f7ff6b8e5..9bafb137b 100644 --- a/src/slave/containerizer/mesos/containerizer.cpp +++ b/src/slave/containerizer/mesos/containerizer.cpp @@ -369,16 +369,21 @@ Try MesosContainerizer::create( Shared provisioner = _provisioner->share(); #ifdef __linux__ + Owned device_manager = +Owned(CHECK_NOTERROR(DeviceManager::create(flags))); + // Initialize either the cgroups v2 or cgroups v1 isolator, based on what // is available on the host machine. - auto cgroupsIsolatorSelector = [] (const Flags& flags) -> Try { + auto cgroupsIsolatorSelector = [device_manager] (const Flags& flags) + -> Try + { Try mounted = cgroups2::mounted(); if (mounted.isError()) { return Error("Failed to determine if the cgroup2 filesystem is mounted: " + mounted.error()); } if (*mounted) { - return Cgroups2IsolatorProcess::create(flags); + return Cgroups2IsolatorProcess::create(flags, device_manager); } return CgroupsIsolatorProcess::create(flags); }; diff --git a/src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.cpp b/src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.cpp index 2ca388079..3cf3645c4 100644 --- a/src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.cpp +++ b/src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.cpp @@ -73,7 +73,9 @@ Cgroups2IsolatorProcess::Cgroups2IsolatorProcess( Cgroups2IsolatorProcess::~Cgroups2IsolatorProcess() {} -Try Cgroups2IsolatorProcess::create(const Flags& flags) +Try Cgroups2IsolatorProcess::create( +const Flags& flags, +const Owned& deviceManager) { hashmap>(*)(const Flags&)> creators = { {"core", ::create}, @@ -82,6 +84,10 @@ Try Cgroups2IsolatorProcess::create(const Flags& flags) {"perf_event", ::create} }; + hashmap>(*) + (const Flags&, const Owned)> +creatorsWithDeviceManager = {}; + hashmap> controllers; // The "core" controller is always enabled because the "cgroup.*" control @@ -105,9 +111,10 @@ Try Cgroups2IsolatorProcess::create(const Flags& flags) } isolator = strings::remove(isolator, "cgroups/", strings::Mode::PREFIX); - if (!creators.contains(isolator)) { + if (!creators.contains(isolator) + && !creatorsWithDeviceManager.contains(isolator)) { return Error( -"Unknown or unsupported isolator 'cgroups/" + isolator + "'"); + "Unknown or unsupported isolator 'cgroups/" + isolator + "'"); } controllersToCreate.insert(isolator); @@ -115,12 +122,15 @@ Try Cgroups2IsolatorProcess::create(const Flags& flags) } foreach (const string& controllerName, controllersToCreate) { -if (creators.count(controllerName) == 0) { +if (creators.count(controllerName) == 0 +&& creatorsWithDeviceManager.count(controllerName) == 0) { return Error( - "Cgroups v2 controller '" + controllerName + "' is not supported."); +"Cgroups v2 controller '" + controllerName + "' is not supported."); } -Try> process = creators.at(controllerName)
(mesos) branch master updated: [devices] Add ability to remove cgroup from DeviceManager state.
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 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 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 DeviceManager::remove(const std::string& cgroup) +{ + return dispatch( + *process, + ::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& additions, const std::vector& removals); + // Remove the cgroup from the DeviceManager state if the state contains it. + process::Future remove(const std::string& cgroup); + private: explicit DeviceManager(const process::Owned& process); process::Owned 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 dm = +Owned(CHECK_NOTERROR(DeviceManager::create(flags))); + + vector allow_list = {*devices::Entry::parse("c 1:3 w")}; + vector 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 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 removal = dm->remove(TEST_CGROUP); + AWAIT_ASSERT_READY(removal); + + Future> 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{}, cgroup_state->allow_list); + EXPECT_EQ(vector{}, cgroup_state->deny_list); +} + + using DeviceManagerGetDiffStateTestParams = tuple< vector, // Allow list for initial configure. vector, // Deny list for initial configure.
(mesos) branch master updated: [reviewbot] Fix reviewbot build error.
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 60283f0a2 [reviewbot] Fix reviewbot build error. 60283f0a2 is described below commit 60283f0a27edfb54c2d1c39aee20a3fabbc88ae5 Author: Jason Zhou AuthorDate: Wed Jul 31 13:04:38 2024 -0700 [reviewbot] Fix reviewbot build error. Currently, reviewbot is failing from a 'control reaches end of non-void function' error due to a switch case inside a lambda in the device manager code. We use an UNREACHABLE macro to stop this error. Review: https://reviews.apache.org/r/75128/ --- src/slave/containerizer/device_manager/device_manager.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/slave/containerizer/device_manager/device_manager.cpp b/src/slave/containerizer/device_manager/device_manager.cpp index 9c3e6c00c..5c7c88b98 100644 --- a/src/slave/containerizer/device_manager/device_manager.cpp +++ b/src/slave/containerizer/device_manager/device_manager.cpp @@ -24,6 +24,7 @@ #include #include +#include #include "slave/containerizer/device_manager/device_manager.hpp" #include "slave/paths.hpp" @@ -59,6 +60,7 @@ vector convert_to_entries( case DeviceManager::NonWildcardEntry::Selector::Type::CHARACTER: return Entry::Selector::Type::CHARACTER; } + UNREACHABLE(); }(); entry.selector.major = non_wildcards_entry.selector.major; entry.selector.minor = non_wildcards_entry.selector.minor;
(mesos) 01/02: [devices] Fix DeviceManager tests.
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 commit e29cf7bb9f26ceec418c1880c6fa73c352bc5fcb Author: Jason Zhou AuthorDate: Fri Jul 26 22:38:33 2024 -0400 [devices] Fix DeviceManager tests. Changes when merging previous changes caused some DeviceManager testcases to fail. This patch updates the tests to pass it. Review: https://reviews.apache.org/r/75117/ --- src/tests/device_manager_tests.cpp | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/tests/device_manager_tests.cpp b/src/tests/device_manager_tests.cpp index aebf9014c..3778eeff8 100644 --- a/src/tests/device_manager_tests.cpp +++ b/src/tests/device_manager_tests.cpp @@ -393,7 +393,7 @@ INSTANTIATE_TEST_CASE_P( vector{}, vector{ *devices::Entry::parse("c 3:* rm"), -*devices::Entry::parse("c 3:1 rwm") +*devices::Entry::parse("c 3:1 rw") }, vector{*devices::Entry::parse("c 3:1 m")} }, @@ -424,6 +424,12 @@ TEST(DeviceManagerCgroupDeviceAccessTest, IsAccessGrantedTest) EXPECT_FALSE( cgroup_device_access.is_access_granted(*devices::Entry::parse("b 1:3 w")) ); + + // Character devices with minor number 3 can do write only: + cgroup_device_access = CHECK_NOTERROR( + DeviceManager::CgroupDeviceAccess::create( + {*devices::Entry::parse("c *:3 w")}, {} + )); EXPECT_TRUE( cgroup_device_access.is_access_granted(*devices::Entry::parse("c 4:3 w")) );
(mesos) branch master updated (13000010e -> 3aab3999d)
This is an automated email from the ASF dual-hosted git repository. bmahler pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/mesos.git from 1310e [devices] Add CgroupDeviceAccess::create helper which checks normalization. new e29cf7bb9 [devices] Fix DeviceManager tests. new 3aab3999d [style] Add newlines for readability. The 2 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. Summary of changes: src/tests/device_manager_tests.cpp | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-)
(mesos) 02/02: [style] Add newlines for readability.
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 commit 3aab3999d0d6a8da8fc23d2b5827e7d5986d9b1b Author: Jason Zhou AuthorDate: Fri Jul 26 22:39:41 2024 -0400 [style] Add newlines for readability. --- src/tests/device_manager_tests.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/tests/device_manager_tests.cpp b/src/tests/device_manager_tests.cpp index 3778eeff8..4f925f3ff 100644 --- a/src/tests/device_manager_tests.cpp +++ b/src/tests/device_manager_tests.cpp @@ -436,6 +436,7 @@ TEST(DeviceManagerCgroupDeviceAccessTest, IsAccessGrantedTest) EXPECT_TRUE( cgroup_device_access.is_access_granted(*devices::Entry::parse("c *:3 w")) ); + // Character devices with major number 5 can do write only: cgroup_device_access = CHECK_NOTERROR( DeviceManager::CgroupDeviceAccess::create( @@ -447,14 +448,15 @@ TEST(DeviceManagerCgroupDeviceAccessTest, IsAccessGrantedTest) EXPECT_TRUE( cgroup_device_access.is_access_granted(*devices::Entry::parse("c 5:2 w")) ); + // All devices will match the catch-all and can perform all operations: cgroup_device_access = CHECK_NOTERROR( DeviceManager::CgroupDeviceAccess::create( {*devices::Entry::parse("a *:* rwm")}, {} )); - EXPECT_TRUE( cgroup_device_access.is_access_granted(*devices::Entry::parse("c 6:2 w"))); + // Deny all accesses to character device with major and numbers 1:3. cgroup_device_access = CHECK_NOTERROR( DeviceManager::CgroupDeviceAccess::create(
(mesos) branch master updated: [devices] Add CgroupDeviceAccess::create helper which checks normalization.
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 1310e [devices] Add CgroupDeviceAccess::create helper which checks normalization. 1310e is described below commit 1310e07bf6e2cad5c04cf58546f18bb66e3a Author: Jason Zhou AuthorDate: Fri Jul 26 18:19:25 2024 -0400 [devices] Add CgroupDeviceAccess::create helper which checks normalization. Currently, CgroupDeviceAccess instances can be directly constructed without verifying that its allow and deny lists are normalized. To codify our normalization constraints, CgroupDeviceAccess can now only be created with a create() helper. Review: https://reviews.apache.org/r/75116/ --- .../device_manager/device_manager.cpp | 48 +--- .../device_manager/device_manager.hpp | 10 src/tests/device_manager_tests.cpp | 67 ++ 3 files changed, 93 insertions(+), 32 deletions(-) diff --git a/src/slave/containerizer/device_manager/device_manager.cpp b/src/slave/containerizer/device_manager/device_manager.cpp index c25af0adc..9c3e6c00c 100644 --- a/src/slave/containerizer/device_manager/device_manager.cpp +++ b/src/slave/containerizer/device_manager/device_manager.cpp @@ -99,8 +99,13 @@ public: } } -device_access_per_cgroup[cgroup].allow_list = allow_list; -device_access_per_cgroup[cgroup].deny_list = deny_list; +auto result = device_access_per_cgroup.emplace( +cgroup, +CHECK_NOTERROR( + DeviceManager::CgroupDeviceAccess::create(allow_list, deny_list))); +if (!result.second) { + return Failure("cgroup entry already exists"); +} Try commit = commit_device_access_changes(cgroup); if (commit.isError()) { @@ -131,10 +136,19 @@ public: } } -device_access_per_cgroup[cgroup] = DeviceManager::apply_diff( -device_access_per_cgroup[cgroup], -non_wildcard_additions, -non_wildcard_removals); +auto it = device_access_per_cgroup.find(cgroup); +if (it != device_access_per_cgroup.end()) { + it->second = DeviceManager::apply_diff( + it->second, non_wildcard_additions, non_wildcard_removals); +} else { + auto result = device_access_per_cgroup.emplace( +cgroup, +DeviceManager::apply_diff( + CHECK_NOTERROR(DeviceManager::CgroupDeviceAccess::create({}, {})), + non_wildcard_additions, + non_wildcard_removals)); + CHECK(result.second); +} Try commit = commit_device_access_changes(cgroup); if (commit.isError()) { @@ -156,7 +170,7 @@ public: { return device_access_per_cgroup.contains(cgroup) ? device_access_per_cgroup.at(cgroup) - : DeviceManager::CgroupDeviceAccess(); + : CHECK_NOTERROR(DeviceManager::CgroupDeviceAccess::create({}, {})); } private: @@ -366,6 +380,26 @@ bool DeviceManager::CgroupDeviceAccess::is_access_granted( return allowed() && !denied(); } + +DeviceManager::CgroupDeviceAccess::CgroupDeviceAccess( + const std::vector& _allow_list, + const std::vector& _deny_list) + : allow_list(_allow_list), deny_list(_deny_list) {} + + +Try +DeviceManager::CgroupDeviceAccess::create( +const vector& allow_list, +const vector& deny_list) +{ + if (!cgroups2::devices::normalized(allow_list) + || !(cgroups2::devices::normalized(deny_list))) { +return Error("Failed to create CgroupDeviceAccess:" + " The allow or deny list is not normalized"); + } + return CgroupDeviceAccess(allow_list, deny_list); +} + } // 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 c0ffcc20f..6d5a7693b 100644 --- a/src/slave/containerizer/device_manager/device_manager.hpp +++ b/src/slave/containerizer/device_manager/device_manager.hpp @@ -72,6 +72,16 @@ public: // A device access is granted if it is encompassed by an allow entry // and does not have access overlaps with any deny entry. bool is_access_granted(const cgroups::devices::Entry& entry) const; + +// Returns an error if it the allow or deny lists are not normalized. +static Try create( + const std::vector& allow_list, + const std::vector& deny_list); + + private: +CgroupDeviceAccess( + const std::vector& allow_list, + const std::vector& deny_list); }; static Try create(const Flags& flags); diff --git a/src/tests/device_manager_tests.cpp b/src/tests/device_manager_tests.cpp index da1ef64fe..aebf9014c 100644 --- a/src/tests/device_manager_tests.cpp +++ b/src/t
(mesos) 03/04: [cgroups2] Enforce normalization in configure.
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 commit e9bab29626da4de1293eaa1c916b9b15a89c94ff Author: Jason Zhou AuthorDate: Fri Jul 26 17:24:17 2024 -0400 [cgroups2] Enforce normalization in configure. We currently do not enforce normalized allow and deny in configure. However, to ensure that we can generate an ebpf program that behaves correctly, we have to ensure that allow and deny are normalized. This patch adds a validation check to ensure that the allow and deny are normalized before attempting to generate the ebpf program. Review: https://reviews.apache.org/r/75114/ --- src/linux/cgroups2.cpp | 5 src/tests/containerizer/cgroups2_tests.cpp | 44 ++ 2 files changed, 49 insertions(+) diff --git a/src/linux/cgroups2.cpp b/src/linux/cgroups2.cpp index 5b027c5fb..9dd100aa6 100644 --- a/src/linux/cgroups2.cpp +++ b/src/linux/cgroups2.cpp @@ -1466,6 +1466,11 @@ Try configure( const vector& allow, const vector& deny) { + if (!normalized(allow) || !normalized(deny)) { +return Error( +"Failed to validate arguments: allow or deny lists are not normalized"); + } + Try program = DeviceProgram::build(allow, deny); if (program.isError()) { diff --git a/src/tests/containerizer/cgroups2_tests.cpp b/src/tests/containerizer/cgroups2_tests.cpp index de31a330a..fc3899526 100644 --- a/src/tests/containerizer/cgroups2_tests.cpp +++ b/src/tests/containerizer/cgroups2_tests.cpp @@ -972,6 +972,50 @@ TEST(Cgroups2DevicesTest, NormalizeTest) cgroups2::devices::normalize(already_normalized)); } + +TEST_F(Cgroups2Test, CGROUPS2_ConfigureValidation) +{ + const string& cgroup = TEST_CGROUP; + + // Error if there is empty accesses in any entry. + devices::Entry empty_entry = CHECK_NOTERROR(devices::Entry::parse("c 1:3 w")); + empty_entry.access.read = false; + empty_entry.access.write = false; + empty_entry.access.mknod = false; + vector allow = {empty_entry}; + vector deny = { +CHECK_NOTERROR(devices::Entry::parse("c 1:3 w")) + }; + Try configure_status = devices::configure(cgroup, allow, deny); + EXPECT_ERROR(configure_status); + EXPECT_EQ("Failed to validate arguments: allow or deny lists are not" +" normalized", +configure_status.error()); + + // Error if there is any entry that shares the same type, major, and minor + // numbers with another entry in the same list. + allow = { +CHECK_NOTERROR(devices::Entry::parse("b 3:1 rw")), +CHECK_NOTERROR(devices::Entry::parse("b 3:1 m"))}; + deny = {CHECK_NOTERROR(devices::Entry::parse("c 1:3 w"))}; + configure_status = devices::configure(cgroup, allow, deny); + EXPECT_ERROR(configure_status); + EXPECT_EQ("Failed to validate arguments: allow or deny lists are not" +" normalized", +configure_status.error()); + + // Error if there are entries which are encompassed by another on the same + // list. + allow = { +CHECK_NOTERROR(devices::Entry::parse("c *:* rw")), +CHECK_NOTERROR(devices::Entry::parse("c 3:1 w"))}; + deny = {CHECK_NOTERROR(devices::Entry::parse("c 1:3 w"))}; + configure_status = devices::configure(cgroup, allow, deny); + EXPECT_ERROR(configure_status); + EXPECT_EQ("Failed to validate arguments: allow or deny lists are not" +" normalized", +configure_status.error()); +} } // namespace tests { } // namespace internal {
(mesos) branch master updated (c4f2890af -> fc2536283)
This is an automated email from the ASF dual-hosted git repository. bmahler pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/mesos.git from c4f2890af [cgroups2] Helper to check device entry normalization. new 2241e64ca [cgroups2] Add helper to normalize allow/deny list. new 2458d2f9d [cgroups2] Helper to check device access. new e9bab2962 [cgroups2] Enforce normalization in configure. new fc2536283 [devices] Enforce normalization for DeviceManager configure & reconfigure. The 4 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. Summary of changes: src/linux/cgroups2.cpp | 64 src/linux/cgroups2.hpp | 5 + .../device_manager/device_manager.cpp | 44 +++-- .../device_manager/device_manager.hpp | 4 + src/tests/containerizer/cgroups2_tests.cpp | 109 + src/tests/device_manager_tests.cpp | 77 +-- 6 files changed, 284 insertions(+), 19 deletions(-)
(mesos) 04/04: [devices] Enforce normalization for DeviceManager configure & reconfigure.
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 commit fc25362836f7b3c922bdb13497dbbcbaeea10c20 Author: Jason Zhou AuthorDate: Fri Jul 26 17:28:57 2024 -0400 [devices] Enforce normalization for DeviceManager configure & reconfigure. Currently in the configure() and reconfigure() functions in device manager, we do not ensure that the device access state at the end of the function call is normalized. So we incorporate normalized() and normalize() calls to ensure that the allow and deny lists are always normalized at the end of a configure() or reconfigure() call. Review: https://reviews.apache.org/r/75115/ --- .../device_manager/device_manager.cpp | 21 +++- src/tests/device_manager_tests.cpp | 29 ++ 2 files changed, 28 insertions(+), 22 deletions(-) diff --git a/src/slave/containerizer/device_manager/device_manager.cpp b/src/slave/containerizer/device_manager/device_manager.cpp index 800e50243..c25af0adc 100644 --- a/src/slave/containerizer/device_manager/device_manager.cpp +++ b/src/slave/containerizer/device_manager/device_manager.cpp @@ -81,6 +81,13 @@ public: const vector& non_wildcard_deny_list) { vector deny_list = convert_to_entries(non_wildcard_deny_list); + +if (!cgroups2::devices::normalized(allow_list) +|| !cgroups2::devices::normalized(deny_list)) { + return Failure("Failed to configure allow and deny devices:" + " the input allow or deny list is not normalized"); +} + foreach (const Entry& allow_entry, allow_list) { foreach (const Entry& deny_entry, deny_list) { if (deny_entry.encompasses(allow_entry)) { @@ -324,18 +331,8 @@ DeviceManager::CgroupDeviceAccess DeviceManager::apply_diff( } } - auto strip_empties = [](const vector& entries) { -vector 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); + new_state.allow_list = cgroups2::devices::normalize(new_state.allow_list); + new_state.deny_list = cgroups2::devices::normalize(new_state.deny_list); return new_state; } diff --git a/src/tests/device_manager_tests.cpp b/src/tests/device_manager_tests.cpp index b30618c98..da1ef64fe 100644 --- a/src/tests/device_manager_tests.cpp +++ b/src/tests/device_manager_tests.cpp @@ -352,7 +352,8 @@ INSTANTIATE_TEST_CASE_P( vector{}, vector{*devices::Entry::parse("c 3:1 rm")}, vector{*devices::Entry::parse("c 3:1 w")}, - vector{}}, + vector{} +}, // Remove existing deny entry accesses: DeviceManagerGetDiffStateTestParams{ vector{*devices::Entry::parse("c 3:* rwm")}, @@ -360,9 +361,10 @@ INSTANTIATE_TEST_CASE_P( vector{*devices::Entry::parse("c 3:1 rm")}, vector{}, vector{ -*devices::Entry::parse("c 3:* rwm"), -*devices::Entry::parse("c 3:1 rm")}, - vector{*devices::Entry::parse("c 3:1 w")}}, +*devices::Entry::parse("c 3:* rwm") + }, + vector{*devices::Entry::parse("c 3:1 w")} +}, // Remove entire existing allow entry: DeviceManagerGetDiffStateTestParams{ vector{*devices::Entry::parse("c 3:1 rm")}, @@ -370,7 +372,8 @@ INSTANTIATE_TEST_CASE_P( vector{}, vector{*devices::Entry::parse("c 3:1 rwm")}, vector{}, - vector{}}, + vector{} +}, // Remove entire existing deny entry: DeviceManagerGetDiffStateTestParams{ vector{*devices::Entry::parse("c 3:* rm")}, @@ -378,8 +381,10 @@ INSTANTIATE_TEST_CASE_P( vector{*devices::Entry::parse("c 3:1 rm")}, vector{}, vector{ -*devices::Entry::parse("c 3:* rm"), *devices::Entry::parse("c 3:1 rm")}, - vector{}}, +*devices::Entry::parse("c 3:* rm") + }, + vector{} +}, // Overlapping entries where none encompasses the other: DeviceManagerGetDiffStateTestParams{ vector{*devices::Entry::parse("c 3:* rm")}, @@ -387,8 +392,11 @@ INSTANTIATE_TEST_CASE_P( vector{*devices::Entry::parse("c 3:1 rw")}, vector{}, vector{ -*devices::Entry::parse("c 3:* rm"), *devices::Entry::parse("c 3:1 rw")}, - vector{*devices::Entry::parse("c 3:1 m")}}, +*devices::Entry::parse("c 3:* rm"), +*devices::Entry::parse("c 3:1 rwm") + }, + vector{*devices::Entry::parse
(mesos) 01/04: [cgroups2] Add helper to normalize allow/deny list.
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 commit 2241e64caf7ecd735d9a8aaabf02c3f5f3a35ab8 Author: Jason Zhou AuthorDate: Fri Jul 26 16:53:34 2024 -0400 [cgroups2] Add helper to normalize allow/deny list. This patch adds a public helper function to abstract away the logic used to make a list comply with the 'normalized' requirements. As a reminder, an allow or deny list is 'normalized' iff everything below are true: 1. No Entry has empty accesses specified. 2. No two entries on the same list can have the same selector (type, major & minor numbers). 3. No two entries on the same list can be encompassed by the other entry (see Entry::encompasses). Review: https://reviews.apache.org/r/75104/ --- src/linux/cgroups2.cpp | 59 +++ src/linux/cgroups2.hpp | 5 +++ src/tests/containerizer/cgroups2_tests.cpp | 65 ++ 3 files changed, 129 insertions(+) diff --git a/src/linux/cgroups2.cpp b/src/linux/cgroups2.cpp index bd6018050..5b027c5fb 100644 --- a/src/linux/cgroups2.cpp +++ b/src/linux/cgroups2.cpp @@ -30,6 +30,7 @@ #include #include +#include #include #include #include @@ -1530,6 +1531,64 @@ bool normalized(const vector& query) return true; } + +vector normalize(const vector& to_normalize) +{ + auto strip_empties = [](const vector& entries) { +vector stripped = {}; +foreach (const Entry& entry, entries) { + if (!entry.access.none()) { +stripped.push_back(entry); + } +} +return stripped; + }; + + auto deduplicate = [](const vector& entries) { +LinkedHashMap deduplicated; +foreach (const Entry& entry, entries) { + if (!deduplicated.contains(stringify(entry.selector))) { +deduplicated[stringify(entry.selector)] = entry; + } + + Entry& e = deduplicated.at(stringify(entry.selector)); + e.access.write |= entry.access.write; + e.access.read |= entry.access.read; + e.access.mknod |= entry.access.mknod; +} + +return deduplicated.values(); + }; + + auto strip_encompassed = [](const vector& entries) { +vector result = {}; +foreach (const Entry& entry, entries) { + bool is_encompassed = [&]() { +foreach (const Entry& other, entries) { + if (!cgroups::devices::operator==(entry.selector, other.selector) + && other.encompasses(entry)) { +return true; + } +} +return false; + }(); + + // Skip entries that are encompassed by other entries. + if (!is_encompassed) { +result.push_back(entry); + } +} +return result; + }; + + vector result = to_normalize; + result = strip_empties(result); + result = deduplicate(result); + result = strip_encompassed(result); + CHECK(normalized(result)); + return result; +} + } // namespace devices { } // namespace cgroups2 { diff --git a/src/linux/cgroups2.hpp b/src/linux/cgroups2.hpp index ef4e67932..e13e57076 100644 --- a/src/linux/cgroups2.hpp +++ b/src/linux/cgroups2.hpp @@ -451,6 +451,11 @@ Try configure( // entry (see Entry::encompasses). bool normalized(const std::vector& entries); + +// Modifies the given entries such that the three normalization requirements +// are fulfilled. +std::vector normalize(const std::vector& entries); + } // namespace devices { } // namespace cgroups2 { diff --git a/src/tests/containerizer/cgroups2_tests.cpp b/src/tests/containerizer/cgroups2_tests.cpp index 388639bed..de31a330a 100644 --- a/src/tests/containerizer/cgroups2_tests.cpp +++ b/src/tests/containerizer/cgroups2_tests.cpp @@ -907,6 +907,71 @@ TEST(Cgroups2DevicesTest, NormalizedTest) })); } + +TEST(Cgroups2DevicesTest, NormalizeTest) +{ + // Empty entries are eliminated. + devices::Entry empty_entry; + empty_entry.selector.type = devices::Entry::Selector::Type::CHARACTER; + empty_entry.selector.major = 1; + empty_entry.selector.minor = 3; + empty_entry.access.read = false; + empty_entry.access.write = false; + empty_entry.access.mknod = false; + EXPECT_EQ( + vector{}, + cgroups2::devices::normalize({empty_entry})); + + // Entries with matching type, major & minor numbers are combined into one + EXPECT_EQ( + vector{ +CHECK_NOTERROR(devices::Entry::parse("b 3:1 rwm")) + }, + cgroups2::devices::normalize({ +CHECK_NOTERROR(devices::Entry::parse("b 3:1 rw")), +CHECK_NOTERROR(devices::Entry::parse("b 3:1 m")) + })); + + // Entries that are encompassed by another are eliminated + EXPECT_EQ( + vector{CHECK_NOTERROR(devices::Entry::parse("c *:* rw"))}, + cgroups2::devices::normalize({ +
(mesos) 02/04: [cgroups2] Helper to check device access.
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 commit 2458d2f9d367f4b4b6a7a79da24bf45ca6f7ebcb Author: Jason Zhou AuthorDate: Fri Jul 26 17:17:31 2024 -0400 [cgroups2] Helper to check device access. A device access is granted if it is encompassed by an allow entry and does not have access overlaps with any deny entry. The current process of manually checking if a device access would be granted given a state is tedious and leads to worse readability. A new helper function is added to check if an entry would be granted access in a CgroupDeviceAccess instance, and requires the state to be normalized. Review: https://reviews.apache.org/r/75113/ --- .../device_manager/device_manager.cpp | 29 + .../device_manager/device_manager.hpp | 4 ++ src/tests/device_manager_tests.cpp | 48 ++ 3 files changed, 81 insertions(+) diff --git a/src/slave/containerizer/device_manager/device_manager.cpp b/src/slave/containerizer/device_manager/device_manager.cpp index 4c9b86393..800e50243 100644 --- a/src/slave/containerizer/device_manager/device_manager.cpp +++ b/src/slave/containerizer/device_manager/device_manager.cpp @@ -340,6 +340,35 @@ DeviceManager::CgroupDeviceAccess DeviceManager::apply_diff( return new_state; } + +bool DeviceManager::CgroupDeviceAccess::is_access_granted( +const Entry& query) const +{ + CHECK(cgroups2::devices::normalized(allow_list)); + CHECK(cgroups2::devices::normalized(deny_list)); + + auto allowed = [&]() { +foreach (const Entry& allow, allow_list) { + if (allow.encompasses(query)) { +return true; + } +} +return false; + }; + + auto denied = [&]() { +foreach (const Entry& deny, deny_list) { + if (deny.selector.encompasses(query.selector) + && deny.access.overlaps(query.access)) { +return true; + } +} +return false; + }; + + return allowed() && !denied(); +} + } // 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 7c8523d8b..c0ffcc20f 100644 --- a/src/slave/containerizer/device_manager/device_manager.hpp +++ b/src/slave/containerizer/device_manager/device_manager.hpp @@ -68,6 +68,10 @@ public: { std::vector allow_list; std::vector deny_list; + +// A device access is granted if it is encompassed by an allow entry +// and does not have access overlaps with any deny entry. +bool is_access_granted(const cgroups::devices::Entry& entry) const; }; static Try create(const Flags& flags); diff --git a/src/tests/device_manager_tests.cpp b/src/tests/device_manager_tests.cpp index 54d464e97..b30618c98 100644 --- a/src/tests/device_manager_tests.cpp +++ b/src/tests/device_manager_tests.cpp @@ -398,6 +398,54 @@ INSTANTIATE_TEST_CASE_P( vector{*devices::Entry::parse("c 3:* rm")}, vector{*devices::Entry::parse("c 3:1 r")}})); + +TEST(DeviceManagerCgroupDeviceAccessTest, IsAccessGrantedTest) +{ + DeviceManager::CgroupDeviceAccess cgroup_device_access; + + // Character devices with major minor numbers 1:3 can do write only: + cgroup_device_access.allow_list = {*devices::Entry::parse("c 1:3 w")}; + EXPECT_TRUE( +cgroup_device_access.is_access_granted(*devices::Entry::parse("c 1:3 w"))); + EXPECT_FALSE( +cgroup_device_access.is_access_granted(*devices::Entry::parse("c 1:3 rw"))); + EXPECT_FALSE( +cgroup_device_access.is_access_granted(*devices::Entry::parse("b 1:3 w"))); + + // Character devices with minor number 3 can do write only: + cgroup_device_access.allow_list = {*devices::Entry::parse("c *:3 w")}; + EXPECT_TRUE( +cgroup_device_access.is_access_granted(*devices::Entry::parse("c 4:3 w"))); + EXPECT_TRUE( +cgroup_device_access.is_access_granted(*devices::Entry::parse("c *:3 w"))); + + // Character devices with major number 5 can do write only: + cgroup_device_access.allow_list = {(*devices::Entry::parse("c 5:* w"))}; + EXPECT_TRUE( +cgroup_device_access.is_access_granted(*devices::Entry::parse("c 5:* w"))); + EXPECT_TRUE( +cgroup_device_access.is_access_granted(*devices::Entry::parse("c 5:2 w"))); + + // All devices will match the catch-all and can perform all operations: + cgroup_device_access.allow_list = {*devices::Entry::parse("a *:* rwm")}; + EXPECT_TRUE( +cgroup_device_access.is_access_granted(*devices::Entry::parse("c 6:2 w"))); + + // Deny all accesses to character device with major and numbers 1:3. + c
(mesos) branch master updated: [cgroups2] Helper to check device entry normalization.
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 c4f2890af [cgroups2] Helper to check device entry normalization. c4f2890af is described below commit c4f2890afafc747462189d76c5ff2bd7bb5e5018 Author: Jason Zhou AuthorDate: Fri Jul 26 16:41:50 2024 -0400 [cgroups2] Helper to check device entry normalization. Currently we assume that a device state is normalized before using it for generating ebpf files. However, we have not been enforcing these constraints. We add a helper to check if a device state is normalized so that we can enforce these constraints. An allow or deny list is 'normalized' iff everything below are true: 1. No Entry has empty accesses specified. 2. No two entries on the same list can have the same selector (type, major & minor numbers). 3. No two entries on the same list can be encompassed by the other entry (see Entry::encompasses). Review: https://reviews.apache.org/r/75099/ --- src/linux/cgroups2.cpp | 47 ++ src/linux/cgroups2.hpp | 11 +++ src/tests/containerizer/cgroups2_tests.cpp | 45 3 files changed, 103 insertions(+) diff --git a/src/linux/cgroups2.cpp b/src/linux/cgroups2.cpp index ac4678261..bd6018050 100644 --- a/src/linux/cgroups2.cpp +++ b/src/linux/cgroups2.cpp @@ -1483,6 +1483,53 @@ Try configure( return Nothing(); } + +bool normalized(const vector& query) +{ + auto has_empties = [](const vector& entries) { +foreach (const Entry& entry, entries) { + if (entry.access.none()) { +return true; + } +} +return false; + }; + + if (has_empties(query)) { +return false; + } + + auto has_duplicate_selectors = [](const vector& entries) { +hashset selectors; +foreach (const Entry& entry, entries) { + selectors.insert(stringify(entry.selector)); +} +return selectors.size() != entries.size(); + }; + + if (has_duplicate_selectors(query)) { +return false; + } + + auto has_encompassed_entries = [](const vector& entries) { +foreach (const Entry& entry, entries) { + foreach (const Entry& other, entries) { +if ((!cgroups::devices::operator==(entry, other)) +&& entry.encompasses(other)) { + return true; +} + } +} +return false; + }; + + if (has_encompassed_entries(query)) { +return false; + } + + return true; +} + } // namespace devices { } // namespace cgroups2 { diff --git a/src/linux/cgroups2.hpp b/src/linux/cgroups2.hpp index accaebdad..ef4e67932 100644 --- a/src/linux/cgroups2.hpp +++ b/src/linux/cgroups2.hpp @@ -440,6 +440,17 @@ Try configure( const std::vector& allow, const std::vector& deny); + +// Checks if the list of entries passed in is normalized. +// A list of entries is normalized if: +// +// 1. No Entry has empty accesses specified. +// 2. No two entries on the same list can have the same selector (type, +//major & minor numbers). +// 3. No two entries on the same list can be encompassed by the other +// entry (see Entry::encompasses). +bool normalized(const std::vector& entries); + } // namespace devices { } // namespace cgroups2 { diff --git a/src/tests/containerizer/cgroups2_tests.cpp b/src/tests/containerizer/cgroups2_tests.cpp index 39f17460c..388639bed 100644 --- a/src/tests/containerizer/cgroups2_tests.cpp +++ b/src/tests/containerizer/cgroups2_tests.cpp @@ -862,6 +862,51 @@ TEST_F(Cgroups2Test, ROOT_CGROUPS2_GetBpfFdById) ASSERT_SOME(result); } + +TEST(Cgroups2DevicesTest, NormalizedTest) +{ + // Not normalized if there is an entry with no accesses specified. + devices::Entry empty_entry; + empty_entry.selector.type = devices::Entry::Selector::Type::CHARACTER; + empty_entry.selector.major = 1; + empty_entry.selector.minor = 3; + empty_entry.access.read = false; + empty_entry.access.write = false; + empty_entry.access.mknod = false; + EXPECT_FALSE(cgroups2::devices::normalized({empty_entry})); + + // Not normalized if there is any entry that shares the same selector + // with another entry in the same list. + EXPECT_FALSE(cgroups2::devices::normalized( +{CHECK_NOTERROR(devices::Entry::parse("b 3:1 rw")), + CHECK_NOTERROR(devices::Entry::parse("b 3:1 m"))})); + + // Not normalized if there are entries which are encompassed by + // another on the same list. + EXPECT_FALSE(cgroups2::devices::normalized({ +CHECK_NOTERROR(devices::Entry::parse("c *:* rw")), +CHECK_NOTERROR(devices::Entry::parse("c 3:1 w")) + })); + + // Normalized if all of the above are false. + EXPECT_TRUE(cgroups2::devi
(mesos) 02/02: Revert "[cgroups2] Add allow / deny list normalization validation."
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 commit 907aab5645830aac6f164b903da5945a7191ec29 Author: Benjamin Mahler AuthorDate: Fri Jul 26 16:41:27 2024 -0400 Revert "[cgroups2] Add allow / deny list normalization validation." This reverts commit 45d290aeff6912c8e6a4b1a7358c4e9772c447b4. --- src/linux/cgroups2.cpp | 12 -- src/linux/cgroups2.hpp | 10 +- .../device_manager/device_manager.cpp | 138 ++--- .../device_manager/device_manager.hpp | 12 -- src/tests/containerizer/cgroups2_tests.cpp | 42 --- src/tests/device_manager_tests.cpp | 95 -- 6 files changed, 8 insertions(+), 301 deletions(-) diff --git a/src/linux/cgroups2.cpp b/src/linux/cgroups2.cpp index 2c8290727..ac4678261 100644 --- a/src/linux/cgroups2.cpp +++ b/src/linux/cgroups2.cpp @@ -41,8 +41,6 @@ #include "linux/ebpf.hpp" #include "linux/fs.hpp" -#include "slave/containerizer/device_manager/device_manager.hpp" - using std::ostream; using std::set; using std::string; @@ -1467,16 +1465,6 @@ Try configure( const vector& allow, const vector& deny) { - using mesos::internal::slave::DeviceManager; - DeviceManager::CgroupDeviceAccess state = -CHECK_NOTERROR(DeviceManager::CgroupDeviceAccess::create({}, {})); - state.allow_list = allow; - state.deny_list = deny; - if (!state.normalized()) { -return Error( - "Failed to validate arguments: allow or deny lists are not normalized."); - } - Try program = DeviceProgram::build(allow, deny); if (program.isError()) { diff --git a/src/linux/cgroups2.hpp b/src/linux/cgroups2.hpp index a814bc03c..accaebdad 100644 --- a/src/linux/cgroups2.hpp +++ b/src/linux/cgroups2.hpp @@ -434,15 +434,7 @@ using cgroups::devices::Entry; // are hierarchical. I.e. if a parent cgroup does not allow an access then // 'this' cgroup will be denied access. // For access to be granted, the requested access must match an entry in the -// allow list, and not match with any entry on the deny list. If the device -// type, major, minor numbers match with an entry on the deny list, there must -// be no overlap between the requested access and the deny entry's accesses -// -// We additionally enforce the following constraints on both allow and deny: -// 1. No Entry can have no accesses specified -// 2. No two entries on the same list can have the same type, major & minor -//numbers. -// 3. No two entries on the same list can be encompassed by the other entry. +// allow list, and not match with any entry on the deny list. Try configure( const std::string& cgroup, const std::vector& allow, diff --git a/src/slave/containerizer/device_manager/device_manager.cpp b/src/slave/containerizer/device_manager/device_manager.cpp index 6f013d4bc..4c9b86393 100644 --- a/src/slave/containerizer/device_manager/device_manager.cpp +++ b/src/slave/containerizer/device_manager/device_manager.cpp @@ -91,21 +91,9 @@ public: } } } -auto it = device_access_per_cgroup.find(cgroup); -if (it != device_access_per_cgroup.end()) { - return Failure( -"Failed to configure allow and deny devices: cgroup entry already " -"exists."); -} else { - auto result = device_access_per_cgroup.emplace( -cgroup, -CHECK_NOTERROR( - DeviceManager::CgroupDeviceAccess::create(allow_list, deny_list))); - if (!result.second) { -return Failure("Failed to insert new element for key '" + cgroup + "'"); - } -} +device_access_per_cgroup[cgroup].allow_list = allow_list; +device_access_per_cgroup[cgroup].deny_list = deny_list; Try commit = commit_device_access_changes(cgroup); if (commit.isError()) { @@ -136,21 +124,10 @@ public: } } -auto it = device_access_per_cgroup.find(cgroup); -if (it != device_access_per_cgroup.end()) { - it->second = DeviceManager::apply_diff( -it->second, non_wildcard_additions, non_wildcard_removals); -} else { - auto result = device_access_per_cgroup.emplace( -cgroup, -DeviceManager::apply_diff( - CHECK_NOTERROR(DeviceManager::CgroupDeviceAccess::create({}, {})), - non_wildcard_additions, - non_wildcard_removals)); - if (!result.second) { -return Failure("Failed to insert new element for key '" + cgroup + "'"); - } -} +device_access_per_cgroup[cgroup] = DeviceManager::apply_diff( +device_access_per_cgroup[cgroup], +non_wildcard_additions, +non_wildcard_removals); Try commit = commit_device_ac
(mesos) 01/02: Revert "[cgroups2] Clarify device documentation."
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 commit 4b96eaaacf211b4a23fd508ae3968a96c1a0f68e Author: Benjamin Mahler AuthorDate: Fri Jul 26 16:41:23 2024 -0400 Revert "[cgroups2] Clarify device documentation." This reverts commit fd17efe3402fc859efc63d3cd32658d1ec61a015. --- src/linux/cgroups2.hpp | 12 +--- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/linux/cgroups2.hpp b/src/linux/cgroups2.hpp index a06534432..a814bc03c 100644 --- a/src/linux/cgroups2.hpp +++ b/src/linux/cgroups2.hpp @@ -436,15 +436,13 @@ using cgroups::devices::Entry; // For access to be granted, the requested access must match an entry in the // allow list, and not match with any entry on the deny list. If the device // type, major, minor numbers match with an entry on the deny list, there must -// be no overlap between the requested access and the deny entry's accesses. +// be no overlap between the requested access and the deny entry's accesses // // We additionally enforce the following constraints on both allow and deny: -// -// 1. Entries must not have empty accesses specified. -// 2. No two entries on the same list can have the same selector -// (type, major & minor numbers). -// 3. No two entries on the same list can be encompassed by the other entry -// (See Entry::encompassed). +// 1. No Entry can have no accesses specified +// 2. No two entries on the same list can have the same type, major & minor +//numbers. +// 3. No two entries on the same list can be encompassed by the other entry. Try configure( const std::string& cgroup, const std::vector& allow,
(mesos) branch master updated (fd17efe34 -> 907aab564)
This is an automated email from the ASF dual-hosted git repository. bmahler pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/mesos.git from fd17efe34 [cgroups2] Clarify device documentation. new 4b96eaaac Revert "[cgroups2] Clarify device documentation." new 907aab564 Revert "[cgroups2] Add allow / deny list normalization validation." The 2 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. Summary of changes: src/linux/cgroups2.cpp | 12 -- src/linux/cgroups2.hpp | 12 +- .../device_manager/device_manager.cpp | 138 ++--- .../device_manager/device_manager.hpp | 12 -- src/tests/containerizer/cgroups2_tests.cpp | 42 --- src/tests/device_manager_tests.cpp | 95 -- 6 files changed, 8 insertions(+), 303 deletions(-)
(mesos) branch master updated (acb5a8a50 -> fd17efe34)
This is an automated email from the ASF dual-hosted git repository. bmahler pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/mesos.git from acb5a8a50 [ebpf] Correct ebpf deny block behavior. new 45d290aef [cgroups2] Add allow / deny list normalization validation. new fd17efe34 [cgroups2] Clarify device documentation. The 2 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. Summary of changes: src/linux/cgroups2.cpp | 12 ++ src/linux/cgroups2.hpp | 12 +- .../device_manager/device_manager.cpp | 138 +++-- .../device_manager/device_manager.hpp | 12 ++ src/tests/containerizer/cgroups2_tests.cpp | 42 +++ src/tests/device_manager_tests.cpp | 95 ++ 6 files changed, 303 insertions(+), 8 deletions(-)
(mesos) 02/02: [cgroups2] Clarify device documentation.
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 commit fd17efe3402fc859efc63d3cd32658d1ec61a015 Author: Benjamin Mahler AuthorDate: Fri Jul 26 16:34:34 2024 -0400 [cgroups2] Clarify device documentation. --- src/linux/cgroups2.hpp | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/linux/cgroups2.hpp b/src/linux/cgroups2.hpp index a814bc03c..a06534432 100644 --- a/src/linux/cgroups2.hpp +++ b/src/linux/cgroups2.hpp @@ -436,13 +436,15 @@ using cgroups::devices::Entry; // For access to be granted, the requested access must match an entry in the // allow list, and not match with any entry on the deny list. If the device // type, major, minor numbers match with an entry on the deny list, there must -// be no overlap between the requested access and the deny entry's accesses +// be no overlap between the requested access and the deny entry's accesses. // // We additionally enforce the following constraints on both allow and deny: -// 1. No Entry can have no accesses specified -// 2. No two entries on the same list can have the same type, major & minor -//numbers. -// 3. No two entries on the same list can be encompassed by the other entry. +// +// 1. Entries must not have empty accesses specified. +// 2. No two entries on the same list can have the same selector +// (type, major & minor numbers). +// 3. No two entries on the same list can be encompassed by the other entry +// (See Entry::encompassed). Try configure( const std::string& cgroup, const std::vector& allow,
(mesos) 01/02: [cgroups2] Add allow / deny list normalization validation.
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 commit 45d290aeff6912c8e6a4b1a7358c4e9772c447b4 Author: Jason Zhou AuthorDate: Fri Jul 26 01:45:30 2024 -0400 [cgroups2] Add allow / deny list normalization validation. Currently we assume that a device state is normalized before using it for generating ebpf files. However, we have not been enforcing these constraints on the device access state. We enforce some basic validation on cgroups2::configure on the state to ensure that we are able to generate a correct ebpf program. If the lists are not normalized, we generate incorrect programs! An allow or deny list is 'normalized' iff everything below are true: 1. No entries have empty accesses specified. 2. No two entries on the same list can have the same selector (type, major & minor numbers). 3. No two entries on the same list can be encompassed by the other entry. See Entry::encompassed. This patch adds helpers to check if a device state is normalized, and will only allow users to create new CgroupDeviceAccess instances using a helper that checks that the allow and deny lists are normalized. A new helper function is added to check if an entry would be granted access, and requires the state to be normalized. Review: https://reviews.apache.org/r/75099/ --- src/linux/cgroups2.cpp | 12 ++ src/linux/cgroups2.hpp | 10 +- .../device_manager/device_manager.cpp | 138 +++-- .../device_manager/device_manager.hpp | 12 ++ src/tests/containerizer/cgroups2_tests.cpp | 42 +++ src/tests/device_manager_tests.cpp | 95 ++ 6 files changed, 301 insertions(+), 8 deletions(-) diff --git a/src/linux/cgroups2.cpp b/src/linux/cgroups2.cpp index ac4678261..2c8290727 100644 --- a/src/linux/cgroups2.cpp +++ b/src/linux/cgroups2.cpp @@ -41,6 +41,8 @@ #include "linux/ebpf.hpp" #include "linux/fs.hpp" +#include "slave/containerizer/device_manager/device_manager.hpp" + using std::ostream; using std::set; using std::string; @@ -1465,6 +1467,16 @@ Try configure( const vector& allow, const vector& deny) { + using mesos::internal::slave::DeviceManager; + DeviceManager::CgroupDeviceAccess state = +CHECK_NOTERROR(DeviceManager::CgroupDeviceAccess::create({}, {})); + state.allow_list = allow; + state.deny_list = deny; + if (!state.normalized()) { +return Error( + "Failed to validate arguments: allow or deny lists are not normalized."); + } + Try program = DeviceProgram::build(allow, deny); if (program.isError()) { diff --git a/src/linux/cgroups2.hpp b/src/linux/cgroups2.hpp index accaebdad..a814bc03c 100644 --- a/src/linux/cgroups2.hpp +++ b/src/linux/cgroups2.hpp @@ -434,7 +434,15 @@ using cgroups::devices::Entry; // are hierarchical. I.e. if a parent cgroup does not allow an access then // 'this' cgroup will be denied access. // For access to be granted, the requested access must match an entry in the -// allow list, and not match with any entry on the deny list. +// allow list, and not match with any entry on the deny list. If the device +// type, major, minor numbers match with an entry on the deny list, there must +// be no overlap between the requested access and the deny entry's accesses +// +// We additionally enforce the following constraints on both allow and deny: +// 1. No Entry can have no accesses specified +// 2. No two entries on the same list can have the same type, major & minor +//numbers. +// 3. No two entries on the same list can be encompassed by the other entry. Try configure( const std::string& cgroup, const std::vector& allow, diff --git a/src/slave/containerizer/device_manager/device_manager.cpp b/src/slave/containerizer/device_manager/device_manager.cpp index 4c9b86393..6f013d4bc 100644 --- a/src/slave/containerizer/device_manager/device_manager.cpp +++ b/src/slave/containerizer/device_manager/device_manager.cpp @@ -91,9 +91,21 @@ public: } } } +auto it = device_access_per_cgroup.find(cgroup); +if (it != device_access_per_cgroup.end()) { + return Failure( +"Failed to configure allow and deny devices: cgroup entry already " +"exists."); +} else { + auto result = device_access_per_cgroup.emplace( +cgroup, +CHECK_NOTERROR( + DeviceManager::CgroupDeviceAccess::create(allow_list, deny_list))); + if (!result.second) { +return Failure("Failed to insert new element for key '" + cgroup + "'"); + } +} -device_access_per_cgroup[cgroup]
(mesos) branch master updated: [ebpf] Correct ebpf deny block behavior.
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 acb5a8a50 [ebpf] Correct ebpf deny block behavior. acb5a8a50 is described below commit acb5a8a50cc285146726ce5518fd42499e373f87 Author: Jason Zhou AuthorDate: Thu Jul 25 19:52:55 2024 -0400 [ebpf] Correct ebpf deny block behavior. Currently, the deny block matches a device access iff all accesses match on the deny block. For example, a rw access would not match the deny block even if the deny block had w access specified. We would expect that the deny block should deny all accesses if the type, major, and minor number matches, and if any of the device accesses overlap with what's specified in the deny block. Review: https://reviews.apache.org/r/75109/ --- src/linux/cgroups2.cpp | 46 - src/tests/containerizer/cgroups2_tests.cpp | 272 ++--- 2 files changed, 168 insertions(+), 150 deletions(-) diff --git a/src/linux/cgroups2.cpp b/src/linux/cgroups2.cpp index cb3c425a4..ac4678261 100644 --- a/src/linux/cgroups2.cpp +++ b/src/linux/cgroups2.cpp @@ -1250,8 +1250,8 @@ public: short allow_block_trailer_size = allow_block_trailer(0).size(); foreach (const Entry& entry, allow) { -vector allow_block = - add_device_checks(entry, allow_block_trailer_size); +vector allow_block = add_device_checks( +entry, allow_block_trailer_size, DeviceCheckType::ALLOW); allow_device_check_blocks.push_back(allow_block); start_of_deny_jmp_size += allow_block.size() + allow_block_trailer_size; @@ -1273,7 +1273,8 @@ public: // We are either following the normal code flow or special case 1 (see // diagram above) if we reached this section. foreach (const Entry& entry, deny) { - program.append(add_device_checks(entry, deny_block_trailer().size())); + program.append(add_device_checks( + entry, deny_block_trailer().size(), DeviceCheckType::DENY)); program.append(deny_block_trailer()); } @@ -1289,9 +1290,16 @@ public: } private: + enum DeviceCheckType + { +ALLOW, +DENY + }; + static vector add_device_checks( const Entry& entry, - short trailer_length) + short trailer_length, + DeviceCheckType device_check_type) { // We create a block of bytecode with the format: // 1. Major Version Check @@ -1324,7 +1332,25 @@ private: }); }; -auto check_access_instructions = +auto check_deny_access_instructions = + [](short jmp_size, const Entry::Access& access) +{ + int bpf_access = 0; + bpf_access |= access.read ? BPF_DEVCG_ACC_READ : 0; + bpf_access |= access.write ? BPF_DEVCG_ACC_WRITE : 0; + bpf_access |= access.mknod ? BPF_DEVCG_ACC_MKNOD : 0; + return vector({ + BPF_MOV32_REG(BPF_REG_1, BPF_REG_3), + BPF_ALU32_IMM(BPF_AND, BPF_REG_1, bpf_access), + BPF_JMP_IMM( +BPF_JEQ, +BPF_REG_1, +0, +static_cast(jmp_size - 2)), + }); +}; + +auto check_allow_access_instructions = [](short jmp_size, const Entry::Access& access) { int bpf_access = 0; @@ -1368,10 +1394,14 @@ private: // The jump sizes here correspond to the size of the bpf instructions // that each check adds to the program. The total size of the block is // the trailer length plus the total length of all checks. +size_t access_insn_size = + device_check_type == DeviceCheckType::ALLOW +? check_allow_access_instructions(0, access).size() +: check_deny_access_instructions(0, access).size(); short nxt_blk_jmp_size = trailer_length + (check_major ? check_major_instructions(0, 0).size() : 0) + (check_minor ? check_minor_instructions(0, 0).size() : 0) - + (check_access ? check_access_instructions(0, access).size() : 0) + + (check_access ? access_insn_size : 0) + (check_type ? check_type_instructions(0, selector).size() : 0); // We subtract one because the program counter will be one ahead when it @@ -1414,7 +1444,9 @@ private: // 4. Check access (r3) against entry. if (check_access) { vector insert_instructions = -check_access_instructions(nxt_blk_jmp_size, access); +device_check_type == DeviceCheckType::ALLOW + ? check_allow_access_instructions(nxt_blk_jmp_size, access) + : check_deny_access_instructions(nxt_blk_jmp_size, access); foreach (const bpf_insn& insn, insert_instructions) { device_check_block.push_back(insn); } diff --git a/src/tests/containerizer/cgroups2_tests.cpp b/src/tests/containerizer/cgroups2_tests.cpp index c73045632..39f17460
(mesos) branch master updated: [cgroups] Add Device::Selector::encompasses.
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 40e531522 [cgroups] Add Device::Selector::encompasses. 40e531522 is described below commit 40e531522edc76d6d47b71760ea72c46a0aeb2d5 Author: Jason Zhou AuthorDate: Wed Jul 24 16:10:49 2024 -0400 [cgroups] Add Device::Selector::encompasses. Currently, we have to check via Selector's member variables if the devices represented by one Selector encompasses those represented by another. We add a helper function to simplify the logic which differ depending on whether one Selector encompasses the other. Review: https://reviews.apache.org/r/75106/ --- src/linux/cgroups.cpp | 29 +-- src/linux/cgroups.hpp | 5 ++- src/tests/containerizer/cgroups_tests.cpp | 59 +++ 3 files changed, 82 insertions(+), 11 deletions(-) diff --git a/src/linux/cgroups.cpp b/src/linux/cgroups.cpp index e58c2977a..203a1f6e1 100644 --- a/src/linux/cgroups.cpp +++ b/src/linux/cgroups.cpp @@ -2920,16 +2920,7 @@ bool Entry::encompasses(const Entry& other) const return true; } - if (selector.major.isSome() && selector.major != other.selector.major) { -return false; - } - - if (selector.minor.isSome() && selector.minor != other.selector.minor) { -return false; - } - - if (selector.type != Entry::Selector::Type::ALL - && selector.type != other.selector.type) { + if (!selector.encompasses(other.selector)) { return false; } @@ -2960,6 +2951,24 @@ bool Entry::Selector::has_wildcard() const } +bool Entry::Selector::encompasses(const Entry::Selector& other) const +{ + if (type != Entry::Selector::Type::ALL && type != other.type) { +return false; + } + + if (major.isSome() && major != other.major) { +return false; + } + + if (minor.isSome() && minor != other.minor) { +return false; + } + + return true; +} + + Try> list(const string& hierarchy, const string& cgroup) { Try read = cgroups::read(hierarchy, cgroup, "devices.list"); diff --git a/src/linux/cgroups.hpp b/src/linux/cgroups.hpp index c50ec59e3..53bdf9d6a 100644 --- a/src/linux/cgroups.hpp +++ b/src/linux/cgroups.hpp @@ -953,8 +953,11 @@ struct Entry Option major; // Matches all `major` numbers if None. Option minor; // Matches all `minor` numbers if None. -// Returns iff major or minor are wildcards or if type == ALL. +// Returns true iff major or minor are wildcards or if type == ALL. bool has_wildcard() const; + +// Returns true iff selector encompasses the other selector. +bool encompasses(const Selector& other) const; }; struct Access diff --git a/src/tests/containerizer/cgroups_tests.cpp b/src/tests/containerizer/cgroups_tests.cpp index 7e6bb0659..ff8a2ed91 100644 --- a/src/tests/containerizer/cgroups_tests.cpp +++ b/src/tests/containerizer/cgroups_tests.cpp @@ -1547,6 +1547,65 @@ TEST(DeviceTest, SelectorWildcardTest) EXPECT_FALSE(selector.has_wildcard()); } + +TEST(DeviceTest, SelectorEncompassTest) +{ + cgroups::devices::Entry::Selector other; + other.type = cgroups::devices::Entry::Selector::Type::CHARACTER; + other.major = 1; + other.minor = 1; + + // Same selector should be encompass each other. + cgroups::devices::Entry::Selector selector; + selector.type = cgroups::devices::Entry::Selector::Type::CHARACTER; + selector.major = 1; + selector.minor = 1; + EXPECT_TRUE(selector.encompasses(other)); + EXPECT_TRUE(other.encompasses(selector)); + + // Wildcard in type + selector.type = cgroups::devices::Entry::Selector::Type::ALL; + selector.major = 1; + selector.minor = 1; + EXPECT_TRUE(selector.encompasses(other)); + EXPECT_FALSE(other.encompasses(selector)); + + // Wildcard in major + selector.type = cgroups::devices::Entry::Selector::Type::CHARACTER; + selector.major = Option::none(); + selector.minor = 1; + EXPECT_TRUE(selector.encompasses(other)); + EXPECT_FALSE(other.encompasses(selector)); + + // Wildcard in minor + selector.type = cgroups::devices::Entry::Selector::Type::CHARACTER; + selector.major = 1; + selector.minor = Option::none(); + EXPECT_TRUE(selector.encompasses(other)); + EXPECT_FALSE(other.encompasses(selector)); + + // Mismatch in type + selector.type = cgroups::devices::Entry::Selector::Type::BLOCK; + selector.major = Option::none(); + selector.minor = Option::none(); + EXPECT_FALSE(selector.encompasses(other)); + EXPECT_FALSE(other.encompasses(selector)); + + // Mismatch in major + selector.type = cgroups::devices::Entry::Selector::Type::ALL; + selector.major = 2; + selector.minor = Option::none(); + EXPECT_FALSE(selector.encompasses(other)); + EXPECT_FALSE(o
(mesos) branch master updated: [cgroups] Add helper to find overlapping device access.
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 fbb120740 [cgroups] Add helper to find overlapping device access. fbb120740 is described below commit fbb120740e05b4dc45b3873f3bca9f8632a53e6b Author: Jason Zhou AuthorDate: Wed Jul 24 16:08:31 2024 -0400 [cgroups] Add helper to find overlapping device access. Currently, we have to directly compare member variables to see if one Access object would overlap that of another, which isn't very clear to people that would be reading the code. We add a helper to abstract away the logic to see if the accesses specified in one Access instance would overlap with that of another. Review: https://reviews.apache.org/r/75107/ --- src/linux/cgroups.cpp | 8 src/linux/cgroups.hpp | 1 + src/tests/containerizer/cgroups_tests.cpp | 30 ++ 3 files changed, 39 insertions(+) diff --git a/src/linux/cgroups.cpp b/src/linux/cgroups.cpp index 3dae320bd..e58c2977a 100644 --- a/src/linux/cgroups.cpp +++ b/src/linux/cgroups.cpp @@ -2946,6 +2946,14 @@ bool Entry::encompasses(const Entry& other) const bool Entry::Access::none() const { return !mknod && !read && !write; } +bool Entry::Access::overlaps(const Entry::Access& other) const +{ + return (read && other.read) + || (write && other.write) + || (mknod && other.mknod); +} + + bool Entry::Selector::has_wildcard() const { return major.isNone() || minor.isNone() || type == Entry::Selector::Type::ALL; diff --git a/src/linux/cgroups.hpp b/src/linux/cgroups.hpp index 4fefb5c3b..c50ec59e3 100644 --- a/src/linux/cgroups.hpp +++ b/src/linux/cgroups.hpp @@ -963,6 +963,7 @@ struct Entry bool write; bool mknod; bool none() const; +bool overlaps(const Access& other) const; }; Selector selector; diff --git a/src/tests/containerizer/cgroups_tests.cpp b/src/tests/containerizer/cgroups_tests.cpp index e4ca7933b..7e6bb0659 100644 --- a/src/tests/containerizer/cgroups_tests.cpp +++ b/src/tests/containerizer/cgroups_tests.cpp @@ -1493,6 +1493,36 @@ TEST(DevicesTest, AccessNoneTest) } +TEST(DeviceTest, AccessOverlapsTest) +{ + cgroups::devices::Entry::Access access; + cgroups::devices::Entry::Access other; + access.read = true; + access.write = true; + access.mknod = true; + + other.read = false; + other.write = false; + other.mknod = false; + EXPECT_FALSE(access.overlaps(other)); + + other.read = true; + other.write = false; + other.mknod = false; + EXPECT_TRUE(access.overlaps(other)); + + other.read = false; + other.write = true; + other.mknod = false; + EXPECT_TRUE(access.overlaps(other)); + + other.read = false; + other.write = false; + other.mknod = true; + EXPECT_TRUE(access.overlaps(other)); +} + + TEST(DeviceTest, SelectorWildcardTest) { cgroups::devices::Entry::Selector selector;
(mesos) 02/02: [cgroups2] Fix unsafe Process usage in DeviceManager.
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 commit 682f6a743acb9cde4063e8e2d8d200c8531a21b3 Author: Benjamin Mahler AuthorDate: Wed Jul 24 00:33:55 2024 -0400 [cgroups2] Fix unsafe Process usage in DeviceManager. Calls to the DeviceManager wrapper were directly accessing the state of DeviceManagerProcess. This patch uses the dispatch mechanism instead, and adjusts the tests accordingly. --- .../device_manager/device_manager.cpp | 17 ++-- .../device_manager/device_manager.hpp | 4 +- src/tests/device_manager_tests.cpp | 47 +- 3 files changed, 44 insertions(+), 24 deletions(-) diff --git a/src/slave/containerizer/device_manager/device_manager.cpp b/src/slave/containerizer/device_manager/device_manager.cpp index d28b2c35b..4c9b86393 100644 --- a/src/slave/containerizer/device_manager/device_manager.cpp +++ b/src/slave/containerizer/device_manager/device_manager.cpp @@ -224,16 +224,25 @@ Future DeviceManager::configure( } -hashmap DeviceManager::state() const +Future> + DeviceManager::state() const { - return process->state(); + // Necessary due to overloading of state(). + auto process_copy = process; + return dispatch(*process, [process_copy]() { +return process_copy->state(); + }); } -DeviceManager::CgroupDeviceAccess DeviceManager::state( +Future DeviceManager::state( const string& cgroup) const { - return process->state(cgroup); + // Necessary due to overloading of state(). + auto process_copy = process; + return dispatch(*process, [process_copy, cgroup]() { +return process_copy->state(cgroup); + }); } diff --git a/src/slave/containerizer/device_manager/device_manager.hpp b/src/slave/containerizer/device_manager/device_manager.hpp index eafa4e6f6..7c8523d8b 100644 --- a/src/slave/containerizer/device_manager/device_manager.hpp +++ b/src/slave/containerizer/device_manager/device_manager.hpp @@ -97,10 +97,10 @@ public: // 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; + process::Future state(const std::string& cgroup) const; // Return the cgroup's device access state for all cgroups tracked. - hashmap state() const; + process::Future> state() const; // Returns cgroup device state with additions and removals applied to it. // Exposed for unit testing. diff --git a/src/tests/device_manager_tests.cpp b/src/tests/device_manager_tests.cpp index 19ca0af4c..54d464e97 100644 --- a/src/tests/device_manager_tests.cpp +++ b/src/tests/device_manager_tests.cpp @@ -126,10 +126,12 @@ TEST_F(DeviceManagerTest, ROOT_DeviceManagerConfigure_Normal) allow_list, CHECK_NOTERROR(convert_to_non_wildcards(deny_list; - DeviceManager::CgroupDeviceAccess cgroup_state = dm->state(TEST_CGROUP); + Future cgroup_state = +dm->state(TEST_CGROUP); - EXPECT_EQ(cgroup_state.allow_list, allow_list); - EXPECT_EQ(cgroup_state.deny_list, deny_list); + AWAIT_ASSERT_READY(cgroup_state); + EXPECT_EQ(allow_list, cgroup_state->allow_list); + EXPECT_EQ(deny_list, cgroup_state->deny_list); pid_t pid = ::fork(); ASSERT_NE(-1, pid); @@ -171,10 +173,13 @@ TEST_F(DeviceManagerTest, ROOT_DeviceManagerReconfigure_Normal) 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); + Future 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); vector additions = {*devices::Entry::parse("c 1:3 r")}; vector removals = allow_list; @@ -183,10 +188,12 @@ TEST_F(DeviceManagerTest, ROOT_DeviceManagerReconfigure_Normal) 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); + AWAIT_ASSERT_READY(cgroup_state); + EXPECT_EQ(additions, cgroup_state->allow_list); + EXPECT_EQ(deny_list, cgroup_state->deny_list); pid_t pid = ::fork(); ASSERT_NE(-1, pid); @@ -250,10 +257,12 @@ TEST_F(DeviceManagerTest, ROOT_DeviceManagerConfigure_AllowWildcard) allow_list, CHECK_NOTERROR(convert_to_non_wildcards(deny_list; - DeviceManager::CgroupDeviceAccess dm_state = dm->state(TEST_CGROUP); + Future cgroup_state = +dm->state(TEST_CGROUP); - EXPECT_
(mesos) branch master updated (78454b221 -> 682f6a743)
This is an automated email from the ASF dual-hosted git repository. bmahler pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/mesos.git from 78454b221 [cgroups2] Introduce a device manager. new 9d3541c1b [cgroups2] Add ebpf program attachment to the DeviceManager. new 682f6a743 [cgroups2] Fix unsafe Process usage in DeviceManager. The 2 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. Summary of changes: .../device_manager/device_manager.cpp | 49 ++- .../device_manager/device_manager.hpp | 8 +- src/tests/device_manager_tests.cpp | 99 +- 3 files changed, 127 insertions(+), 29 deletions(-)
(mesos) 01/02: [cgroups2] Add ebpf program attachment to the DeviceManager.
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 commit 9d3541c1b3668d15ddee18a03fa8be15c9812550 Author: Jason Zhou AuthorDate: Tue Jul 23 23:57:00 2024 -0400 [cgroups2] Add ebpf program attachment to the DeviceManager. Currently, the device manager only keeps track of the state in memory, and does not commit the changes by attaching an ebpf file to the corresponding cgroup. We will now generate and attach the ebpf file when configure and reconfigure are called. Review: https://reviews.apache.org/r/75102/ --- .../device_manager/device_manager.cpp | 32 + .../device_manager/device_manager.hpp | 4 +- src/tests/device_manager_tests.cpp | 52 +- 3 files changed, 83 insertions(+), 5 deletions(-) diff --git a/src/slave/containerizer/device_manager/device_manager.cpp b/src/slave/containerizer/device_manager/device_manager.cpp index a392000ea..d28b2c35b 100644 --- a/src/slave/containerizer/device_manager/device_manager.cpp +++ b/src/slave/containerizer/device_manager/device_manager.cpp @@ -27,6 +27,7 @@ #include "slave/containerizer/device_manager/device_manager.hpp" #include "slave/paths.hpp" +#include "linux/cgroups2.hpp" using std::string; using std::vector; @@ -94,6 +95,14 @@ public: device_access_per_cgroup[cgroup].allow_list = allow_list; device_access_per_cgroup[cgroup].deny_list = deny_list; +Try commit = commit_device_access_changes(cgroup); +if (commit.isError()) { + // We do not rollback the state when something goes wrong in the + // update because the container will be destroyed when this fails. + return Failure("Failed to commit cgroup device access changes: " + + commit.error()); +} + return Nothing(); } @@ -120,6 +129,14 @@ public: non_wildcard_additions, non_wildcard_removals); +Try commit = commit_device_access_changes(cgroup); +if (commit.isError()) { + // We do not rollback the state when something goes wrong in the + // update because the container will be destroyed when this fails. + return Failure("Failed to commit cgroup device access changes: " + + commit.error()); +} + return Nothing(); } @@ -139,6 +156,21 @@ private: const string meta_dir; hashmap device_access_per_cgroup; + + // TODO(jasonzhou): persist device_access_per_cgroup on disk. + Try commit_device_access_changes(const string& cgroup) const + { +Try status = cgroups2::devices::configure( +cgroup, +device_access_per_cgroup.at(cgroup).allow_list, +device_access_per_cgroup.at(cgroup).deny_list); + +if (status.isError()) { + return Error("Failed to configure device access: " + status.error()); +} + +return Nothing(); + } }; diff --git a/src/slave/containerizer/device_manager/device_manager.hpp b/src/slave/containerizer/device_manager/device_manager.hpp index a6e87dd54..eafa4e6f6 100644 --- a/src/slave/containerizer/device_manager/device_manager.hpp +++ b/src/slave/containerizer/device_manager/device_manager.hpp @@ -45,9 +45,7 @@ class DeviceManagerProcess; // 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. +// TODO(jasonzhou): Add checkpointing / recovery support in follow-on patches. class DeviceManager { public: diff --git a/src/tests/device_manager_tests.cpp b/src/tests/device_manager_tests.cpp index 398846bdd..19ca0af4c 100644 --- a/src/tests/device_manager_tests.cpp +++ b/src/tests/device_manager_tests.cpp @@ -21,6 +21,7 @@ #include #include +#include #include #include @@ -110,13 +111,14 @@ TEST(NonWildcardEntry, NonWildcardFromWildcard) TEST_F(DeviceManagerTest, ROOT_DeviceManagerConfigure_Normal) { + typedef std::pair OpenArgs; ASSERT_SOME(cgroups2::create(TEST_CGROUP)); slave::Flags flags; flags.work_dir = *sandbox; Owned dm = Owned(CHECK_NOTERROR(DeviceManager::create(flags))); - vector allow_list = {*devices::Entry::parse("c 1:3 w")}; + vector allow_list = {*devices::Entry::parse("c 1:3 r")}; vector deny_list = {*devices::Entry::parse("c 3:1 w")}; AWAIT_ASSERT_READY(dm->configure( @@ -128,6 +130,29 @@ TEST_F(DeviceManagerTest, ROOT_DeviceManagerConfigure_Normal) EXPECT_EQ(cgroup_state.allow_list, allow_list); EXPECT_EQ(cgroup_state.deny_list, deny_list); + + pid_t pid = ::fork(); + ASSERT_NE(-1, pid); + + if (pid == 0) { +// Move the child process into the newly cre
(mesos) branch master updated: [cgroups2] Introduce a device manager.
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 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 0..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. +
(mesos) branch master updated: [cgroups] Add helper functions for device Entry.
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 991fb042e [cgroups] Add helper functions for device Entry. 991fb042e is described below commit 991fb042e763f21dd3f695ed2ed5814b8a2a5190 Author: Jason Zhou AuthorDate: Fri Jul 19 16:26:20 2024 -0400 [cgroups] Add helper functions for device Entry. Currently, the Entry class does not have readable helper functions for determining whether the device accesses represented by one Entry would be a subset of that of another. In addition, we want more readable ways to determine if a device has wildcards present and if it has any accesses specified. These additions will streamline the logic in the DeviceManager DeviceManager, which will heavily utilize the Entry class, improving code readability. Review: https://reviews.apache.org/r/75096/ --- src/linux/cgroups.cpp | 42 +++- src/linux/cgroups.hpp | 9 +++ src/tests/containerizer/cgroups_tests.cpp | 110 ++ 3 files changed, 158 insertions(+), 3 deletions(-) diff --git a/src/linux/cgroups.cpp b/src/linux/cgroups.cpp index c42894933..3dae320bd 100644 --- a/src/linux/cgroups.cpp +++ b/src/linux/cgroups.cpp @@ -2914,9 +2914,45 @@ bool Entry::is_catch_all() const } -Try> list( -const string& hierarchy, -const string& cgroup) +bool Entry::encompasses(const Entry& other) const +{ + if (*this == other) { +return true; + } + + if (selector.major.isSome() && selector.major != other.selector.major) { +return false; + } + + if (selector.minor.isSome() && selector.minor != other.selector.minor) { +return false; + } + + if (selector.type != Entry::Selector::Type::ALL + && selector.type != other.selector.type) { +return false; + } + + if ((!access.mknod && other.access.mknod) + || (!access.read && other.access.read) + || (!access.write && other.access.write)) { +return false; + } + + return true; +} + + +bool Entry::Access::none() const { return !mknod && !read && !write; } + + +bool Entry::Selector::has_wildcard() const +{ + return major.isNone() || minor.isNone() || type == Entry::Selector::Type::ALL; +} + + +Try> list(const string& hierarchy, const string& cgroup) { Try read = cgroups::read(hierarchy, cgroup, "devices.list"); diff --git a/src/linux/cgroups.hpp b/src/linux/cgroups.hpp index eaa4cdf9f..4fefb5c3b 100644 --- a/src/linux/cgroups.hpp +++ b/src/linux/cgroups.hpp @@ -933,8 +933,13 @@ namespace devices { struct Entry { static Try parse(const std::string& s); + + // Returns true iff entry matches all devices accesses. bool is_catch_all() const; + // Returns true iff this entry fully contains the other's device accesses. + bool encompasses(const Entry& other) const; + struct Selector { enum class Type @@ -947,6 +952,9 @@ struct Entry Type type; Option major; // Matches all `major` numbers if None. Option minor; // Matches all `minor` numbers if None. + +// Returns iff major or minor are wildcards or if type == ALL. +bool has_wildcard() const; }; struct Access @@ -954,6 +962,7 @@ struct Entry bool read; bool write; bool mknod; +bool none() const; }; Selector selector; diff --git a/src/tests/containerizer/cgroups_tests.cpp b/src/tests/containerizer/cgroups_tests.cpp index 0f4967ff1..e4ca7933b 100644 --- a/src/tests/containerizer/cgroups_tests.cpp +++ b/src/tests/containerizer/cgroups_tests.cpp @@ -1407,6 +1407,116 @@ TEST_F(CgroupsAnyHierarchyDevicesTest, ROOT_CGROUPS_Devices) ASSERT_SOME(cgroups::assign(hierarchy, "", ::getpid())); } + +TEST(DevicesTest, SubsetHelperTest) +{ + cgroups::devices::Entry subset = +CHECK_NOTERROR(cgroups::devices::Entry::parse("c 3:1 rwm")); + cgroups::devices::Entry superset = +CHECK_NOTERROR(cgroups::devices::Entry::parse("c *:1 rwm")); + + EXPECT_TRUE(subset.encompasses(subset)); + EXPECT_TRUE(superset.encompasses(superset)); + EXPECT_FALSE(subset.encompasses(superset)); + EXPECT_TRUE(superset.encompasses(subset)); + + superset = CHECK_NOTERROR(cgroups::devices::Entry::parse("c 3:* rwm")); + EXPECT_TRUE(subset.encompasses(subset)); + EXPECT_TRUE(superset.encompasses(superset)); + EXPECT_FALSE(subset.encompasses(superset)); + EXPECT_TRUE(superset.encompasses(subset)); + + superset = CHECK_NOTERROR(cgroups::devices::Entry::parse("a")); + EXPECT_TRUE(subset.encompasses(subset)); + EXPECT_TRUE(superset.encompasses(superset)); + EXPECT_FALSE(subset.encompasses(superset)); + EXPECT_TRUE(superset.encompasses(subset)); + +
(mesos) branch master updated: [build] Use clang-14 for non ubuntu 16.04 targets in docker-build.sh.
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 7f9311f78 [build] Use clang-14 for non ubuntu 16.04 targets in docker-build.sh. 7f9311f78 is described below commit 7f9311f78ce364797fe0df23fc2c635eed001a31 Author: Jason Zhou AuthorDate: Wed Jul 17 01:34:25 2024 -0400 [build] Use clang-14 for non ubuntu 16.04 targets in docker-build.sh. As we migrate to ubuntu 22.04, clang-10 is no longer available via apt-get install. As such, we will move to clang-14, which should allow us to run the docker-build.sh file with OS equal to ubuntu:22.04. This should allow coverity bot to compile and return to normal. Review: https://reviews.apache.org/r/75091/ --- support/docker-build.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/support/docker-build.sh b/support/docker-build.sh index a0757b98e..b68081a46 100755 --- a/support/docker-build.sh +++ b/support/docker-build.sh @@ -84,7 +84,7 @@ case $OS in if [[ "$OS" = "ubuntu:16.04" ]]; then CLANG_PKG=clang-3.5 else -CLANG_PKG=clang-10 +CLANG_PKG=clang-14 fi else CLANG_PKG=
(mesos) branch master updated: [jenkins] Create dockerfile compatible with 22.04 build.
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 f0d402846 [jenkins] Create dockerfile compatible with 22.04 build. f0d402846 is described below commit f0d4028468a5038cb508820a6479d6205c368a79 Author: Jason Zhou AuthorDate: Mon Jul 15 18:05:38 2024 -0400 [jenkins] Create dockerfile compatible with 22.04 build. For review #75080, we made use of replace_bpf_fd and BPF_F_REPLACE which were added in kernel 5.6. Our current ubuntu 20.04 base image uses kernel 5.4. As such we will be upgrading the ubuntu version used in Jenkins to 22.04, whose base image uses kernel 5.15, so that we can make mesos on the updated pipeline, enabling reviewbot, tidybot, and coverity. Review: https://reviews.apache.org/r/75088/ --- configure.ac | 9 +- support/docker-build.sh| 33 ++ support/jenkins/reviewbot.sh | 2 +- ...-arm.dockerfile => ubuntu-22.04-arm.dockerfile} | 4 +-- ...tu-20.04.dockerfile => ubuntu-22.04.dockerfile} | 4 +-- support/mesos-tidy/Dockerfile | 4 +-- support/verify-reviews.py | 2 +- 7 files changed, 31 insertions(+), 27 deletions(-) diff --git a/configure.ac b/configure.ac index 7c0597f56..4307f004d 100644 --- a/configure.ac +++ b/configure.ac @@ -2310,8 +2310,15 @@ if test -n "`echo $with_zlib`" ; then LDFLAGS="$ZLIB_LINKERFLAGS $LDFLAGS" fi +# We just check for one function here because in ubuntu 22.04, checking multiple +# functions will generate incorrect code where the namespace for other functions +# is not properly specified. +# +# Hence we just check for inflate here as an indicator of whether we have the +# other functions available to us, rather than writing separate checks for +# each function which will be very cumbersome. AC_CHECK_HEADERS([zlib.h], - [AC_CHECK_LIB([z], [deflate, gzread, gzwrite, inflate], [], + [AC_CHECK_LIB([z], [inflate], [], [AC_MSG_ERROR([cannot find libz --- libz is required for Mesos to build. diff --git a/support/docker-build.sh b/support/docker-build.sh index cb0b05b6e..a0757b98e 100755 --- a/support/docker-build.sh +++ b/support/docker-build.sh @@ -67,7 +67,7 @@ case $OS in append_dockerfile "FROM $OS" case $OS in -*20.04*) +*22.04*) append_dockerfile "ARG DEBIAN_FRONTEND=noninteractive" ;; esac @@ -91,13 +91,13 @@ case $OS in fi append_dockerfile "RUN apt-get update" append_dockerfile "RUN apt-get install -y build-essential $CLANG_PKG git maven autoconf libtool software-properties-common" -append_dockerfile "RUN apt-get install -y python-dev python-six libcurl4-nss-dev libsasl2-dev libapr1-dev libsvn-dev libevent-dev libev-dev libssl-dev" +append_dockerfile "RUN apt-get install -y python-six libcurl4-nss-dev libsasl2-dev libapr1-dev libsvn-dev libevent-dev libev-dev libssl-dev" append_dockerfile "RUN apt-get install -y wget curl sed" case $OS in *16.04*) echo "Install Ubuntu 16.04 LTS (Xenial Xerus) specific packages" -append_dockerfile "RUN apt-get install -y openjdk-8-jdk zlib1g-dev" +append_dockerfile "RUN apt-get install -y openjdk-8-jdk zlib1g-dev python-dev" # Install ping required by OsTest.Which append_dockerfile "RUN apt-get install -y iputils-ping" @@ -114,25 +114,22 @@ case $OS in # Install pip for Python 3.6. append_dockerfile "RUN curl https://bootstrap.pypa.io/pip/3.6/get-pip.py | python3" ;; - *20.04*) -echo "Install Ubuntu 20.04 LTS (Focal Fossa) specific packages" -append_dockerfile "RUN apt-get install -y openjdk-11-jdk zlib1g-dev" + *22.04*) +echo "Install Ubuntu 22.04 LTS (Focal Fossa) specific packages" +append_dockerfile "RUN apt-get install -y openjdk-11-jdk zlib1g-dev python2-dev" # Install ping required by OsTest.Which append_dockerfile "RUN apt-get install -y iputils-ping" -append_dockerfile "RUN add-apt-repository -y ppa:deadsnakes/ppa && \ -apt-get update && \ -apt-get install -qy \ - python3.6 \ - python3.6-dev \ - python3.6-distutils \ - python3.6-venv && \ -add-apt-repository --remove -y ppa:deadsnakes/ppa && \ -apt-get clean && \ -
(mesos) branch master updated: [veth] Provide the ability to set veth peer link MAC address on creation.
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 70d9da223 [veth] Provide the ability to set veth peer link MAC address on creation. 70d9da223 is described below commit 70d9da223a204dc89431aa7b26db7beeb53a9f3c Author: Jason Zhou AuthorDate: Mon Jul 15 18:02:54 2024 -0400 [veth] Provide the ability to set veth peer link MAC address on creation. This addresses the previous todo where we want to set the MAC address of the peer link when we are creating a veth pair so that we can avoid the race condition we are racing against udev to see who will set the MAC address of the interface last. See: https://reviews.apache.org/r/75087/ See: https://issues.apache.org/jira/browse/MESOS-10243 Review: https://reviews.apache.org/r/75090/ --- src/linux/routing/link/veth.cpp| 47 +++ src/linux/routing/link/veth.hpp| 6 +- .../mesos/isolators/network/port_mapping.cpp | 23 ++--- src/tests/containerizer/routing_tests.cpp | 97 +- 4 files changed, 77 insertions(+), 96 deletions(-) diff --git a/src/linux/routing/link/veth.cpp b/src/linux/routing/link/veth.cpp index 36aad5fec..12c00cd16 100644 --- a/src/linux/routing/link/veth.cpp +++ b/src/linux/routing/link/veth.cpp @@ -38,42 +38,55 @@ Try create( const string& veth, const string& peer, const Option& pid, -const Option& veth_mac) +const Option& veth_mac, +const Option& peer_mac) { + auto set_addr_from_mac = [](struct rtnl_link* link, const net::MAC& mac) + { +struct nl_addr* addr; +unsigned char mac_addr_uchar[6]; +for (int i = 0; i < 6; i++) { + mac_addr_uchar[i] = (unsigned char) mac[i]; +} +addr = nl_addr_build(AF_LLC, mac_addr_uchar, 6); +rtnl_link_set_addr(link, addr); +return addr; + }; + Try> socket = routing::socket(); if (socket.isError()) { return Error(socket.error()); } struct nl_sock *sock = socket->get(); - struct rtnl_link *link, *peer_link; + struct rtnl_link *veth_link, *peer_link; int error = -NLE_NOMEM; - if (!(link = rtnl_link_veth_alloc())) { + if (!(veth_link = rtnl_link_veth_alloc())) { return Error(nl_geterror(error)); } - peer_link = rtnl_link_veth_get_peer(link); - - struct nl_addr *addr; + peer_link = rtnl_link_veth_get_peer(veth_link); + Option veth_addr, peer_addr; if (veth_mac.isSome()) { -unsigned char mac_addr_uchar[6]; -for (int i = 0; i < 6; i++) { - mac_addr_uchar[i] = (unsigned char) (*veth_mac)[i]; -} -addr = nl_addr_build(AF_LLC, mac_addr_uchar, 6); -rtnl_link_set_addr(link, addr); +veth_addr = set_addr_from_mac(veth_link, *veth_mac); + } + if (peer_mac.isSome()) { +peer_addr = set_addr_from_mac(peer_link, *peer_mac); } - rtnl_link_set_name(link, veth.c_str()); + rtnl_link_set_name(veth_link, veth.c_str()); rtnl_link_set_name(peer_link, peer.c_str()); rtnl_link_set_ns_pid(peer_link, pid.getOrElse(getpid())); - error = rtnl_link_add(sock, link, NLM_F_CREATE | NLM_F_EXCL); + error = rtnl_link_add(sock, veth_link, NLM_F_CREATE | NLM_F_EXCL); - rtnl_link_put(link); - if (veth_mac.isSome()) { -nl_addr_put(addr); + rtnl_link_put(veth_link); + if (veth_addr.isSome()) { +nl_addr_put(*veth_addr); + } + if (peer_addr.isSome()) { +nl_addr_put(*peer_addr); } if (error != 0) { diff --git a/src/linux/routing/link/veth.hpp b/src/linux/routing/link/veth.hpp index 1e54ce696..693033623 100644 --- a/src/linux/routing/link/veth.hpp +++ b/src/linux/routing/link/veth.hpp @@ -24,6 +24,7 @@ #include #include #include +#include namespace routing { namespace link { @@ -51,8 +52,9 @@ namespace veth { Try create( const std::string& veth, const std::string& peer, -const Option& pid, -const Option& veth_mac); +const Option& pid = None(), +const Option& veth_mac = None(), +const Option& peer_mac = None()); } // namespace veth { } // namespace link { diff --git a/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp b/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp index 894d77c56..4dd0b00ad 100644 --- a/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp +++ b/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp @@ -3598,10 +3598,8 @@ Future PortMappingIsolatorProcess::isolate( // Create a virtual ethernet pair for this container, also set the MAC // address of the host network namespace veth interface to match // that of the host network namespace eth0. - // - // TODO(jasonzhou): Also set the mac address of the container network - // namespace eth0 here on creation, to
(mesos) branch master updated: [ssl_tests] Correct ubuntu version on comment.
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 d630e8c3e [ssl_tests] Correct ubuntu version on comment. d630e8c3e is described below commit d630e8c3ec2ab7575c6cd4b21665e73588ccd52b Author: Jason Zhou AuthorDate: Mon Jul 15 13:24:46 2024 -0400 [ssl_tests] Correct ubuntu version on comment. This commit is part of our effort to upgrade jenkins ubuntu version to 22.04. Review: https://reviews.apache.org/r/75089/ --- 3rdparty/libprocess/src/tests/ssl_tests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/3rdparty/libprocess/src/tests/ssl_tests.cpp b/3rdparty/libprocess/src/tests/ssl_tests.cpp index 10e609d8a..f2c1e680a 100644 --- a/3rdparty/libprocess/src/tests/ssl_tests.cpp +++ b/3rdparty/libprocess/src/tests/ssl_tests.cpp @@ -123,7 +123,7 @@ static const vector protocols = { #ifndef OPENSSL_NO_SSL3 "LIBPROCESS_SSL_ENABLE_SSL_V3", #endif - // Removed TLS 1.0 and 1.1 as they're not supported by ubuntu 20.04 + // Removed TLS 1.0 and 1.1 as they're not supported by ubuntu >= 20.04. // https://discourse.ubuntu.com/t/spec-tls-1-0-and-1-1-are-disabled-by-default/41868 "LIBPROCESS_SSL_ENABLE_TLS_V1_2", // On some platforms, we need to build against OpenSSL versions that
(mesos) branch master updated: [veth] Avoid udev race condtion on systems with systemd version > 242.
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 25f35402a [veth] Avoid udev race condtion on systems with systemd version > 242. 25f35402a is described below commit 25f35402a3a45e4499ee24c1938ad7f73290ec7f Author: Jason Zhou AuthorDate: Mon Jul 15 13:06:54 2024 -0400 [veth] Avoid udev race condtion on systems with systemd version > 242. In systems with systemd version above 242, there is a potential data race where udev will try to update the MAC address of the device at the same time as us if the systemd's MacAddressPolicy is set to 'persistent'. To prevent udev from trying to set the veth device's MAC address by itself, we must set the device MAC address on creation so that addr_assign_type will be set to NET_ADDR_SET, which prevents udev from attempting to change the MAC address of the veth device. See: https://github.com/torvalds/linux/commit/2afb9b533423a9b97f84181e773cf9361d98fed6 See: https://lore.kernel.org/netdev/cahxsexy8lkzocbdbzss_vjopc_tqmyzm87kc192hpmuhmcq...@mail.gmail.com/T/ See: https://issues.apache.org/jira/browse/MESOS-10243 Review: https://reviews.apache.org/r/75086/ --- src/linux/routing/link/link.cpp| 121 - src/linux/routing/link/link.hpp| 5 + src/linux/routing/link/veth.cpp| 40 ++- src/linux/routing/link/veth.hpp| 18 ++- .../mesos/isolators/network/port_mapping.cpp | 18 ++- src/tests/containerizer/routing_tests.cpp | 104 +- 6 files changed, 163 insertions(+), 143 deletions(-) diff --git a/src/linux/routing/link/link.cpp b/src/linux/routing/link/link.cpp index bb97e3281..f62b37a18 100644 --- a/src/linux/routing/link/link.cpp +++ b/src/linux/routing/link/link.cpp @@ -247,124 +247,49 @@ Try setUp(const string& link) -// There seems to be a bug that is occassionally preventing ioctl and libnl -// from setting correct MAC addresses, despite them not returning errors. -// -// Observed scenarios with incorrectly assigned MAC addresses: -// -// 1. After setting the mac address: ioctl returns the correct MAC address, but -//net::mac returns an incorrect MAC address (different from the original!) -// 2. After setting the mac address: both ioctl and net::mac return the same MAC -//address, but are both wrong (and different from the original one!) -// 3. After setting the mac address: there are no cases where ioctl or net::mac -//come back with the same MAC address as before we set the address. -// 4. Before we set the mac address: there is a possibility that ioctl and -//net::mac results disagree with each other! -// 5. There is a possibility that the MAC address we set ends up overwritten by -//a garbage value after setMAC has already completed and checked that the -//mac address was set correctly. Since this error happens after this -//function has finished, we cannot log nor detect it in setMAC because we -//have not yet studied at what point this occurs. -// -// Notes: -// -// 1. We have observed this behavior only on CentOS 9 systems at the moment, -//CentOS 7 systems under various kernels do not seem to have the issue -//(which is quite strange if this was purely a kernel bug). -// 2. We have tried kernels 5.15.147, 5.15.160, 5.15.161, all of these have -//this issue on CentOS 9. -// -// To workaround this bug, we check that the MAC address is set correctly -// after the ioctl call, and retry the address setting if necessary. In our -// testing, this workaround appears to workaround scenarios (1), (2), (3), -// and (4) above, but it does not address scenario (5). -// -// See MESOS-10243 for additional details, follow-ups. Try setMAC(const string& link, const net::MAC& mac) { - auto getMacViaIoctl = [](int fd, const string& link) -> Try { -ifreq ifr; -memset(, 0, sizeof(ifr)); -strncpy(ifr.ifr_name, link.c_str(), IFNAMSIZ); -if (ioctl(fd, SIOCGIFHWADDR, ) == -1) { - return ErrnoError(); -} -return ifr; - }; - // Since loopback interface has sa_family ARPHRD_LOOPBACK, we need // to get the MAC address of the link first to decide what value the // sa_family should be. // // TODO(jieyu): We use ioctl to set the MAC address because the // interfaces in libnl have some issues with virtual devices. + struct ifreq ifr; + memset(, 0, sizeof(ifr)); + + strncpy(ifr.ifr_name, link.c_str(), IFNAMSIZ); + int fd = ::socket(AF_INET, SOCK_STREAM, 0); if (fd == -1) { return ErrnoError(); } - Try if_request = getMacViaIoctl(fd, link); - if (if_request.isError()) { + // Since loopback interface has sa_family ARPHRD_LOOPBACK, we need +
(mesos) branch master updated: [veth] Add todo to set mac address on create for peer link.
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 d0db66540 [veth] Add todo to set mac address on create for peer link. d0db66540 is described below commit d0db6654094821cfb7520720041b30abef974d5b Author: Jason Zhou AuthorDate: Mon Jul 15 13:02:55 2024 -0400 [veth] Add todo to set mac address on create for peer link. Due to a systemd-induced race-condition related to the MacAddressPolicy being set to 'persistent' on versions >= 242, we will have to set the peer link MAC address of the peer link (eth0) when we create the eth0 peer link so that the udev will not try to overwrite it when it is notified that this device was created, which would lead to a race condition here where us and udev are racing to see who is the last one to write our MAC address to eth0. see: https://issues.apache.org/jira/browse/MESOS-10243 Review: https://reviews.apache.org/r/75087/ --- .../containerizer/mesos/isolators/network/port_mapping.cpp | 12 1 file changed, 12 insertions(+) diff --git a/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp b/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp index 3b3b899fb..52b63f524 100644 --- a/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp +++ b/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp @@ -5181,6 +5181,18 @@ string PortMappingIsolatorProcess::scripts(Info* info) // checksum offloading ensures the TCP layer will checksum and drop // it. script << "ethtool -K " << eth0 << " rx off\n"; + // TODO(jasonzhou): Set the link for the container eth0 in veth::create on + // creation to prevent race condition against udev + // + // Due to a systemd-induced race-condition related to the + // MacAddressPolicy being set to 'persistent' on versions >= 242, + // we will have to set the peer link MAC address of the peer link (eth0) + // when we create the eth0 peer link so that the udev will not try to + // overwrite it when it is notified that this device was created, which + // would lead to a race condition here where us and udev are racing to see + // who is the last one to write our MAC address to eth0. + // + // see: https://issues.apache.org/jira/browse/MESOS-10243 script << "ip link set " << eth0 << " address " << hostMAC << " mtu " << hostEth0MTU << " up\n"; script << "ip addr add " << hostIPNetwork << " dev " << eth0 << "\n";
(mesos) branch master updated: [ebpf] Implement atomic replacement of cgroup device programs.
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 3990bf745 [ebpf] Implement atomic replacement of cgroup device programs. 3990bf745 is described below commit 3990bf745ba95f8f9a9bafc9c3a983f191b4c12d Author: Jason Zhou AuthorDate: Sun Jul 14 21:02:52 2024 -0400 [ebpf] Implement atomic replacement of cgroup device programs. Currently, if we try to attach device ebpf files to the same cgroup multiple times, they will all be attached, and they will all be run when a device requests access. This conflicts with our design to have one ebpf file per cgroup that represents all the files they want to allow or deny, where that file is updated when the cgroup adds or removes a device. So we add a patch to atomically replace any existing ebpf file already attached to our target cgroup using our new ebpf file. Review: https://reviews.apache.org/r/75080/ --- src/linux/cgroups2.cpp | 2 +- src/linux/ebpf.cpp | 67 ++-- src/tests/containerizer/cgroups2_tests.cpp | 81 ++ 3 files changed, 135 insertions(+), 15 deletions(-) diff --git a/src/linux/cgroups2.cpp b/src/linux/cgroups2.cpp index 133eec1b2..cb3c425a4 100644 --- a/src/linux/cgroups2.cpp +++ b/src/linux/cgroups2.cpp @@ -1440,7 +1440,7 @@ Try configure( } Try attach = ebpf::cgroups2::attach( - cgroups2::path(cgroup), + cgroup, *program); if (attach.isError()) { diff --git a/src/linux/ebpf.cpp b/src/linux/ebpf.cpp index 50f2c6743..e74768e80 100644 --- a/src/linux/ebpf.cpp +++ b/src/linux/ebpf.cpp @@ -15,6 +15,7 @@ // limitations under the License. #include "linux/ebpf.hpp" +#include "linux/cgroups2.hpp" #include #include @@ -25,9 +26,11 @@ #include "stout/check.hpp" #include "stout/error.hpp" +#include "stout/none.hpp" #include "stout/nothing.hpp" #include "stout/os/close.hpp" #include "stout/os/open.hpp" +#include "stout/stringify.hpp" #include "stout/try.hpp" using std::string; @@ -125,25 +128,44 @@ Try bpf_get_fd_by_id(uint32_t prog_id) // Attaches the eBPF program identified by the provided fd to a cgroup. -// -// TODO(dleamy): This currently does not replace existing programs attached -// to the cgroup, we will need to add replacement to support adding / removing -// device access dynamically. -Try attach(const string& cgroup, int fd) +// If there is an existing ebpf program at the cgroup, we will replace +// it atomically with the provided ebpf program. +Try attach(const string& cgroup, int new_program_fd) { - Try cgroup_fd = os::open(cgroup, O_DIRECTORY | O_RDONLY | O_CLOEXEC); + string cgroup_path = ::cgroups2::path(cgroup); + + Try cgroup_fd = +os::open(cgroup_path, O_DIRECTORY | O_RDONLY | O_CLOEXEC); if (cgroup_fd.isError()) { -return Error("Failed to open '" + cgroup + "': " + cgroup_fd.error()); +return Error("Failed to open cgroup: " + cgroup_fd.error()); + } + + Try> attached = ebpf::cgroups2::attached(cgroup_path); + if (attached.isError()) { +return Error("Failed to retrieve attached bpf device programs: " + + attached.error()); + } else if (attached->size() > 1) { +return Error("Detected multiple (" + stringify(attached->size()) + ")" + " BPF_CGROUP_DEVICE attached to cgroup"); + } + + Result old_program_fd = None(); + + if (attached->size() == 1) { +uint32_t old_program_id = attached->at(0); +old_program_fd = bpf_get_fd_by_id(old_program_id); +if (old_program_fd.isError()) { + return Error("Failed to open existing program fd for id " + + stringify(old_program_id) + ": " + old_program_fd.error()); +} } bpf_attr attr; memset(, 0, sizeof(attr)); attr.attach_type = BPF_CGROUP_DEVICE; attr.target_fd = *cgroup_fd; - attr.attach_bpf_fd = fd; + attr.attach_bpf_fd = new_program_fd; - // TODO(dleamy): Replace any existing attached programs here! - // // BPF_F_ALLOW_MULTI allows multiple eBPF programs to be attached to a single // cgroup and determines how the programs up and down the hierarchy will run. // @@ -163,13 +185,24 @@ Try attach(const string& cgroup, int fd) // cgroup4 run order: F, D, E, A, B // cgroup2 run order: C, A, B // + // If any program in the run order returns a non-successful code, the device + // access will be denied. Thus, we opt to atomically replace the existing + // ebpf device file so that we have exactly one file as our source of truth + // for device access. // For
(mesos) branch master updated: [cgroups2] Remove accidental tabs.
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 5791b35e4 [cgroups2] Remove accidental tabs. 5791b35e4 is described below commit 5791b35e497f51ddbf037db925389bf08c83e81c Author: Benjamin Mahler AuthorDate: Thu Jul 11 15:56:32 2024 -0400 [cgroups2] Remove accidental tabs. --- src/linux/cgroups2.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/linux/cgroups2.cpp b/src/linux/cgroups2.cpp index faf3a5853..133eec1b2 100644 --- a/src/linux/cgroups2.cpp +++ b/src/linux/cgroups2.cpp @@ -1167,7 +1167,7 @@ public: // +-+ static Try build( const vector& allow, - const vector& deny) + const vector& deny) { // The BPF_PROG_TYPE_CGROUP_DEVICE program takes in // `struct bpf_cgroup_dev_ctx*` as input. We extract the fields into @@ -1291,7 +1291,7 @@ public: private: static vector add_device_checks( const Entry& entry, - short trailer_length) + short trailer_length) { // We create a block of bytecode with the format: // 1. Major Version Check @@ -1326,7 +1326,7 @@ private: auto check_access_instructions = [](short jmp_size, const Entry::Access& access) - { +{ int bpf_access = 0; bpf_access |= access.read ? BPF_DEVCG_ACC_READ : 0; bpf_access |= access.write ? BPF_DEVCG_ACC_WRITE : 0;
(mesos) branch master updated: [cgroups2] Make cgroups2::path take both absolute and relative paths.
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 25029f009 [cgroups2] Make cgroups2::path take both absolute and relative paths. 25029f009 is described below commit 25029f0095774f654293d13c3f0fb2b6e21cea16 Author: Jason Zhou AuthorDate: Thu Jul 11 15:08:34 2024 -0400 [cgroups2] Make cgroups2::path take both absolute and relative paths. Currently, cgroups2::path assumes the path in the argument is relative. We want the function to be able to distinguish between absolute and relative paths, where we only prepend the mounting point on the relative path. Review: https://reviews.apache.org/r/75081/ --- src/linux/cgroups2.cpp | 4 +++- src/tests/containerizer/cgroups2_tests.cpp | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/linux/cgroups2.cpp b/src/linux/cgroups2.cpp index 31fee0459..faf3a5853 100644 --- a/src/linux/cgroups2.cpp +++ b/src/linux/cgroups2.cpp @@ -564,7 +564,9 @@ Try> threads(const string& cgroup) string path(const string& cgroup) { - return path::join(cgroups2::MOUNT_POINT, cgroup); + return (!cgroup.empty() && cgroup.at(0) == '/') + ? cgroup + : path::join(cgroups2::MOUNT_POINT, cgroup); } namespace controllers { diff --git a/src/tests/containerizer/cgroups2_tests.cpp b/src/tests/containerizer/cgroups2_tests.cpp index e4796b6ca..2ee39b342 100644 --- a/src/tests/containerizer/cgroups2_tests.cpp +++ b/src/tests/containerizer/cgroups2_tests.cpp @@ -129,6 +129,7 @@ TEST_F(Cgroups2Test, CGROUPS2_Path) EXPECT_EQ("/sys/fs/cgroup/", cgroups2::path(cgroups2::ROOT_CGROUP)); EXPECT_EQ("/sys/fs/cgroup/foo", cgroups2::path("foo")); EXPECT_EQ("/sys/fs/cgroup/foo/bar", cgroups2::path("foo/bar")); + EXPECT_EQ("/mount/cgroup/foo/bar", cgroups2::path("/mount/cgroup/foo/bar")); }
(mesos) branch master updated: [ebpf] Add helper function for getting bpf fd by program id.
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 4fb4788ee [ebpf] Add helper function for getting bpf fd by program id. 4fb4788ee is described below commit 4fb4788ee9f47d4b7146de1384d49c7a2b15b246 Author: Jason Zhou AuthorDate: Thu Jul 11 15:01:43 2024 -0400 [ebpf] Add helper function for getting bpf fd by program id. Introduces a helper function to help abstract away the logic for getting bpf program file descriptor by its program id. Review: https://reviews.apache.org/r/75083/ --- src/linux/ebpf.cpp | 29 --- src/linux/ebpf.hpp | 6 + src/tests/containerizer/cgroups2_tests.cpp | 37 ++ 3 files changed, 64 insertions(+), 8 deletions(-) diff --git a/src/linux/ebpf.cpp b/src/linux/ebpf.cpp index 3f7f74df2..50f2c6743 100644 --- a/src/linux/ebpf.cpp +++ b/src/linux/ebpf.cpp @@ -107,6 +107,23 @@ Try load(const Program& program) namespace cgroups2 { +Try bpf_get_fd_by_id(uint32_t prog_id) +{ + bpf_attr attr; + memset(, 0, sizeof(attr)); + attr.prog_id = prog_id; + + Try fd = bpf(BPF_PROG_GET_FD_BY_ID, , sizeof(attr)); + + if (fd.isError()) { +return Error("bpf syscall to BPF_PROG_GET_FD_BY_ID failed: " + + fd.error().message); + } + + return *fd; +} + + // Attaches the eBPF program identified by the provided fd to a cgroup. // // TODO(dleamy): This currently does not replace existing programs attached @@ -226,18 +243,14 @@ Try detach(const string& cgroup, uint32_t program_id) return Error("Failed to open '" + cgroup + "': " + cgroup_fd.error()); } - bpf_attr attr; - memset(, 0, sizeof(attr)); - attr.prog_id = program_id; - - Try program_fd = -bpf(BPF_PROG_GET_FD_BY_ID, , sizeof(attr)); + Try program_fd = bpf_get_fd_by_id(program_id); if (program_fd.isError()) { -return Error("bpf syscall to BPF_PROG_GET_FD_BY_ID failed: " + - program_fd.error().message); +return Error("Could not get bpf fd from program id: "+ + program_fd.error()); } + bpf_attr attr; memset(, 0, sizeof(attr)); attr.attach_type = BPF_CGROUP_DEVICE; attr.target_fd = *cgroup_fd; diff --git a/src/linux/ebpf.hpp b/src/linux/ebpf.hpp index 5bf9d34d4..5a81ee55a 100644 --- a/src/linux/ebpf.hpp +++ b/src/linux/ebpf.hpp @@ -30,6 +30,9 @@ namespace ebpf { +Try bpf(int cmd, bpf_attr* attr, size_t size); + + // eBPF program. class Program { @@ -49,6 +52,9 @@ public: namespace cgroups2 { +// Get the file descriptor of a valid bpf program by its program id. +Try bpf_get_fd_by_id(uint32_t prog_id); + // Load and attach a BPF_CGROUP_DEVICE eBPF program to a cgroup. Try attach(const std::string& cgroup, const Program& program); diff --git a/src/tests/containerizer/cgroups2_tests.cpp b/src/tests/containerizer/cgroups2_tests.cpp index 751c0e389..e4796b6ca 100644 --- a/src/tests/containerizer/cgroups2_tests.cpp +++ b/src/tests/containerizer/cgroups2_tests.cpp @@ -758,6 +758,43 @@ INSTANTIATE_TEST_CASE_P( } )); + +TEST_F(Cgroups2Test, ROOT_CGROUPS2_GetBpfFdById) +{ + const string& cgroup = TEST_CGROUP; + + ASSERT_SOME(cgroups2::create(cgroup)); + string path = cgroups2::path(cgroup); + + Try> attached = ebpf::cgroups2::attached(path); + ASSERT_SOME(attached); + ASSERT_EQ(0u, attached->size()); + + ASSERT_SOME(devices::configure(cgroup, {}, {})); + attached = ebpf::cgroups2::attached(path); + ASSERT_SOME(attached); + ASSERT_EQ(1u, attached->size()); + + Try cgroup_fd = os::open(path, O_DIRECTORY | O_RDONLY | O_CLOEXEC); + ASSERT_SOME(cgroup_fd); + + Try program_fd = ebpf::cgroups2::bpf_get_fd_by_id(attached->at(0)); + ASSERT_SOME(program_fd); + + bpf_attr attr; + memset(, 0, sizeof(attr)); + attr.attach_type = BPF_CGROUP_DEVICE; + attr.target_fd = *cgroup_fd; + attr.attach_bpf_fd = *program_fd; + + Try result = ebpf::bpf(BPF_PROG_DETACH, , sizeof(attr)); + + os::close(*cgroup_fd); + os::close(*program_fd); + + ASSERT_SOME(result); +} + } // namespace tests { } // namespace internal {
(mesos) branch master updated: Minor edits missed in r/75026.
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 7fcd3c9a5 Minor edits missed in r/75026. 7fcd3c9a5 is described below commit 7fcd3c9a5d19dc1d0223a168c128c3d6b19e02e5 Author: Benjamin Mahler AuthorDate: Wed Jul 10 16:23:52 2024 -0400 Minor edits missed in r/75026. --- src/linux/cgroups2.cpp | 27 --- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/linux/cgroups2.cpp b/src/linux/cgroups2.cpp index 59c212ebc..31fee0459 100644 --- a/src/linux/cgroups2.cpp +++ b/src/linux/cgroups2.cpp @@ -1312,16 +1312,14 @@ private: auto check_major_instructions = [](short jmp_size, int major) { return vector({ - BPF_JMP_IMM( -BPF_JNE, BPF_REG_4, major, jmp_size), + BPF_JMP_IMM(BPF_JNE, BPF_REG_4, major, jmp_size), }); }; auto check_minor_instructions = [](short jmp_size, int minor) { return vector({ - BPF_JMP_IMM( -BPF_JNE, BPF_REG_5, minor, jmp_size), -}); +BPF_JMP_IMM(BPF_JNE, BPF_REG_5, minor, jmp_size) + }); }; auto check_access_instructions = @@ -1354,8 +1352,9 @@ private: }(); return { BPF_JMP_IMM(BPF_JNE, BPF_REG_2, bpf_type, jmp_size), -}; + }; }; + const Entry::Selector& selector = entry.selector; const Entry::Access& access = entry.access; @@ -1365,15 +1364,13 @@ private: bool check_access = !access.mknod || !access.read || !access.write; // The jump sizes here correspond to the size of the bpf instructions -// that each check adds to the program -// the total size of the block is the trailer length plus the total length -// of all checks. -short nxt_blk_jmp_size = - trailer_length + - (check_major ? check_major_instructions(0, 0).size() : 0) + - (check_minor ? check_minor_instructions(0, 0).size() : 0) + - (check_access ? check_access_instructions(0, access).size() : 0) + - (check_type ? check_type_instructions(0, selector).size() : 0); +// that each check adds to the program. The total size of the block is +// the trailer length plus the total length of all checks. +short nxt_blk_jmp_size = trailer_length + + (check_major ? check_major_instructions(0, 0).size() : 0) + + (check_minor ? check_minor_instructions(0, 0).size() : 0) + + (check_access ? check_access_instructions(0, access).size() : 0) + + (check_type ? check_type_instructions(0, selector).size() : 0); // We subtract one because the program counter will be one ahead when it // is executing the code in this code block, so we need to jump one less
(mesos) branch master updated: [cgroups2] Fix allow deny semantics for device access.
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 f91700d71 [cgroups2] Fix allow deny semantics for device access. f91700d71 is described below commit f91700d71987203c2dd9f0917ad53be0eb39a655 Author: Jason Zhou AuthorDate: Wed Jul 10 15:37:50 2024 -0400 [cgroups2] Fix allow deny semantics for device access. Currently, the EBPF program we generate has the behavior where the deny list has no effect, as we will allow device access iff the device matched with an allow entry. Instead we want to grant access to a device iff it is in a cgroup's allow list *and not in its deny list.* This means that we need to change our existing logic, which exits on the first match. It is not our desired behavior because the current EBPF program construction logic puts the allow-device checks before the deny-device checks, meaning that if a device is on both allow and deny lists for a cgroup, it will be granted access. This change revamps the EBPF program construction to now check both the allow and deny list of a cgroup before determining whether access may be granted. Specifically, if a device is matched with an entry inside the allow list, we will also be checking if it matches with any entry on the deny list, and deny the device's access if that is the case. We also avoid generating specific parts of the EBPF program code to avoid creating unreachable code, explanations with a diagram are attached above the cgroups2::devices::DeviceProgram::build function. Review: https://reviews.apache.org/r/75026/ --- src/linux/cgroups.cpp | 8 + src/linux/cgroups.hpp | 1 + src/linux/cgroups2.cpp | 372 + src/linux/cgroups2.hpp | 2 + src/tests/containerizer/cgroups2_tests.cpp | 74 ++ 5 files changed, 364 insertions(+), 93 deletions(-) diff --git a/src/linux/cgroups.cpp b/src/linux/cgroups.cpp index c1272fbca..c42894933 100644 --- a/src/linux/cgroups.cpp +++ b/src/linux/cgroups.cpp @@ -2906,6 +2906,14 @@ Try Entry::parse(const string& s) } +bool Entry::is_catch_all() const +{ + return !selector.major.isSome() && !selector.minor.isSome() + && selector.type == Entry::Selector::Type::ALL + && access.mknod && access.read && access.write; +} + + Try> list( const string& hierarchy, const string& cgroup) diff --git a/src/linux/cgroups.hpp b/src/linux/cgroups.hpp index 9be53e31e..eaa4cdf9f 100644 --- a/src/linux/cgroups.hpp +++ b/src/linux/cgroups.hpp @@ -933,6 +933,7 @@ namespace devices { struct Entry { static Try parse(const std::string& s); + bool is_catch_all() const; struct Selector { diff --git a/src/linux/cgroups2.cpp b/src/linux/cgroups2.cpp index d1fc2638c..59c212ebc 100644 --- a/src/linux/cgroups2.cpp +++ b/src/linux/cgroups2.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include @@ -1086,7 +1087,85 @@ namespace devices { class DeviceProgram { public: - DeviceProgram() : program{ebpf::Program(BPF_PROG_TYPE_CGROUP_DEVICE)} + // We will generate one allow block for each entry in the allow list + // and one deny block for each entry in the deny list. + // + // There are special cases for catch-all values or empty allow lists + // Which are done to avoid generating unreachable code which are prohibited + // by the verifier: + // 1. If we have a catch-all in the allow entries, we will not generate any + //code for allow section, since there is no need to check if any entries + //match anything in allow. + // 2. If we have a catch-all in the deny entries, we will immediately return + //with the deny value still in R0 to indicate that access is denied. + // 3. If we have an empty allow list, we will immediately return with the deny + //value still in R0 to indicate that access is denied. + // + // --- + // Normal code flow + // +-+ + // |Initialize R0 to deny value | + // +-+---+ + // |Allow Block | | + // |Check each register, jump to next block if there is no match | | + // | | | + // |If each register has matched, jump over the exit instruction | | + //
(mesos) branch master updated: [ssl] Remove TLS 1.0 and 1.1 tests.
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 8cf287778 [ssl] Remove TLS 1.0 and 1.1 tests. 8cf287778 is described below commit 8cf287778371c13ee7e88fa428424b3c0fbc7ff0 Author: Jason Zhou AuthorDate: Tue Jul 2 23:53:11 2024 -0400 [ssl] Remove TLS 1.0 and 1.1 tests. Currently the SSLProtocolTest with TLS v1.0 and v1.1 do not pass because those versions were disabled in ubuntu 20.04, see: https://discourse.ubuntu.com/t/spec-tls-1-0-and-1-1-are-disabled-by-default/41868 https://github.com/SoftEtherVPN/SoftEtherVPN/issues/1358#issuecomment-851427905 Review: https://reviews.apache.org/r/75075/ --- 3rdparty/libprocess/src/tests/ssl_tests.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/3rdparty/libprocess/src/tests/ssl_tests.cpp b/3rdparty/libprocess/src/tests/ssl_tests.cpp index c90d1da62..10e609d8a 100644 --- a/3rdparty/libprocess/src/tests/ssl_tests.cpp +++ b/3rdparty/libprocess/src/tests/ssl_tests.cpp @@ -123,8 +123,8 @@ static const vector protocols = { #ifndef OPENSSL_NO_SSL3 "LIBPROCESS_SSL_ENABLE_SSL_V3", #endif - "LIBPROCESS_SSL_ENABLE_TLS_V1_0", - "LIBPROCESS_SSL_ENABLE_TLS_V1_1", + // Removed TLS 1.0 and 1.1 as they're not supported by ubuntu 20.04 + // https://discourse.ubuntu.com/t/spec-tls-1-0-and-1-1-are-disabled-by-default/41868 "LIBPROCESS_SSL_ENABLE_TLS_V1_2", // On some platforms, we need to build against OpenSSL versions that // do not support TLS 1.3 yet.
(mesos) branch master updated: [build] Fix libevent-enabled cmake builds on ubuntu 20.04.
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 afb84f8ca [build] Fix libevent-enabled cmake builds on ubuntu 20.04. afb84f8ca is described below commit afb84f8cac8ad6d9e4c3b2a02af053b5f12ba071 Author: Jason Zhou AuthorDate: Tue Jul 2 17:57:11 2024 -0400 [build] Fix libevent-enabled cmake builds on ubuntu 20.04. Our current libevent-enabled cmake builds cannot complete on jenkins as it gets the 'incomplete definition of type 'struct bio_st'' error. This is because the upgrade to ubuntu 20.04 also upgraded our openSSL version from 1.0.2 to 1.1.1, which breaks compatibility with libevent 2.1.5 that was previously used. A compatibility patch for openSSL 1.1+ was released with libevent 2.1.7, but the closest tarball that includes a CMakeLists.txt file is 2.1.9, which is what we will upgrade to. With the new libevent library, builds are able to complete using cmake and with libevents enabled. But it still sees the test failures we see on other builds (such as autotools) and operating system (CentOS 7). We also need to link libevent_pthread and libevent_openssl with libprocess, otherwise we will get errors like: ``` ld: 3rdparty/libprocess/src/libprocess.so: undefined reference to bufferevent_openssl_get_ssl ``` Review: https://reviews.apache.org/r/75070/ --- 3rdparty/CMakeLists.txt| 53 +++--- 3rdparty/cmake/Versions.cmake | 4 +-- 3rdparty/libprocess/src/CMakeLists.txt | 13 - cmake/CompilationConfigure.cmake | 5 4 files changed, 68 insertions(+), 7 deletions(-) diff --git a/3rdparty/CMakeLists.txt b/3rdparty/CMakeLists.txt index e1f71283e..bec004618 100644 --- a/3rdparty/CMakeLists.txt +++ b/3rdparty/CMakeLists.txt @@ -48,8 +48,7 @@ set(ZOOKEEPER_URL ${FETCH_URL}/zookeeper-${ZOOKEEPER_VERSION}.tar.gz) # NOTE: libevent doesn't come rebundled, so this URL is always the same. But, # it's only downloaded if `ENABLE_LIBEVENT` is set. -set(LIBEVENT_URL ${3RDPARTY_DEPENDENCIES}/libevent-release-${LIBEVENT_VERSION}.tar.gz) - +set(LIBEVENT_URL ${LIBEVENT_RELEASES}/release-${LIBEVENT_VERSION}/libevent-${LIBEVENT_VERSION}.tar.gz) if (WIN32) # NOTE: These dependencies are only rebundled on Windows because they # are available as installable packages on Linux; so they live @@ -700,6 +699,22 @@ if (ENABLE_LIBEVENT) add_library(libevent ${LIBRARY_LINKAGE} IMPORTED) add_dependencies(libevent ${LIBEVENT_TARGET}) +if (ENABLE_SSL) + add_library(libevent_openssl ${LIBRARY_LINKAGE} IMPORTED) + set_target_properties( +libevent_openssl PROPERTIES +INTERFACE_INCLUDE_DIRECTORIES "${LIBEVENT_ROOT}/include;${LIBEVENT_ROOT}-build/include" + ) +endif () + +if (BUILD_SHARED_LIBS) + add_library(libevent_pthreads ${LIBRARY_LINKAGE} IMPORTED) + set_target_properties( +libevent_pthreads PROPERTIES +INTERFACE_INCLUDE_DIRECTORIES "${LIBEVENT_ROOT}/include;${LIBEVENT_ROOT}-build/include" + ) +endif () + set_target_properties( libevent PROPERTIES INTERFACE_INCLUDE_DIRECTORIES "${LIBEVENT_ROOT}/include;${LIBEVENT_ROOT}-build/include") @@ -721,16 +736,32 @@ if (ENABLE_LIBEVENT) IMPORTED_LOCATION ${LIBEVENT_ROOT}-build/lib/libevent${LIBRARY_SUFFIX}) endif () +if (ENABLE_SSL) + set_target_properties( +libevent_openssl PROPERTIES +IMPORTED_LOCATION ${LIBEVENT_ROOT}-build/lib/libevent_openssl${LIBRARY_SUFFIX}) +endif () + +if (BUILD_SHARED_LIBS) + set_target_properties( +libevent_pthreads PROPERTIES +IMPORTED_LOCATION ${LIBEVENT_ROOT}-build/lib/libevent_pthreads${LIBRARY_SUFFIX}) + set(EVENT__LIBRARY_TYPE SHARED) +else () + set(EVENT__LIBRARY_TYPE STATIC) +endif () + set(LIBEVENT_CMAKE_FORWARD_ARGS ${CMAKE_C_FORWARD_ARGS} ${CMAKE_SSL_FORWARD_ARGS} # NOTE: Libevent does not respect the BUILD_SHARED_LIBS global flag. - -DEVENT__BUILD_SHARED_LIBRARIES=${BUILD_SHARED_LIBS} + -DEVENT__LIBRARY_TYPE=${EVENT__LIBRARY_TYPE} -DEVENT__DISABLE_OPENSSL=$> -DEVENT__DISABLE_BENCHMARK=ON -DEVENT__DISABLE_REGRESS=ON -DEVENT__DISABLE_SAMPLES=ON - -DEVENT__DISABLE_TESTS=ON) + -DEVENT__DISABLE_TESTS=ON + -DEVENT__DISABLE_THREAD_SUPPORT=OFF) if (CMAKE_C_COMPILER_ID MATCHES GNU OR CMAKE_C_COMPILER_ID MATCHES Clang) list(APPEND LIBEVENT_CMAKE_FORWARD_ARGS -DCMAKE_C_FLAGS=-fPIC) @@ -752,6 +783,20 @@ if (ENABLE_LIBEVENT) FILES $ DESTINATION ${3RDPARTY_LIBS_INSTALL_DIR}) +if (ENABLE_SSL) + install( +
(mesos) branch master updated: [build] Fix docker-build.sh failing to compile with distcheck.
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 abc38a685 [build] Fix docker-build.sh failing to compile with distcheck. abc38a685 is described below commit abc38a685d9a7ffb1196e1c0d6b58eff296702af Author: Jason Zhou AuthorDate: Tue Jul 2 17:55:05 2024 -0400 [build] Fix docker-build.sh failing to compile with distcheck. Review: https://reviews.apache.org/r/75066/ --- support/docker-build.sh | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/support/docker-build.sh b/support/docker-build.sh index 1273848d9..cb0b05b6e 100755 --- a/support/docker-build.sh +++ b/support/docker-build.sh @@ -78,7 +78,17 @@ case $OS in # Install dependencies. # IBM Power only supports Ubuntu 14.04 and gcc compiler. -[ "$(uname -m)" = "x86_64" ] && CLANG_PKG=clang-3.5 || CLANG_PKG= +if [ "$(uname -m)" = "x86_64" ]; then + # We install clang-10 on non-ubuntu-16.04 systems because newer OS + # no longer support clang 3.5 + if [[ "$OS" = "ubuntu:16.04" ]]; then +CLANG_PKG=clang-3.5 + else +CLANG_PKG=clang-10 + fi +else + CLANG_PKG= +fi append_dockerfile "RUN apt-get update" append_dockerfile "RUN apt-get install -y build-essential $CLANG_PKG git maven autoconf libtool software-properties-common" append_dockerfile "RUN apt-get install -y python-dev python-six libcurl4-nss-dev libsasl2-dev libapr1-dev libsvn-dev libevent-dev libev-dev libssl-dev"
(mesos) branch master updated: [build] Use ubuntu:20.04 for verify-reviews.py.
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 c3d0d710c [build] Use ubuntu:20.04 for verify-reviews.py. c3d0d710c is described below commit c3d0d710cbbd4e0b716425b4d8b136f3ce728b96 Author: Jason Zhou AuthorDate: Tue Jul 2 16:02:40 2024 -0400 [build] Use ubuntu:20.04 for verify-reviews.py. Moving to ubuntu 20.04 so so that we can get the ebpf header files for our build. Review: https://reviews.apache.org/r/75063/ --- support/verify-reviews.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/support/verify-reviews.py b/support/verify-reviews.py index 6a28e8279..8e7c54faa 100755 --- a/support/verify-reviews.py +++ b/support/verify-reviews.py @@ -244,7 +244,7 @@ def verify_review(review_request): else: # Launch docker build script. configuration = ("export " - "OS='ubuntu:16.04' " + "OS='ubuntu:20.04' " "BUILDTOOL='autotools' " "COMPILER='gcc' " "CONFIGURATION='--verbose "
(mesos) branch master updated: [build] Fix make distcheck for ubuntu 20.04.
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 64aba8a83 [build] Fix make distcheck for ubuntu 20.04. 64aba8a83 is described below commit 64aba8a83821d2ea9b7bccd2d7b2dfa9ba6a3635 Author: Jason Zhou AuthorDate: Tue Jul 2 15:59:17 2024 -0400 [build] Fix make distcheck for ubuntu 20.04. Fixes the makefiles for ubuntu 20.04 so that make distcheck works properly with its protobuf dependencies generated as part of make distcheck. The reason that a change was needed in the makefile is because the upgrade from ubuntu 16.04 to 20.04 also caused the automake version to be updated when dependencies were being installed during docker build. The change in the automake version created slight changes in the generated makefile. Speficically, the distcheck on the new automake-generated makefile now depends on `BUILT_SOURCES` which causes an error as the CSI protobuf files are not ready when distcheck is called. So we add the csi build stamps to `BUILT_SOURCES` to ensure that the protobuf files will be ready when distcheck's dependencies are made. The additional chmod change for java is because for some reason when distcheck attempts to build the mesos-1.12.0.jar from its created distribution, some folders are missing write permissions, causing the build to terminate. Review: https://reviews.apache.org/r/75062/ --- 3rdparty/Makefile.am | 2 ++ src/Makefile.am | 2 ++ 2 files changed, 4 insertions(+) diff --git a/3rdparty/Makefile.am b/3rdparty/Makefile.am index 55f49ee59..25950e979 100644 --- a/3rdparty/Makefile.am +++ b/3rdparty/Makefile.am @@ -578,6 +578,8 @@ $(CSI_V1)-build-stamp: $(CSI_V1)-stamp mv $(CSI_V1)/spec-$(CSI_V1_VERSION)/csi.proto $(CSI_V1)/csi/v1 touch $@ +BUILT_SOURCES += $(CSI_V0)-build-stamp $(CSI_V1)-build-stamp + # Mesos depends on CSI. Install the CSI proto file into $PREFIX/include but # don't add it to the source tarball. csi_v0dir = $(includedir)/csi/v0 diff --git a/src/Makefile.am b/src/Makefile.am index b6acfb5ea..03eb0cc28 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -2211,6 +2211,8 @@ if HAS_JAVAH -classpath $(MESOS_JAR):@PROTOBUF_JAR@\ $(subst _,.,$(*F)) else + @chmod u+w $(srcdir)/java/src/$(subst _,/,$(*D)) && \ + find $(srcdir)/java/src/$(subst _,/,$(*D)) -type d -exec chmod u+w {} + $(JAVA_HOME)/bin/javac -h java/jni \ -classpath $(MESOS_JAR):@PROTOBUF_JAR@\ $(srcdir)/java/src/$(subst _,/,$(*F)).java
(mesos) branch master updated: CHANGE: add default network if no net options was set.
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 b6750fe61 CHANGE: add default network if no net options was set. b6750fe61 is described below commit b6750fe619f3b573e7e00a361117091da0fbc6de Author: Andreas Peters AuthorDate: Mon May 27 22:33:32 2024 +0200 CHANGE: add default network if no net options was set. --- src/docker/docker.cpp | 30 +++--- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/src/docker/docker.cpp b/src/docker/docker.cpp index 8ad58339b..c2a92099a 100644 --- a/src/docker/docker.cpp +++ b/src/docker/docker.cpp @@ -947,18 +947,26 @@ Try Docker::RunOptions::create( options.name = name; bool dnsSpecified = false; + bool networkSpecified = false; foreach (const Parameter& parameter, dockerInfo.parameters()) { -options.additionalOptions.push_back( -"--" + parameter.key() + "=" + parameter.value()); - -// In Docker 1.13.0, `--dns-option` was added and `--dns-opt` was hidden -// (but it can still be used), so here we need to check both of them. -if (!dnsSpecified && -(parameter.key() == "dns" || - parameter.key() == "dns-search" || - parameter.key() == "dns-opt" || - parameter.key() == "dns-option")) { - dnsSpecified = true; +if (!networkSpecified && +(parameter.key() == "net" || + parameter.key() == "network")) { + options.network = parameter.value(); + networkSpecified = true; +} else { + options.additionalOptions.push_back( + "--" + parameter.key() + "=" + parameter.value()); + + // In Docker 1.13.0, `--dns-option` was added and `--dns-opt` was hidden + // (but it can still be used), so here we need to check both of them. + if (!dnsSpecified && + (parameter.key() == "dns" || + parameter.key() == "dns-search" || + parameter.key() == "dns-opt" || + parameter.key() == "dns-option")) { +dnsSpecified = true; + } } }
(mesos) branch master updated: [port mapping isolator] Work around apparent MAC address kernel bug.
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 8b202bbeb [port mapping isolator] Work around apparent MAC address kernel bug. 8b202bbeb is described below commit 8b202bbebdc89429ad82c6983aa1c514eb1b8d95 Author: Jason Zhou AuthorDate: Thu Jun 20 00:25:18 2024 -0400 [port mapping isolator] Work around apparent MAC address kernel bug. It seems that there are scenarios where, when using the port mapping isolator, mesos containers sometimes cannot communicate with the mesos agent as the MAC address of the veth interface is set incorrectly, leading to dropped packets by the kernel. This was discovered with the use of tcpdump (which reveals that the kernel marks the packets as destined for another host), and the latter of which reveals that the kernel is indeed dropping the packets due to this. We then found that when we set the mac address on the veth interface, it sometimes does not "stick" despite ioctl returning successfully. Observed scenarios with incorrectly assigned MAC addresses: 1. After setting the mac address: ioctl returns the correct MAC address, but net::mac returns an incorrect MAC address (different from the original!) 2. After setting the mac address: both ioctl and net::mac return the same MAC address, but are both wrong (and different from the original one!) 3. After setting the mac address: there are no cases where ioctl or net::mac come back with the same MAC address as before we set the address. 4. Before we set the mac address: there is a possibility that ioctl and net::mac results disagree with each other! 5. There is a possibility that the MAC address we set ends up overwritten by a garbage value after setMAC has already completed and checked that the mac address was set correctly. Since this error happens after this function has finished, we cannot log nor detect it in setMAC because we have not yet studied at what point this occurs. Notes: 1. We have observed this behavior only on CentOS 9 systems at the moment, CentOS 7 systems under various kernels do not seem to have the issue (which is quite strange if this was purely a kernel bug). 2. We have tried kernels 5.15.147, 5.15.160, 5.15.161, all of these have this issue on CentOS 9. This patch adds a workaround for this bug, which is to check that the MAC address is set correctly after the ioctl call, and retry the address setting if necessary. In our testing, this workaround appears to workaround scenarios (1), (2), (3), and (4) above, but it does not address scenario (5). See MESOS-10243 for additional details, follow-ups. Review: https://reviews.apache.org/r/75057/ --- src/linux/routing/link/link.cpp | 126 1 file changed, 103 insertions(+), 23 deletions(-) diff --git a/src/linux/routing/link/link.cpp b/src/linux/routing/link/link.cpp index 87ca26c54..bb97e3281 100644 --- a/src/linux/routing/link/link.cpp +++ b/src/linux/routing/link/link.cpp @@ -246,45 +246,125 @@ Try setUp(const string& link) } + +// There seems to be a bug that is occassionally preventing ioctl and libnl +// from setting correct MAC addresses, despite them not returning errors. +// +// Observed scenarios with incorrectly assigned MAC addresses: +// +// 1. After setting the mac address: ioctl returns the correct MAC address, but +//net::mac returns an incorrect MAC address (different from the original!) +// 2. After setting the mac address: both ioctl and net::mac return the same MAC +//address, but are both wrong (and different from the original one!) +// 3. After setting the mac address: there are no cases where ioctl or net::mac +//come back with the same MAC address as before we set the address. +// 4. Before we set the mac address: there is a possibility that ioctl and +//net::mac results disagree with each other! +// 5. There is a possibility that the MAC address we set ends up overwritten by +//a garbage value after setMAC has already completed and checked that the +//mac address was set correctly. Since this error happens after this +//function has finished, we cannot log nor detect it in setMAC because we +//have not yet studied at what point this occurs. +// +// Notes: +// +// 1. We have observed this behavior only on CentOS 9 systems at the moment, +//CentOS 7 systems under various kernels do not seem to have the issue +//(which is quite strange if this was purely a kernel bug). +// 2. We have tried kernels 5.15.147, 5.15.160, 5.15.161, all of these have +//this issue on CentOS 9. +// +// To workaround this bug, we check that t
(mesos) branch master updated: Support constructing net::MAC objects from sockaddr.sa_data.
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 fe2f38f2e Support constructing net::MAC objects from sockaddr.sa_data. fe2f38f2e is described below commit fe2f38f2eae0f8be21507ab298935d53d93a07da Author: Jason Zhou AuthorDate: Wed Jun 19 13:47:36 2024 -0400 Support constructing net::MAC objects from sockaddr.sa_data. When using ioctl, we get a char[14] sa_data array from sockaddr that holds the information necessary to construct a net::MAC object, this patch adds support for using the sa_data field to directly create a net::MAC object. Review: https://reviews.apache.org/r/75058/ --- 3rdparty/stout/include/stout/mac.hpp | 12 3rdparty/stout/tests/mac_tests.cpp | 7 +++ 2 files changed, 19 insertions(+) diff --git a/3rdparty/stout/include/stout/mac.hpp b/3rdparty/stout/include/stout/mac.hpp index 298cc1422..eb5e4d66e 100644 --- a/3rdparty/stout/include/stout/mac.hpp +++ b/3rdparty/stout/include/stout/mac.hpp @@ -45,6 +45,7 @@ #include #include +#include #include #include #include @@ -120,6 +121,17 @@ public: } } + // Template-based constructor for constructing from character array. + // This is useful for sockaddr.sa_data (char[14]). + template + explicit MAC(const char (&_bytes)[N]) + { +CHECK_GE(N, 6u); +for (size_t i = 0; i < 6; i++) { + bytes[i] = _bytes[i]; +} + } + // Returns the byte at the given index. For example, for a MAC // address 01:23:45:67:89:ab, mac[0] = 01, mac[1] = 23 and etc. uint8_t operator[](size_t index) const diff --git a/3rdparty/stout/tests/mac_tests.cpp b/3rdparty/stout/tests/mac_tests.cpp index ebd50a005..83d180ff2 100644 --- a/3rdparty/stout/tests/mac_tests.cpp +++ b/3rdparty/stout/tests/mac_tests.cpp @@ -65,6 +65,13 @@ TEST(NetTest, ConstructMAC) uint8_t bytes[6] = {0x12, 0x34, 0x56, 0x78, 0x9a, 0xbc}; EXPECT_EQ("12:34:56:78:9a:bc", stringify(net::MAC(bytes))); + + // Mimicking a sockaddr.sa_data field which contains a MAC address, + // this is how ioctl exposes mac addresses. + char sa_data[14] = {0x12, 0x34, 0x56, 0x78, 0x0a, 0x14, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; + + EXPECT_EQ("12:34:56:78:0a:14", stringify(net::MAC(sa_data))); }
(mesos) branch master updated: [routing] Change link::setMAC to return Try.
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 fd38b0b35 [routing] Change link::setMAC to return Try. fd38b0b35 is described below commit fd38b0b351b4e95135d6208f58c81902dbd9ce7f Author: Jason Zhou AuthorDate: Tue Jun 18 16:29:32 2024 -0400 [routing] Change link::setMAC to return Try. We have noticed that our code does not treat the setMAC bool return value differently based on whether it returns true or false. As such, we are changing the return type to return Nothing so that we either return Error or Nothing, rather than Error or True or False. As a consequence of this we are also removing the special case of returning False but not Error when we get ENODEV from ioctl. Review: https://reviews.apache.org/r/75056/ --- src/linux/routing/link/link.cpp| 30 -- src/linux/routing/link/link.hpp| 2 +- .../mesos/isolators/network/port_mapping.cpp | 4 +-- src/tests/containerizer/routing_tests.cpp | 6 ++--- 4 files changed, 16 insertions(+), 26 deletions(-) diff --git a/src/linux/routing/link/link.cpp b/src/linux/routing/link/link.cpp index bff172dea..87ca26c54 100644 --- a/src/linux/routing/link/link.cpp +++ b/src/linux/routing/link/link.cpp @@ -246,7 +246,7 @@ Try setUp(const string& link) } -Try setMAC(const string& link, const net::MAC& mac) +Try setMAC(const string& link, const net::MAC& mac) { // TODO(jieyu): We use ioctl to set the MAC address because the // interfaces in libnl have some issues with virtual devices. @@ -264,15 +264,10 @@ Try setMAC(const string& link, const net::MAC& mac) // to get the MAC address of the link first to decide what value the // sa_family should be. if (ioctl(fd, SIOCGIFHWADDR, ) == -1) { -if (errno == ENODEV) { - os::close(fd); - return false; -} else { - // Save the error string as os::close may overwrite errno. - const string message = os::strerror(errno); - os::close(fd); - return Error(message); -} +// Save the error string as os::close may overwrite errno. +const string message = os::strerror(errno); +os::close(fd); +return Error(message); } ifr.ifr_hwaddr.sa_data[0] = mac[0]; @@ -283,19 +278,14 @@ Try setMAC(const string& link, const net::MAC& mac) ifr.ifr_hwaddr.sa_data[5] = mac[5]; if (ioctl(fd, SIOCSIFHWADDR, ) == -1) { -if (errno == ENODEV) { - os::close(fd); - return false; -} else { - // Save the error string as os::close may overwrite errno. - const string message = os::strerror(errno); - os::close(fd); - return Error(message); -} +// Save the error string as os::close may overwrite errno. +const string message = os::strerror(errno); +os::close(fd); +return Error(message); } os::close(fd); - return true; + return Nothing(); } diff --git a/src/linux/routing/link/link.hpp b/src/linux/routing/link/link.hpp index 35639afa4..09790f317 100644 --- a/src/linux/routing/link/link.hpp +++ b/src/linux/routing/link/link.hpp @@ -82,7 +82,7 @@ Try setUp(const std::string& link); // Sets the MAC address of the link. Returns false if the link is not // found. -Try setMAC(const std::string& link, const net::MAC& mac); +Try setMAC(const std::string& link, const net::MAC& mac); // Returns the Maximum Transmission Unit (MTU) of the link. Returns diff --git a/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp b/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp index 1fd2e226c..3b3b899fb 100644 --- a/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp +++ b/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp @@ -2504,7 +2504,7 @@ Try PortMappingIsolatorProcess::create(const Flags& flags) // recent kernel patch is needed for this operation to succeed: // https://git.kernel.org/cgit/linux/kernel/git/davem/net.git/: // 25f929fbff0d1bcebf2e92656d33025cd330cbf8 - Try setHostLoMAC = link::setMAC(lo.get(), hostMAC.get()); + Try setHostLoMAC = link::setMAC(lo.get(), hostMAC.get()); if (setHostLoMAC.isError()) { return Error( "Failed to set the MAC address of " + lo.get() + @@ -3625,7 +3625,7 @@ Future PortMappingIsolatorProcess::isolate( // Sets the MAC address of veth to match the MAC address of the host // public interface (eth0). - Try setVethMAC = link::setMAC(veth(pid), hostMAC); + Try setVethMAC = link::setMAC(veth(pid), hostMAC); if (setVethMAC.isError()) { return Failure( "Failed to set the MAC address of " + veth(pid) + diff --git a/src/tests/containerizer/routing_tests.cpp b/src/tests/c
(mesos) branch master updated: [route] Use nl_addr_iszero helper when checking for destination IP network.
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 82bff25ef [route] Use nl_addr_iszero helper when checking for destination IP network. 82bff25ef is described below commit 82bff25ef26559e36b9bfa26fade96a53a492f79 Author: Jason Zhou AuthorDate: Wed Jun 12 11:11:09 2024 -0400 [route] Use nl_addr_iszero helper when checking for destination IP network. Previously, when grabbing the destination, we would filter out the default address at 0.0.0.0/0 by checking that the destination pointer is pointing at an empty struct. On newer Linux, it seems to be possible that the destination pointer can be pointing at a valid struct that corresponds to 0.0.0.0/0. To ensure that we are able accurately filter out the default route, we switch to the libnl function nl_addr_iszero to determine if the nl_addr struct corresponds to 0.0.0.0/0. We also apply this change to other areas where nl_addr_get_len is used to ensure that non-empty nl_addr with only zeroes are accounted for. Review: https://reviews.apache.org/r/75046/ --- src/linux/routing/diagnosis/diagnosis.cpp | 2 +- src/linux/routing/route.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/linux/routing/diagnosis/diagnosis.cpp b/src/linux/routing/diagnosis/diagnosis.cpp index eed84533c..cd5a31ff7 100644 --- a/src/linux/routing/diagnosis/diagnosis.cpp +++ b/src/linux/routing/diagnosis/diagnosis.cpp @@ -41,7 +41,7 @@ namespace socket { static Option IP(nl_addr* _ip) { - if (_ip != nullptr && nl_addr_get_len(_ip) != 0) { + if (_ip != nullptr && !nl_addr_iszero(_ip)) { if (nl_addr_get_family(_ip) == AF_INET) { struct in_addr* addr = (struct in_addr*)nl_addr_get_binary_addr(_ip); return net::IP(*addr); diff --git a/src/linux/routing/route.cpp b/src/linux/routing/route.cpp index bdf29a9c7..03cefaff9 100644 --- a/src/linux/routing/route.cpp +++ b/src/linux/routing/route.cpp @@ -77,7 +77,7 @@ Try> table() // Get the destination IP network if exists. Option destination; struct nl_addr* dst = rtnl_route_get_dst(route); - if (dst != nullptr && nl_addr_get_len(dst) != 0) { + if (dst != nullptr && !nl_addr_iszero(dst)) { struct in_addr* addr = (struct in_addr*) nl_addr_get_binary_addr(dst); Try network = net::IP::Network::create( net::IP(*addr), @@ -96,7 +96,7 @@ Try> table() Option gateway; struct rtnl_nexthop* hop = rtnl_route_nexthop_n(route, 0); struct nl_addr* gw = rtnl_route_nh_get_gateway(CHECK_NOTNULL(hop)); - if (gw != nullptr && nl_addr_get_len(gw) != 0) { + if (gw != nullptr && !nl_addr_iszero(gw)) { struct in_addr* addr = (struct in_addr*) nl_addr_get_binary_addr(gw); gateway = net::IP(*addr); }
(mesos) branch master updated: [cgroups2] Fix a compilation error on CentOS 7 due to move operations.
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 1990f11da [cgroups2] Fix a compilation error on CentOS 7 due to move operations. 1990f11da is described below commit 1990f11dacebc32930d31ec794897f019bc31723 Author: Benjamin Mahler AuthorDate: Tue Jun 11 01:28:38 2024 -0400 [cgroups2] Fix a compilation error on CentOS 7 due to move operations. --- src/tests/mesos.cpp | 13 + 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/tests/mesos.cpp b/src/tests/mesos.cpp index 53f35be16..6c29e1b29 100644 --- a/src/tests/mesos.cpp +++ b/src/tests/mesos.cpp @@ -795,20 +795,17 @@ void ContainerizerTest::SetUpCgroupsV2() ASSERT_SOME(cgroups2::create(TEST_CGROUPS_ROOT)); } - Try> _controllers = cgroups2::controllers::available( + Try> controllers = cgroups2::controllers::available( cgroups2::ROOT_CGROUP); - ASSERT_SOME(_controllers); - subsystems = *_controllers; - set controllers( - std::make_move_iterator(_controllers->begin()), - std::make_move_iterator(_controllers->end())); + ASSERT_SOME(controllers); + subsystems = *controllers; // Enable all of the controllers inside of the test root cgroup so they // are accessible from the child container cgroups. ASSERT_TRUE(cgroups2::exists(TEST_CGROUPS_ROOT)); ASSERT_SOME( - cgroups2::controllers::enable(cgroups2::ROOT_CGROUP, controllers)); - ASSERT_SOME(cgroups2::controllers::enable(TEST_CGROUPS_ROOT, controllers)); + cgroups2::controllers::enable(cgroups2::ROOT_CGROUP, *controllers)); + ASSERT_SOME(cgroups2::controllers::enable(TEST_CGROUPS_ROOT, *controllers)); }
(mesos) branch master updated: [cgroups2] Fix multi-line comment compilation warning.
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 8dd469b53 [cgroups2] Fix multi-line comment compilation warning. 8dd469b53 is described below commit 8dd469b5336073763654a281fe852118d131be4b Author: Benjamin Mahler AuthorDate: Tue Jun 11 00:44:51 2024 -0400 [cgroups2] Fix multi-line comment compilation warning. This fixes a compilation warning due to the comment line ending with a backslash character. --- src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.hpp | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.hpp b/src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.hpp index e193f3f1f..d60215ec1 100644 --- a/src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.hpp +++ b/src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.hpp @@ -46,11 +46,12 @@ namespace slave { // https://docs.kernel.org/admin-guide/cgroup-v2.html#no-internal-process-constraint // NOLINT // // Example cgroups: -// containerA non-leaf cgroup -// / \ /\ -// processes containerB leaf cgroup non-leaf child cgroup -// | | -//processes leaf-cgroup +// +// containerA non-leaf cgroup +// / \ / | +// processes containerB leaf cgroup non-leaf child cgroup +// | | +// processes leaf-cgroup // // TODO(dleamy): Nested containers are not yet supported. class Cgroups2IsolatorProcess : public MesosIsolatorProcess
(mesos) 01/02: [cgroups2] Fix control reaches end of non-void function.
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 commit 2c9c8180fb342690110c918944de0fe7ed3f69c7 Author: Jason Zhou AuthorDate: Mon Jun 10 18:53:36 2024 -0400 [cgroups2] Fix control reaches end of non-void function. This change moves the UNREACHABLE macro out of the switch case to fix the "control reaches end of non-void function" error in the lambda function for addDevice. Review: https://reviews.apache.org/r/75043/ --- src/linux/cgroups2.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/linux/cgroups2.cpp b/src/linux/cgroups2.cpp index 9e2ca2207..d1fc2638c 100644 --- a/src/linux/cgroups2.cpp +++ b/src/linux/cgroups2.cpp @@ -1186,8 +1186,9 @@ private: switch (selector.type) { case Entry::Selector::Type::BLOCK: return BPF_DEVCG_DEV_BLOCK; case Entry::Selector::Type::CHARACTER: return BPF_DEVCG_DEV_CHAR; - case Entry::Selector::Type::ALL: UNREACHABLE(); + case Entry::Selector::Type::ALL: break; } +UNREACHABLE(); }(); program.append({
(mesos) 02/02: [port_mapping] Fix SmallEgressLimit test.
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 commit b0d8d3056ec2f2bc87786d712492c502ec8ee755 Author: Jason Zhou AuthorDate: Mon Jun 10 18:54:49 2024 -0400 [port_mapping] Fix SmallEgressLimit test. The test was previously failing as it was timing the echo rather than ncat. This fix measures the time that ncat takes so that the elapsed time does not display as 0s and fail the test. Review: https://reviews.apache.org/r/75044/ --- src/tests/containerizer/port_mapping_tests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/containerizer/port_mapping_tests.cpp b/src/tests/containerizer/port_mapping_tests.cpp index 12d4626ca..44950acb0 100644 --- a/src/tests/containerizer/port_mapping_tests.cpp +++ b/src/tests/containerizer/port_mapping_tests.cpp @@ -1568,7 +1568,7 @@ TEST_F(PortMappingIsolatorTest, ROOT_NC_SmallEgressLimit) << " bytes of data under egress rate limit " << rate.bytes() << "Bytes/s...';"; - command2 << "{ time -p echo " << data << " | nc localhost " + command2 << "{ echo " << data << " | time -p nc localhost " << invalidPort << " ; } 2> " << transmissionTime << " && "; // Touch the guard file.
(mesos) branch master updated (741790f03 -> b0d8d3056)
This is an automated email from the ASF dual-hosted git repository. bmahler pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/mesos.git from 741790f03 [port-mapping] Fix typo port_mapping.cpp. new 2c9c8180f [cgroups2] Fix control reaches end of non-void function. new b0d8d3056 [port_mapping] Fix SmallEgressLimit test. The 2 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. Summary of changes: src/linux/cgroups2.cpp | 3 ++- src/tests/containerizer/port_mapping_tests.cpp | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-)
(mesos) branch master updated: [port-mapping] Fix typo port_mapping.cpp.
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 741790f03 [port-mapping] Fix typo port_mapping.cpp. 741790f03 is described below commit 741790f03f8a95b80b70f2785d93f6da2b7ae535 Author: Jason Zhou AuthorDate: Mon Jun 10 12:59:36 2024 -0400 [port-mapping] Fix typo port_mapping.cpp. Review: https://reviews.apache.org/r/75042/ --- src/slave/containerizer/mesos/isolators/network/port_mapping.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp b/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp index c5e4e6d10..1fd2e226c 100644 --- a/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp +++ b/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp @@ -5194,7 +5194,7 @@ string PortMappingIsolatorProcess::scripts(Info* info) << " > /proc/sys/net/ipv4/ip_local_port_range\n"; // Verify that the port range has been updated correctly - script << "cat /proc/sys/net/ipv4/ip_local_port_range\n" + script << "cat /proc/sys/net/ipv4/ip_local_port_range\n"; // Allow eth0 and lo in the container to accept local packets. We // need this because we will set up filters to redirect packets from
(mesos) branch master updated: [port-mapping] cat back ip_local_port_range after updating ephemeral ports.
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 06e17bd9a [port-mapping] cat back ip_local_port_range after updating ephemeral ports. 06e17bd9a is described below commit 06e17bd9a222902c54643d5bcfdf74f539b94d91 Author: Jason Zhou AuthorDate: Fri Jun 7 16:56:07 2024 -0400 [port-mapping] cat back ip_local_port_range after updating ephemeral ports. This ensures that the update was successful and that the port range is what we expect. Review: https://reviews.apache.org/r/75038/ --- src/slave/containerizer/mesos/isolators/network/port_mapping.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp b/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp index 06f04d10f..c5e4e6d10 100644 --- a/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp +++ b/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp @@ -5193,6 +5193,9 @@ string PortMappingIsolatorProcess::scripts(Info* info) << (info->ephemeralPorts.upper() - 1) << " > /proc/sys/net/ipv4/ip_local_port_range\n"; + // Verify that the port range has been updated correctly + script << "cat /proc/sys/net/ipv4/ip_local_port_range\n" + // Allow eth0 and lo in the container to accept local packets. We // need this because we will set up filters to redirect packets from // lo to eth0 in the container.
(mesos) 02/02: [mesos-build] Remove setting of environment variable in dockerfiles.
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 commit 4e6375bc071a33821d2fdc15c0d2d5a0b555676e Author: Jason Zhou AuthorDate: Fri Jun 7 14:34:32 2024 -0400 [mesos-build] Remove setting of environment variable in dockerfiles. Setting an environment variable as PYTHON_VERSION caused an unexpected behavior in jenkins as the configure.ac script checked for that exact environment variable. PYTHON_VERSION has been renamed and set as an ARG in the dockerfile so that it will not persist after the build. Review: https://reviews.apache.org/r/75036/ --- support/mesos-build/ubuntu-20.04-arm.dockerfile | 10 +- support/mesos-build/ubuntu-20.04.dockerfile | 10 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/support/mesos-build/ubuntu-20.04-arm.dockerfile b/support/mesos-build/ubuntu-20.04-arm.dockerfile index 50f15039f..a3b321fb7 100644 --- a/support/mesos-build/ubuntu-20.04-arm.dockerfile +++ b/support/mesos-build/ubuntu-20.04-arm.dockerfile @@ -46,16 +46,16 @@ RUN apt-get update && \ rm -rf /var/lib/apt/lists # Install Python 3.6. -ENV PYTHON_VERSION=3.6.15 +ARG PYTHON_TARGET_VERSION=3.6.15 # Download and install Python from source -RUN curl https://www.python.org/ftp/python/$PYTHON_VERSION/Python-$PYTHON_VERSION.tgz -o /tmp/Python-$PYTHON_VERSION.tgz && \ +RUN curl https://www.python.org/ftp/python/$PYTHON_TARGET_VERSION/Python-$PYTHON_TARGET_VERSION.tgz -o /tmp/Python-$PYTHON_TARGET_VERSION.tgz && \ cd /tmp && \ -tar xzf Python-$PYTHON_VERSION.tgz && \ -cd Python-$PYTHON_VERSION && \ +tar xzf Python-$PYTHON_TARGET_VERSION.tgz && \ +cd Python-$PYTHON_TARGET_VERSION && \ ./configure --enable-optimizations && \ make altinstall && \ -rm -rf /tmp/Python-$PYTHON_VERSION.tgz /tmp/Python-$PYTHON_VERSION +rm -rf /tmp/Python-$PYTHON_TARGET_VERSION.tgz /tmp/Python-$PYTHON_TARGET_VERSION # Use update-alternatives to set python3.6 as python3. RUN update-alternatives --install /usr/bin/python3 python3 /usr/local/bin/python3.6 1 diff --git a/support/mesos-build/ubuntu-20.04.dockerfile b/support/mesos-build/ubuntu-20.04.dockerfile index 5b03356f5..e183ff742 100644 --- a/support/mesos-build/ubuntu-20.04.dockerfile +++ b/support/mesos-build/ubuntu-20.04.dockerfile @@ -52,16 +52,16 @@ RUN curl -sSL https://cmake.org/files/v3.8/cmake-3.8.2-Linux-x86_64.sh \ rm -f /tmp/install-cmake.sh # Install Python 3.6. -ENV PYTHON_VERSION=3.6.15 +ARG PYTHON_TARGET_VERSION=3.6.15 # Download and install Python from source -RUN curl https://www.python.org/ftp/python/$PYTHON_VERSION/Python-$PYTHON_VERSION.tgz -o /tmp/Python-$PYTHON_VERSION.tgz && \ +RUN curl https://www.python.org/ftp/python/$PYTHON_TARGET_VERSION/Python-$PYTHON_TARGET_VERSION.tgz -o /tmp/Python-$PYTHON_TARGET_VERSION.tgz && \ cd /tmp && \ -tar xzf Python-$PYTHON_VERSION.tgz && \ -cd Python-$PYTHON_VERSION && \ +tar xzf Python-$PYTHON_TARGET_VERSION.tgz && \ +cd Python-$PYTHON_TARGET_VERSION && \ ./configure --enable-optimizations && \ make altinstall && \ -rm -rf /tmp/Python-$PYTHON_VERSION.tgz /tmp/Python-$PYTHON_VERSION +rm -rf /tmp/Python-$PYTHON_TARGET_VERSION.tgz /tmp/Python-$PYTHON_TARGET_VERSION # Use update-alternatives to set python3.6 as python3. RUN update-alternatives --install /usr/bin/python3 python3 /usr/local/bin/python3.6 1
(mesos) 01/02: [mesos-build] Add /SRC/.git as safe directory for tidybot.
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 commit 193dd64580e5e9337ed20218d6a1db4fe5dbac31 Author: Jason Zhou AuthorDate: Fri Jun 7 14:34:05 2024 -0400 [mesos-build] Add /SRC/.git as safe directory for tidybot. This change allows us to bypass the git directory warnings for tidybot. As part of this change we will have to rebuild the tidybot image and push it to dockerhub. Review: https://reviews.apache.org/r/75037/ --- support/mesos-tidy/entrypoint.sh | 5 + 1 file changed, 5 insertions(+) diff --git a/support/mesos-tidy/entrypoint.sh b/support/mesos-tidy/entrypoint.sh index 70e6c3133..86271dfa2 100755 --- a/support/mesos-tidy/entrypoint.sh +++ b/support/mesos-tidy/entrypoint.sh @@ -21,6 +21,11 @@ set -o pipefail SRCDIR=/tmp/SRC +# NOTE: Higher ubuntu versions seem to give the following warning: +# "detected dubious ownership in repository at '/SRC/.git'" +# we will add an exception for this directory. +git config --global --add safe.directory /SRC/.git + # Prepare sources git clone --depth 1 file:///SRC "${SRCDIR}"
(mesos) branch master updated (ac8765c8a -> 4e6375bc0)
This is an automated email from the ASF dual-hosted git repository. bmahler pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/mesos.git from ac8765c8a [mesos-build] Address dependency issues in centos-7 / ubuntu-20.04. new 193dd6458 [mesos-build] Add /SRC/.git as safe directory for tidybot. new 4e6375bc0 [mesos-build] Remove setting of environment variable in dockerfiles. The 2 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. Summary of changes: support/mesos-build/ubuntu-20.04-arm.dockerfile | 10 +- support/mesos-build/ubuntu-20.04.dockerfile | 10 +- support/mesos-tidy/entrypoint.sh| 5 + 3 files changed, 15 insertions(+), 10 deletions(-)
(mesos) branch master updated: [mesos-build] Address dependency issues in centos-7 / ubuntu-20.04.
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 ac8765c8a [mesos-build] Address dependency issues in centos-7 / ubuntu-20.04. ac8765c8a is described below commit ac8765c8a807fc2710d22ed7c7671f16c4ea2a25 Author: Jason Zhou AuthorDate: Fri Jun 7 11:20:04 2024 -0400 [mesos-build] Address dependency issues in centos-7 / ubuntu-20.04. In CentOS 7, the linux/amd64 base image is missing ebpf fields such as BPF_PROG_TYPE_CGROUP_DEVICE, which prevents jenkins from building mesos now that the ENABLE_CGROUPS_V2 macro has been removed as /usr/include/linux/bpf.h is missing the fields required by ebpf.h. We installed dependencies from kernel-ml so that we can have newer headerfiles in /usr/include/linux, which should help us compile the mesos code in jenkins. For ubuntu, there is a dependency on installing jdk-11 instead of the old jdk-8 which is preventing some builds which pull the ubuntu image to build. As part of this change, both the CentOS 7 and ubuntu:20.04 will need to be rebuilt and uploaded to dockerhub for jenkins. Review: https://reviews.apache.org/r/75035/ --- support/docker-build.sh | 2 ++ support/mesos-build/centos-7.dockerfile | 11 +++ support/mesos-build/ubuntu-20.04.dockerfile | 2 +- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/support/docker-build.sh b/support/docker-build.sh index ce643c14a..1273848d9 100755 --- a/support/docker-build.sh +++ b/support/docker-build.sh @@ -43,6 +43,8 @@ case $OS in append_dockerfile "RUN yum install -y which" append_dockerfile "RUN yum groupinstall -y 'Development Tools'" append_dockerfile "RUN yum install -y epel-release" # Needed for clang. +append_dockerfile "RUN yum install -y https://www.elrepo.org/elrepo-release-7.el7.elrepo.noarch.rpm; +append_dockerfile "RUN yum install -y --enablerepo=elrepo-kernel kernel-ml-headers" append_dockerfile "RUN yum install -y clang git maven" append_dockerfile "RUN yum install -y java-1.8.0-openjdk-devel python-devel python-six zlib-devel libcurl-devel openssl-devel cyrus-sasl-devel cyrus-sasl-md5 apr-devel subversion-devel apr-utils-devel libevent-devel libev-devel" diff --git a/support/mesos-build/centos-7.dockerfile b/support/mesos-build/centos-7.dockerfile index c75c5b61d..7142ffda4 100644 --- a/support/mesos-build/centos-7.dockerfile +++ b/support/mesos-build/centos-7.dockerfile @@ -22,6 +22,13 @@ FROM centos:7 RUN curl -sSL https://copr.fedorainfracloud.org/coprs/alonid/llvm-3.9.0/repo/epel-7/alonid-llvm-3.9.0-epel-7.repo \ -o /etc/yum.repos.d/llvm-3.9.0.repo +# Install epel-release and elrepo to get access to kernel-ml headers +RUN yum install -y https://dl.fedoraproject.org/pub/epel/epel-release-latest-7.noarch.rpm && \ +yum install -y https://www.elrepo.org/elrepo-release-7.el7.elrepo.noarch.rpm + +# Install specific kernel headers +RUN yum install -y --enablerepo=elrepo-kernel kernel-ml-headers + # Install dependencies. RUN yum groupinstall -y 'Development Tools' && \ yum install -y centos-release-scl && \ @@ -48,11 +55,7 @@ RUN yum groupinstall -y 'Development Tools' && \ rm -rf /var/cache/yum # Install Python 3.6 and pip. -# We need two separate `yum install` in order for -# the python packages to be installed correctly. RUN yum install -y \ - https://dl.fedoraproject.org/pub/epel/epel-release-latest-7.noarch.rpm && \ -yum install -y \ python36 \ python36-devel && \ yum clean all && \ diff --git a/support/mesos-build/ubuntu-20.04.dockerfile b/support/mesos-build/ubuntu-20.04.dockerfile index 280c93ae6..5b03356f5 100644 --- a/support/mesos-build/ubuntu-20.04.dockerfile +++ b/support/mesos-build/ubuntu-20.04.dockerfile @@ -36,7 +36,7 @@ RUN apt-get update && \ libsvn-dev \ libtool \ maven \ - openjdk-8-jdk \ + openjdk-11-jdk \ python-dev \ python-six \ sed \
(mesos) branch master updated: [mesos-build] Install openjdk 11 on ubuntu 20.04.
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 93a268dc9 [mesos-build] Install openjdk 11 on ubuntu 20.04. 93a268dc9 is described below commit 93a268dc9783750bbd68561dce089b73871663f6 Author: Jason Zhou AuthorDate: Thu Jun 6 20:41:24 2024 -0400 [mesos-build] Install openjdk 11 on ubuntu 20.04. Install openjdk 11 on ubuntu 20.04. Our reviewbot is running into issues where their java 11 installation is missing javac and configure.ac cannot run properly, this fixes that issue. Review: https://reviews.apache.org/r/75032/ --- support/docker-build.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/support/docker-build.sh b/support/docker-build.sh index f1893e0b3..ce643c14a 100755 --- a/support/docker-build.sh +++ b/support/docker-build.sh @@ -104,7 +104,7 @@ case $OS in ;; *20.04*) echo "Install Ubuntu 20.04 LTS (Focal Fossa) specific packages" -append_dockerfile "RUN apt-get install -y openjdk-8-jdk zlib1g-dev" +append_dockerfile "RUN apt-get install -y openjdk-11-jdk zlib1g-dev" # Install ping required by OsTest.Which append_dockerfile "RUN apt-get install -y iputils-ping"
(mesos) branch master updated: Add push instructions to mesos-tidy.
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 f0a8a4522 Add push instructions to mesos-tidy. f0a8a4522 is described below commit f0a8a4522c12129e9f3d3ecf7d84dfc709541be3 Author: bmahler AuthorDate: Wed Jun 5 20:09:28 2024 -0400 Add push instructions to mesos-tidy. --- support/mesos-tidy/README.md | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/support/mesos-tidy/README.md b/support/mesos-tidy/README.md index 043d22044..391f8e42d 100644 --- a/support/mesos-tidy/README.md +++ b/support/mesos-tidy/README.md @@ -22,7 +22,12 @@ $ docker build -t mesos-tidy . On an M1 Mac, you may need to pass `--platform` to avoid an error with `qemu-x86_64: Could not open '/lib64/ld-linux-x86-64.so.2': No such file or directory`: ```bash -$ docker build -t mesos-tidy --platform linux/amd64 . +$ docker build -t mesos/mesos-tidy --platform linux/amd64 . +``` + +To push this image to docker hub (you need permissions), run: +```bash +$ docker push mesos/mesos-tidy ``` A pre-built image is available via Docker Hub as `mesos/mesos-tidy`.
(mesos) branch master updated: [mesos-build] Add .git directory to safe directory.
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 ca025fbd8 [mesos-build] Add .git directory to safe directory. ca025fbd8 is described below commit ca025fbd8136247f6a125d8f938a4642c69ce756 Author: Jason Zhou AuthorDate: Wed Jun 5 19:28:13 2024 -0400 [mesos-build] Add .git directory to safe directory. Review: https://reviews.apache.org/r/75031/ --- support/mesos-build/entrypoint.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/support/mesos-build/entrypoint.sh b/support/mesos-build/entrypoint.sh index aafaaba3e..38c6f5644 100755 --- a/support/mesos-build/entrypoint.sh +++ b/support/mesos-build/entrypoint.sh @@ -16,6 +16,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +set -x set -e set -o pipefail @@ -24,7 +25,7 @@ SRCDIR=/tmp/SRC # NOTE: Higher ubuntu versions seem to give the following warning: # "detected dubious ownership in repository at '/SRC/.git'" # we will add an exception for this directory. -git config --global --add safe.directory /SRC +git config --global --add safe.directory /SRC/.git # Prepare sources git clone --depth 1 file:///SRC "${SRCDIR}"
(mesos) branch master updated: Clarify mesos-build readme instructions.
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 33e6d9c42 Clarify mesos-build readme instructions. 33e6d9c42 is described below commit 33e6d9c4285ed7b6520b75259913c906bc503cb2 Author: bmahler AuthorDate: Wed Jun 5 18:46:30 2024 -0400 Clarify mesos-build readme instructions. Provide more specific commands. --- support/mesos-build/README.md | 21 + 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/support/mesos-build/README.md b/support/mesos-build/README.md index 879f5fe9b..af168c9a9 100644 --- a/support/mesos-build/README.md +++ b/support/mesos-build/README.md @@ -5,16 +5,21 @@ which our workflows, like the buildbot, pulls from. # Building images -To build a dockerfile inside this directory, run the following command: +To build the dockerfiles inside this directory, run the following command: + ```bash -$ docker build -t -f . +docker build -t mesos/mesos-build:centos-7 -f centos-7.dockerfile --platform=linux/amd64 . +docker build -t mesos-build:ubuntu-20.04 -f ubuntu-20.04.dockerfile --platform=linux/amd64 . +docker build -t mesos-build:ubuntu-20.04-arm -f ubuntu-20.04-arm.dockerfile --platform=linux/arm64 . ``` -If you intend on uploading this image to dockerhub mesos/mesos-build: -For Ubuntu 20.04, and CentOS 7 please run docker build with the -`--platform linux/amd64` flag, especially if you are using an Apple Silicon Mac. - # Pushing images -Ensure you have your tag for the image as `mesos/mesos-build:` -Run: `docker push mesos/mesos-build:` +Ensure you have your tag for the image as `mesos/mesos-build:` as shown above. + +```bash +docker push mesos/mesos-build:centos-7 +docker push mesos/mesos-build:ubuntu-20.04 +docker push mesos/mesos-build:ubuntu-20.04-arm +``` + You need to be an authenticated user who is authorized in order to perform docker push successfully
(mesos) branch master updated (d60221f7a -> 2cad8f467)
This is an automated email from the ASF dual-hosted git repository. bmahler pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/mesos.git from d60221f7a [mesos-build] Add correct directory to git safe directory. new bb0c5c30e Remove trailing whitespace in mesos build entrypoint. new 2cad8f467 [mesos-readme] Clarify mesos-build instructions for uploading to dockerhub. The 2 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. Summary of changes: support/mesos-build/README.md | 5 +++-- support/mesos-build/entrypoint.sh | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-)
(mesos) 02/02: [mesos-readme] Clarify mesos-build instructions for uploading to dockerhub.
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 commit 2cad8f467d78cf93b781cc7a9e7c34ebc0e6d032 Author: Jason Zhou AuthorDate: Wed Jun 5 12:59:29 2024 -0400 [mesos-readme] Clarify mesos-build instructions for uploading to dockerhub. Review: https://reviews.apache.org/r/75030/ --- support/mesos-build/README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/support/mesos-build/README.md b/support/mesos-build/README.md index 0132e4562..879f5fe9b 100644 --- a/support/mesos-build/README.md +++ b/support/mesos-build/README.md @@ -11,9 +11,10 @@ $ docker build -t -f . ``` If you intend on uploading this image to dockerhub mesos/mesos-build: -For Ubuntu 20.04, please run docker build with the `--platform linux/amd64` flag. +For Ubuntu 20.04, and CentOS 7 please run docker build with the +`--platform linux/amd64` flag, especially if you are using an Apple Silicon Mac. # Pushing images Ensure you have your tag for the image as `mesos/mesos-build:` Run: `docker push mesos/mesos-build:` -You need to be an authenticated user who is authorized in order to perform docker push successfully \ No newline at end of file +You need to be an authenticated user who is authorized in order to perform docker push successfully
(mesos) 01/02: Remove trailing whitespace in mesos build entrypoint.
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 commit bb0c5c30e4b43ba2e6b08c83c1934ec2090bb9c4 Author: Benjamin Mahler AuthorDate: Wed Jun 5 13:00:42 2024 -0400 Remove trailing whitespace in mesos build entrypoint. --- support/mesos-build/entrypoint.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/support/mesos-build/entrypoint.sh b/support/mesos-build/entrypoint.sh index 38f697e66..aafaaba3e 100755 --- a/support/mesos-build/entrypoint.sh +++ b/support/mesos-build/entrypoint.sh @@ -21,7 +21,7 @@ set -o pipefail SRCDIR=/tmp/SRC -# NOTE: Higher ubuntu versions seem to give the following warning: +# NOTE: Higher ubuntu versions seem to give the following warning: # "detected dubious ownership in repository at '/SRC/.git'" # we will add an exception for this directory. git config --global --add safe.directory /SRC
(mesos) branch master updated: [mesos-build] Add correct directory to git safe directory.
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 d60221f7a [mesos-build] Add correct directory to git safe directory. d60221f7a is described below commit d60221f7acfa84e26662728108fc8854bb14b7f1 Author: Jason Zhou AuthorDate: Wed Jun 5 12:55:36 2024 -0400 [mesos-build] Add correct directory to git safe directory. Previously, the entrypoint.sh added /SRC/.git as a safe directory after the git clone, which was not useful. We now add it /SRC/ as a safe directory before the git clone. Review: https://reviews.apache.org/r/75029/ --- support/mesos-build/entrypoint.sh | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/support/mesos-build/entrypoint.sh b/support/mesos-build/entrypoint.sh index 2e1e26130..38f697e66 100755 --- a/support/mesos-build/entrypoint.sh +++ b/support/mesos-build/entrypoint.sh @@ -21,9 +21,13 @@ set -o pipefail SRCDIR=/tmp/SRC +# NOTE: Higher ubuntu versions seem to give the following warning: +# "detected dubious ownership in repository at '/SRC/.git'" +# we will add an exception for this directory. +git config --global --add safe.directory /SRC + # Prepare sources git clone --depth 1 file:///SRC "${SRCDIR}" -git config --global --add safe.directory /SRC/.git cd "${SRCDIR}"
(mesos) branch master updated: [mesos-build] Add readme to support/mesos-build directory.
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 ec73629fe [mesos-build] Add readme to support/mesos-build directory. ec73629fe is described below commit ec73629fee7135771c6e732846cb44609f0eba50 Author: Jason Zhou AuthorDate: Wed Jun 5 00:44:47 2024 -0400 [mesos-build] Add readme to support/mesos-build directory. Review: https://reviews.apache.org/r/75028/ --- support/mesos-build/README.md | 19 +++ 1 file changed, 19 insertions(+) diff --git a/support/mesos-build/README.md b/support/mesos-build/README.md new file mode 100644 index 0..0132e4562 --- /dev/null +++ b/support/mesos-build/README.md @@ -0,0 +1,19 @@ +# Mesos docker files for Ubuntu 20.04, Centos 7, Ubuntu 18.04 + +We use these dockerfiles to update the images that we have hosted on docker hub +which our workflows, like the buildbot, pulls from. + +# Building images + +To build a dockerfile inside this directory, run the following command: +```bash +$ docker build -t -f . +``` + +If you intend on uploading this image to dockerhub mesos/mesos-build: +For Ubuntu 20.04, please run docker build with the `--platform linux/amd64` flag. + +# Pushing images +Ensure you have your tag for the image as `mesos/mesos-build:` +Run: `docker push mesos/mesos-build:` +You need to be an authenticated user who is authorized in order to perform docker push successfully \ No newline at end of file
(mesos) branch master updated: [mesos-build] Update reviewbot / tidybot / docker-build.sh to support ubuntu 20.04.
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 ac09fa3cf [mesos-build] Update reviewbot / tidybot / docker-build.sh to support ubuntu 20.04. ac09fa3cf is described below commit ac09fa3cf60e55ca0c1771184d127b24de7c670b Author: Jason Zhou AuthorDate: Wed Jun 5 00:43:06 2024 -0400 [mesos-build] Update reviewbot / tidybot / docker-build.sh to support ubuntu 20.04. The reviewbot, tidybot, and our docker-build scripts have been updated to use or accomodate for ubuntu 20.04. Review: https://reviews.apache.org/r/75027/ --- support/docker-build.sh | 28 support/jenkins/reviewbot.sh | 2 +- support/mesos-build/entrypoint.sh | 1 + support/mesos-tidy/Dockerfile | 2 +- 4 files changed, 31 insertions(+), 2 deletions(-) diff --git a/support/docker-build.sh b/support/docker-build.sh index 4f4019001..f1893e0b3 100755 --- a/support/docker-build.sh +++ b/support/docker-build.sh @@ -64,6 +64,12 @@ case $OS in append_dockerfile "FROM $OS" +case $OS in +*20.04*) +append_dockerfile "ARG DEBIAN_FRONTEND=noninteractive" +;; +esac + # NOTE: We need to do this to fix some flakiness while fetching packages. # See https://bugs.launchpad.net/ubuntu/+source/apt/+bug/972077 append_dockerfile "RUN rm -rf /var/lib/apt/lists/*" @@ -96,6 +102,28 @@ case $OS in # Install pip for Python 3.6. append_dockerfile "RUN curl https://bootstrap.pypa.io/pip/3.6/get-pip.py | python3" ;; + *20.04*) +echo "Install Ubuntu 20.04 LTS (Focal Fossa) specific packages" +append_dockerfile "RUN apt-get install -y openjdk-8-jdk zlib1g-dev" +# Install ping required by OsTest.Which +append_dockerfile "RUN apt-get install -y iputils-ping" + +append_dockerfile "RUN add-apt-repository -y ppa:deadsnakes/ppa && \ +apt-get update && \ +apt-get install -qy \ + python3.6 \ + python3.6-dev \ + python3.6-distutils \ + python3.6-venv && \ +add-apt-repository --remove -y ppa:deadsnakes/ppa && \ +apt-get clean && \ +rm -rf /var/lib/apt/lists" + +# Use update-alternatives to set python3.6 as python3. +append_dockerfile "RUN update-alternatives --install /usr/bin/python3 python3 /usr/bin/python3.6 1" +# Install pip for Python 3.6. +append_dockerfile "RUN curl https://bootstrap.pypa.io/pip/3.6/get-pip.py | python3" + ;; *) append_dockerfile "RUN apt-get install -y openjdk-7-jdk" ;; diff --git a/support/jenkins/reviewbot.sh b/support/jenkins/reviewbot.sh index 349e18dfd..03ebcd600 100755 --- a/support/jenkins/reviewbot.sh +++ b/support/jenkins/reviewbot.sh @@ -44,7 +44,7 @@ fi # Build the HEAD first to ensure that there are no errors prior to applying # the review chain. We do not run tests at this stage. -export OS='ubuntu:16.04' +export OS='ubuntu:20.04' export BUILDTOOL='autotools' export COMPILER='gcc' export CONFIGURATION='--verbose --disable-libtool-wrappers --disable-parallel-test-execution' diff --git a/support/mesos-build/entrypoint.sh b/support/mesos-build/entrypoint.sh index 865c44aef..2e1e26130 100755 --- a/support/mesos-build/entrypoint.sh +++ b/support/mesos-build/entrypoint.sh @@ -23,6 +23,7 @@ SRCDIR=/tmp/SRC # Prepare sources git clone --depth 1 file:///SRC "${SRCDIR}" +git config --global --add safe.directory /SRC/.git cd "${SRCDIR}" diff --git a/support/mesos-tidy/Dockerfile b/support/mesos-tidy/Dockerfile index db66a8575..769a22d9e 100644 --- a/support/mesos-tidy/Dockerfile +++ b/support/mesos-tidy/Dockerfile @@ -14,7 +14,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -FROM ubuntu:xenial +FROM ubuntu:20.04 MAINTAINER The Apache Mesos Developers WORKDIR /tmp/build
(mesos) branch master updated: [mesos-build] Move mesos-build to from ubuntu 16.04 to 20.04
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 232980fb4 [mesos-build] Move mesos-build to from ubuntu 16.04 to 20.04 232980fb4 is described below commit 232980fb48bbf322c01ba2c3ac41e4cd7205cfd3 Author: Jason Zhou AuthorDate: Mon Jun 3 17:50:36 2024 -0400 [mesos-build] Move mesos-build to from ubuntu 16.04 to 20.04 Ubuntu 16.04 docker builds were having issues with the jenkins pipeline as it was missing certain fields in /usr/include/linux/bpf.h that are present in more modern linux kernels' which were used inside the ebpf code. We will try to address this along with Ubuntu's EOL issue by upgrading to ubuntu 20.04 Review: https://reviews.apache.org/r/75023/ --- .../{ubuntu-16.04-arm.dockerfile => ubuntu-20.04-arm.dockerfile} | 8 +--- .../{ubuntu-16.04.dockerfile => ubuntu-20.04.dockerfile} | 4 +++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/support/mesos-build/ubuntu-16.04-arm.dockerfile b/support/mesos-build/ubuntu-20.04-arm.dockerfile similarity index 95% rename from support/mesos-build/ubuntu-16.04-arm.dockerfile rename to support/mesos-build/ubuntu-20.04-arm.dockerfile index 332009ff2..50f15039f 100644 --- a/support/mesos-build/ubuntu-16.04-arm.dockerfile +++ b/support/mesos-build/ubuntu-20.04-arm.dockerfile @@ -14,7 +14,9 @@ # See the License for the specific language governing permissions and # limitations under the License. -FROM arm64v8/ubuntu:16.04 +FROM arm64v8/ubuntu:20.04 + +ARG DEBIAN_FRONTEND=noninteractive # Install dependencies. RUN apt-get update && \ @@ -28,6 +30,7 @@ RUN apt-get update && \ libcurl4-nss-dev \ libev-dev \ libevent-dev \ + libncurses5 \ libsasl2-dev \ libssl-dev \ libsvn-dev \ @@ -38,8 +41,7 @@ RUN apt-get update && \ python-six \ sed \ zlib1g-dev \ - software-properties-common \ - python-software-properties && \ + software-properties-common && \ apt-get clean && \ rm -rf /var/lib/apt/lists diff --git a/support/mesos-build/ubuntu-16.04.dockerfile b/support/mesos-build/ubuntu-20.04.dockerfile similarity index 97% rename from support/mesos-build/ubuntu-16.04.dockerfile rename to support/mesos-build/ubuntu-20.04.dockerfile index 70b269b79..280c93ae6 100644 --- a/support/mesos-build/ubuntu-16.04.dockerfile +++ b/support/mesos-build/ubuntu-20.04.dockerfile @@ -14,7 +14,9 @@ # See the License for the specific language governing permissions and # limitations under the License. -FROM ubuntu:16.04 +FROM ubuntu:20.04 + +ARG DEBIAN_FRONTEND=noninteractive # Install dependencies. RUN apt-get update && \
(mesos) branch master updated: [mesos-build] Fix python setup in docker-build.sh.
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 c70de8f5f [mesos-build] Fix python setup in docker-build.sh. c70de8f5f is described below commit c70de8f5ff49d989cd439122a77cc0660ca8e8de Author: Jason Zhou AuthorDate: Mon Jun 3 17:43:38 2024 -0400 [mesos-build] Fix python setup in docker-build.sh. Fixes the python 3.6 install inside docker-build.sh when the detected OS version is Ubuntu v16.04. Review: https://reviews.apache.org/r/75020/ --- support/docker-build.sh | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/support/docker-build.sh b/support/docker-build.sh index 3b8017eeb..4f4019001 100755 --- a/support/docker-build.sh +++ b/support/docker-build.sh @@ -84,11 +84,17 @@ case $OS in append_dockerfile "RUN apt-get install -y iputils-ping" # Install Python 3.6. -append_dockerfile "RUN add-apt-repository -y ppa:deadsnakes/ppa && apt-get update && apt-get install -qy python3.6 python3.6-dev python3.6-venv" +append_dockerfile "RUN curl https://www.python.org/ftp/python/3.6.15/Python-3.6.15.tgz -o /tmp/Python-3.6.15.tgz && \ +cd /tmp && \ +tar xzf Python-3.6.15.tgz && \ +cd Python-3.6.15 && \ +./configure --enable-optimizations && \ +make altinstall && \ +rm -rf /tmp/Python-3.6.15.tgz /tmp/Python-3.6.15" # Use update-alternatives to set python3.6 as python3. -append_dockerfile "RUN update-alternatives --install /usr/bin/python3 python3 /usr/bin/python3.6 1" +append_dockerfile "RUN update-alternatives --install /usr/bin/python3 python3 /usr/local/bin/python3.6 1" # Install pip for Python 3.6. -append_dockerfile "RUN curl https://bootstrap.pypa.io/get-pip.py | python3" +append_dockerfile "RUN curl https://bootstrap.pypa.io/pip/3.6/get-pip.py | python3" ;; *) append_dockerfile "RUN apt-get install -y openjdk-7-jdk"
(mesos) branch master updated: [cgroups2] Remove ENABLE_CGROUPS_V2 ifdefs.
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 bbb27f9c8 [cgroups2] Remove ENABLE_CGROUPS_V2 ifdefs. bbb27f9c8 is described below commit bbb27f9c8cde5df92ef11f157dd16e8fb54b7ce0 Author: Jason Zhou AuthorDate: Wed May 29 20:05:33 2024 -0400 [cgroups2] Remove ENABLE_CGROUPS_V2 ifdefs. This commit removes the sections where ENABLE_CGROUPS_V2 is used to determine the compiled code. Any need to determine whether or not cgroups2 is used will be satisfied using the cgroups2::mounted() function. This guard was only in place temporarily to avoid breaking our CI while we figured out how to ensure that all of the CI docker images have the header. Review: https://reviews.apache.org/r/75021/ --- cmake/CompilationConfigure.cmake | 3 -- configure.ac | 42 +- src/CMakeLists.txt | 25 + src/Makefile.am| 27 +- src/slave/containerizer/mesos/containerizer.cpp| 4 --- src/slave/containerizer/mesos/linux_launcher.cpp | 14 src/slave/flags.cpp| 4 --- src/slave/main.cpp | 6 src/tests/CMakeLists.txt | 8 ++--- src/tests/containerizer/cgroups_isolator_tests.cpp | 25 ++--- src/tests/containerizer/memory_isolator_tests.cpp | 22 ++-- src/tests/environment.cpp | 2 +- src/tests/mesos.cpp| 14 13 files changed, 53 insertions(+), 143 deletions(-) diff --git a/cmake/CompilationConfigure.cmake b/cmake/CompilationConfigure.cmake index 88f3353bb..813b255d2 100644 --- a/cmake/CompilationConfigure.cmake +++ b/cmake/CompilationConfigure.cmake @@ -505,9 +505,6 @@ if (LINUX) "The XFS disk isolator is not yet supported, see MESOS-9117.") endif () - option(ENABLE_CGROUPS_V2 -"Whether to enable cgroups v2." -FALSE) option(ENABLE_LAUNCHER_SEALING "Whether to enable containerizer launcher sealing via memfd." diff --git a/configure.ac b/configure.ac index e0314fb42..7c0597f56 100644 --- a/configure.ac +++ b/configure.ac @@ -396,13 +396,6 @@ AC_ARG_ENABLE([xfs-disk-isolator], [builds the XFS disk isolator]), [], [enable_xfs_disk_isolator=no]) -# TODO(dleamy): This flag should flipped to --disable-cgroups-v2 so using -# cgroups v2 is the default. -AC_ARG_ENABLE([cgroups-v2], - AS_HELP_STRING([--enable-cgroups-v2], - [builds the cgroups2 and ebpf modules]), - [], [enable_cgroups_v2=no]) - ### # Optional packages. ### @@ -2294,36 +2287,17 @@ Please install the libblkid package for XFS disk isolator support. AM_CONDITIONAL([ENABLE_XFS_DISK_ISOLATOR], [test "x$enable_xfs_disk_isolator" = "xyes"]) - -AC_MSG_CHECKING([whether to enable the cgroups v2]) -AS_IF([test "x$enable_cgroups_v2" = "xyes"], - [AC_MSG_RESULT([yes])], - [AC_MSG_RESULT([no])]) - -AS_IF([test "x$enable_cgroups_v2" = "xyes"], [ - # We only support cgroups v2 on Linux. - AS_IF([test "$OS_NAME" = "linux"], -[], -[AC_MSG_ERROR([no cgroups v2 support on $OS_NAME -cgroups v2 is only supported on Linux. - ])]) - - # Check for build dependencies for cgroups v2. We only - # enable this if all the needed headers and libraries are present. - AC_CHECK_HEADERS([linux/bpf.h], - [], [AC_MSG_ERROR([missing eBPF headers +# We only support cgroups v2 on Linux. +AS_IF([test "$OS_NAME" = "linux"], + [ +# Check for build dependencies for cgroups v2. We only +# enable this if all the needed headers and libraries are present. +AC_CHECK_HEADERS([linux/bpf.h], + [], [AC_MSG_ERROR([missing eBPF headers --- Please install the bpf Linux kernel headers for cgroups v2 support. --- - ])]) - - AC_DEFINE([ENABLE_CGROUPS_V2]) -]) - -AM_CONDITIONAL([ENABLE_CGROUPS_V2], [test "x$enable_cgroups_v2" = "xyes"]) - +])])]) # Check if zlib prefix path was provided, and if so, add it to # the CPPFLAGS and LDFLAGS with respe
(mesos) branch master updated: Revert "CHANGE: add default network if no net options was set."
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 e1752d198 Revert "CHANGE: add default network if no net options was set." e1752d198 is described below commit e1752d19805ffd2229c786153a374ed89eb38853 Author: bmahler AuthorDate: Wed May 29 20:01:25 2024 -0400 Revert "CHANGE: add default network if no net options was set." This reverts commit 5c3b039db544e937617cd63810185cc95fc2b34b. --- src/docker/docker.cpp | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/docker/docker.cpp b/src/docker/docker.cpp index 50239bf32..8ad58339b 100644 --- a/src/docker/docker.cpp +++ b/src/docker/docker.cpp @@ -833,12 +833,12 @@ Try Docker::RunOptions::create( ContainerInfo::DockerInfo::Network network; if (dockerInfo.has_network()) { network = dockerInfo.network(); - } else if (!options.network.isSome()) { -// If no network and no docker network options was given, then use the OS specific default. + } else { +// If no network was given, then use the OS specific default. #ifdef __WINDOWS__ - network = ContainerInfo::DockerInfo::BRIDGE; +network = ContainerInfo::DockerInfo::BRIDGE; #else - network = ContainerInfo::DockerInfo::HOST; +network = ContainerInfo::DockerInfo::HOST; #endif // __WINDOWS__ }
(mesos) branch revert-583-add-docker-net-arg created (now f7e74ddae)
This is an automated email from the ASF dual-hosted git repository. bmahler pushed a change to branch revert-583-add-docker-net-arg in repository https://gitbox.apache.org/repos/asf/mesos.git at f7e74ddae Revert "CHANGE: add default network if no net options was set." This branch includes the following new commits: new f7e74ddae Revert "CHANGE: add default network if no net options was set." The 1 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference.
(mesos) 01/01: Revert "CHANGE: add default network if no net options was set."
This is an automated email from the ASF dual-hosted git repository. bmahler pushed a commit to branch revert-583-add-docker-net-arg in repository https://gitbox.apache.org/repos/asf/mesos.git commit f7e74ddae3a1187d75aae150ef980e6169a1e3a8 Author: bmahler AuthorDate: Wed May 29 20:01:25 2024 -0400 Revert "CHANGE: add default network if no net options was set." This reverts commit 5c3b039db544e937617cd63810185cc95fc2b34b. --- src/docker/docker.cpp | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/docker/docker.cpp b/src/docker/docker.cpp index 50239bf32..8ad58339b 100644 --- a/src/docker/docker.cpp +++ b/src/docker/docker.cpp @@ -833,12 +833,12 @@ Try Docker::RunOptions::create( ContainerInfo::DockerInfo::Network network; if (dockerInfo.has_network()) { network = dockerInfo.network(); - } else if (!options.network.isSome()) { -// If no network and no docker network options was given, then use the OS specific default. + } else { +// If no network was given, then use the OS specific default. #ifdef __WINDOWS__ - network = ContainerInfo::DockerInfo::BRIDGE; +network = ContainerInfo::DockerInfo::BRIDGE; #else - network = ContainerInfo::DockerInfo::HOST; +network = ContainerInfo::DockerInfo::HOST; #endif // __WINDOWS__ }
(mesos) branch master updated: CHANGE: add default network if no net options was set.
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 5c3b039db CHANGE: add default network if no net options was set. 5c3b039db is described below commit 5c3b039db544e937617cd63810185cc95fc2b34b Author: Andreas Peters AuthorDate: Mon May 27 22:33:32 2024 +0200 CHANGE: add default network if no net options was set. --- src/docker/docker.cpp | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/docker/docker.cpp b/src/docker/docker.cpp index 8ad58339b..50239bf32 100644 --- a/src/docker/docker.cpp +++ b/src/docker/docker.cpp @@ -833,12 +833,12 @@ Try Docker::RunOptions::create( ContainerInfo::DockerInfo::Network network; if (dockerInfo.has_network()) { network = dockerInfo.network(); - } else { -// If no network was given, then use the OS specific default. + } else if (!options.network.isSome()) { +// If no network and no docker network options was given, then use the OS specific default. #ifdef __WINDOWS__ -network = ContainerInfo::DockerInfo::BRIDGE; + network = ContainerInfo::DockerInfo::BRIDGE; #else -network = ContainerInfo::DockerInfo::HOST; + network = ContainerInfo::DockerInfo::HOST; #endif // __WINDOWS__ }
(mesos) branch master updated: [mesos-build] Fix python setup in Dockerfiles.
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 a60affe3a [mesos-build] Fix python setup in Dockerfiles. a60affe3a is described below commit a60affe3af6e053b8ecf7fcfa3c11076e9bdc333 Author: None AuthorDate: Wed May 29 12:12:31 2024 -0400 [mesos-build] Fix python setup in Dockerfiles. The dockerfile themselves were fixed up to account for updated pip install urls, deprecated deadsnakes PPA for Ubuntu 16.04, and curl misconfigurations for installing clang. Review: https://reviews.apache.org/r/75018/ --- support/mesos-build/centos-7.dockerfile | 2 +- support/mesos-build/ubuntu-16.04-arm.dockerfile | 24 ++-- support/mesos-build/ubuntu-16.04.dockerfile | 23 --- 3 files changed, 27 insertions(+), 22 deletions(-) diff --git a/support/mesos-build/centos-7.dockerfile b/support/mesos-build/centos-7.dockerfile index 306102cf7..c75c5b61d 100644 --- a/support/mesos-build/centos-7.dockerfile +++ b/support/mesos-build/centos-7.dockerfile @@ -62,7 +62,7 @@ RUN yum install -y \ RUN update-alternatives --install /usr/bin/python3 python3 /usr/bin/python3.6 1 # Install pip for Python 3.6. -RUN curl https://bootstrap.pypa.io/get-pip.py | python3 +RUN curl https://bootstrap.pypa.io/pip/3.6/get-pip.py | python3 # Install virtualenv to /usr/bin/virtualenv with pip. RUN pip3 install --no-cache-dir virtualenv diff --git a/support/mesos-build/ubuntu-16.04-arm.dockerfile b/support/mesos-build/ubuntu-16.04-arm.dockerfile index 6a147394d..332009ff2 100644 --- a/support/mesos-build/ubuntu-16.04-arm.dockerfile +++ b/support/mesos-build/ubuntu-16.04-arm.dockerfile @@ -44,18 +44,22 @@ RUN apt-get update && \ rm -rf /var/lib/apt/lists # Install Python 3.6. -RUN add-apt-repository -y ppa:deadsnakes/ppa && \ -apt-get update && \ -apt-get install -qy \ - python3.6 \ - python3.6-dev \ - python3.6-venv && \ -add-apt-repository --remove -y ppa:deadsnakes/ppa && \ -apt-get clean && \ -rm -rf /var/lib/apt/lists +ENV PYTHON_VERSION=3.6.15 + +# Download and install Python from source +RUN curl https://www.python.org/ftp/python/$PYTHON_VERSION/Python-$PYTHON_VERSION.tgz -o /tmp/Python-$PYTHON_VERSION.tgz && \ +cd /tmp && \ +tar xzf Python-$PYTHON_VERSION.tgz && \ +cd Python-$PYTHON_VERSION && \ +./configure --enable-optimizations && \ +make altinstall && \ +rm -rf /tmp/Python-$PYTHON_VERSION.tgz /tmp/Python-$PYTHON_VERSION + +# Use update-alternatives to set python3.6 as python3. +RUN update-alternatives --install /usr/bin/python3 python3 /usr/local/bin/python3.6 1 # Install newer Clang. -RUN curl http://releases.llvm.org/8.0.0/clang+llvm-8.0.0-aarch64-linux-gnu.tar.xz -o /tmp/clang.tar.xz && \ +RUN curl -L http://releases.llvm.org/8.0.0/clang+llvm-8.0.0-aarch64-linux-gnu.tar.xz -o /tmp/clang.tar.xz && \ tar -xf /tmp/clang.tar.xz && \ rm /tmp/clang.tar.xz && \ cp -R clang+llvm-8.0.0-aarch64-linux-gnu/* /usr/ && \ diff --git a/support/mesos-build/ubuntu-16.04.dockerfile b/support/mesos-build/ubuntu-16.04.dockerfile index 0606170ef..70b269b79 100644 --- a/support/mesos-build/ubuntu-16.04.dockerfile +++ b/support/mesos-build/ubuntu-16.04.dockerfile @@ -50,21 +50,22 @@ RUN curl -sSL https://cmake.org/files/v3.8/cmake-3.8.2-Linux-x86_64.sh \ rm -f /tmp/install-cmake.sh # Install Python 3.6. -RUN add-apt-repository -y ppa:deadsnakes/ppa && \ -apt-get update && \ -apt-get install -qy \ - python3.6 \ - python3.6-dev \ - python3.6-venv && \ -add-apt-repository --remove -y ppa:deadsnakes/ppa && \ -apt-get clean && \ -rm -rf /var/lib/apt/lists +ENV PYTHON_VERSION=3.6.15 + +# Download and install Python from source +RUN curl https://www.python.org/ftp/python/$PYTHON_VERSION/Python-$PYTHON_VERSION.tgz -o /tmp/Python-$PYTHON_VERSION.tgz && \ +cd /tmp && \ +tar xzf Python-$PYTHON_VERSION.tgz && \ +cd Python-$PYTHON_VERSION && \ +./configure --enable-optimizations && \ +make altinstall && \ +rm -rf /tmp/Python-$PYTHON_VERSION.tgz /tmp/Python-$PYTHON_VERSION # Use update-alternatives to set python3.6 as python3. -RUN update-alternatives --install /usr/bin/python3 python3 /usr/bin/python3.6 1 +RUN update-alternatives --install /usr/bin/python3 python3 /usr/local/bin/python3.6 1 # Install pip for Python 3.6. -RUN curl https://bootstrap.pypa.io/get-pip.py | python3 +RUN curl https://bootstrap.pypa.io/pip/3.6/get-pip.py | python3 # Add an unprivileged user. RUN adduser --disabled-password --gecos '' mesos
(mesos) branch master updated: FIX: add missing cgroups file.
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 1bdabaf79 FIX: add missing cgroups file. 1bdabaf79 is described below commit 1bdabaf79fa238f23d7a1bbce0899c12d5e5609a Author: Andreas Peters AuthorDate: Thu May 23 22:46:54 2024 +0200 FIX: add missing cgroups file. --- src/CMakeLists.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 49f620851..3973737f1 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -349,10 +349,11 @@ if (ENABLE_XFS_DISK_ISOLATOR) slave/containerizer/mesos/isolators/xfs/utils.cpp) endif () -if (ENABLE_CGROUPS_v2) +if (ENABLE_CGROUPS_V2) list(APPEND LINUX_SRC linux/cgroups2.cpp linux/ebpf.cpp +slave/containerizer/mesos/isolators/cgroups2/cgroups2.cpp slave/containerizer/mesos/isolators/cgroups2/controller.cpp slave/containerizer/mesos/isolators/cgroups2/controllers/core.cpp slave/containerizer/mesos/isolators/cgroups2/controllers/cpu.cpp
(mesos) branch master updated: [agent] Add test for framework_id and executor_id support in /containers.
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 a4ce050f3 [agent] Add test for framework_id and executor_id support in /containers. a4ce050f3 is described below commit a4ce050f3bc791844db48f8282f88992bc4744f1 Author: None AuthorDate: Thu May 23 19:11:08 2024 -0400 [agent] Add test for framework_id and executor_id support in /containers. Review: https://reviews.apache.org/r/75010/ --- src/tests/slave_tests.cpp | 172 +- 1 file changed, 170 insertions(+), 2 deletions(-) diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp index a5e01ef42..89afbb8dc 100644 --- a/src/tests/slave_tests.cpp +++ b/src/tests/slave_tests.cpp @@ -134,6 +134,7 @@ using process::UPID; using process::filter; +using process::http::BadRequest; using process::http::InternalServerError; using process::http::OK; using process::http::Response; @@ -2512,7 +2513,8 @@ TEST_F(SlaveTest, ContainersEndpointNoExecutor) // This is an end-to-end test that verifies that the slave returns the // correct container status and resource statistics based on the currently // running executors, and ensures that '/containers' endpoint returns the -// correct container when it is provided a container ID query parameter. +// correct container when it is provided a container ID, framework ID, or +// executor ID query parameters. TEST_F(SlaveTest, ContainersEndpoint) { Try> master = StartMaster(); @@ -2540,7 +2542,9 @@ TEST_F(SlaveTest, ContainersEndpoint) MesosSchedulerDriver driver( , DEFAULT_FRAMEWORK_INFO, master.get()->pid, DEFAULT_CREDENTIAL); - EXPECT_CALL(sched, registered(, _, _)); + Future frameworkId; + EXPECT_CALL(sched, registered(, _, _)) +.WillOnce(FutureArg<1>()); Future> offers; EXPECT_CALL(sched, resourceOffers(, _)) @@ -2549,6 +2553,7 @@ TEST_F(SlaveTest, ContainersEndpoint) driver.start(); + AWAIT_READY(frameworkId); AWAIT_READY(offers); ASSERT_FALSE(offers->empty()); @@ -2786,6 +2791,169 @@ TEST_F(SlaveTest, ContainersEndpoint) EXPECT_TRUE(value->contains(expected.get())); } + // Will be called once during the third request. + EXPECT_CALL(containerizer, usage(_)) +.WillOnce(Return(statistics2)); + + // Will be called once during the third request and might be called if + // the `TASK_FAILED` update reaches the agent before the test finishes. + EXPECT_CALL(containerizer, status(_)) +.WillOnce(Return(containerStatus2)) +.WillRepeatedly(Return(containerStatus2)); + + { +Future response = process::http::get( + slave.get()->pid, + "containers?executor_id=" + executor2.executor_id().value() + +"_id=" + frameworkId->value(), + None(), + createBasicAuthHeaders(DEFAULT_CREDENTIAL)); + +Try value = JSON::parse(response->body); +ASSERT_SOME(value); + +JSON::Array array = value->as(); +EXPECT_TRUE(array.values.size() == 1); + +Try expected = JSON::parse( +"[{" +"\"executor_id\":\"" + executor2.executor_id().value() + "\"," +"\"framework_id\":\"" + frameworkId->value() + "\"," +"\"statistics\":{" +"}," +"\"status\":{" +"\"container_id\":{" + "\"parent\":{\"value\":\"parent\"}," + "\"value\":\"child2\"" +"}," +"\"cgroup_info\":{\"net_cls\":{\"classid\":42}}," +"\"network_infos\":[{" +"\"ip_addresses\":[{\"ip_address\":\"192.168.1.21\"}]" +"}]" +"}" +"}]"); + +ASSERT_SOME(expected); + +EXPECT_TRUE(value->contains(expected.get())); + } + + EXPECT_CALL(containerizer, usage(_)) +.WillOnce(DoAll(FutureArg<0>(), Return(statistics1))) +.WillOnce(DoAll(FutureArg<0>(), Return(statistics2))); + + EXPECT_CALL(containerizer, status(_)) +.WillOnce(Return(containerStatus1)) +.WillOnce(Return(containerStatus2)); + + { +Future response = process::http::get( + slave.get()->pid, + "containers?framework_id=" + frameworkId->value(), + None(), + createBasicAuthHeaders(DEFAULT_CREDENTIAL)); + +Try value = JSON::parse(response->body); +ASSERT_SOME(value); + +JSON::Array array = value->as(); + +EXPECT_TRUE(array.val
(mesos) branch master updated: [agent] Add executor_id / framework_id query parameters in /containers.
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 145a115b9 [agent] Add executor_id / framework_id query parameters in /containers. 145a115b9 is described below commit 145a115b9440077a93e2186512ec7079aa7c686c Author: None AuthorDate: Thu May 23 19:02:13 2024 -0400 [agent] Add executor_id / framework_id query parameters in /containers. We now allow filtering by framework ID and executor ID in addition to the original functionality of filtering by container ID. Please note that the /containers endooint only allows select combinations of these query parameter fields to be populated at once. We will return a failure if we see that the combination of query paramters is invalid. We currently accept: * no query parameters * only container id * only framework id * only framework id and executor id Review: https://reviews.apache.org/r/75009/ --- src/slave/http.cpp | 29 + src/slave/http.hpp | 2 ++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/src/slave/http.cpp b/src/slave/http.cpp index ff3408f65..b35207c02 100644 --- a/src/slave/http.cpp +++ b/src/slave/http.cpp @@ -2838,6 +2838,8 @@ Future Http::getContainers( return __containers( approvers, None(), + None(), + None(), call.get_containers().show_nested(), call.get_containers().show_standalone()); })) @@ -2856,6 +2858,14 @@ Future Http::_containers( const Option& principal) const { Option containerId = request.url.query.get("container_id"); + Option frameworkId = request.url.query.get("framework_id"); + Option executorId = request.url.query.get("executor_id"); + + if ((containerId.isSome() && !(executorId.isNone() && frameworkId.isNone())) + || (containerId.isNone() && executorId.isSome() && frameworkId.isNone())) { +return BadRequest("Supported query parameters are container_id only," + " framework_id only, or executor_id and framework_id only"); + } return ObjectApprovers::create( slave->authorizer, @@ -2863,12 +2873,16 @@ Future Http::_containers( {VIEW_CONTAINER, VIEW_STANDALONE_CONTAINER}) .then(defer( slave->self(), -[this, containerId](const Owned& approvers) { +[this, containerId, executorId, frameworkId](const Owned& approvers) { IDAcceptor selectContainerId(containerId); + IDAcceptor selectFrameworkId(frameworkId); + IDAcceptor selectExecutorId(executorId); return __containers( approvers, selectContainerId, + selectFrameworkId, + selectExecutorId, false, false); })) @@ -2881,6 +2895,8 @@ Future Http::_containers( Future Http::__containers( const Owned& approvers, Option> selectContainerId, +Option> selectFrameworkId, +Option> selectExecutorId, bool showNestedContainers, bool showStandaloneContainers) const { @@ -2894,6 +2910,10 @@ Future Http::__containers( hashset authorizedExecutorContainerIds; foreachvalue (const Framework* framework, slave->frameworks) { +if (selectFrameworkId.isSome() && !selectFrameworkId->accept(framework->id())) { + continue; +} + foreachvalue (const Executor* executor, framework->executors) { // No need to get statistics and status if we know that the // executor has already terminated. @@ -2902,13 +2922,14 @@ Future Http::__containers( } const ExecutorInfo& info = executor->info; + const ExecutorID& executorId = executor->id; const ContainerID& containerId = executor->containerId; executorContainerIds.insert(containerId); - if ((selectContainerId.isSome() && - !selectContainerId->accept(containerId)) || - !approvers->approved(info, framework->info)) { + if ((selectContainerId.isSome() && !selectContainerId->accept(containerId)) + || (selectExecutorId.isSome() && !selectExecutorId->accept(executorId)) + || !approvers->approved(info, framework->info)) { continue; } diff --git a/src/slave/http.hpp b/src/slave/http.hpp index 92114ea5e..ff39b3f84 100644 --- a/src/slave/http.hpp +++ b/src/slave/http.hpp @@ -123,6 +123,8 @@ private: process::Future __containers( const process::Owned& approvers, Option> selectContainerId, + Option> selectFrameworkId, + Option> selectExecutorId, bool showNestedContainers, bool showStandaloneContainers) const;
(mesos) branch master updated: [contributors] Add Jason Zhou to contributors.yaml.
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 a5b93b341 [contributors] Add Jason Zhou to contributors.yaml. a5b93b341 is described below commit a5b93b341e42e79d63f81a3e7d09ee06c260651f Author: None AuthorDate: Thu May 23 15:20:45 2024 -0400 [contributors] Add Jason Zhou to contributors.yaml. Review: https://reviews.apache.org/r/75011/ --- docs/contributors.yaml | 9 + 1 file changed, 9 insertions(+) diff --git a/docs/contributors.yaml b/docs/contributors.yaml index 1c8e71de1..203b6c348 100644 --- a/docs/contributors.yaml +++ b/docs/contributors.yaml @@ -969,3 +969,12 @@ - bluezhud...@gmail.com jira_user: dzhu reviewboard_user: dzhu + +- name: Jason Zhou + affiliations: +- {organization: X} + emails: +- jasonz...@twitter.com +- j434z...@uwaterloo.ca +- jasonzhou...@gmail.com + reviewboard_user: zhoujas
(mesos) branch master updated: [cgroups2] Ignore manual enabling of perf_event during prepare phase.
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 d9c584c7e [cgroups2] Ignore manual enabling of perf_event during prepare phase. d9c584c7e is described below commit d9c584c7e92cdd3e9dee84cb8aa47b0ade3c5048 Author: None AuthorDate: Tue May 21 00:41:41 2024 -0400 [cgroups2] Ignore manual enabling of perf_event during prepare phase. In Cgroups2IsolatorProcess::prepare, it may manually enable controller by writing to the cgroup.subtree_control process. For perf_event, since is is automatically turned on, it does not appear inside the cgroup.controllers file and hence cannot be written to the cgroup.subtree_control file. For this reason, we skip the enable call for the perf_event controller. Review: https://reviews.apache.org/r/74998/ --- .../mesos/isolators/cgroups2/cgroups2.cpp | 29 ++ 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.cpp b/src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.cpp index cd9c38d9d..2ca388079 100644 --- a/src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.cpp +++ b/src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.cpp @@ -209,20 +209,29 @@ Future> Cgroups2IsolatorProcess::prepare( foreachvalue (const Owned& controller, controllers) { if (controller->name() == "core") { // The "core" controller is always enabled because the "cgroup.*" control - // files exist for all cgroups. Additionally, since "core" isn't a - // valid controller name (i.e. it doesn't exist in "cgroup.controllers"), - // calling `cgroups2::controllers::enable` with the "core" cgroup will - // fail with "Invalid argument". + // files exist for all cgroups. // - // For that reason, we skip enabling the "core" controller here. + // Additionally, since "core" and "perf_event" aren't valid controller + // names (i.e. they don't exist in "cgroup.controllers"), calling + // `cgroups2::controllers::enable` with these cgroups will fail with + // "Invalid argument". + // + // Therefore, we skip enabling the "core" and "perf_event" controller here. continue; } -Try enable = - cgroups2::controllers::enable(nonLeafCgroup, {controller->name()}); -if (enable.isError()) { - return Failure("Failed to enable controller '" + controller->name() + "'" - " in cgroup '" + nonLeafCgroup + "': " + enable.error()); +// Similar to "core", "perf_event" does not exist in cgroup.controllers, +// and therefore we cannot call cgroups2::controllers::enable with it +// as it cannot be written into cgroup.subtree_control, but we still +// need to push it into the controllers of the containers, so we will +// only skip the call for cgroups2::controllers::enable +if (controller->name() != "perf_event") { + Try enable = +cgroups2::controllers::enable(nonLeafCgroup, {controller->name()}); + if (enable.isError()) { +return Failure("Failed to enable controller '" + controller->name() + "'" + " in cgroup '" + nonLeafCgroup + "': " + enable.error()); + } } // We don't enable the controllers in the leaf cgroup because of the
(mesos) branch master updated: [cgroups2] Introduce the PerfEventControllerProcess.
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 eb3b5a16d [cgroups2] Introduce the PerfEventControllerProcess. eb3b5a16d is described below commit eb3b5a16dc7d5d96775ac04bbfa0554a3c6e2051 Author: None AuthorDate: Tue May 21 00:35:40 2024 -0400 [cgroups2] Introduce the PerfEventControllerProcess. Introduces the controller process for perf event which was also present in cgroups1. The controller is automatically enabled, and should not be visible inside the cgroups.controllers file in the root cgroup. As a consequence, we will not be able to manually enable or disable this controller via writing to the cgroup.subtree_control file. References: * perf_event section in https://docs.kernel.org/admin-guide/cgroup-v2.html * slide 34 in https://man7.org/conf/ndctechtown2021/cgroups-v2-part-1-intro-NDC-TechTown-2021-Kerrisk.pdf Review: https://reviews.apache.org/r/74997/ --- src/CMakeLists.txt | 4 +- src/Makefile.am| 5 +- .../mesos/isolators/cgroups2/cgroups2.cpp | 4 +- .../mesos/isolators/cgroups2/constants.hpp | 1 + .../isolators/cgroups2/controllers/perf_event.cpp | 223 + .../isolators/cgroups2/controllers/perf_event.hpp | 103 ++ 6 files changed, 337 insertions(+), 3 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 963d4201a..49f620851 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -356,7 +356,9 @@ if (ENABLE_CGROUPS_v2) slave/containerizer/mesos/isolators/cgroups2/controller.cpp 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/memory.cpp +slave/containerizer/mesos/isolators/cgroups2/controllers/perf_event.cpp) + endif () diff --git a/src/Makefile.am b/src/Makefile.am index 779b893fc..68a93674f 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1505,7 +1505,10 @@ MESOS_LINUX_FILES += \ slave/containerizer/mesos/isolators/cgroups2/controllers/cpu.cpp\ slave/containerizer/mesos/isolators/cgroups2/controllers/cpu.hpp\ slave/containerizer/mesos/isolators/cgroups2/controllers/memory.cpp\ - slave/containerizer/mesos/isolators/cgroups2/controllers/memory.hpp + 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 + endif if ENABLE_SECCOMP_ISOLATOR diff --git a/src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.cpp b/src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.cpp index 76c8df9b1..cd9c38d9d 100644 --- a/src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.cpp +++ b/src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.cpp @@ -21,6 +21,7 @@ #include "slave/containerizer/mesos/isolators/cgroups2/controllers/core.hpp" #include "slave/containerizer/mesos/isolators/cgroups2/controllers/cpu.hpp" #include "slave/containerizer/mesos/isolators/cgroups2/controllers/memory.hpp" +#include "slave/containerizer/mesos/isolators/cgroups2/controllers/perf_event.hpp" #include #include @@ -77,7 +78,8 @@ Try Cgroups2IsolatorProcess::create(const Flags& flags) hashmap>(*)(const Flags&)> creators = { {"core", ::create}, {"cpu", ::create}, -{"mem", ::create} +{"mem", ::create}, +{"perf_event", ::create} }; hashmap> controllers; diff --git a/src/slave/containerizer/mesos/isolators/cgroups2/constants.hpp b/src/slave/containerizer/mesos/isolators/cgroups2/constants.hpp index 1fb713837..bad79ad0c 100644 --- a/src/slave/containerizer/mesos/isolators/cgroups2/constants.hpp +++ b/src/slave/containerizer/mesos/isolators/cgroups2/constants.hpp @@ -38,6 +38,7 @@ const Bytes CGROUPS2_MIN_MEMORY = Megabytes(32); const std::string CGROUPS2_CONTROLLER_CORE_NAME = "core"; const std::string CGROUPS2_CONTROLLER_CPU_NAME = "cpu"; const std::string CGROUPS2_CONTROLLER_MEMORY_NAME = "memory"; +const std::string CGROUPS2_CONTROLLER_PERF_EVENT_NAME = "perf_event"; } // namespace slave { } // namespace internal { diff --git a/src/slave/containerizer/mesos/isolators/cgroups2/controllers/perf_event.cpp b/src/slave/containerizer/mesos/isolators/cgroups2/controllers/perf_event.cpp new file mode 100644 index 0
(mesos) branch master updated: [cgroups2] Allow cgroups2::enable() to take in a set.
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 2ada2fb5c [cgroups2] Allow cgroups2::enable() to take in a set. 2ada2fb5c is described below commit 2ada2fb5cf4d5563f169a6fd8e629ce079a05a4c Author: None AuthorDate: Tue May 21 00:30:06 2024 -0400 [cgroups2] Allow cgroups2::enable() to take in a set. Modifies the cgroups2::controllers::enable function to take in a set of strings for controllers. This helps eliminate the possibility of duplicate controllers in the argument, and brings it in line with the cgroups2::controllers::disable function Review: https://reviews.apache.org/r/74981/ --- src/linux/cgroups2.cpp | 4 ++-- src/linux/cgroups2.hpp | 2 +- src/slave/main.cpp | 6 +- src/tests/containerizer/cgroups2_tests.cpp | 2 +- src/tests/mesos.cpp| 5 +++-- 5 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/linux/cgroups2.cpp b/src/linux/cgroups2.cpp index e9845bb56..9e2ca2207 100644 --- a/src/linux/cgroups2.cpp +++ b/src/linux/cgroups2.cpp @@ -128,7 +128,7 @@ struct State // We don't return errors here because enabling something // unknown will fail when writing it back out. - void enable(const vector& controllers) + void enable(const set& controllers) { foreach (const string& controller, controllers) { enable(controller); @@ -591,7 +591,7 @@ Try> available(const string& cgroup) } -Try enable(const string& cgroup, const vector& controllers) +Try enable(const string& cgroup, const set& controllers) { using State = control::subtree_control::State; Try control = cgroups2::control::subtree_control::read(cgroup); diff --git a/src/linux/cgroups2.hpp b/src/linux/cgroups2.hpp index 97260c0d6..64254d04f 100644 --- a/src/linux/cgroups2.hpp +++ b/src/linux/cgroups2.hpp @@ -119,7 +119,7 @@ Try> available(const std::string& cgroup); // controllers. Errors if a requested controller is not available. Try enable( const std::string& cgroup, -const std::vector& controllers); +const std::set& controllers); // Disables controllers in the cgroup. No-op if the controller is not enabled. diff --git a/src/slave/main.cpp b/src/slave/main.cpp index f4e7f3b64..8a0e161ff 100644 --- a/src/slave/main.cpp +++ b/src/slave/main.cpp @@ -276,8 +276,12 @@ static Try initializeCgroups2(const slave::Flags& flags) const vector requestedControllers = strings::tokenize( *flags.agent_subsystems, ","); + const set requestedControllersSet( + requestedControllers.begin(), requestedControllers.end()); + + Try enable = cgroups2::controllers::enable( - cgroups2::ROOT_CGROUP, requestedControllers); + cgroups2::ROOT_CGROUP, requestedControllersSet); if (enable.isError()) { return Error("Failed to enable the requested cgroup v2 controllers: " + enable.error()); diff --git a/src/tests/containerizer/cgroups2_tests.cpp b/src/tests/containerizer/cgroups2_tests.cpp index e36f99b64..cb1e229f7 100644 --- a/src/tests/containerizer/cgroups2_tests.cpp +++ b/src/tests/containerizer/cgroups2_tests.cpp @@ -79,7 +79,7 @@ protected: Try result = cgroups2::controllers::enable( cgroups2::ROOT_CGROUP, -vector(to_enable.begin(), to_enable.end())); +to_enable); if (result.isSome()) { enabled_controllers = enabled_controllers | to_enable; diff --git a/src/tests/mesos.cpp b/src/tests/mesos.cpp index ffbb30b95..d33c7b477 100644 --- a/src/tests/mesos.cpp +++ b/src/tests/mesos.cpp @@ -806,8 +806,9 @@ void ContainerizerTest::SetUpCgroupsV2() cgroups2::ROOT_CGROUP); ASSERT_SOME(_controllers); subsystems = *_controllers; - vector controllers(std::make_move_iterator(_controllers->begin()), - std::make_move_iterator(_controllers->end())); + set controllers( + std::make_move_iterator(_controllers->begin()), + std::make_move_iterator(_controllers->end())); // Enable all of the controllers inside of the test root cgroup so they // are accessible from the child container cgroups.
(mesos) branch master updated: [cgroups2] crash when root folder is not detected when creating cgroups
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 a5473726f [cgroups2] crash when root folder is not detected when creating cgroups a5473726f is described below commit a5473726f863b151d11b7436676a46a37c3e55f0 Author: None AuthorDate: Thu May 16 20:35:37 2024 -0400 [cgroups2] crash when root folder is not detected when creating cgroups Based on this ticket (https://issues.apache.org/jira/browse/MESOS-9305) and the ROOT_CGROUPS_CreateRecursively test in CgroupsIsolatorTest, there seems to be a possibility that the root folder may be deleted and new cgroups cannot be properly created. In v1, this was addressed by enabling recursively creating the groups. In v2, since we make use of cgroup.subtree_control to determine a cgroup and its descendents' access to controllers, we cannot recover this effectively if the root folder is deleted, so we cant just recursively create the folders. Hence, we elected to crash if the root folder is not found, as it will allow us to restart and go through the logic that takes care of setting all the values inside cgroup.subtree_control again. Review: https://reviews.apache.org/r/74995/ --- src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.cpp | 8 1 file changed, 8 insertions(+) diff --git a/src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.cpp b/src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.cpp index 6fce8c984..76c8df9b1 100644 --- a/src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.cpp +++ b/src/slave/containerizer/mesos/isolators/cgroups2/cgroups2.cpp @@ -163,6 +163,14 @@ Future> Cgroups2IsolatorProcess::prepare( CHECK(containerConfig.container_class() != ContainerClass::DEBUG); + // Based on MESOS-9305, there seems to be a possibility that the root + // folder may be deleted underneath us. Since we make use of subtree_control + // to determine a cgroup and its descendents' access to controllers, we + // we can't just recursively create the folders. Hence, we crash if the root + // folder is not found, as it will allow us to restart and go through agent's + // main logic which sets up the root cgroup and its subtree control. + CHECK(cgroups2::exists(flags.cgroups_root)); + // Create the non-leaf and leaf cgroups for the container, enable // controllers in the non-leaf cgroup, and `prepare` each of the controllers. const string nonLeafCgroup = cgroups2_paths::container(
(mesos) branch master updated: [cgroups2] Fix cgroups isolator test for RevocableCpu.
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 9e0a37dbb [cgroups2] Fix cgroups isolator test for RevocableCpu. 9e0a37dbb is described below commit 9e0a37dbbadaa4e99a213b5bc0790a7ccfabeda5 Author: None AuthorDate: Thu May 16 15:51:25 2024 -0400 [cgroups2] Fix cgroups isolator test for RevocableCpu. This patch fixes the RevocableCpu test for cgroups2 by conditionally skipping the hierarchy check which is only relevant to cgroups1 systems. Review: https://reviews.apache.org/r/74994/ --- src/tests/containerizer/cgroups_isolator_tests.cpp | 16 +--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/tests/containerizer/cgroups_isolator_tests.cpp b/src/tests/containerizer/cgroups_isolator_tests.cpp index 01e410a94..6a0443404 100644 --- a/src/tests/containerizer/cgroups_isolator_tests.cpp +++ b/src/tests/containerizer/cgroups_isolator_tests.cpp @@ -32,6 +32,9 @@ #include "slave/containerizer/mesos/containerizer.hpp" #include "slave/containerizer/mesos/paths.hpp" +#include "linux/cgroups2.hpp" + +#include "slave/containerizer/mesos/isolators/cgroups2/constants.hpp" #include "slave/containerizer/mesos/isolators/cgroups/constants.hpp" #include "slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.hpp" @@ -60,6 +63,7 @@ using mesos::internal::slave::CPU_SHARES_PER_CPU_REVOCABLE; using mesos::internal::slave::CPU_CFS_PERIOD; using mesos::internal::slave::DEFAULT_EXECUTOR_CPUS; using mesos::internal::slave::DEFAULT_EXECUTOR_MEM; +using mesos::internal::slave::CGROUPS2_CPU_WEIGHT_PER_CPU_REVOCABLE; using mesos::internal::slave::Containerizer; using mesos::internal::slave::Fetcher; @@ -371,15 +375,21 @@ TEST_F(CgroupsIsolatorTest, ROOT_CGROUPS_RevocableCpu) ContainerID containerId = *(containers->begin()); - Result cpuHierarchy = cgroups::hierarchy("cpu"); - ASSERT_SOME(cpuHierarchy); - string cpuCgroup = path::join(flags.cgroups_root, containerId.value()); double totalCpus = cpus.cpus().get() + DEFAULT_EXECUTOR_CPUS; +#ifdef ENABLE_CGROUPS_V2 + EXPECT_SOME_EQ( + static_cast(CGROUPS2_CPU_WEIGHT_PER_CPU_REVOCABLE * totalCpus), + cgroups2::cpu::weight(cpuCgroup)); +#else + Result cpuHierarchy = cgroups::hierarchy("cpu"); + ASSERT_SOME(cpuHierarchy); + EXPECT_SOME_EQ( CPU_SHARES_PER_CPU_REVOCABLE * totalCpus, cgroups::cpu::shares(cpuHierarchy.get(), cpuCgroup)); +#endif // ENABLE_CGROUPS_V2 driver.stop(); driver.join();
(mesos) branch master updated: [cgroups2] Rename constants in cgroups2 isolator.
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 5922e01bd [cgroups2] Rename constants in cgroups2 isolator. 5922e01bd is described below commit 5922e01bdf2157d42edb46adee20dcc2f0a02e15 Author: None AuthorDate: Thu May 16 15:49:46 2024 -0400 [cgroups2] Rename constants in cgroups2 isolator. Specify that the cgroups2 constants are cgroups2 in their names. This helps avoid redefinition of constants inside test files that may import constant files from both cgroups v1 and v2. Review: https://reviews.apache.org/r/74993/ --- .../mesos/isolators/cgroups2/constants.hpp | 18 +- .../mesos/isolators/cgroups2/controllers/core.cpp | 2 +- .../mesos/isolators/cgroups2/controllers/cpu.cpp | 16 .../mesos/isolators/cgroups2/controllers/memory.cpp| 6 +++--- 4 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/slave/containerizer/mesos/isolators/cgroups2/constants.hpp b/src/slave/containerizer/mesos/isolators/cgroups2/constants.hpp index b82f8cd74..1fb713837 100644 --- a/src/slave/containerizer/mesos/isolators/cgroups2/constants.hpp +++ b/src/slave/containerizer/mesos/isolators/cgroups2/constants.hpp @@ -26,18 +26,18 @@ namespace internal { namespace slave { // CPU controller constants. -const uint64_t CPU_WEIGHT_PER_CPU = 100; -const uint64_t CPU_WEIGHT_PER_CPU_REVOCABLE = 1; -const uint64_t MIN_CPU_WEIGHT = 1; // Linux constant. -const Duration CPU_CFS_PERIOD = Milliseconds(100); // Linux default. -const Duration MIN_CPU_CFS_QUOTA = Milliseconds(1); +const uint64_t CGROUPS2_CPU_WEIGHT_PER_CPU = 100; +const uint64_t CGROUPS2_CPU_WEIGHT_PER_CPU_REVOCABLE = 1; +const uint64_t CGROUPS2_MIN_CPU_WEIGHT = 1; // Linux constant. +const Duration CGROUPS2_CPU_CFS_PERIOD = Milliseconds(100); // Linux default. +const Duration CGROUPS2_MIN_CPU_CFS_QUOTA = Milliseconds(1); // Memory controller constants. -const Bytes MIN_MEMORY = Megabytes(32); +const Bytes CGROUPS2_MIN_MEMORY = Megabytes(32); -const std::string CGROUPS_V2_CONTROLLER_CORE_NAME = "core"; -const std::string CGROUPS_V2_CONTROLLER_CPU_NAME = "cpu"; -const std::string CGROUPS_V2_CONTROLLER_MEMORY_NAME = "memory"; +const std::string CGROUPS2_CONTROLLER_CORE_NAME = "core"; +const std::string CGROUPS2_CONTROLLER_CPU_NAME = "cpu"; +const std::string CGROUPS2_CONTROLLER_MEMORY_NAME = "memory"; } // namespace slave { } // namespace internal { diff --git a/src/slave/containerizer/mesos/isolators/cgroups2/controllers/core.cpp b/src/slave/containerizer/mesos/isolators/cgroups2/controllers/core.cpp index 54f60c835..54d7c8977 100644 --- a/src/slave/containerizer/mesos/isolators/cgroups2/controllers/core.cpp +++ b/src/slave/containerizer/mesos/isolators/cgroups2/controllers/core.cpp @@ -49,7 +49,7 @@ CoreControllerProcess::CoreControllerProcess(const Flags& _flags) string CoreControllerProcess::name() const { - return CGROUPS_V2_CONTROLLER_CORE_NAME; + return CGROUPS2_CONTROLLER_CORE_NAME; } diff --git a/src/slave/containerizer/mesos/isolators/cgroups2/controllers/cpu.cpp b/src/slave/containerizer/mesos/isolators/cgroups2/controllers/cpu.cpp index adebab744..0fcb9f2bc 100644 --- a/src/slave/containerizer/mesos/isolators/cgroups2/controllers/cpu.cpp +++ b/src/slave/containerizer/mesos/isolators/cgroups2/controllers/cpu.cpp @@ -57,7 +57,7 @@ CpuControllerProcess::CpuControllerProcess(const Flags& _flags) string CpuControllerProcess::name() const { - return CGROUPS_V2_CONTROLLER_CPU_NAME; + return CGROUPS2_CONTROLLER_CPU_NAME; } @@ -79,9 +79,9 @@ Future CpuControllerProcess::update( double cpuRequest = *resourceRequests.cpus(); bool revocable = resourceRequests.revocable().cpus().isSome(); uint64_t weightPerCpu = (revocable && flags.revocable_cpu_low_priority) ? - CPU_WEIGHT_PER_CPU_REVOCABLE : CPU_WEIGHT_PER_CPU; + CGROUPS2_CPU_WEIGHT_PER_CPU_REVOCABLE : CGROUPS2_CPU_WEIGHT_PER_CPU; uint64_t weight = std::max( - static_cast(weightPerCpu * cpuRequest), MIN_CPU_WEIGHT); + static_cast(weightPerCpu * cpuRequest), CGROUPS2_MIN_CPU_WEIGHT); Try update = cgroups2::cpu::weight(cgroup, weight); if (update.isError()) { @@ -98,23 +98,23 @@ Future CpuControllerProcess::update( cpuLimit = resourceLimits.at("cpus").value(); } BandwidthLimit limit = [&] () { -uint64_t min_quota = static_cast(MIN_CPU_CFS_QUOTA.us()); +uint64_t min_quota = static_cast(CGROUPS2_MIN_CPU_CFS_QUOTA.us()); if (cpuLimit.isSome()) { if (std::isinf(*cpuLimit)) { return BandwidthLimit(); // (1) } - uint64_t quota = static_cast(*cpuLimit * CPU_CFS_PERIOD.us()); + uint64_t qu
(mesos) branch master updated: [cgroups2] Add memory usage reporting to the MemoryControllerProcess
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 04f973ba1 [cgroups2] Add memory usage reporting to the MemoryControllerProcess 04f973ba1 is described below commit 04f973ba1526224aab3031b9502507abf74d9864 Author: None AuthorDate: Wed May 15 17:47:31 2024 -0400 [cgroups2] Add memory usage reporting to the MemoryControllerProcess Introduces `::usage` to the MemoryControllerProcess to report the total memory usage of a cgroup as well as memory usage statistics provided by `cgroups2::memory:stats`. Review: https://reviews.apache.org/r/74985/ --- include/mesos/mesos.proto | 11 + include/mesos/v1/mesos.proto | 9 .../isolators/cgroups2/controllers/memory.cpp | 49 ++ .../isolators/cgroups2/controllers/memory.hpp | 4 ++ src/tests/containerizer/memory_isolator_tests.cpp | 10 +++-- src/tests/mesos.cpp| 4 +- 6 files changed, 81 insertions(+), 6 deletions(-) diff --git a/include/mesos/mesos.proto b/include/mesos/mesos.proto index 2aad66d67..01f37ae38 100644 --- a/include/mesos/mesos.proto +++ b/include/mesos/mesos.proto @@ -1836,20 +1836,31 @@ message ResourceStatistics { // TODO(chzhcn) mem_file_bytes and mem_anon_bytes are deprecated in // 0.23.0 and will be removed in 0.24.0. + // Amount of memory used to cache filesystem data, including tmpfs and shared + // memory, in bytes. optional uint64 mem_file_bytes = 10; + // Amount of memory used in anonymous mappings in bytes. optional uint64 mem_anon_bytes = 11; // mem_cache_bytes is added in 0.23.0 to represent page cache usage. + // Amount of memory used to cache filesystem data, including tmpfs and shared + // memory, in bytes. optional uint64 mem_cache_bytes = 39; // Since 0.23.0, mem_rss_bytes is changed to represent only // anonymous memory usage. Note that neither its requiredness, type, // name nor numeric tag has been changed. + // Amount of memory used in anonymous mappings in bytes. optional uint64 mem_rss_bytes = 5; + // Amount of cached filesystem data mapped with mmap() in bytes. optional uint64 mem_mapped_file_bytes = 12; + // This is only set if swap is enabled. + // Amount of swap usage in bytes. optional uint64 mem_swap_bytes = 40; + + // Amount of unevictable usage in bytes. optional uint64 mem_unevictable_bytes = 41; // Number of occurrences of different levels of memory pressure diff --git a/include/mesos/v1/mesos.proto b/include/mesos/v1/mesos.proto index d4ab6f35f..ef4920a47 100644 --- a/include/mesos/v1/mesos.proto +++ b/include/mesos/v1/mesos.proto @@ -1800,20 +1800,29 @@ message ResourceStatistics { // TODO(chzhcn) mem_file_bytes and mem_anon_bytes are deprecated in // 0.23.0 and will be removed in 0.24.0. + // Amount of memory used to cache filesystem data, including tmpfs and shared + // memory, in bytes. optional uint64 mem_file_bytes = 10; + // Amount of memory used in anonymous mappings in bytes. optional uint64 mem_anon_bytes = 11; // mem_cache_bytes is added in 0.23.0 to represent page cache usage. + // Amount of memory used to cache filesystem data, including tmpfs and shared + // memory, in bytes. optional uint64 mem_cache_bytes = 39; // Since 0.23.0, mem_rss_bytes is changed to represent only // anonymous memory usage. Note that neither its requiredness, type, // name nor numeric tag has been changed. + // Amount of memory used in anonymous mappings in bytes. optional uint64 mem_rss_bytes = 5; + // Amount of cached filesystem data mapped with mmap() in bytes. optional uint64 mem_mapped_file_bytes = 12; // This is only set if swap is enabled. + // Amount of swap usage in bytes. optional uint64 mem_swap_bytes = 40; + // Amount of unevictable usage in bytes. optional uint64 mem_unevictable_bytes = 41; // Number of occurrences of different levels of memory pressure diff --git a/src/slave/containerizer/mesos/isolators/cgroups2/controllers/memory.cpp b/src/slave/containerizer/mesos/isolators/cgroups2/controllers/memory.cpp index 872a585e2..4da2ab79e 100644 --- a/src/slave/containerizer/mesos/isolators/cgroups2/controllers/memory.cpp +++ b/src/slave/containerizer/mesos/isolators/cgroups2/controllers/memory.cpp @@ -197,6 +197,55 @@ Future MemoryControllerProcess::update( } +Future MemoryControllerProcess::usage( +const ContainerID& containerId, +const string& cgroup) +{ + if (!infos.contains(containerId)) { +return Failure("Unknown container"); + } + + // TODO (jasonzhou): Report a greater variety of stats which are available in + // cgroups2 memory.stat file. + ResourceStatistics stats; + + // Current memory u
(mesos) branch master updated: [cgroup2] Fix CPU isolator tests on cgroups2 systems
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 ba16e64ef [cgroup2] Fix CPU isolator tests on cgroups2 systems ba16e64ef is described below commit ba16e64ef390f0f694c1c0c4446a147cdba2d00d Author: None AuthorDate: Wed May 15 17:46:21 2024 -0400 [cgroup2] Fix CPU isolator tests on cgroups2 systems The change involves migrating the isolator tests from MesosTests to ContainerizerTest which inherit from MesosTests This allows cgroups2 tests to create cgroups in appropriate directories during tests. Review: https://reviews.apache.org/r/74989/ --- src/tests/containerizer/cpu_isolator_tests.cpp| 2 +- src/tests/containerizer/memory_isolator_tests.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tests/containerizer/cpu_isolator_tests.cpp b/src/tests/containerizer/cpu_isolator_tests.cpp index 9498a23da..1131af754 100644 --- a/src/tests/containerizer/cpu_isolator_tests.cpp +++ b/src/tests/containerizer/cpu_isolator_tests.cpp @@ -45,7 +45,7 @@ namespace internal { namespace tests { class CpuIsolatorTest - : public MesosTest, + : public ContainerizerTest, public WithParamInterface {}; diff --git a/src/tests/containerizer/memory_isolator_tests.cpp b/src/tests/containerizer/memory_isolator_tests.cpp index ec0f359a2..6e0e51455 100644 --- a/src/tests/containerizer/memory_isolator_tests.cpp +++ b/src/tests/containerizer/memory_isolator_tests.cpp @@ -45,7 +45,7 @@ namespace internal { namespace tests { class MemoryIsolatorTest - : public MesosTest, + : public ContainerizerTest, public WithParamInterface {};
(mesos) branch master updated: [cgroups2] populate unevictable field from memory.stat
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 43e65760a [cgroups2] populate unevictable field from memory.stat 43e65760a is described below commit 43e65760af52dd286e30a753fd87325c1e67cd6e Author: None AuthorDate: Wed May 15 17:44:12 2024 -0400 [cgroups2] populate unevictable field from memory.stat Review: https://reviews.apache.org/r/74991/ --- src/linux/cgroups2.cpp | 1 + src/linux/cgroups2.hpp | 3 +++ 2 files changed, 4 insertions(+) diff --git a/src/linux/cgroups2.cpp b/src/linux/cgroups2.cpp index 2057b3292..e9845bb56 100644 --- a/src/linux/cgroups2.cpp +++ b/src/linux/cgroups2.cpp @@ -895,6 +895,7 @@ Try parse(const string& content) else if (key == "vmalloc") { stats.vmalloc = bytes; } else if (key == "file_mapped") { stats.file_mapped = bytes; } else if (key == "slab") { stats.slab = bytes; } +else if (key == "unevictable") { stats.unevictable = bytes; } kernel_found |= key == "kernel"; } diff --git a/src/linux/cgroups2.hpp b/src/linux/cgroups2.hpp index ecac09d9b..97260c0d6 100644 --- a/src/linux/cgroups2.hpp +++ b/src/linux/cgroups2.hpp @@ -297,6 +297,9 @@ struct Stats // Amount of memory used for storing in-kernel data structures. Bytes slab; + + // Amount of memory that cannot be reclaimed. + Bytes unevictable; };
(mesos) branch master updated: [cgroups2] adjust CPU weight values from v1 to v2 default
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 15959a137 [cgroups2] adjust CPU weight values from v1 to v2 default 15959a137 is described below commit 15959a137d45a008c63bfd196a119efacbde57f3 Author: None AuthorDate: Wed May 15 17:43:44 2024 -0400 [cgroups2] adjust CPU weight values from v1 to v2 default Modifies the cgroups CPU weights to reflect change from cpu.shares to cpu.weight. In v1, cgroups used cpu.shares which has a default of 1024. In v2, cgroups use cpu.weight which has a default of 100 The range for the cpu.weight is [1,1], the minimum weight has been updated to reflect this. The revocable CPU weight has been scaled down from 10 to 1 to reflect a similar scale to the default. Review: https://reviews.apache.org/r/74992/ --- src/slave/containerizer/mesos/isolators/cgroups2/constants.hpp | 6 +++--- .../containerizer/mesos/isolators/cgroups2/controllers/cpu.cpp | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/slave/containerizer/mesos/isolators/cgroups2/constants.hpp b/src/slave/containerizer/mesos/isolators/cgroups2/constants.hpp index 9498a4779..b82f8cd74 100644 --- a/src/slave/containerizer/mesos/isolators/cgroups2/constants.hpp +++ b/src/slave/containerizer/mesos/isolators/cgroups2/constants.hpp @@ -26,9 +26,9 @@ namespace internal { namespace slave { // CPU controller constants. -const uint64_t CPU_SHARES_PER_CPU = 1024; -const uint64_t CPU_SHARES_PER_CPU_REVOCABLE = 10; -const uint64_t MIN_CPU_SHARES = 2; // Linux constant. +const uint64_t CPU_WEIGHT_PER_CPU = 100; +const uint64_t CPU_WEIGHT_PER_CPU_REVOCABLE = 1; +const uint64_t MIN_CPU_WEIGHT = 1; // Linux constant. const Duration CPU_CFS_PERIOD = Milliseconds(100); // Linux default. const Duration MIN_CPU_CFS_QUOTA = Milliseconds(1); diff --git a/src/slave/containerizer/mesos/isolators/cgroups2/controllers/cpu.cpp b/src/slave/containerizer/mesos/isolators/cgroups2/controllers/cpu.cpp index 85fe063a7..adebab744 100644 --- a/src/slave/containerizer/mesos/isolators/cgroups2/controllers/cpu.cpp +++ b/src/slave/containerizer/mesos/isolators/cgroups2/controllers/cpu.cpp @@ -79,9 +79,9 @@ Future CpuControllerProcess::update( double cpuRequest = *resourceRequests.cpus(); bool revocable = resourceRequests.revocable().cpus().isSome(); uint64_t weightPerCpu = (revocable && flags.revocable_cpu_low_priority) ? - CPU_SHARES_PER_CPU_REVOCABLE : CPU_SHARES_PER_CPU; + CPU_WEIGHT_PER_CPU_REVOCABLE : CPU_WEIGHT_PER_CPU; uint64_t weight = std::max( - static_cast(weightPerCpu * cpuRequest), MIN_CPU_SHARES); + static_cast(weightPerCpu * cpuRequest), MIN_CPU_WEIGHT); Try update = cgroups2::cpu::weight(cgroup, weight); if (update.isError()) {
(mesos) branch master updated: [cgroups2] Add OOM listening to the MemoryControllerProcess.
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 5a56015fb [cgroups2] Add OOM listening to the MemoryControllerProcess. 5a56015fb is described below commit 5a56015fb24993ac92d32e2e56f5db7935fcbebf Author: None AuthorDate: Tue May 14 16:41:16 2024 -0400 [cgroups2] Add OOM listening to the MemoryControllerProcess. Introduces OOM listening to the MemoryControllerProcess so that we detect, report, and respond to OOM events. Review: https://reviews.apache.org/r/74979/ --- .../isolators/cgroups2/controllers/memory.cpp | 120 + .../isolators/cgroups2/controllers/memory.hpp | 19 +++- 2 files changed, 138 insertions(+), 1 deletion(-) diff --git a/src/slave/containerizer/mesos/isolators/cgroups2/controllers/memory.cpp b/src/slave/containerizer/mesos/isolators/cgroups2/controllers/memory.cpp index 732b1c65f..872a585e2 100644 --- a/src/slave/containerizer/mesos/isolators/cgroups2/controllers/memory.cpp +++ b/src/slave/containerizer/mesos/isolators/cgroups2/controllers/memory.cpp @@ -74,6 +74,8 @@ Future MemoryControllerProcess::prepare( infos.put(containerId, Info()); + oomListen(containerId, cgroup); + return Nothing(); } @@ -105,10 +107,24 @@ Future MemoryControllerProcess::recover( infos.put(containerId, Info()); infos[containerId].hardLimitUpdated = true; + oomListen(containerId, cgroup); + return Nothing(); } +Future MemoryControllerProcess::watch( +const ContainerID& containerId, +const string& cgroup) +{ + if (!infos.contains(containerId)) { +return Failure("Unknown container"); + } + + return infos[containerId].limitation.future(); +} + + Future MemoryControllerProcess::update( const ContainerID& containerId, const string& cgroup, @@ -192,12 +208,116 @@ Future MemoryControllerProcess::cleanup( return Nothing(); } + if (infos[containerId].oom.isPending()) { +infos[containerId].oom.discard(); + } + infos.erase(containerId); return Nothing(); } +void MemoryControllerProcess::oomListen( +const ContainerID& containerId, +const string& cgroup) +{ + if (!infos.contains(containerId)) { +LOG(ERROR) << "Cannot listen for OOM events for unknown container " + << containerId; +return; + } + + infos[containerId].oom = cgroups2::memory::oom(cgroup); + + LOG(INFO) << "Listening for OOM events for container " +<< containerId; + + infos[containerId].oom.onAny( + defer(PID(this), +::oomed, +containerId, +cgroup, +lambda::_1)); +} + + +void MemoryControllerProcess::oomed( +const ContainerID& containerId, +const string& cgroup, +const Future& oom) +{ + if (oom.isDiscarded()) { +LOG(INFO) << "OOM event listener discarded"; +return; + } + + if (oom.isFailed()) { +LOG(ERROR) << "OOM event listener failed: " << oom.failure(); +return; + } + + if (!infos.contains(containerId)) { +// It is likely that process exited is executed before this +// function (e.g. The kill and OOM events happen at the same time, +// and the process exit event arrives first). Therefore, we should +// not report a fatal error here. +LOG(INFO) << "OOM event received for terminated container"; +return; + } + + LOG(INFO) << "OOM detected for container" << containerId; + + // Construct a message for the limitation to help with debugging the OOM. + ostringstream limitMessage; + limitMessage << "Memory limit exceeded: "; + + // TODO(dleamy): Report the peak memory usage of the container. The + // 'memory.peak' control is only available on newer Linux kernels. + + // Report memory statistics if successfully retrieved. + Try stats = cgroups2::memory::stats(cgroup); + if (stats.isError()) { +LOG(ERROR) << "Failed to get cgroup memory stats for container " + << containerId << ": " << stats.error(); + } else { +limitMessage << "\nMEMORY STATISTICS post-OOM: \n"; +limitMessage << "anon: " << stats->anon << "\n"; +limitMessage << "file: " << stats->file << "\n"; +limitMessage << "kernel: " << stats->kernel << "\n"; + } + + LOG(INFO) << limitMessage.str(); + + Result hardLimit = cgroups2::memory::max(cgroup); + if (hardLimit.isError()) { +LOG(ERROR) << "Failed to get hard memory limit for container " + << containerId &l