From c1a5b3e1095b090b7ee3a9030d58eb135e672fcf Mon Sep 17 00:00:00 2001 From: Odin Ugedal Date: Mon, 5 Jul 2021 16:18:07 +0200 Subject: [PATCH 1/2] libct/cg/fs/freezer.GetState: report current cgroup state If a control group is frozen, all its descendants will report FROZEN in freezer.state cgroup file. OTOH cgroup v2 cgroup.freeze is not reporting the cgroup as frozen unless it is frozen directly (i.e. not via an ancestor). Fix the discrepancy between v1 and v2 drivers behavior by looking into freezer.self_freezing cgroup file, which, according to kernel documentation, will show 1 iff the cgroup was frozen directly. Co-authored-by: Kir Kolyshkin Signed-off-by: Odin Ugedal Signed-off-by: Kir Kolyshkin (cherry picked from commit 294c4866ea93392dd81f68f97fe6d94bbbf8f9c1) Signed-off-by: Kir Kolyshkin --- libcontainer/cgroups/fs/freezer.go | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/libcontainer/cgroups/fs/freezer.go b/libcontainer/cgroups/fs/freezer.go index 519bd5136..4baa2798a 100644 --- a/libcontainer/cgroups/fs/freezer.go +++ b/libcontainer/cgroups/fs/freezer.go @@ -129,7 +129,25 @@ func (s *FreezerGroup) GetState(path string) (configs.FreezerState, error) { case "THAWED": return configs.Thawed, nil case "FROZEN": - return configs.Frozen, nil + // Find out whether the cgroup is frozen directly, + // or indirectly via an ancestor. + self, err := cgroups.ReadFile(path, "freezer.self_freezing") + if err != nil { + // If the kernel is too old, then we just treat + // it as being frozen. + if errors.Is(err, os.ErrNotExist) || errors.Is(err, unix.ENODEV) { + err = nil + } + return configs.Frozen, err + } + switch self { + case "0\n": + return configs.Thawed, nil + case "1\n": + return configs.Frozen, nil + default: + return configs.Undefined, fmt.Errorf(`unknown "freezer.self_freezing" state: %q`, self) + } case "FREEZING": // Make sure we get a stable freezer state, so retry if the cgroup // is still undergoing freezing. This should be a temporary delay. From 3f40fbff17558fb5fd65c22de1d60a226e1463bd Mon Sep 17 00:00:00 2001 From: Odin Ugedal Date: Wed, 7 Jul 2021 11:33:02 +0200 Subject: [PATCH 2/2] libct/cg/sd: Add freezer tests This test the issues fixed by the two preceding commits. Co-Authored-By: Kir Kolyshkin Signed-off-by: Odin Ugedal Signed-off-by: Kir Kolyshkin (cherry picked from commit 3e5c19970838b96a55b1474dd54b273eea46acb7) Signed-off-by: Kir Kolyshkin --- libcontainer/cgroups/systemd/systemd_test.go | 141 +++++++++++++++++++ 1 file changed, 141 insertions(+) diff --git a/libcontainer/cgroups/systemd/systemd_test.go b/libcontainer/cgroups/systemd/systemd_test.go index cee176579..a444ff613 100644 --- a/libcontainer/cgroups/systemd/systemd_test.go +++ b/libcontainer/cgroups/systemd/systemd_test.go @@ -1,6 +1,7 @@ package systemd import ( + "bufio" "bytes" "os" "os/exec" @@ -193,3 +194,143 @@ func TestUnitExistsIgnored(t *testing.T) { } } } + +func TestFreezePodCgroup(t *testing.T) { + if !IsRunningSystemd() { + t.Skip("Test requires systemd.") + } + if os.Geteuid() != 0 { + t.Skip("Test requires root.") + } + + podConfig := &configs.Cgroup{ + Parent: "system.slice", + Name: "system-runc_test_pod.slice", + Resources: &configs.Resources{ + SkipDevices: true, + Freezer: configs.Frozen, + }, + } + // Create a "pod" cgroup (a systemd slice to hold containers), + // which is frozen initially. + pm := newManager(podConfig) + defer pm.Destroy() //nolint:errcheck + if err := pm.Apply(-1); err != nil { + t.Fatal(err) + } + + if err := pm.Freeze(configs.Frozen); err != nil { + t.Fatal(err) + } + if err := pm.Set(podConfig.Resources); err != nil { + t.Fatal(err) + } + + // Check the pod is frozen. + pf, err := pm.GetFreezerState() + if err != nil { + t.Fatal(err) + } + if pf != configs.Frozen { + t.Fatalf("expected pod to be frozen, got %v", pf) + } + + // Create a "container" within the "pod" cgroup. + // This is not a real container, just a process in the cgroup. + containerConfig := &configs.Cgroup{ + Parent: "system-runc_test_pod.slice", + ScopePrefix: "test", + Name: "inner-contianer", + Resources: &configs.Resources{}, + } + + cmd := exec.Command("bash", "-c", "while read; do echo $REPLY; done") + cmd.Env = append(os.Environ(), "LANG=C") + + // Setup stdin. + stdinR, stdinW, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + cmd.Stdin = stdinR + + // Setup stdout. + stdoutR, stdoutW, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + cmd.Stdout = stdoutW + rdr := bufio.NewReader(stdoutR) + + // Setup stderr. + var stderr bytes.Buffer + cmd.Stderr = &stderr + + err = cmd.Start() + stdinR.Close() + stdoutW.Close() + defer func() { + _ = stdinW.Close() + _ = stdoutR.Close() + }() + if err != nil { + t.Fatal(err) + } + // Make sure to not leave a zombie. + defer func() { + // These may fail, we don't care. + _ = cmd.Process.Kill() + _ = cmd.Wait() + }() + + // Put the process into a cgroup. + cm := newManager(containerConfig) + defer cm.Destroy() //nolint:errcheck + + if err := cm.Apply(cmd.Process.Pid); err != nil { + t.Fatal(err) + } + if err := cm.Set(containerConfig.Resources); err != nil { + t.Fatal(err) + } + // Check that we put the "container" into the "pod" cgroup. + if !strings.HasPrefix(cm.Path("freezer"), pm.Path("freezer")) { + t.Fatalf("expected container cgroup path %q to be under pod cgroup path %q", + cm.Path("freezer"), pm.Path("freezer")) + } + // Check the container is not reported as frozen despite the frozen parent. + cf, err := cm.GetFreezerState() + if err != nil { + t.Fatal(err) + } + if cf != configs.Thawed { + t.Fatalf("expected container to be thawed, got %v", cf) + } + + // Unfreeze the pod. + if err := pm.Freeze(configs.Thawed); err != nil { + t.Fatal(err) + } + + cf, err = cm.GetFreezerState() + if err != nil { + t.Fatal(err) + } + if cf != configs.Thawed { + t.Fatalf("expected container to be thawed, got %v", cf) + } + + // Check the "container" works. + marker := "one two\n" + _, err = stdinW.WriteString(marker) + if err != nil { + t.Fatal(err) + } + reply, err := rdr.ReadString('\n') + if err != nil { + t.Fatalf("reading from container: %v", err) + } + if reply != marker { + t.Fatalf("expected %q, got %q", marker, reply) + } +}