Skip to content

Conversation

nalind
Copy link
Member

@nalind nalind commented Aug 14, 2025

What type of PR is this?

/kind failing-test

What this PR does / why we need it:

Add a CI task that tests inside of an unprivileged container. We previously ran our only inside-of-a-container task inside of a privileged container.

How to verify it

New CI task!

Which issue(s) this PR fixes:

None

Special notes for your reviewer:

We might be missing some things in CI, particularly #6331.

Does this PR introduce a user-facing change?

None

@openshift-ci openshift-ci bot added the kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. label Aug 14, 2025
Copy link
Contributor

openshift-ci bot commented Aug 14, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nalind

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@nalind nalind force-pushed the unprivileged-containers-ci branch from 3acf4c1 to 97f9572 Compare August 14, 2025 21:23
Copy link

Ephemeral COPR build failed. @containers/packit-build please check.

@nalind nalind force-pushed the unprivileged-containers-ci branch 4 times, most recently from c205291 to 9f85e1e Compare August 15, 2025 18:29
@nalind nalind force-pushed the unprivileged-containers-ci branch 19 times, most recently from 4d7249b to a0c479e Compare August 20, 2025 20:54
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 20, 2025
@nalind nalind force-pushed the unprivileged-containers-ci branch from a0c479e to 2b36e6e Compare August 20, 2025 20:55
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 20, 2025
@nalind nalind force-pushed the unprivileged-containers-ci branch 7 times, most recently from 8aa977b to ab69fe1 Compare August 27, 2025 20:02
nalind added 15 commits August 27, 2025 16:02
Add a missing space between "[" and its first argument.

Signed-off-by: Nalin Dahyabhai <[email protected]>
Signed-off-by: Nalin Dahyabhai <[email protected]>
Add -authfile and -tls-verify flags to imgtype so that it can
authenticate to registries when we call it in tests.

Signed-off-by: Nalin Dahyabhai <[email protected]>
Add --authfile, --preserve-digests, and --tls-verify flags to copy so
that it can authenticate to registries when we call it in tests.

Signed-off-by: Nalin Dahyabhai <[email protected]>
Use imgtype, instead of running skopeo in a container, to do a
spot-check on an image that we've just pushed to a temporary registry.

Signed-off-by: Nalin Dahyabhai <[email protected]>
Use copy, instead of running skopeo in a container, to copy an image to
a local directory.

Signed-off-by: Nalin Dahyabhai <[email protected]>
Whenever we use podman or buildah to run something in the host network
namespace, use the host cgroup namespace, too.

Signed-off-by: Nalin Dahyabhai <[email protected]>
The VM stands up a registry on port 60333 that holds copies of images
that we pre-identify as being used by tests, and our
registries-cached.conf test file points to using it as the primary
source for those images.
The CI environment starts another registry on port 5000.  Update so that
clients don't have to use --tls-verify=false to talk to it.

Signed-off-by: Nalin Dahyabhai <[email protected]>
Signed-off-by: Nalin Dahyabhai <[email protected]>
Replace a bind-mounted /dev directory with a tmpfs /dev directory with
bind-mounted device nodes.

Signed-off-by: Nalin Dahyabhai <[email protected]>
If we try to mknod() a device that was specified in the runtime spec,
and we get a permissions error, look for a corresponding node under
/sys/dev that we can bind mount instead.  The ownership and permissions
might still be wrong, but at least we tried.

In practice, buildah's Run() logic will do this for us, but we still do
it here for the sake of unit tests and other possible callers.

Signed-off-by: Nalin Dahyabhai <[email protected]>
Check that device nodes we specify show up in the new mount namespace.

Signed-off-by: Nalin Dahyabhai <[email protected]>
When handling bind mounts, use the nodev/noexec/nosuid/ro or not state
of the source location as the defaults for the flags that we request
when creating the new bind mount, subject to requests to change them
passed in as options.

Recognize more recursive versions of known flags, and recognize the
sync/async and atime/diratime/lazytime/relatime/strictatime mount
options, too.

Stop passing "bind" in as a filesystem name when using unix.MS_BIND,
since it's ignored.

Stop passing a source location in when using unix.MS_REMOUNT, since it's
ignored.

Set mount propagation after creating a bind/tmpfs/overlay mount as a
distinct step before messing with other mount flags.

Use the makeReadOnly() helper function when remounting locations
specifically to make them read-only, and make sure it doesn't pass in
mount propagation flags when it's trying to change mount flags.

Make sure we always pass SELinux-specific mount options to unix.Mount().

Signed-off-by: Nalin Dahyabhai <[email protected]>
Teach the "chroot mount flags" test that it might not have the full
32-bit range of IDs to pass down when creating a new user namespace, and
to not try to mount /proc because SELinux policy might not allow it to,
as happens when it starts as UID 0 without cap_sys_admin and creates
only a new user and mount namespace, but doesn't chroot.

Also check bind-mounting from locations with assorted sync and
atime-related flags applied.

Signed-off-by: Nalin Dahyabhai <[email protected]>
@nalind nalind force-pushed the unprivileged-containers-ci branch from ab69fe1 to 5e9d085 Compare August 27, 2025 20:05
Add unprivileged versions of the test-in-a-container task that help
ensure that we're not missing expected functionality.  Test with both
overlay and regular vfs when unprivileged, and test with chroot
isolation.

Signed-off-by: Nalin Dahyabhai <[email protected]>
@nalind nalind force-pushed the unprivileged-containers-ci branch from 5e9d085 to f8510d9 Compare August 27, 2025 20:11
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 30, 2025
@openshift-merge-robot
Copy link
Collaborator

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

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.

CI is failing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved do-not-merge/work-in-progress kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants