V2 backports #7289
Merged
V2 backports #7289
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fix a bug in the error handling which returned nil instead of an error and ultimately lead to nil dereferences in the client. To prevent future regressions, add a test and check for the error message. Fixes: containers#7271 Signed-off-by: Valentin Rothberg <[email protected]>
Listing images has shown increasing performance penalties with an increasing number of images. Unless `--all` is specified, Podman will filter intermediate images. Determining intermediate images has been done by finding (and comparing!) parent images which is expensive. We had to query the storage many times which turned it into a bottleneck. Instead, create a layer tree and assign one or more images to nodes that match the images' top layer. Determining the children of an image is now exponentially faster as we already know the child images from the layer graph and the images using the same top layer, which may also be considered child images based on their history. On my system with 510 images, a rootful image list drops from 6 secs down to 0.3 secs. Also use the tree to compute parent nodes, and to filter intermediate images for pruning. Signed-off-by: Valentin Rothberg <[email protected]>
Signed-off-by: Paul Holzinger <[email protected]>
In local Podman, the frontend interprets the error and exit code given by the Exec API to determine the appropriate exit code to set for Podman itself; special cases like a missing executable receive special exit codes. Exec for the remote API, however, has to do this inside Libpod itself, as Libpod will be directly queried (via the Inspect API for exec sessions) to get the exit code. This was done correctly when the exec session started properly, but we did not properly handle cases where the OCI runtime fails before the exec session can properly start. Making two error returns that would otherwise not set exit code actually do so should resolve the issue. Fixes containers#6893 Signed-off-by: Matthew Heon <[email protected]>
when loading an image with podman-remote load, we need to send a name and a tag to the endpoint Fixes: containers#7124 Backported-by: Valentin Rothberg <[email protected]> Signed-off-by: Brent Baude <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vrothberg 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 |
openshift-ci-robot
added
the
approved
vrothberg
force-pushed
the
v2-backports
branch
from
2300e60 to
dc0ec24
Compare
|
@baude @giuseppe @jwhonce @mheon @QiWang19 @rhatdan @TomSweeneyRedHat @zhangguanzhang PTAL ... went through the log and picked up commits for 2.0 |
- fix the bud podman not using specified --directory as signature storage. - use manifest and image referce to set repo@digest. close containers#6994 close containers#6993 Backported-by: Valentin Rothberg <[email protected]> Signed-off-by: Qi Wang <[email protected]>
when there is a pause process running, let the "system service" podman instance join immediately the existing namespaces. Closes: containers#7180 Closes: containers#6660 Signed-off-by: Giuseppe Scrivano <[email protected]>
even though the official documentation suggests that application/x-tar should be used for tar files, it seems docker-compose uses application/tar. we now accept them and issue a warning. Fixes: containers#7185 Signed-off-by: Brent Baude <[email protected]>
the exists code was plagued by a missing return statement meant to trigger an early exit. Fixes: containers#7197 Signed-off-by: Brent Baude <[email protected]>
Docker and CNI have very different ideas of what 0.0.0.0 means.
Docker takes it to be 0.0.0.0/0 - that is, bind to every IPv4
address on the host. CNI (and, thus, root Podman) take it to mean
the literal IP 0.0.0.0. Instead, CNI interprets the empty string
("") as "bind to all IPs".
We could ask CNI to change, but given this is established
behavior, that's unlikely. Instead, let's just catch 0.0.0.0 and
turn it into "" when we parse ports.
Fixes containers#7014
Signed-off-by: Matthew Heon <[email protected]>
Signed-off-by: zhangguanzhang <[email protected]>
vrothberg
force-pushed
the
v2-backports
branch
from
dc0ec24 to
273e580
Compare
- Issue containers#6735 : problem with multiple namespaces; confirms combinations of --userns=keep-id, --privileged, --user=XX - Issue containers#6829 : --userns=keep-id will add a /etc/passwd entry - Issue containers#6593 : podman exec, with --userns=keep-id, errors (test is currently skipped because issue remains live) ...and, addendum: add new helper function, remove_same_dev_warning. Some CI systems issue a warning on podman run --privileged: WARNING: The same type, major and minor should not be used for multiple devices. We already had special-case code to ignore than in the SELinux test, but now we're seeing it in the new run tests I added, so I've refactored the "ignore this warning" code and written tests for the removal code. Signed-off-by: Ed Santiago <[email protected]>
vrothberg
force-pushed
the
v2-backports
branch
from
273e580 to
9d165b0
Compare
NOTE: the remote tests are not reenabled but the changes are applied. Future commits depend on some of the changes and having the commit applied will likely facilitate future backports as well. podman-remote is in better shape now. Let's see what needs to be done to reenable remote system tests. - logs test: skip multilog, it doesn't work remote - diff test: use -l only when local, not with remote - many other tests: skip_if_remote, with 'FIXME: pending #xxxx' where xxxx is a filed issue. Unrelated: added new helper to skip_if_remote and _if_rootless, where we check if the source message includes "remote"/"rootless" and insert it if missing. This is a minor usability enhancement to make it easier to understand at-a-glance why a skip triggers. Backported-by: Valentin Rothberg <[email protected]> Signed-off-by: Ed Santiago <[email protected]>
the go binding for remove container was using 'vols' for a key to remove volumes associated to the container. the correct key should be "v" and is documented as such. Fixes: containers#7128 Signed-off-by: Brent Baude <[email protected]>
We need to remove the container after it has exited for podman-remote run --rm commands. If we don't remove this container at this step, we open ourselves up to race conditions. Signed-off-by: Daniel J Walsh <[email protected]>
podman.service is socket activated through podman.socket. It should not have its own [Install] section, it does not make sense to systemctl enable podman.service. This leads to podman.service always running on a Debian system, as Debian's policy is to enable/start running services by default. We don't want a daemon :^) Fixes: containers#7190 Reported-by: @martinpitt Signed-off-by: Valentin Rothberg <[email protected]>
Backported-by: Valentin Rothberg <[email protected]> Signed-off-by: zhangguanzhang <[email protected]>
`/usr/lib/systemd/system` should only be used by the package manager administrators should use: `/etc/systemd/system` or `/usr/local/lib/systemd/system` see: man systemd.unit Signed-off-by: Paul Holzinger <[email protected]>
Fixes containers#7008 Signed-off-by: Jhon Honce <[email protected]>
Documentation and unit files call for a millisecond timeout while the code was using a second resolution. Code change is smaller given varlink has been deprecated. Signed-off-by: Jhon Honce <[email protected]>
When you execute podman create/run with the --pod new:<name> syntax the pod was created but the namespaces where not shared and therefore containers could not communicate over localhost. Add the default namespaces and pass the network options to the pod create options. Signed-off-by: Paul Holzinger <[email protected]>
Fix the closing of fds from --preserve-fds to avoid the operation on unrelated fds. Signed-off-by: Qi Wang <[email protected]>
vrothberg
force-pushed
the
v2-backports
branch
from
9d165b0 to
92d01d7
Compare
If I enter a continer with --userns keep-id, my UID will be present inside of the container, but most likely my user will not be defined. This patch will take information about the user and stick it into the container. Signed-off-by: Daniel J Walsh <[email protected]>
Bind-mounting /etc/passwd into the container is problematic becuase of how system utilities like `useradd` work. They want to make a copy and then rename to try to prevent breakage; this is, unfortunately, impossible when the file they want to rename is a bind mount. The current behavior is fine for read-only containers, though, because we expect useradd to fail in those cases. Instead of bind-mounting, we can edit /etc/passwd in the container's rootfs. This is kind of gross, because the change will show up in `podman diff` and similar tools, and will be included in images made by `podman commit`. However, it's a lot better than breaking important system tools. Fixes containers#6953 Signed-off-by: Matthew Heon <[email protected]>
Reversion of one part of containers#6679: my handling of 'realpath' would not work when $PODMAN is 'podman-remote --url etc'. Trying to handle that case got unmaintainable; so instead let's just force 'make {local,remote}system' to invoke with a full PODMAN path. This breaks down if someone runs the tests with a manual 'bats' invocation, but I think I'm the only one who ever does that. Since podman path will now be very long in the logs, add code to logformatter to abbreviate it like we do for the ginkgo logs. And, one thing that has bugged me for a long time: in the error logs, show a different prompt ('#' vs '$') to distinguish root vs rootless. This should make it much easier to see at-a-glance whether a log file is root or not. Add tests for it. Signed-off-by: Ed Santiago <[email protected]>
|
I'm a little iffy about the userns keep-id makes changes to |
@rhatdan WDYT? |
|
I'm OK with it. |
|
I'd lean towards not including the user-id that @mheon questioned too. However, I'm OK if it goes in if that's the decision. Otherwise: |
|
I also wasn't 100 percent sure about the passwd nature but as #6953 was marked as a bug, I figure to pull that in. Maybe it was a bug by an earlier feature commit though. |
|
/lgtm I am fine with adding them. |
github-actions
bot
added
the
locked - please file new issue/PR
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport a number of fixes:
TODO: