-
Notifications
You must be signed in to change notification settings - Fork 2.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
libct/cg/sd: fix SkipDevices for systemd #2958
Conversation
Commit 108ee85 adds SkipDevices flag, which is used by kubernetes to create cgroups for pods. Unfortunately the above commit falls short, and systemd DevicePolicy and DeviceAllow properties are still set, which requires kubernetes to set "allow everything" rule. This commit fixes this: if SkipDevices flag is set, we return Device* properties to allow all devices. Fixes: 108ee85 Signed-off-by: Kir Kolyshkin <[email protected]>
fb538d6
to
752e7a8
Compare
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.
LGTM.
Would it be possible to have some tests? |
I started working on something that mimics the kubelet behavior... |
99fde82
to
b9347d1
Compare
CentOS failure seems legit -- SELinux?
|
Once this is merged I will send out the v1.0.0 vote (it would be nice to do the release -- as @h-vetinari suggested -- on the 3rd of June since that's the 5-year anniversary of 1.0.0-rc1). |
The idea is to mimic what kubelet is doing, with minimum amount of code. First, create a slice with SkipDevices=true. It should have access to all devices. Next, create a scope within the above slice, allowing access to /dev/full only. Check that within that scope we can only access /dev/full and not other devices (such as /dev/null). Repeat the test with SkipDevices=false, make sure we can not access any devices (as they are disallowed by a parent cgroup). This is done only to assess the test correctness. NOTE that cgroup v1 and v2 behave differently for SkipDevices=false case, and thus the check is different. Cgroup v1 returns EPERM on writing to devices.allow, so cgroup manager's Set() fails, and we check for a particular error from m.Set(). Cgroup v2 allows to create a child cgroup, but denies access to any device (despite access being enabled) -- so we check the error from the shell script running in that cgroup. Again, this is only about SkipDevices=false case. Signed-off-by: Kir Kolyshkin <[email protected]>
b9347d1
to
0e16e7c
Compare
Test case fixed, PR description updated. |
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.
LGTM. Will send the v1.0.0 vote out today.
libct/cg/sd: fix SkipDevices for systemd
Commit 108ee85 (PR libct/cgroups: add SkipDevices to Resources #2490) adds
SkipDevices
flag, which is used by kubernetesto create cgroups for pods.
Unfortunately the above commit falls short, and systemd
DevicePolicy
andDeviceAllow
properties are still set, which requires kubernetes to set"allow everything" rule.
This commit fixes this: if
SkipDevices
flag is set, we returnDevice*
properties to allow all devices.libct/cg/sd: add SkipDevices unit test
The idea is to mimic what kubelet is doing, with minimum amount of code.
First, create a slice with
SkipDevices=true
. It should have access toall devices.
Next, create a scope within the above slice, allowing access to
/dev/full
only.
Check that within that scope we can only access
/dev/full
and not otherdevices (such as
/dev/null
).Repeat the test with
SkipDevices=false
, make sure we can not access anydevices (as they are disallowed by a parent cgroup). This is done only
to assess the test correctness.
NOTE that cgroup v1 and v2 behave differently for
SkipDevices=false
case, and thus the check is different. Cgroup v1 returns
EPERM
onwriting to
devices.allow
, so cgroup manager'sSet()
fails, and we checkfor a particular error from
m.Set()
. Cgroup v2 allows to create a childcgroup, but denies access to any device (despite access being enabled)
-- so we check the error from the shell script running in that cgroup.
Again, this is only about
SkipDevices=false
case.Previous discussions on topic: #2490 (comment), kubernetes/kubernetes#92862 (comment)
Fixes: 108ee85