On Fri, 7 Nov 2025 16:41:26 GMT, Severin Gehwolf <[email protected]> wrote:
> Please review this test-only enhancement. It creates a regression test for > the Amazon ECS setup on cgroups v1 where the parent memory limit isn't > visible inside the container and, thus, needs to rely on the cg v1 specific > `hierarchical_memory_limit` token in `memory.stat`. The proposed test is cg > v1 only and needs to be run as root. It's skipped otherwise. It's useful to > have when working on refactorings like #27743 so as not to regress. > > The other changes are an effort to reduce code duplication in the test code > where similar patterns have been used in other container tests. > > Testing (all on Linux x86_64): > - [x] CG version 2, run as root. Engine: docker. Test is skipped. > - [x] CG version 1, run as root. Engine: docker. Test passes and fails > without the product fix of > [JDK-8370572](https://bugs.openjdk.org/browse/JDK-8370572) > - [x] CG version 1, run as root. Engine: podman. Test passes and fails > without the product fix of > [JDK-8370572](https://bugs.openjdk.org/browse/JDK-8370572) > - [X] CG version 1, run as non-root. Test skipped. > - [x] GHA, though I don't think this is very useful for this change. > > Thoughts? Looks fine, a few nits. test/hotspot/jtreg/containers/docker/TestMemoryInvisibleParent.java line 96: > 94: opts.addDockerOpts("--cgroup-parent=/" + cgroupParent); > 95: Common.run(opts) > 96: .shouldContain("Hierarchical Memory Limit is: " + > expectedValue); Indenting a a bit off here. test/hotspot/jtreg/containers/docker/TestMemoryInvisibleParent.java line 107: > 105: Path sysFsMemory = Path.of("/", "sys", "fs", "cgroup", "memory"); > 106: Path cgroupParentPath = sysFsMemory.resolve(cgroupParent); > 107: ProcessBuilder pb = new ProcessBuilder("mkdir", "-p", > cgroupParentPath.toString()); So I am guessing we are fine with leaving this cgroup behind, after the test is done? ------------- Marked as reviewed by shade (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/28201#pullrequestreview-3441857489 PR Review Comment: https://git.openjdk.org/jdk/pull/28201#discussion_r2509492642 PR Review Comment: https://git.openjdk.org/jdk/pull/28201#discussion_r2509498605
