On Tue, 25 Jun 2024 13:54:46 GMT, Severin Gehwolf <sgehw...@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 with a new target base due to a 
> merge or a rebase. The pull request now contains 17 commits:
> 
>  - Refactor mount info matching to helper function
>  - Merge branch 'master' into jdk-8261242-is-containerized-fix
>  - Remove problem listing of PlainRead which is reworked here
>  - Merge branch 'master' into jdk-8261242-is-containerized-fix
>  - Merge branch 'master' into jdk-8261242-is-containerized-fix
>  - Add doc for mountinfo scanning.
>  - Unify naming of variables
>  - Merge branch 'master' into jdk-8261242-is-containerized-fix
>  - Merge branch 'master' into jdk-8261242-is-containerized-fix
>  - jcheck fixes
>  - ... and 7 more: https://git.openjdk.org/jdk/compare/baafa662...532ea33b

src/hotspot/share/prims/jvm.cpp line 504:

> 502: JVM_LEAF(jboolean, JVM_IsContainerized(void))
> 503: #ifdef LINUX
> 504:   if (OSContainer::is_containerized()) {

// nit: personal preference...

return OSContainer::isContainerized() ? JNI_TRUE : JNI_FALSE;

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18201#discussion_r1657650139

Reply via email to