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

[Severin Gehwolf]

On Fri, 28 Jun 2024 15:41:48 GMT, Severin Gehwolf  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 18 commits:
> 
>  - Merge branch 'master' into jdk-8261242-is-containerized-fix
>  - 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
>  - ... and 8 more: https://git.openjdk.org/jdk/compare/486aa11e...1017da35

Thank you for the review!

-

PR Comment: https://git.openjdk.org/jdk/pull/18201#issuecomment-2199581201

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

[Ioi Lam]

On Fri, 28 Jun 2024 15:41:48 GMT, Severin Gehwolf  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 18 commits:
> 
>  - Merge branch 'master' into jdk-8261242-is-containerized-fix
>  - 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
>  - ... and 8 more: https://git.openjdk.org/jdk/compare/486aa11e...1017da35

Looks reasonable to me

-

Marked as reviewed by iklam (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18201#pullrequestreview-2149956104

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

[Larry Cable]

On Fri, 28 Jun 2024 15:41:48 GMT, Severin Gehwolf  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 18 commits:
> 
>  - Merge branch 'master' into jdk-8261242-is-containerized-fix
>  - 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
>  - ... and 8 more: https://git.openjdk.org/jdk/compare/486aa11e...1017da35

On 6/28/24 8:47 AM, Severin Gehwolf wrote:
>
> @adinn 
> 
>  
> @iklam 
> 
>  
> Could one of you please help with a second review, please? Not sure if 
> @larry-cable 
> 
>  
> review gets recorded with him having no OpenJDK project role :-/ 
> Thanks in advance!
>

yeah sorry - I'm a "newbie" ... only since 1.1 ... :)
>
> —
> Reply to this email directly, view it on GitHub 
> ,
>  
> or unsubscribe 
> .
> You are receiving this because you were mentioned.Message ID: 
> ***@***.***>
>

--UaLc7Fb3y3GBgvf0paImu5tU
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: 8bit



  
  


On 6/28/24 8:47 AM, Severin Gehwolf
  wrote:


  
  https://urldefense.com/v3/__https://github.com/adinn__;!!ACWV5N9M2RV99hQ!JFioMWt3M387QFQK8UEgSmPMiy736aUVfLbZdVscJ-BDxoO9GjUqLFHtvCkNB5oG3MBadzdxxytGTNNNu9cWG6X3BQ$;
 ***@***.*** https://urldefense.com/v3/__https://github.com/iklam__;!!ACWV5N9M2RV99hQ!JFioMWt3M387QFQK8UEgSmPMiy736aUVfLbZdVscJ-BDxoO9GjUqLFHtvCkNB5oG3MBadzdxxytGTNNNu9dyhXpCrw$;
 ***@***.*** Could one of you please help
   

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

[Severin Gehwolf]

On Fri, 28 Jun 2024 15:41:48 GMT, Severin Gehwolf  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 18 commits:
> 
>  - Merge branch 'master' into jdk-8261242-is-containerized-fix
>  - 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
>  - ... and 8 more: https://git.openjdk.org/jdk/compare/486aa11e...1017da35

@adinn @iklam Could one of you please help with a second review, please? Not 
sure if @larry-cable review gets recorded with him having no OpenJDK project 
role :-/ Thanks in advance!

-

PR Comment: https://git.openjdk.org/jdk/pull/18201#issuecomment-2197212014

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

[Severin Gehwolf]

> 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 18 commits:

 - Merge branch 'master' into jdk-8261242-is-containerized-fix
 - 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
 - ... and 8 more: https://git.openjdk.org/jdk/compare/486aa11e...1017da35

-

Changes: https://git.openjdk.org/jdk/pull/18201/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk=18201=07
  Stats: 411 lines in 20 files changed: 305 ins; 79 del; 27 mod
  Patch: https://git.openjdk.org/jdk/pull/18201.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18201/head:pull/18201

PR: https://git.openjdk.org/jdk/pull/18201