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

state: fix race condition when reading cgroup #474

Merged
merged 1 commit into from
Aug 27, 2020

Conversation

giuseppe
Copy link
Member

by the time crun attempts to read from the cgroup, systemd might have
already cleaned it up. When using systemd, on ENOENT state reports
the container as "stopped" instead of an error.

Closes: containers/podman#7148

Signed-off-by: Giuseppe Scrivano gscrivan@redhat.com

by the time crun attempts to read from the cgroup, systemd might have
already cleaned it up.  When using systemd, on ENOENT state reports
the container as "stopped" instead of an error.

Closes: containers/podman#7148

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

LGTM

I had to quickly crash-course on crun_error_release(), ..._get_errno() and look for the possibility of a constant instead of "stopped" (none found, not worth fixing for this PR). The logic looks sound. Thanks @giuseppe !

@giuseppe giuseppe merged commit ee8746d into containers:master Aug 27, 2020
@rhatdan
Copy link
Member

rhatdan commented Aug 31, 2020

@giuseppe Can you cut a release? Is this something we should race into stable, IE RHEL8 (8.3)? Or can we wait until RHEL8.3.1?

@edsantiago
Copy link
Member

edsantiago commented Aug 31, 2020

Does RHEL even ship crun? (EDIT: I checked the module. Yes, crun 0.14.1-2 is in the container-tools rhel8-rhel-8.3.0 module. But is it used?)

@giuseppe
Copy link
Member Author

@giuseppe Can you cut a release? Is this something we should race into stable, IE RHEL8 (8.3)? Or can we wait until RHEL8.3.1?

I think we can wait for 8.3.1. The race just causes an error message to be reported but no functionality is affected.

Does RHEL even ship crun? (EDIT: I checked the module. Yes, crun 0.14.1-2 is in the container-tools rhel8-rhel-8.3.0 module. But is it used?)

It is tech preview. Who is interested in the additional crun functionalities will have the opportunity to try it out.

@edsantiago
Copy link
Member

The race just causes an error message to be reported but no functionality is affected.

Well, to be fully honest, podman build does fail at that step. It usually succeeds on rerun. But it's not just a spurious error message, it does lead to failure.

It is tech preview.

Aha, thank you. So presumably cgroups v2 is also tech preview then, and users would need to enable it explicitly?

@giuseppe
Copy link
Member Author

Aha, thank you. So presumably cgroups v2 is also tech preview then, and users would need to enable it explicitly?

yes, AFAIK cgroup v1 is still the default for RHEL 8.x.

@rhatdan
Copy link
Member

rhatdan commented Sep 2, 2020

cgroup V1 will always be the default in RHEL8. Not even supported until RHEL8.4. crun is shipping for the first time in RHEL 8.3 as I understand it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI: cgroup.freeze flake is back
3 participants