Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8272124: Cgroup v1 initialization causes NullPointerException when cgroup path contains colon #5127

Closed
wants to merge 4 commits into from
Closed
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -196,9 +196,10 @@ public static Optional<CgroupTypeResult> determineType(String mountInfo,
if (isCgroupsV2) {
action = (tokens -> setCgroupV2Path(infos, tokens));
}
selfCgroupLines.map(line -> line.split(":"))
.filter(tokens -> (tokens.length >= 3))
.forEach(action);
// The limit value of 3 is because /proc/self/cgroup contains three
// colon-separated tokens per line. The last token, cgroup path, might
// contain a ':'.
selfCgroupLines.map(line -> line.split(":", 3)).forEach(action);
}

CgroupTypeResult result = new CgroupTypeResult(isCgroupsV2,
@@ -72,6 +72,7 @@ public class TestCgroupSubsystemFactory {
private Path cgroupv1MntInfoDoubleCpusets;
private Path cgroupv1MntInfoDoubleCpusets2;
private Path cgroupv1SelfCgroup;
private Path cgroupv1SelfColons;
private Path cgroupv2SelfCgroup;
private Path cgroupv1SelfCgroupJoinCtrl;
private Path cgroupv1CgroupsOnlyCPUCtrl;
@@ -184,6 +185,23 @@ public class TestCgroupSubsystemFactory {
"2:cpu,cpuacct:/\n" +
"1:name=systemd:/user.slice/user-1000.slice/user@1000.service/apps.slice/apps-org.gnome.Terminal.slice/vte-spawn-3c00b338-5b65-439f-8e97-135e183d135d.scope\n" +
"0::/user.slice/user-1000.slice/user@1000.service/apps.slice/apps-org.gnome.Terminal.slice/vte-spawn-3c00b338-5b65-439f-8e97-135e183d135d.scope\n";

// `/proc/self/cgroup` should contain **three** colon-separated fields,
// `hierarchy-ID:controller-list:cgroup-path`. This cgroup-path intentionally
// contains a colon to ensure that the correct path is being extracted by the
// login in CgroupSubsystemFactory.
Copy link
Contributor

@jerboaa jerboaa Aug 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/login/logic

private String cgroupv1SelfColonsContent = "11:memory:/system.slice/containerd.service/kubepods-burstable-podf65e797d_d5f9_4604_9773_94f4bb9946a0.slice:cri-containerd:86ac6260f9f8a9c1276748250f330ae9c2fcefe5ae809364ad1e45f3edf7e08a\n" +
"10:hugetlb:/\n" +
"9:cpuset:/\n" +
"8:pids:/user.slice/user-1000.slice/user@1000.service\n" +
"7:freezer:/\n" +
"6:blkio:/\n" +
"5:net_cls,net_prio:/\n" +
"4:devices:/user.slice\n" +
"3:perf_event:/\n" +
"2:cpu,cpuacct:/\n" +
"1:name=systemd:/user.slice/user-1000.slice/user@1000.service/apps.slice/apps-org.gnome.Terminal.slice/vte-spawn-3c00b338-5b65-439f-8e97-135e183d135d.scope\n" +
"0::/user.slice/user-1000.slice/user@1000.service/apps.slice/apps-org.gnome.Terminal.slice/vte-spawn-3c00b338-5b65-439f-8e97-135e183d135d.scope\n";
private String cgroupv2SelfCgroupContent = "0::/user.slice/user-1000.slice/session-2.scope";

@Before
@@ -224,6 +242,9 @@ public void setup() {
cgroupv1SelfCgroup = Paths.get(existingDirectory.toString(), "self_cgroup_cgv1");
Files.writeString(cgroupv1SelfCgroup, cgroupv1SelfCgroupContent);

cgroupv1SelfColons = Paths.get(existingDirectory.toString(), "self_colons_cgv1");
Files.writeString(cgroupv1SelfColons, cgroupv1SelfColonsContent);

cgroupv2SelfCgroup = Paths.get(existingDirectory.toString(), "self_cgroup_cgv2");
Files.writeString(cgroupv2SelfCgroup, cgroupv2SelfCgroupContent);

@@ -340,6 +361,20 @@ public void testHybridCgroupsV1() throws IOException {
assertEquals("/sys/fs/cgroup/memory", memoryInfo.getMountPoint());
}

@Test
public void testColonsCgroupsV1() throws IOException {
String cgroups = cgroupv1CgInfoNonZeroHierarchy.toString();
String mountInfo = cgroupv1MntInfoNonZeroHierarchy.toString();
String selfCgroup = cgroupv1SelfColons.toString();
Optional<CgroupTypeResult> result = CgroupSubsystemFactory.determineType(mountInfo, cgroups, selfCgroup);

assertTrue("Expected non-empty cgroup result", result.isPresent());
CgroupTypeResult res = result.get();
CgroupInfo memoryInfo = res.getInfos().get("memory");
assertEquals(memoryInfo.getCgroupPath(), "/system.slice/containerd.service/kubepods-burstable-podf65e797d_d5f9_4604_9773_94f4bb9946a0.slice:cri-containerd:86ac6260f9f8a9c1276748250f330ae9c2fcefe5ae809364ad1e45f3edf7e08a");
assertEquals(memoryInfo.getMountRoot(), memoryInfo.getMountRoot());
Copy link
Contributor

@jerboaa jerboaa Aug 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I meant here is to also change the mountinfo entry for memory and then use

assertEquals(memoryInfo.getMountRoot(), memoryInfo.getCgroupPath());

as that's really what was happening in the wild. mount root - containing colons - would correctly parse, but the cgroup path - containing colons - would not and the assertion would fail (without the change in CgroupSubsystemFactory). Thanks!

Copy link
Contributor

@jerboaa jerboaa Aug 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hseigel it still has assertEquals(memoryInfo.getMountRoot(), memoryInfo.getMountRoot()); - tautology - should be: assertEquals(memoryInfo.getMountRoot(), memoryInfo.getCgroupPath());

}

@Test
public void testZeroHierarchyCgroupsV1() throws IOException {
String cgroups = cgroupv1CgInfoZeroHierarchy.toString();