- fix a few missing safenames
- eliminate 'container rm -a'
- when running ps, do substring match, not exact
- where possible, add ci:parallel tags
- when not possible, explain
Also, fix a completely broken inspect test
Signed-off-by: Ed Santiago <santiago@redhat.com>
This started off as an attempt to make `podman stop` on a
container started with `--rm` actually remove the container,
instead of just cleaning it up and waiting for the cleanup
process to finish the removal.
In the process, I realized that `podman run --rmi` was rather
broken. It was only done as part of the Podman CLI, not the
cleanup process (meaning it only worked with attached containers)
and the way it was wired meant that I was fairly confident that
it wouldn't work if I did a `podman stop` on an attached
container run with `--rmi`. I rewired it to use the same
mechanism that `podman run --rm` uses, so it should be a lot more
durable now, and I also wired it into `podman inspect` so you can
tell that a container will remove its image.
Tests have been added for the changes to `podman run --rmi`. No
tests for `stop` on a `run --rm` container as that would be racy.
Fixes#22852
Fixes RHEL-39513
Signed-off-by: Matt Heon <mheon@redhat.com>
The current code did several complicated state checks that simply do not
work properly on a fast restarting container. It uses a special case for
--restart=always but forgot to take care of --restart=on-failure which
always hang for 20s until it run into the timeout.
The old logic also used to call CheckConmonRunning() but synced the
state before which means it may check a new conmon every time and thus
misses exits.
To fix the new the code is much simpler. Check the conmon pid, if it is
no longer running then get then check exit file and get exit code.
This is related to #23473 but I am not sure if this fixes it because we
cannot reproduce.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
if idmap is specified for a volume, reverse the mappings when copying
up from the container, so that the original permissions are maintained.
Closes: https://github.com/containers/podman/issues/23467
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Many system tests use hardcoded names for containers, images,
and everything. This has worked because system tests run
serially. It will not work if we ever run in parallel.
Create a new safename() helper, and use it as follows:
myctr=c_$(safename)
myvol1=v1_$(safename)
...
Find current instances of hardcoded names, and replace
with safe ones.
Whether or not we ever end up parallelizing system tests,
this is simply good practice.
There are far too many instances to fix in one (reviewable) PR.
This is commit 1 of N.
Signed-off-by: Ed Santiago <santiago@redhat.com>
- fix test name to reflect that it's not pasta-only
(followup from #21563)
- in one podman-update test run in OpenQA, defer assertion
failures so we can gather better data on regressions.
This would've been helpful in diagnosing bz2281805.
- add an error-message check to one test that needed it
(found by accident)
- add distro-integration test tag to a handful of new tests,
so they run in OpenQA. Found via 'git diff 33891e8 test/system'
and scanning for '^\+@test '. I only added tests that IMO
have some risk of interacting poorly with kernel or systemd
updates, e.g. quadlet, modules, tmpfs+noswap.
Signed-off-by: Ed Santiago <santiago@redhat.com>
Remove leaking containers and remove unessesary push/pull args. For push
it tries to push an image as argument which makes no sense and for pull
we try to pull argument as image which is also wrong.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
We don't have two loop twice for the stat call we can just stat both
dirs at once. This means we only have to create half of the containers
so the test is twice as fast.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Instead of iterating over all tmp dirs and creating test containers for
each one we can just pass all files to one touch call. With that we have
to create much less containers while still checking the same thing. This
speeds up the test by about 4 seconds.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
There is no reason for this check to wait 4 seconds for the container to
run, instead make sure to have a running process and then stop it
directly with -t0 not have any delay.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
There is really no point in waiting 10s for the kill, let's use 2 this
should be good enough to observe the timing.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
The scenario for inducing this is as follows:
1. Start a container with a long stop timeout and a PID1 that
ignores SIGTERM
2. Use `podman stop` to stop that container
3. Simultaneously, in another terminal, kill -9 `pidof podman`
(the container is now in ContainerStateStopping)
4. Now kill that container's Conmon with SIGKILL.
5. No commands are able to move the container from Stopping to
Stopped now.
The cause is a logic bug in our exit-file handling logic. Conmon
being dead without an exit file causes no change to the state.
Add handling for this case that tries to clean up, including
stopping the container if it still seems to be running.
Fixes#19629
Signed-off-by: Matt Heon <mheon@redhat.com>
The change in healthcheck_run_test.go, depends on the
containers/image change:
commit b6afa8ca7b324aca8fd5a7b5b206fc05c0c04874
Author: Mikhail Sokolov <msokolov@evolution.com>
Date: Fri Mar 15 13:37:44 2024 +0200
Add support for Docker HealthConfig.StartInterval (v25.0.0+)
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Two system tests were relying on $SYSTEMD_IMAGE but were not
running _prefetch. This led to baffling flakes that wasted
my time. (Quay flakes, of course. New manifestation.)
Signed-off-by: Ed Santiago <santiago@redhat.com>
if the volume is mounted with "idmap", there should not be any mapping
using the user namespace mappings since this is done at runtime using
the "idmap" kernel feature.
Closes: https://github.com/containers/podman/issues/22228
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Currently if a user specifies a negative time to stop a container the
code ends up specifying the negative time to time.Duration which treats
it as 0. By settine the default to max.Unint32 we end up with a positive
number which indicates > 68 years which is probably close enough to
infinity for our use case.
Fixes: https://github.com/containers/podman/issues/21811
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
Given that we can have multiple image digests,
fix the inspect test to check whether the digest
given matches one of the digests of the image.
Signed-off-by: Urvashi Mohnani <umohnani@redhat.com>
Some OCI runtimes (cf. [1]) may tolerate container images that don't
specify an entrypoint even if no entrypoint is given on the command
line. In those cases, it's annoying for the user to have to pass a ""
argument to podman.
If no entrypoint is given, make the behavior the same as if an empty ""
entrypoint was given.
[1] https://github.com/containers/crun-vm
Signed-off-by: Alberto Faria <afaria@redhat.com>
Currently, if the container creation failed with
either run or create and you've used --pod with new:
the pod would be created nonetheless. This change ensures
the pod just created is also cleaned up in case
of container creation failure
Fixes#21228
Signed-off-by: danishprakash <danish.prakash@suse.com>
add a new option --preserve-fd that allows to specify a list of FDs to
pass down to the container.
It is similar to --preserve-fds but it allows to specify a list of FDs
instead of the maximum FD number to preserve.
--preserve-fd and --preserve-fds are mutually exclusive.
It requires crun since runc would complain if any fd below
--preserve-fds is not preserved.
Closes: https://github.com/containers/podman/issues/20844
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Two newly-added tests, fail in gating:
- system connection: difference in how sockets are set up
between CI and gating
- ulimit: gating seems to run with ulimit -c -H 0. Check, and
skip if ulimit is less than what we need
Signed-off-by: Ed Santiago <santiago@redhat.com>
Some system tests run deep loops:
for x in a b c; do
for y in d e f; do
.... check condition $x + $y
Normally, if one of these fails, game over. This can be frustrating
to a developer looking for failure patterns.
Here we introduce a new defer-assertion-failure function, meant
to be called before loops like these. Everything is the same,
except that tests will continue running even after failure.
When test finishes, or if test runs immediate-assertion-failure,
a new message indicates that multiple tests failed:
FAIL: X test assertions failed. Search for 'FAIL': above this line.
Signed-off-by: Ed Santiago <santiago@redhat.com>
Docker allows the passing of -1 to indicate the maximum limit
allowed for the current process.
Fixes: https://github.com/containers/podman/issues/19319
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
commit 7ade972102 introduced the change
that caused an issue in crun since it forces the root user session
instead of the system one when DBUS_SESSION_BUS_ADDRESS is set.
I am addressing it in crun, but for the time being, let's also not
pass the variable down to conmon since the assumption is that when
running as root the containers must be created on the system bus.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Updated the error message to suggest user to use --replace option to instruct Podman to replace the existsing external container with a newly created one.
closes#16759
Signed-off-by: Chetan Giradkar <cgiradka@redhat.com>
Pass the _entire_ environment to conmon instead of selectively enabling
only specific variables. The main reasoning is to make sure that conmon
and the podman-cleanup callback process operate in the exact same
environment than the initial podman process. Some configuration files
may be passed via environment variables. Podman not passing those down
to conmon has led to subtle and hard to debug issues in the past, so
passing all down will avoid such kinds of issues in the future.
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
The processing and setting of the static and volume directories was
scattered across the code base (including c/common) leading to subtle
errors that surfaced in #19938.
There were multiple issues that I try to summarize below:
- c/common loaded the graphroot from c/storage to set the defaults for
static and volume dir. That ignored Podman's --root flag and
surfaced in #19938 and other bugs. c/common does not set the
defaults anymore which gives Podman the ability to detect when the
user/admin configured a custom directory (not empty value).
- When parsing the CLI, Podman (ab)uses containers.conf structures to
set the defaults but also to override them in case the user specified
a flag. The --root flag overrode the static dir which is wrong and
broke a couple of use cases. Now there is a dedicated field for in
the "PodmanConfig" which also includes a containers.conf struct.
- The defaults for static and volume dir and now being set correctly
and adhere to --root.
- The CONTAINERS_CONF_OVERRIDE env variable has not been passed to the
cleanup process. I believe that _all_ env variables should be passed
to conmon to avoid such subtle bugs.
Overall I find that the code and logic is scattered and hard to
understand and follow. I refrained from larger refactorings as I really
just want to get #19938 fixed and then go back to other priorities.
https://github.com/containers/common/pull/1659 broke three pkg/machine
tests. Those have been commented out until getting fixed.
Fixes: #19938
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
commit cf364703fc changed the way
/sys/fs/cgroup is mounted when there is not a netns and it now honors
the ro flag. The mount was created using a bind mount that is a
problem when using a cgroup namespace, fix that by mounting a fresh
cgroup file system.
Closes: https://github.com/containers/podman/issues/20073
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
The --syslog flag has not been passed to the cleanup process (i.e.,
conmon's exit args) complicating debugging quite a bit.
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
Part of RUN-1906.
Followup to #19878 (check stderr in system tests): allow_warnings()
and require_warning() functions to make sure no unexpected messages
fall through the cracks.
Signed-off-by: Ed Santiago <santiago@redhat.com>
When the "rmi" part of "run --rmi" fails due to image being in use
by another container (or for any reason, actually), issue a warning
message, not an error.
Signed-off-by: Ed Santiago <santiago@redhat.com>
Under some circumstances podman tries to kill a container
using signal 37, for which unix.SignalName() returns "".
Not helpful. So, when that happens, show "(signal number)".
Signed-off-by: Ed Santiago <santiago@redhat.com>
With few exceptions, commands that exit 0 should not emit any
messages with level=warning or =error. Let's start enforcing
that in run_podman.
Allow one-off exceptions, typically when we're testing an
actual warning condition (usual case: "podman stop" where it
times out to SIGKILL). Exceptions are specified via:
run_podman 0+w subcommand...
^^^---- or, rarely, 0+e
"0" stands for "expect exit status 0", which is the default
so it's implicit anyway. The +w / +e (or even +we) is the
new part. I have added it to tests where necessary.
And, because life is what it is, add two global exceptions:
- Debian. Because runc has too many flakes.
- kube. Ditto. Kube commands emit lots of nasty error
messages (yes, level=error) that don't seem to affect
results.
Similar to #18442
Signed-off-by: Ed Santiago <santiago@redhat.com>
when running rootless, if the specified oom_score_adj for the
container process is lower than the current value, clamp it to the
current value and print a warning.
Closes: https://github.com/containers/podman/issues/19829
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Fix unquoted string vars. Something like this:
is $output "what we expect"
...will fail with a misleading error message if $output is "".
Also fix typos in a diagnostic; this was causing unhelpful message
on failure
Signed-off-by: Ed Santiago <santiago@redhat.com>
Compat api for containers/stop should take -1 value
Add support for `podman stop --time -1`
Add support for `podman restart --time -1`
Add support for `podman rm --time -1`
Add support for `podman pod stop --time -1`
Add support for `podman pod rm --time -1`
Add support for `podman volume rm --time -1`
Add support for `podman network rm --time -1`
Fixes: https://github.com/containers/podman/issues/17542
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
Ed Santiago
Matt Heon
Paul Holzinger
Giuseppe Scrivano
Daniel J Walsh
Urvashi Mohnani
Alberto Faria
danishprakash
openshift-merge-bot[bot]
Alex Jia
Karuboniru
Miloslav Trmač
openshift-ci[bot]
Chetan Giradkar
Valentin Rothberg
OpenShift Merge Robot