-
Notifications
You must be signed in to change notification settings - Fork 25.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Handle cgroups v2 in OsProbe
#76883
Handle cgroups v2 in OsProbe
#76883
Conversation
Closes elastic#76812. `OsProbe` was only capable of handle cgroup data in the v1 format. However, Debian 11 uses cgroups v2 by default, and Elasticsearch isn't capable of reporting any cgroup information. Therefore, add support for the v2 layout.
Pinging @elastic/es-core-infra (Team:Core/Infra) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 I assume there's no need to add explicit unit test coverage as we're getting coverage simply by running this on a system using cgroups v2 (Debian 11)?
@mark-vieira I added testing for v2. |
@rjernst I'd appreciate you view on this PR. I'm particularly concerned that the security manager policy changes aren't accurate or sufficient, because the wildcard matching isn't sophisticated enough. For example, if
then the code needs to access:
The best that the policy file could manage is:
...which would work, but is significantly more open than what we currently allow. |
@elasticmachine update branch |
List<String> lines = readProcSelfCgroup(); | ||
|
||
// cgroup v2 | ||
if (lines.size() == 1 && lines.get(0).startsWith("0::")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is fine since here we are not actually parsing the line, but elsewhere in this file we do a bare split on the control group line, which doesn’t allow for colons in the underlying path. Looks like an existing issue in this code, but thought I would mention. To ensure we protect against it (could be a followup). See https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8272124
This commit re-applies e827ec6.
Backported to |
This had to be reverted in the end since the policy changes didn't work. |
Closes #76812.
OsProbe
was only capable of handle cgroup data in the v1 format.However, Debian 11 uses cgroups v2 by default, and Elasticsearch isn't
capable of reporting any cgroup information. Therefore, add support for
the v2 layout.