DevinLeamy commented on code in PR #567:
URL: https://github.com/apache/mesos/pull/567#discussion_r1578178126
##########
src/tests/mesos.cpp:
##########
@@ -579,7 +585,13 @@ slave::Flags
ContainerizerTest<slave::MesosContainerizer>::CreateSlaveFlags()
// Use cgroup isolators if they're available and we're root.
// TODO(idownes): Refactor the cgroups/non-cgroups code.
- if (cgroups::enabled() && user.get() == "root") {
+ if (cgroupsV2() && *user == "root") {
+ // TODO(dleamy): Add the memory isolator once it's supported by the cgroups
+ // v2 isolator.
+ flags.isolation = "cgroups/cpu";
+ flags.cgroups_hierarchy = "/sys/fs/cgroup";
+ flags.cgroups_root = TEST_CGROUPS_ROOT;
Review Comment:
We don't create a unique `flags.cgroups_root` like in cgroups v1. The source
of that change [[1]](https://issues.apache.org/jira/browse/MESOS-926),
[[2]](https://github.com/apache/mesos/commit/0f3f8f35a73dd5f9b35d657b3912449f570243d3#diff-a6984932a4ddcf84bf808f053133927e627d81583011efe943ba46531181a4d4)
doesn't draw specific attention to the change. My view, in cgroups v2, is that
we can trust our creation and destruction fixtures to create and destroy the
test cgroup hierarchy on every test run, removing the need for this.
The reason I don't _just follow the convention in cgroups v1_ here is
because we need to know the _exact_ name of the root test cgroup to create it,
inside of `SetupCgroupsV2`, and we don't have access the flag
`flags.cgroups_root` directly. We _could_ store the root cgroup inside of a
member variable; I thought this was simpler.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
