Skip to content
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

conmon writes oom file to cwd #504

Closed
Luap99 opened this issue Apr 29, 2024 · 1 comment · Fixed by #514
Closed

conmon writes oom file to cwd #504

Luap99 opened this issue Apr 29, 2024 · 1 comment · Fixed by #514

Comments

@Luap99
Copy link
Member

Luap99 commented Apr 29, 2024

I have no idea if there is a reason for it but this seems rather strange behavior, why do we write the oom file to the cwd? This clutters the working directory of users with a random oom file in case of a oom container kill which makes no sense to a end user.

conmon/src/cgroup.c

Lines 331 to 334 in e21e7c8

_cleanup_close_ int oom_fd = open("oom", O_CREAT | O_CLOEXEC, 0666);
if (oom_fd < 0) {
nwarn("Failed to write oom file");
}

For podman something simple as
podman run --rm --memory 1m --oom-score-adj 1000 quay.io/libpod/testimage:20240123 sort /dev/urandom will reproduce the issue and writes it the cwd as podman does not change the cwd for conmon.

I tried to lookup the history and chased this all the way back to the inial commit, then continued in crio and eventually found cri-o/cri-o@7700a62

I am not sure did cri-o set a different cwd for each conmon instance so cri-o could just read this without conflicts?

IMO I like to remove this assuming cri-o does no depend on it? Using the persistent path should be better?
Alternatively do we have to change the cwd in podman before launching conmon? Not sure if this breaks anything.

giuseppe added a commit to giuseppe/conmon that referenced this issue Jul 4, 2024

Verified

This commit was signed with the committer’s verified signature.
giuseppe Giuseppe Scrivano
if a persist_path was provided, then use only that to write the oom
file instead of creating it also in the current working directory.

Closes: containers#504

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
@giuseppe
Copy link
Member

giuseppe commented Jul 4, 2024

seems weird to create it twice, I've opened a PR: #513

giuseppe added a commit to giuseppe/conmon that referenced this issue Jul 4, 2024

Verified

This commit was signed with the committer’s verified signature.
giuseppe Giuseppe Scrivano
if a persist_path was provided, then use only that to write the oom
file instead of creating it also in the current working directory.

Closes: containers#504

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
giuseppe added a commit to giuseppe/conmon that referenced this issue Jul 10, 2024

Verified

This commit was signed with the committer’s verified signature.
giuseppe Giuseppe Scrivano
instead use the bundle path to create the second (shurgh!) file since
this is what CRI-O uses.

Closes: containers#504

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
giuseppe added a commit to giuseppe/conmon that referenced this issue Jul 10, 2024

Verified

This commit was signed with the committer’s verified signature.
giuseppe Giuseppe Scrivano
instead use the bundle path to create the second (shurgh!) file since
this is what CRI-O uses.

Closes: containers#504

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
giuseppe added a commit to giuseppe/conmon that referenced this issue Jul 11, 2024

Verified

This commit was signed with the committer’s verified signature.
giuseppe Giuseppe Scrivano
instead use the bundle path to create the second (shurgh!) file since
this is what CRI-O uses.

Closes: containers#504

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
giuseppe added a commit to giuseppe/conmon that referenced this issue Jul 16, 2024

Verified

This commit was signed with the committer’s verified signature.
giuseppe Giuseppe Scrivano
instead use the bundle path to create the second (shurgh!) file since
this is what CRI-O uses.

Closes: containers#504

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
haircommander pushed a commit that referenced this issue Jul 16, 2024
instead use the bundle path to create the second (shurgh!) file since
this is what CRI-O uses.

Closes: #504

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants