Skip to content

Commit

Permalink
client: change test to not poke cgroupv2 edge case
Browse files Browse the repository at this point in the history
This PR tweaks the TestCpusetManager_AddAlloc unit test to not break
when being run on a machine using cgroupsv2. The behavior of writing
an empty cpuset.cpu changes in cgroupv2, where such a group now inherits
the value of its parent group, rather than remaining empty.

The test in question was written such that a task would consume all available
cores shared on an alloc, causing the empty set to be written to the shared
group, which works fine on cgroupsv1 but breaks on cgroupsv2. By adjusting
the test to consume only 1 core instead of all cores, it no longer triggers
that edge case.

The actual fix for the new cgroupsv2 behavior will be in #11933
  • Loading branch information
shoenig committed Jan 26, 2022
1 parent 94b744c commit e8e4055
Showing 1 changed file with 20 additions and 5 deletions.
25 changes: 20 additions & 5 deletions client/lib/cgutil/cpuset_manager_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,27 +57,31 @@ func TestCpusetManager_Init(t *testing.T) {
require.DirExists(t, filepath.Join(manager.cgroupParentPath, ReservedCpusetCgroupName))
}

func TestCpusetManager_AddAlloc(t *testing.T) {
func TestCpusetManager_AddAlloc_single(t *testing.T) {
manager, cleanup := tmpCpusetManager(t)
defer cleanup()
require.NoError(t, manager.Init())

alloc := mock.Alloc()
alloc.AllocatedResources.Tasks["web"].Cpu.ReservedCores = manager.parentCpuset.ToSlice()
// reserve just one core (the 0th core, which probably exists)
alloc.AllocatedResources.Tasks["web"].Cpu.ReservedCores = cpuset.New(0).ToSlice()
manager.AddAlloc(alloc)

// force reconcile
manager.reconcileCpusets()

// check that no more cores exist in the shared cgroup
// check that the 0th core is no longer available in the shared group
// actual contents of shared group depends on machine core count
require.DirExists(t, filepath.Join(manager.cgroupParentPath, SharedCpusetCgroupName))
require.FileExists(t, filepath.Join(manager.cgroupParentPath, SharedCpusetCgroupName, "cpuset.cpus"))
sharedCpusRaw, err := ioutil.ReadFile(filepath.Join(manager.cgroupParentPath, SharedCpusetCgroupName, "cpuset.cpus"))
require.NoError(t, err)
sharedCpus, err := cpuset.Parse(string(sharedCpusRaw))
require.NoError(t, err)
require.Empty(t, sharedCpus.ToSlice())
require.NotEmpty(t, sharedCpus.ToSlice())
require.NotContains(t, sharedCpus.ToSlice(), uint16(0))

// check that all cores are allocated to reserved cgroup
// check that the 0th core is allocated to reserved cgroup
require.DirExists(t, filepath.Join(manager.cgroupParentPath, ReservedCpusetCgroupName))
reservedCpusRaw, err := ioutil.ReadFile(filepath.Join(manager.cgroupParentPath, ReservedCpusetCgroupName, "cpuset.cpus"))
require.NoError(t, err)
Expand All @@ -100,6 +104,17 @@ func TestCpusetManager_AddAlloc(t *testing.T) {
require.Exactly(t, alloc.AllocatedResources.Tasks["web"].Cpu.ReservedCores, taskCpus.ToSlice())
}

func TestCpusetManager_AddAlloc_subset(t *testing.T) {
t.Skip("todo: add test for #11933")
}

func TestCpusetManager_AddAlloc_all(t *testing.T) {
// cgroupsv2 changes behavior of writing empty cpuset.cpu, which is what
// happens to the /shared group when one or more allocs consume all available
// cores.
t.Skip("todo: add test for #11933")
}

func TestCpusetManager_RemoveAlloc(t *testing.T) {
manager, cleanup := tmpCpusetManager(t)
defer cleanup()
Expand Down

0 comments on commit e8e4055

Please sign in to comment.