Skip to content

fs2: fix cgroup.subtree_control EPERM on rootless + add CI#2340

Merged
mrunalp merged 1 commit intoopencontainers:masterfrom
AkihiroSuda:fix-2339
Apr 28, 2020
Merged

fs2: fix cgroup.subtree_control EPERM on rootless + add CI#2340
mrunalp merged 1 commit intoopencontainers:masterfrom
AkihiroSuda:fix-2339

Conversation

@AkihiroSuda
Copy link
Member

@AkihiroSuda AkihiroSuda commented Apr 21, 2020

Fix #2339 (regression in 813cb3e)

@AkihiroSuda AkihiroSuda mentioned this pull request Apr 21, 2020
Closed
kolyshkin
kolyshkin approved these changes Apr 21, 2020
View reviewed changes
Copy link
Contributor

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

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

LGTM

@kolyshkin
Copy link
Contributor

kolyshkin commented Apr 21, 2020 •
edited
Loading

Can we add a test case for this (apparently we don't have one or it is disabled, since #2299 CI was green)?

@AkihiroSuda
Copy link
Member Author

AkihiroSuda commented Apr 21, 2020

I will open separate PR for CI

@AkihiroSuda AkihiroSuda changed the title fs2: ignore cgroup.subtree_control EPERM when rootless fs2: ignore cgroup.subtree_control EPERM when rootless + add CI Apr 21, 2020
@AkihiroSuda
Copy link
Member Author

AkihiroSuda commented Apr 21, 2020

added CI

@kolyshkin kolyshkin mentioned this pull request Apr 21, 2020
Closed
kolyshkin
kolyshkin reviewed Apr 21, 2020
View reviewed changes
kolyshkin
kolyshkin reviewed Apr 21, 2020
View reviewed changes
kolyshkin
kolyshkin requested changes Apr 21, 2020
View reviewed changes
Copy link
Contributor

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

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

left a few comments

kolyshkin
kolyshkin reviewed Apr 21, 2020
View reviewed changes
@AkihiroSuda AkihiroSuda changed the title fs2: ignore cgroup.subtree_control EPERM when rootless + add CI fs2: fix cgroup.subtree_control EPERM on rootless + add CI Apr 21, 2020
@AkihiroSuda AkihiroSuda force-pushed the fix-2339 branch 2 times, most recently from 26d44d9 to 473a18d Compare April 21, 2020 22:27
@AkihiroSuda AkihiroSuda mentioned this pull request Apr 21, 2020
Merged
AkihiroSuda
AkihiroSuda commented Apr 21, 2020
View reviewed changes
kolyshkin
kolyshkin reviewed Apr 21, 2020
View reviewed changes
kolyshkin
kolyshkin reviewed Apr 21, 2020
View reviewed changes
@AkihiroSuda AkihiroSuda force-pushed the fix-2339 branch 4 times, most recently from 5e157cd to ce64430 Compare April 21, 2020 23:59
@AkihiroSuda AkihiroSuda force-pushed the fix-2339 branch 4 times, most recently from 9867192 to 753857c Compare April 22, 2020 00:58
kolyshkin
kolyshkin reviewed Apr 22, 2020
View reviewed changes
kolyshkin
kolyshkin reviewed Apr 22, 2020
View reviewed changes
kolyshkin
kolyshkin reviewed Apr 22, 2020
View reviewed changes
kolyshkin
kolyshkin reviewed Apr 22, 2020
View reviewed changes
@AkihiroSuda
Copy link
Member Author

AkihiroSuda commented Apr 22, 2020

Weird failure, fails only with idmap mode.

[03] run rootless tests ... (idmap)
path: /usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
1..78
ok 1 runc update --kernel-memory{,-tcp} (initialized) # skip test requires rootless_cgroup
ok 2 runc update --kernel-memory (uninitialized) # skip test requires rootless_cgroup
ok 3 runc create (no limits + no cgrouppath + no permission) succeeds
not ok 4 runc create (rootless + no limits + cgrouppath + no permission) fails with permission error
# (in test file tests/integration/cgroups.bats, line 86)
#   `[ "$status" -eq 1 ]' failed
...

@AkihiroSuda AkihiroSuda force-pushed the fix-2339 branch 4 times, most recently from cbf350e to 603bd8c Compare April 24, 2020 07:36
@AkihiroSuda
Copy link
Member Author

AkihiroSuda commented Apr 24, 2020

CI green. Ready for review.

@kolyshkin
Copy link
Contributor

kolyshkin commented Apr 24, 2020

Needs a rebase

@AkihiroSuda
Copy link
Member Author

AkihiroSuda commented Apr 24, 2020

rebased

@AkihiroSuda
Copy link
Member Author

AkihiroSuda commented Apr 26, 2020

@opencontainers/runc-maintainers PTAL?

I'd like to get almost all cgroup v2 issues to be resolved before Docker 20.0X freeze (May 1) and Podman 2.0

@AkihiroSuda
fs2: fix cgroup.subtree_control EPERM on rootless + add CI
60c647e 
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
kolyshkin
kolyshkin reviewed Apr 27, 2020
View reviewed changes
kolyshkin
kolyshkin requested changes Apr 27, 2020
View reviewed changes
Copy link
Contributor

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

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

Do we need to test rootless+systemd? Otherwise LGTM

@AkihiroSuda
Copy link
Member Author

AkihiroSuda commented Apr 27, 2020

@kolyshkin
No, the systemd PR is not merged yet: #2281

Let's merge this PR now so that we can rebase #2281 on top of this and add test for that

kolyshkin
kolyshkin approved these changes Apr 28, 2020
View reviewed changes
Copy link
Contributor

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

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

LGTM

@kolyshkin
Copy link
Contributor

kolyshkin commented Apr 28, 2020 •
edited by AkihiroSuda
Loading

LGTM

Approved with PullApprove

@kolyshkin
Copy link
Contributor

kolyshkin commented Apr 28, 2020

@mrunalp PTAL

@mrunalp
Copy link
Contributor

mrunalp commented Apr 28, 2020 •
edited by AkihiroSuda
Loading

LGTM

Approved with PullApprove

@mrunalp mrunalp merged commit a1f007e into opencontainers:master Apr 28, 2020
@cyphar cyphar mentioned this pull request May 8, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kolyshkin kolyshkin kolyshkin approved these changes

Assignees

No one assigned

Labels

area/cgroupv2 area/ci area/rootless kind/bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

cgroup2: \"open /sys/fs/cgroup/cgroup.subtree_control: permission denied\"": unknown.

3 participants

@AkihiroSuda @kolyshkin @mrunalp