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

fix data inconsistent when runc update in systemd driven cgroup #2343

Merged
merged 3 commits into from
Apr 25, 2020

Conversation

lifubang
Copy link
Member

fix #2287
runc --systemd-cgroup update does not update systemd scope

As pointed out by @kolyshkin The cause of this is Set() method of systemd.UnifiedManager, which only uses fsManager to set the updated values.
So when we use systemd driven cgroup v2, we should use SetUnitProperties method to update container resource constraints.

Signed-off-by: lifubang [email protected]

@lifubang
Copy link
Member Author

If it makes sense, the same changes can be added to cgroup v1.

}
return fsMgr.Set(container)
Copy link
Contributor

Choose a reason for hiding this comment

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

some resources are not controlled by systemd so we still need to call this I'm afraid.

Copy link
Contributor

Choose a reason for hiding this comment

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

and this is why CI is failing (systemd does not control cpuset)

Copy link
Member Author

Choose a reason for hiding this comment

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

Force pushed now.
I don't know whether we should change to only write cpuset cgroup files or not?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the best is to keep it like it is now (with writing all of them), and then we can look at it in another PR/issue. This also applies to other resources not set via systemd like hugetlb.

More info about it here: https://systemd.io/CGROUP_DELEGATION/

Copy link
Contributor

@odinuge odinuge left a comment

Choose a reason for hiding this comment

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

Thanks for doing this @lifubang! This change looks good to me, but as you say it also applies to cgroup v1, so it would be nice if you could fix that at the same time!

Issue depending on this in kubernetes: kubernetes/kubernetes#88197

@lifubang
Copy link
Member Author

but as you say it also applies to cgroup v1, so it would be nice if you could fix that at the same time!

OK, I'll fix and test it in cgroup v1.

@lifubang lifubang changed the title fix data inconsistent when runc update in systemd driven cgroup v2 fix data inconsistent when runc update in systemd driven cgroup Apr 23, 2020
@kolyshkin
Copy link
Contributor

kolyshkin commented Apr 24, 2020

LGTM

Approved with PullApprove

@kolyshkin
Copy link
Contributor

@lifubang maybe we can add some simple tests, based on what I described in #2287.

@lifubang
Copy link
Member Author

maybe we can add some simple tests

OK, I'll do it later.

@lifubang
Copy link
Member Author

Three tests has been added.

@AkihiroSuda
Copy link
Member

AkihiroSuda commented Apr 24, 2020

LGTM

Approved with PullApprove

@kolyshkin
Copy link
Contributor

kolyshkin commented Apr 25, 2020

LGTM

Approved with PullApprove

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

Successfully merging this pull request may close these issues.

runc --systemd-cgroup update does not update systemd scope
4 participants