Have one function without a `defer lock.unlock()` as one of the
commands in it calls a function that also takes the same lock,
so the unlock has to happen prior to function completion.
Unfortunately, this is prone to errors, like the one here: I
missed a case, and we could return without unlocking, causing a
deadlock later in the cleanup code as we tried to take the same
lock again.
Refactor the command to use `defer unlock()` to simplify and
avoid any further errors of this type.
Introduced by e66b788a514fb8df2c8b8d3c000e0d543bbd60df - this
should be included in any backports of that commit.
Fixes#25585
Signed-off-by: Matt Heon <mheon@redhat.com>
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
(cherry picked from commit c9c44d400c2870e9a6c966647be1c414dc773b66)
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
unify the error codes returned by runc and crun.
Fix the tests to work with both runtimes, as well as the
https://github.com/containers/crun/pull/1672 changes in progress for
crun.
Follow-up for https://github.com/containers/podman/pull/25340
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
(cherry picked from commit 4695564730abf8432102f8a07546afc9f87f855b)
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Needed-by: https://github.com/containers/crun/pull/1672
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
(cherry picked from commit c65bb903b63c60a1ef2ccd3c21e118c4784d2f6b)
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
GoLang sets unset values to the default value of the type. This means that the destination of the log is an empty string and the count and size are set to 0. However, this means that size and count are unbounded, and this is not the default behavior.
Fixes: https://github.com/containers/podman/issues/25473
Fixes: https://issues.redhat.com/browse/RHEL-83262
Signed-off-by: Jan Rodák <hony.com@seznam.cz>
There are multiple concurrent goroutinces which produce result and they
race agains each other, while producing different results.
This commit addresses at least a part of the problem - producing
different results for competing "sources".
Fixes: #25479
Signed-off-by: Yuri Timenkov <yuri@timenkov.pro>
When waiting for container to be not-running, sometimes wait retuns code
-1 with an empty error instead of actual exit code.
It turned out that syncContainer returns ErrCtrRemoved for a removed
container instead of ErrNoSuchCtr, while data can still be pulled from
the database.
This fixes the issue by taking into account both codes.
Fixes: #25479
Signed-off-by: Yuri Timenkov <yuri@timenkov.pro>
This resolves an ordering issue that prevented quotas from being
applied. XFS quotas are applied recursively, but only for
subdirectories created after the quota is applied; if we create
`_data` before the quota, and then use `_data` for all data in
the volume, the quota will never be used by the volume.
Also, add a test that volume quotas are working as designed using
an XFS formatted loop device in the system tests. This should
prevent any further regressions on basic quota functionality,
such as quotas being shared between volumes.
Fixes#25368
Fixes https://issues.redhat.com/browse/RHEL-82198
Fixes https://issues.redhat.com/browse/RHEL-82199
Signed-off-by: Matt Heon <mheon@redhat.com>
This seems to have been added as part of the cleanup of our
handling of OOM files, but code was never added to remove it, so
we leaked a single directory with an exit file and OOM file per
container run. Apparently have been doing this for a while - I'd
guess since March of '23 - so I'm surprised more people didn't
notice.
Fixes#25291
Signed-off-by: Matt Heon <mheon@redhat.com>
As part of our database init, we perform a check of the current
values for a few fields (graph driver, graph root, static dir,
and a few more) to validate that Libpod is being started with a
sane & sensible config, and the user's containers can actually be
expected to work. Basically, we take the current runtime config
and compare against values cached in the database from the first
time Podman was run.
We've had some issues with this logic before this year around
symlink resolution, but this is a new edge case. Somehow, the
database is being loaded with the empty string for some fields
(at least graph driver) which is causing comparisons to fail
because we will never compare against "" for those fields - we
insert the default value instead, assuming we have one.
Having a value of "" in the database largely invalidates the
check so arguably we could just drop it, but what BoltDB did -
and what SQLite does after this patch - is to use the default
value for comparison instead of "". This should still catch some
edge cases, and shouldn't be too harmful.
What this does not do is identify or solve the reason that we are
seeing the empty string in the database at all. From my read on
the logic, it must mean that the graph driver is explicitly set
to "" in the c/storage config at the time Podman is first run and
I'm not precisely sure how that happens.
Fixes#24738
Signed-off-by: Matt Heon <mheon@redhat.com>
BuildOrigin is a field that can be set at build time by packagers. This helps us trace how and where the binary was built and installed from, allowing us to see if the issue is due to a specfic installation or a general podman bug. This field shows up in podman version and in podman info when populated. Note that podman info has a new field, Client, that only appears when running podman info using the remote client.
Automatically set the BuildOrigin field when building the macOS pkginstaller to pkginstaller.
Usage: make podman-remote BUILD_ORIGIN="mypackaging"
Signed-off-by: Ashley Cui <acui@redhat.com>
podman exec support detaching early via the detach key sequence. In that
case the podman process should exit successfully but the container exec
process keeps running.
Now I wrote automated test for both podman run and exec detach but this
uncovered several larger issues:
- detach sequence parsing is broken[1]
- podman-remote exec detach is broken[2]
- detach in general seems to be buggy/racy, seeing lot of flakes that
fail to restore the terminal and get an EIO instead, i.e.
"Unable to restore terminal: input/output error"
Thus I cannot add tests for now but this commit should at least fix the
obvoius case as reported by the user so I like to get this in regardless
and I will work through the other issues once I have more time.
Fixes#24895
[1] https://github.com/containers/common/pull/2302
[2] https://github.com/containers/podman/issues/25089
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
The `podman system prune` command is able to remove build containers that were created during the build, but were not removed because the build terminated unexpectedly.
By default, build containers are not removed to prevent interference with builds in progress. Use the **--build** flag when running the command to remove build containers as well.
Fixes: https://issues.redhat.com/browse/RHEL-62009
Signed-off-by: Jan Rodák <hony.com@seznam.cz>
This did not have a JSON tag prior to being added by #25008. By
adding one we risk a breaking change in the DB (particularly
given the change in case - useImageHosts vs UseImageHosts) which
we should try to avoid.
Remove the tag given this.
Signed-off-by: Matt Heon <mheon@redhat.com>
Fixes: https://github.com/containers/podman/issues/25002
Also add the ability to inspect containers for
UseImageHosts and UseImageHostname.
Finally fixed some bugs in handling of --no-hosts for Pods,
which I descovered.
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
Fix new issues found by usetesting, mainly we should use t.TempDir() in
test which makes the code better as this will be removed on test end
automatically so no need for defer or any error checking.
Also fix issues reported by exptostd, these mainly show where we can
switch the imports to the std maps/slices packages instead of the
golang.org/x/exp/... packages.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Passing the hostname allows netavark to include it in DHCP lease
requests which, in an environment where DDNS is used, can cause
DNS entries to be created automatically.
* The current Hostname() function in container.go was updated to
check the new `container_name_as_hostname` option in the
CONTAINERS table of containers.conf. If set and no hostname
was configured for the container, it causes the hostname to be
set to a version of the container's name with the characters not
valid for a hostname removed. If not set (the default), the original
behavior of setting the hostname to the short container ID is
preserved.
* Because the Hostname() function can return the host's hostname
if the container isn't running in a private UTS namespace, and we'd
NEVER want to send _that_ in a DHCP request for a container, a new
function NetworkHostname() was added which functions like Hostname()
except that it will return an empty string instead of the host's
hostname if the container is not running in a private UTS namespace.
* networking_common.getNetworkOptions() now uses NetworkHostname()
to set the ContainerHostname member of the NetworkOptions structure.
That member was added to the structure in a corresponding commit in
common/libnetwork/types/network.go.
* Added test to containers_conf_test.go
Signed-off-by: George Joseph <g.devel@wxy78.net>
with defaults values when changes configuration with podman update.
The new LinuxResource structure does not represent the current unchanged configuration, which was not affected by the change.
Fixes: https://issues.redhat.com/browse/RUN-2375
Signed-off-by: Jan Rodák <hony.com@seznam.cz>
The command `podman info` returned only one imagestore in
`store.graphOptions.<driver>.imagestore` even if multiple
image stores were configured.
This change replaces the field `<driver>.imagestore` with
the field `<driver>.additionalImageStores`, that instead
of a string is an array of strings and that includes all
the configured additional image stores.
Fix https://github.com/containers/storage/issues/2094
Signed-off-by: Mario Loriedo <mario.loriedo@gmail.com>
Move error checking of possible null returned value before
its dereference in importBuilder.Format
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Tigran Sogomonian <tsogomonian@astralinux.ru>
commit 5ebba75dbd4462da47283b3f018804b7361d52bf implemented this
behaviour for rootless users and later commit
0a69aefa41d55d2aa30333d6a4ce76b178d1ed5b changed it when in a user
namespace, but the same limitation exists for root without
CAP_SYS_RESOURCE. Change the check to use the clamp to the current
values if running without CAP_SYS_RESOURCE.
Closes: https://github.com/containers/podman/issues/24692
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
This solves several problems with copying into volumes on a
container that is not running.
The first, and most obvious, is that we were previously entirely
unable to copy into a volume that required mounting - like
image volumes, volume plugins, and volumes that specified mount
options.
The second is that this fixed several permissions and content
issues with a fresh volume and a container that has not been run
before. A copy-up will not have occurred, so permissions on the
volume root will not have been set and content will not have been
copied into the volume.
If the container is running, this is very low cost - we maintain
a mount counter for named volumes, so it's just an increment in
the DB if the volume actually needs mounting, and a no-op if it
doesn't.
Unfortunately, we also have to fix permissions, and that is
rather more complicated. This involves an ugly set of manual
edits to the volume state to ensure that the permissions fixes
actually worked, as the code was never meant to be used in this
way. It's really ugly, but necessary to reach full Docker
compatibility.
Fixes#24405
Signed-off-by: Matthew Heon <matthew.heon@pm.me>
This reverts commit 5de7b7c3f379987faa9cb06a58be8914f2d56cbe.
We now require the Unregister shutdown handler function for
handling unmounting named volumes after `podman cp` into a
stopped container.
Signed-off-by: Matt Heon <mheon@redhat.com>
1. Completed the EventerType comment.
2. Changed EventerType to be represented as a string.
3. Since EventerType is designed to be entirely lowercase, changed the comparison to use lowercase instead of uppercase.
4. Renamed newEventJournalD to newJournalDEventer.
5. Removed redundant error-checking steps in events_linux.go.
Signed-off-by: ksw2000 <13825170+ksw2000@users.noreply.github.com>
* Add --hosts-file flag to container create, container run and pod create
* Add HostsFile field to pod inspect and container inspect results
* Test BaseHostsFile config in containers.conf
Signed-off-by: Gavin Lam <gavin.oss@tutamail.com>
New flags in a `podman update` can change the configuration of HealthCheck when the container is started, without having to restart or recreate the container.
This can help determine why a given container suddenly started failing HealthCheck without interfering with the services it provides. For example, reconfigure HealthCheck to keep logs longer than the usual last X results, store logs to other destinations, etc.
Fixes: https://issues.redhat.com/browse/RHEL-60561
Signed-off-by: Jan Rodák <hony.com@seznam.cz>
In theory RootlessNetnsInfo() should never return nil here. However that
was actually only true when the rootless netns was set up before and
wrote the right cache file with the ip addresses.
Given this cache file is a new feature just added in 5.3 if you updated
from 5.2 or earlier the file will not exists thus cause failures for all
following started containers.
The fix for this is to stop all containers and make sure the
rootless-netns was removed so the next start creates it new with the
proper 5.3 cache file. However as there is no way to rely on users doing
that and it is also not requirement so simply handle the nil deref here.
The only way to test this would be to run the old version then the new
version which we cannot really do in CI. We do have upgrade test for
that but they are root only and likely need a lot more work to get them
going rootless but certainly worth to explore to prevent such problems
in the future.
Fixes: a1e6603133 ("libpod: make use of new pasta option from c/common")
Fixes: #24566
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
commit 5ebba75dbd4462da47283b3f018804b7361d52bf implemented this
behaviour for rootless users, but the same limitation exists for any
user in a user namespace. Change the check to use the clamp to the
current values anytime podman runs in a user namespace.
Closes: https://github.com/containers/podman/issues/24508
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
All the backend work was done a while back for image volumes, so
this is effectively just plumbing the option in for volumes in
the parser logic. We do need to change the return type of the
volume parser as it only worked on spec.Mount before (which does
not have subpath support, so we'd have to pass it as an option
and parse it again) but that is cleaner than the alternative.
Fixes#20661
Signed-off-by: Matt Heon <mheon@redhat.com>
Add support for inspecting Mounts which include SubPaths.
Handle SubPaths for kubernetes image volumes.
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
