RFR: 8261242: [Linux] OSContainer::is_containerized() returns true when run outside a container [v6]

[Thomas Stuefe]

On Thu, 20 Jun 2024 12:06:43 GMT, Severin Gehwolf <sgehwolf at openjdk.org> wrote:

>> Please review this enhancement to the container detection code which allows it to figure out whether the JVM is actually running inside a container (`podman`, `docker`, `crio`), or with some other means that enforces memory/cpu limits by means of the cgroup filesystem. If neither of those conditions hold, the JVM runs in not containerized mode, addressing the issue described in the JBS tracker. For example, on my Linux system `is_containerized() == false" is being indicated with the following trace log line:
>> 
>> 
>> [0.001s][debug][os,container] OSContainer::init: is_containerized() = false because no cpu or memory limit is present
>> 
>> 
>> This state is being exposed by the Java `Metrics` API class using the new (still JDK internal) `isContainerized()` method. Example:
>> 
>> 
>> java -XshowSettings:system --version
>> Operating System Metrics:
>>     Provider: cgroupv1
>>     System not containerized.
>> openjdk 23-internal 2024-09-17
>> OpenJDK Runtime Environment (fastdebug build 23-internal-adhoc.sgehwolf.jdk-jdk)
>> OpenJDK 64-Bit Server VM (fastdebug build 23-internal-adhoc.sgehwolf.jdk-jdk, mixed mode, sharing)
>> 
>> 
>> The basic property this is being built on is the observation that the cgroup controllers typically get mounted read only into containers. Note that the current container tests assert that `OSContainer::is_containerized() == true` in various tests. Therefore, using the heuristic of "is any memory or cpu limit present" isn't sufficient. I had considered that in an earlier iteration, but many container tests failed.
>> 
>> Overall, I think, with this patch we improve the current situation of claiming a containerized system being present when it's actually just a regular Linux system.
>> 
>> Testing:
>> 
>> - [x] GHA (risc-v failure seems infra related)
>> - [x] Container tests on Linux x86_64 of cgroups v1 and cgroups v2 (including gtests)
>> - [x] Some manual testing using cri-o
>> 
>> Thoughts?
>
> Severin Gehwolf has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Remove problem listing of PlainRead which is reworked here

Seems okay. I don't understand the depths of V1 vs V2 controller files as well as you do, @jerboaa , but I trust you there. The mechanics seem fine.

src/hotspot/os/linux/cgroupSubsystem_linux.cpp line 373:

> 371:      * (11)   mount source:    matched with '%*s' and discarded
> 372:      * (12)   super options:   matched with '%*s' and discarded
> 373:      */

Thanks for the good comment. That scanf line is a brain teaser.

src/hotspot/os/linux/cgroupSubsystem_linux.cpp line 422:

> 420:      * (12)   super options:   matched with '%s' and captured in 'tmpcgroups'
> 421:      */
> 422:     if (sscanf(p, "%*d %*d %*d:%*d %s %s %s%*[^-]- %s %*s %s", tmproot, tmpmount, mount_opts, tmp_fs_type, tmpcgroups) == 5) {

The only difference to v1 is that we parse the super options (12), right? Could we factor out the parsing into a helper function? Or, alternatively, at least `#define` the scanf format somewhere up top, including the nice comment, and reuse that format string?

-------------

Marked as reviewed by stuefe (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18201#pullrequestreview-2130943896
PR Review Comment: https://git.openjdk.org/jdk/pull/18201#discussion_r1647881202
PR Review Comment: https://git.openjdk.org/jdk/pull/18201#discussion_r1647925120