-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Remove some SkipIfRootless flags from tests #7760
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
LGTM |
6c99c57
to
bdf59c1
Compare
We need to get more tests running in rootless mode. Since cgroupsV2 allows management of cgroups in rootless environments a lot of more tests can be run. Signed-off-by: Daniel J Walsh <[email protected]>
@edsantiago PTAL turns on a few more tests, we need to get the same fix for skipifrootless() that you did for skipifremote() @containers/podman-maintainers PTAL and merge. |
session := podmanTest.Podman([]string{"run", "--rm", ALPINE, "id"}) | ||
session.WaitWithDefaultTimeout() | ||
Expect(session.ExitCode()).To(Equal(0)) | ||
Expect(session.OutputToString()).To(Equal("uid=0(root) gid=0(root) groups=0(root),1(bin),2(daemon),3(sys),4(adm),6(disk),10(wheel),11(floppy),20(dialout),26(tape),27(video)")) |
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.
This seems a lot less specific
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.
Yes, they are very different in rootless versus rootful, but I do not know why. But the current test is fragile since the ALPINE image can change in the future. We just want to make sure that the container is running as root.
But you could very well be pointing out a rootless bug.
$ podman (pullpolicy) $ sudo podman run --rm alpine id
uid=0(root) gid=0(root) groups=0(root),1(bin),2(daemon),3(sys),4(adm),6(disk),10(wheel),11(floppy),20(dialout),26(tape),27(video)
$ podman (pullpolicy) $ podman run --rm alpine id
uid=0(root) gid=0(root)
I still think we should get this in, and I will open an issue above.
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.
the ALPINE image can change in the future
I am establishing what I hope is a robust convention in which quay.io/libpod/testimage:YYYYMMDD
is an alpine-based image, entirely under our control, to avoid precisely this sort of problem. See https://github.com/containers/podman/blob/master/test/system/build-testimage
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.
Opened an issue on this one, I believe we should merge this for and fix this specific issue later.
#7782
/lgtm |
We need to get more tests running in rootless mode. Since cgroupsV2 allows
management of cgroups in rootless environments a lot of more tests can be run.
Signed-off-by: Daniel J Walsh [email protected]