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

cgroup: set the memory limit on the system scope #1217

Merged
merged 2 commits into from
May 19, 2023

Conversation

giuseppe
Copy link
Member

when updating the memory limit, we now set it also on the systemd scope.

giuseppe added a commit to giuseppe/libpod that referenced this pull request May 19, 2023
b25b330 introduced this behaviour.

It was fine at the time because we didn't support "container update",
so the limit could not be changed at runtime.  Since it is not
possible to change the memory limit at runtime, read the limit as
reported from the cgroup.

containers/crun#1217 is required for crun.

[NO NEW TESTS NEEDED] needs a new crun release

Closes: containers#18621

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
giuseppe added a commit to giuseppe/libpod that referenced this pull request May 19, 2023
b25b330 introduced this behaviour.

It was fine at the time because we didn't support "container update",
so the limit could not be changed at runtime.  Since it is not
possible to change the memory limit at runtime, read the limit as
reported from the cgroup.

containers/crun#1217 is required for crun.

[NO NEW TESTS NEEDED] needs a new crun release

Closes: containers#18621

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
giuseppe added a commit to giuseppe/libpod that referenced this pull request May 19, 2023
b25b330 introduced this behaviour.

It was fine at the time because we didn't support "container update",
so the limit could not be changed at runtime.  Since it is not
possible to change the memory limit at runtime, read the limit as
reported from the cgroup.

containers/crun#1217 is required for crun.

[NO NEW TESTS NEEDED] needs a new crun release

Closes: containers#18621

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
@giuseppe giuseppe force-pushed the set-mem-limit-cgroup-systemd branch from 4a24b6a to 950b4f6 Compare May 19, 2023 10:12
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
when updating the memory limit, we now set it also on the systemd
scope.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
@giuseppe giuseppe force-pushed the set-mem-limit-cgroup-systemd branch from 950b4f6 to 6494b69 Compare May 19, 2023 10:13
giuseppe added a commit to giuseppe/libpod that referenced this pull request May 19, 2023
b25b330 introduced this behaviour.

It was fine at the time because we didn't support "container update",
so the limit could not be changed at runtime.  Since it is not
possible to change the memory limit at runtime, read the limit as
reported from the cgroup.

containers/crun#1217 is required for crun.

[NO NEW TESTS NEEDED] needs a new crun release

Closes: containers#18621

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

rhatdan commented May 19, 2023

LGTM

@rhatdan
Copy link
Member

rhatdan commented May 19, 2023

@flouthoc PTAL

giuseppe added a commit to giuseppe/libpod that referenced this pull request May 19, 2023
b25b330 introduced this behaviour.

It was fine at the time because we didn't support "container update",
so the limit could not be changed at runtime.  Since it is not
possible to change the memory limit at runtime, read the limit as
reported from the cgroup.

containers/crun#1217 is required for crun.

Closes: containers#18621

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

@flouthoc flouthoc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@flouthoc flouthoc merged commit 23ba994 into containers:main May 19, 2023
24 checks passed
@rhatdan
Copy link
Member

rhatdan commented May 19, 2023

Time to cut a crun release?

giuseppe added a commit to giuseppe/libpod that referenced this pull request May 19, 2023
b25b330 introduced this behaviour.

It was fine at the time because we didn't support "container update",
so the limit could not be changed at runtime.  Since it is not
possible to change the memory limit at runtime, read the limit as
reported from the cgroup.

containers/crun#1217 is required for crun.

Closes: containers#18621

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
giuseppe added a commit to giuseppe/libpod that referenced this pull request May 19, 2023
b25b330 introduced this behaviour.

It was fine at the time because we didn't support "container update",
so the limit could not be changed at runtime.  Since it is not
possible to change the memory limit at runtime, read the limit as
reported from the cgroup.

containers/crun#1217 is required for crun.

Closes: containers#18621

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

Time to cut a crun release?

opened #1220

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.

None yet

3 participants