Since conmon-rs also uses this code we moved it to c/common. Now podman
should has this also to prevent duplication.
[NO NEW TESTS NEEDED]
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
We now use the golang error wrapping format specifier `%w` instead of
the deprecated github.com/pkg/errors package.
[NO NEW TESTS NEEDED]
Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
Make sure `Sync()` handles state transitions and exit codes correctly.
The function was only being called when batching which could render
containers in an unusable state when running concurrently with other
state-altering functions/commands since the state must be re-read from
the database before acting upon it.
Fixes: #14761
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
This bug was introduced in https://github.com/containers/podman/pull/8906.
When we use 'podman rm/restart/stop/kill etc...' command to
the container running with --rm, the OCI runtime directory
remains at /run/<runtime name> (root user) or
/run/user/<user id>/<runtime name> (rootless user).
This bug could cause other bugs.
For example, when we checkpoint the container running with
--rm (podman checkpoint --export) and restore it
(podman restore --import) with crun, error message
"Error: OCI runtime error: crun: container `<container id>`
already exists" is outputted.
This error is caused by an attempt to restore the container with
the same container ID as the remaining OCI runtime's container ID.
Therefore, I fix that the cleanupRuntime() function runs to
remove the OCI runtime directory,
even if the container has already been removed by --rm option.
Signed-off-by: Toshiki Sonoda <sonoda.toshiki@fujitsu.com>
This commit addresses three intertwined bugs to fix an issue when using
Gitlab runner on Podman. The three bug fixes are not split into
separate commits as tests won't pass otherwise; avoidable noise when
bisecting future issues.
1) Podman conflated states: even when asking to wait for the `exited`
state, Podman returned as soon as a container transitioned to
`stopped`. The issues surfaced in Gitlab tests to fail [1] as
`conmon`'s buffers have not (yet) been emptied when attaching to a
container right after a wait. The race window was extremely narrow,
and I only managed to reproduce with the Gitlab runner [1] unit
tests.
2) The clearer separation between `exited` and `stopped` revealed a race
condition predating the changes. If a container is configured for
autoremoval (e.g., via `run --rm`), the "run" process competes with
the "cleanup" process running in the background. The window of the
race condition was sufficiently large that the "cleanup" process has
already removed the container and storage before the "run" process
could read the exit code and hence waited indefinitely.
Address the exit-code race condition by recording exit codes in the
main libpod database. Exit codes can now be read from a database.
When waiting for a container to exit, Podman first waits for the
container to transition to `exited` and will then query the database
for its exit code. Outdated exit codes are pruned during cleanup
(i.e., non-performance critical) and when refreshing the database
after a reboot. An exit code is considered outdated when it is older
than 5 minutes.
While the race condition predates this change, the waiting process
has apparently always been fast enough in catching the exit code due
to issue 1): `exited` and `stopped` were conflated. The waiting
process hence caught the exit code after the container transitioned
to `stopped` but before it `exited` and got removed.
3) With 1) and 2), Podman is now waiting for a container to properly
transition to the `exited` state. Some tests did not pass after 1)
and 2) which revealed the third bug: `conmon` was executed with its
working directory pointing to the OCI runtime bundle of the
container. The changed working directory broke resolving relative
paths in the "cleanup" process. The "cleanup" process error'ed
before actually cleaning up the container and waiting "main" process
ran indefinitely - or until hitting a timeout. Fix the issue by
executing `conmon` with the same working directory as Podman.
Note that fixing 3) *may* address a number of issues we have seen in the
past where for *some* reason cleanup processes did not fire.
[1] https://gitlab.com/gitlab-org/gitlab-runner/-/issues/27119#note_970712864
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
[MH: Minor reword of commit message]
Signed-off-by: Matthew Heon <mheon@redhat.com>
With conmon-rs on the horizon, we need to disentangle Libpod from
legacy Conmon to the greatest extent possible. There are
definitely opportunities for codesharing between the two, but we
have to assume the implementations will be largely disjoint given
the different architectures.
Fortunately, most of the work has already been done in the past.
The conmon-managed OCI runtime mostly sits behind an interface,
with a few exceptions - the most notable of those being attach.
This PR thus moves Attach behind the interface, to ensure that we
can have attach implementations that don't use our existing unix
socket streaming if necessary.
Still to-do is conmon cleanup. There's a lot of code that removes
Conmon-specific files, or kills the Conmon PID, and all of it
will need to be refactored behind the interface.
[NO NEW TESTS NEEDED] Just moving some things around.
Signed-off-by: Matthew Heon <matthew.heon@pm.me>
Most of these are no longer relevant, just drop the comments.
Most notable change: allow `podman kill` on paused containers.
Works just fine when I test it.
Signed-off-by: Matthew Heon <mheon@redhat.com>
The linter ensures a common code style.
- use switch/case instead of else if
- use if instead of switch/case for single case statement
- add space between comment and text
- detect the use of defer with os.Exit()
- use short form var += "..." instead of var = var + "..."
- detect problems with append()
```
newSlice := append(orgSlice, val)
```
This could lead to nasty bugs because the orgSlice will be changed in
place if it has enough capacity too hold the new elements. Thus we
newSlice might not be a copy.
Of course most of the changes are just cosmetic and do not cause any
logic errors but I think it is a good idea to enforce a common style.
This should help maintainability.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
This is an enhancement proposal for the checkpoint / restore feature of
Podman that enables container migration across multiple systems with
standard image distribution infrastructure.
A new option `--create-image <image>` has been added to the
`podman container checkpoint` command. This option tells Podman to
create a container image. This is a standard image with a single layer,
tar archive, that that contains all checkpoint files. This is similar to
the current approach with checkpoint `--export`/`--import`.
This image can be pushed to a container registry and pulled on a
different system. It can also be exported locally with `podman image
save` and inspected with `podman inspect`. Inspecting the image would
display additional information about the host and the versions of
Podman, criu, crun/runc, kernel, etc.
`podman container restore` has also been extended to support image
name or ID as input.
Suggested-by: Adrian Reber <areber@redhat.com>
Signed-off-by: Radostin Stoyanov <radostin@redhat.com>
CRIU supports checkpoint/restore of file locks. This feature is
required to checkpoint/restore containers running applications
such as MySQL.
Signed-off-by: Radostin Stoyanov <radostin@redhat.com>
This adds the parameter '--print-stats' to 'podman container restore'.
With '--print-stats' Podman will measure how long Podman itself, the OCI
runtime and CRIU requires to restore a checkpoint and print out these
information. CRIU already creates process restore statistics which are
just read in addition to the added measurements. In contrast to just
printing out the ID of the restored container, Podman will now print
out JSON:
# podman container restore --latest --print-stats
{
"podman_restore_duration": 305871,
"container_statistics": [
{
"Id": "47b02e1d474b5d5fe917825e91ac653efa757c91e5a81a368d771a78f6b5ed20",
"runtime_restore_duration": 140614,
"criu_statistics": {
"forking_time": 5,
"restore_time": 67672,
"pages_restored": 14
}
}
]
}
The output contains 'podman_restore_duration' which contains the
number of microseconds Podman required to restore the checkpoint. The
output also includes 'runtime_restore_duration' which is the time
the runtime needed to restore that specific container. Each container
also includes 'criu_statistics' which displays the timing information
collected by CRIU.
Signed-off-by: Adrian Reber <areber@redhat.com>
This adds the parameter '--print-stats' to 'podman container checkpoint'.
With '--print-stats' Podman will measure how long Podman itself, the OCI
runtime and CRIU requires to create a checkpoint and print out these
information. CRIU already creates checkpointing statistics which are
just read in addition to the added measurements. In contrast to just
printing out the ID of the checkpointed container, Podman will now print
out JSON:
# podman container checkpoint --latest --print-stats
{
"podman_checkpoint_duration": 360749,
"container_statistics": [
{
"Id": "25244244bf2efbef30fb6857ddea8cb2e5489f07eb6659e20dda117f0c466808",
"runtime_checkpoint_duration": 177222,
"criu_statistics": {
"freezing_time": 100657,
"frozen_time": 60700,
"memdump_time": 8162,
"memwrite_time": 4224,
"pages_scanned": 20561,
"pages_written": 2129
}
}
]
}
The output contains 'podman_checkpoint_duration' which contains the
number of microseconds Podman required to create the checkpoint. The
output also includes 'runtime_checkpoint_duration' which is the time
the runtime needed to checkpoint that specific container. Each container
also includes 'criu_statistics' which displays the timing information
collected by CRIU.
Signed-off-by: Adrian Reber <areber@redhat.com>
The backend for `ps --sync` has been nonfunctional for a long
while now - probably since v2.0. It's questionable how useful the
flag is in modern Podman (the original case it was intended to
catch, Conmon gone via SIGKILL, should be handled now via pinging
the process with a signal to ensure it's still alive) but having
the ability to force a refresh of container state from the OCI
runtime is still useful.
Signed-off-by: Matthew Heon <mheon@redhat.com>
This allows you to stop a container after a `podman stop` process
started, but did not finish, stopping the container (probably an
ignored stop signal, with no time to SIGKILL?). This is a very
narrow case, but once you're in it the only way to recover is a
`podman rm -f` of the container or extensive manual remediation
(you'd have to kill the container yourself, manually, and then
force a `podman ps --all --sync` to update its status from the
OCI runtime).
[NO NEW TESTS NEEDED] I have no idea how to verify this one -
we need to test that it actually started *during* the other stop
command, and that's nontrivial.
Signed-off-by: Matthew Heon <mheon@redhat.com>
it allows to pass the current std streams down to the container.
conmon support: https://github.com/containers/conmon/pull/289
[NO TESTS NEEDED] it needs a new conmon.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
This adds support to checkpoint containers out of pods and restore
container into pods.
It is only possible to restore a container into a pod if it has been
checkpointed out of pod. It is also not possible to restore a non pod
container into a pod.
The main reason this does not work is the PID namespace. If a non pod
container is being restored in a pod with a shared PID namespace, at
least one process in the restored container uses PID 1 which is already
in use by the infrastructure container. If someone tries to restore
container from a pod with a shared PID namespace without a shared PID
namespace it will also fail because the resulting PID namespace will not
have a PID 1.
Signed-off-by: Adrian Reber <areber@redhat.com>
Implement container to container copy. Previously data could only be
copied from/to the host.
Fixes: #7370
Co-authored-by: Mehul Arora <aroram18@mcmaster.ca>
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
The checkpoint archive compression was hardcoded to `archive.Gzip`.
There have been requests to make the used compression algorithm
selectable. There was especially the request to not compress the
checkpoint archive to be able to create faster checkpoints when not
compressing it.
This also changes the default from `gzip` to `zstd`. This change should
not break anything as the restore code path automatically handles
whatever compression the user provides during restore.
Signed-off-by: Adrian Reber <areber@redhat.com>
The --trace has helped in early stages analyze Podman code. However,
it's contributing to dependency and binary bloat. The standard go
tooling can also help in profiling, so let's turn `--trace` into a NOP.
[NO TESTS NEEDED]
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
Traditionally, the path resolution for containers has been resolved on
the *host*; relative to the container's mount point or relative to
specified bind mounts or volumes.
While this works nicely for non-running containers, it poses a problem
for running ones. In that case, certain kinds of mounts (e.g., tmpfs)
will not resolve correctly. A tmpfs is held in memory and hence cannot
be resolved relatively to the container's mount point. A copy operation
will succeed but the data will not show up inside the container.
To support these kinds of mounts, we need to join the *running*
container's mount namespace (and PID namespace) when copying.
Note that this change implies moving the copy and stat logic into
`libpod` since we need to keep the container locked to avoid race
conditions. The immediate benefit is that all logic is now inside
`libpod`; the code isn't scattered anymore.
Further note that Docker does not support copying to tmpfs mounts.
Tests have been extended to cover *both* path resolutions for running
and created containers. New tests have been added to exercise the
tmpfs-mount case.
For the record: Some tests could be improved by using `start -a` instead
of a start-exec sequence. Unfortunately, `start -a` is flaky in the CI
which forced me to use the more expensive start-exec option.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
prune a dependency that was only being used for a simple struct. Should
correct checksum issue on tarballs
[NO TESTS NEEDED]
Fixes: #9355
Signed-off-by: baude <bbaude@redhat.com>
We missed bumping the go module, so let's do it now :)
* Automated go code with github.com/sirkon/go-imports-rename
* Manually via `vgrep podman/v2` the rest
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
Add an API to libpod to resolve a path on the container. We can
refactor the code that was originally written for copy. Other
functions are requiring a proper path resolution, so libpod seems
like a reasonable home for sharing that code.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
Podman defers stopping the container to the runtime, which can take some
time. Keeping the lock while waiting for the runtime to complete the
stop procedure, prevents other commands from acquiring the lock as shown
in #8501.
To improve the user experience, release the lock before invoking the
runtime, and re-acquire the lock when the runtime is finished. Also
introduce an intermediate "stopping" to properly distinguish from
"stopped" containers etc.
Fixes: #8501
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
When migrating a container with associated volumes, the content of
these volumes should be made available on the destination machine.
This patch enables container checkpoint/restore with named volumes
by including the content of volumes in checkpoint file. On restore,
volumes associated with container are created and their content is
restored.
The --ignore-volumes option is introduced to disable this feature.
Example:
# podman container checkpoint --export checkpoint.tar.gz <container>
The content of all volumes associated with the container are included
in `checkpoint.tar.gz`
# podman container checkpoint --export checkpoint.tar.gz --ignore-volumes <container>
The content of volumes is not included in `checkpoint.tar.gz`. This is
useful, for example, when the checkpoint/restore is performed on the
same machine.
# podman container restore --import checkpoint.tar.gz
The associated volumes will be created and their content will be
restored. Podman will exit with an error if volumes with the same
name already exist on the system or the content of volumes is not
included in checkpoint.tar.gz
# podman container restore --ignore-volumes --import checkpoint.tar.gz
Volumes associated with container must already exist. Podman will not
create them or restore their content.
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
This adds a new command, 'podman network reload', to reload the
networks of existing containers, forcing recreation of firewall
rules after e.g. `firewall-cmd --reload` wipes them out.
Under the hood, this works by calling CNI to tear down the
existing network, then recreate it using identical settings. We
request that CNI preserve the old IP and MAC address in most
cases (where the container only had 1 IP/MAC), but there will be
some downtime inherent to the teardown/bring-up approach. The
architecture of CNI doesn't really make doing this without
downtime easy (or maybe even possible...).
At present, this only works for root Podman, and only locally.
I don't think there is much of a point to adding remote support
(this is very much a local debugging command), but I think adding
rootless support (to kill/recreate slirp4netns) could be
valuable.
Signed-off-by: Matthew Heon <matthew.heon@pm.me>
Signed-off-by: Paul Holzinger <paul.holzinger@web.de>
We only use this channel in terminal attach, and it was not a
buffered channel originally, so it would block on trying to send
unless a receiver was ready. In the non-terminal case, there was
no receiver, so attach blocked forever. Buffer the channel for a
single bool so that it will never block, even if unused.
Fixes#8154
Signed-off-by: Matthew Heon <mheon@redhat.com>
Basically, we want to force the application in the container to
(iff the container was made with a terminal) redraw said terminal
immediately after an attach completes, so the fresh Attach
session will be able to see what's going on (e.g. will have a
shell prompt). Our current attach functions are unfortunately
geared more towards `podman run` than `podman attach` and will
start forwarding resize events *immediately* instead of waiting
until the attach session is alive (much safer for short-lived
`podman run` sessions, but broken for the `podman attach` case).
To avoid a major rewrite, let's just manually send a SIGWINCH
after attach succeeds to force a redraw.
Fixes#6253
Signed-off-by: Matthew Heon <matthew.heon@pm.me>
Our previous flow was to perform a hijack before passing a
connection into Libpod, and then Libpod would attach to the
container's attach socket and begin forwarding traffic.
A problem emerges: we write the attach header as soon as the
attach complete. As soon as we write the header, the client
assumes that all is ready, and sends a Start request. This Start
may be processed *before* we successfully finish attaching,
causing us to lose output.
The solution is to handle hijacking inside Libpod. Unfortunately,
this requires a downright extensive refactor of the Attach and
HTTP Exec StartAndAttach code. I think the result is an
improvement in some places (a lot more errors will be handled
with a proper HTTP error code, before the hijack occurs) but
other parts, like the relocation of printing container logs, are
just *bad*. Still, we need this fixed now to get CI back into
good shape...
Fixes#7195
Signed-off-by: Matthew Heon <matthew.heon@pm.me>
This was inspired by https://github.com/cri-o/cri-o/pull/3934 and
much of the logic for it is contained there. However, in brief,
a named return called "err" can cause lots of code confusion and
encourages using the wrong err variable in defer statements,
which can make them work incorrectly. Using a separate name which
is not used elsewhere makes it very clear what the defer should
be doing.
As part of this, remove a large number of named returns that were
not used anywhere. Most of them were once needed, but are no
longer necessary after previous refactors (but were accidentally
retained).
Signed-off-by: Matthew Heon <matthew.heon@pm.me>
Add a `context.Context` to the log APIs to allow for cancelling
streaming (e.g., via `podman logs -f`). This fixes issues for
the remote API where some go routines of the server will continue
writing and produce nothing but heat and waste CPU cycles.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
With the advent of Podman 2.0.0 we crossed the magical barrier of go
modules. While we were able to continue importing all packages inside
of the project, the project could not be vendored anymore from the
outside.
Move the go module to new major version and change all imports to
`github.com/containers/libpod/v2`. The renaming of the imports
was done via `gomove` [1].
[1] https://github.com/KSubedi/gomove
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
As part of APIv2 Attach, we need to be able to attach to freshly
created containers (in ContainerStateConfigured). This isn't
something Libpod is interested in supporting, so we use Init() to
get the container into ContainerStateCreated, in which attach is
possible. Problem: Init() will fail if dependencies are not
started, so a fresh container in a fresh pod will fail. The
simplest solution is to extend the existing recursive start code
from Start() to Init(), allowing dependency containers to be
started when we initialize the container (optionally, controlled
via bool).
Also, update some comments in container_api.go to make it more
clear how some of our major API calls work.
Fixes#6646
Signed-off-by: Matthew Heon <mheon@redhat.com>
* Add ErrLostSync to report lost of sync when de-mux'ing stream
* Add logus.SetLevel(logrus.DebugLevel) when `go test -v` given
* Add context to debugging messages
Signed-off-by: Jhon Honce <jhonce@redhat.com>
A few major fixes here:
- Support for attaching to Configured containers, to match Docker
behavior.
- Support for stream parameter has been improved (we now properly
handle cases where it is not set).
- Initial support for logs parameter has been added.
- Setting attach streams when the container has a terminal is now
supported.
- Errors are properly reported once the hijack has begun.
Signed-off-by: Matthew Heon <mheon@redhat.com>
add the ability to attach to a running container. the tunnel side of this is not enabled yet as we have work on the endpoints and plumbing to do yet.
add the ability to exec a command in a running container. the tunnel side is also being deferred for same reason.
Signed-off-by: Brent Baude <bbaude@redhat.com>
As part of the rework of exec sessions, we need to address them
independently of containers. In the new API, we need to be able
to fetch them by their ID, regardless of what container they are
associated with. Unfortunately, our existing exec sessions are
tied to individual containers; there's no way to tell what
container a session belongs to and retrieve it without getting
every exec session for every container.
This adds a pointer to the container an exec session is
associated with to the database. The sessions themselves are
still stored in the container.
Exec-related APIs have been restructured to work with the new
database representation. The originally monolithic API has been
split into a number of smaller calls to allow more fine-grained
control of lifecycle. Support for legacy exec sessions has been
retained, but in a deprecated fashion; we should remove this in
a few releases.
Signed-off-by: Matthew Heon <matthew.heon@pm.me>
This reverts commit d3d97a25e8.
This does not resolve the issues we expected it would, and has
some unexpected side effects with the upcoming exec rework.
Signed-off-by: Matthew Heon <matthew.heon@pm.me>
This reverts commit 4632b81c81.
We are reverting #5373 as well, which lays the foundation for
this commit, so it has to go as well.
Signed-off-by: Matthew Heon <matthew.heon@pm.me>
this is a cosmetic change that makes sure podman returns a sane error code when conmon dies underneath it
Signed-off-by: Peter Hunt <pehunt@redhat.com>
Before, we were using -1 as a bogus value in podman to signify something went wrong when reading from a conmon pipe. However, conmon uses negative values to indicate the runtime failed, and return the runtime's exit code.
instead, we should use a bogus value that is actually bogus. Define that value in the define package as MinInt32 (-1<< 31 - 1), which is outside of the range of possible pids (-1 << 31)
Signed-off-by: Peter Hunt <pehunt@redhat.com>
Before, we were getting the exit code from the file, in which we waited an arbitrary amount of time (5 seconds) for the file, and segfaulted if we didn't find it. instead, we should be a bit more certain conmon has sent the exit code. Luckily, it sends the exit code along the sync pipe fd, so we can read it from there
Adapt the ExecContainer interface to pass along a channel to get the pid and exit code from conmon, to be able to read both from the pipe
Signed-off-by: Peter Hunt <pehunt@redhat.com>
This patch allows users to specify the list of capabilities required
to run their container image.
Setting a image/container label "io.containers.capabilities=setuid,setgid"
tells podman that the contained image should work fine with just these two
capabilties, instead of running with the default capabilities, podman will
launch the container with just these capabilties.
If the user or image specified capabilities that are not in the default set,
the container will print an error message and will continue to run with the
default capabilities.
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
This corrects a regression from Podman 1.4.x where container exec
sessions inherited supplemental groups from the container, iff
the exec session did not specify a user.
Signed-off-by: Matthew Heon <matthew.heon@pm.me>
added the ability to wait on a condition (stopped, running, paused...) for a container. if a condition is not provided, wait will default to the stopped condition which uses the original wait code paths. if the condition is stopped, the container exit code will be returned.
also, correct a mux issue we discovered.
Signed-off-by: Brent Baude <bbaude@redhat.com>
Add pkg/capabibilities to deal with capabilities. The code has been
copied from Docker (and attributed with the copyright) but changed
significantly to only do what we really need. The code has also been
simplified and will perform better due to removed redundancy.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
The new APIv2 branch provides an HTTP-based remote API to Podman.
The requirements of this are, unfortunately, incompatible with
the existing Attach API. For non-terminal attach, we need append
a header to what was copied from the container, to multiplex
STDOUT and STDERR; to do this with the old API, we'd need to copy
into an intermediate buffer first, to handle the headers.
To avoid this, provide a new API to handle all aspects of
terminal and non-terminal attach, including closing the hijacked
HTTP connection. This might be a bit too specific, but for now,
it seems to be the simplest approach.
At the same time, add a Resize endpoint. This needs to be a
separate endpoint, so our existing channel approach does not work
here.
I wanted to rework the rest of attach at the same time (some
parts of it, particularly how we start the Attach session and how
we do resizing, are (in my opinion) handled much better here.
That may still be on the table, but I wanted to avoid breaking
existing APIs in this already massive change.
Signed-off-by: Matthew Heon <matthew.heon@pm.me>
When a container is in a PID namespace, it is enought to send
the stop signal to the PID 1 of the namespace, only send signals
to all processes in the container when the container is not in
a pid namespace.
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
We currently rely on exec sessions being removed from the state
by the Exec() API itself, on detecting the session stopping. This
is not a reliable method, though. The Podman frontend for exec
could be killed before the session ended, or another Podman
process could be holding the lock and prevent update (most
notable in `run --rm`, when a container with an active exec
session is stopped).
To resolve this, add a function to reap active exec sessions from
the state, and use it on cleanup (to clear sessions after the
container stops) and remove (to do the same when --rm is passed).
This is a bit more complicated than it ought to be because Kata
and company exist, and we can't guarantee the exec session has a
PID on the host, so we have to plumb this through to the OCI
runtime.
Fixes#4666
Signed-off-by: Matthew Heon <matthew.heon@pm.me>
When Libpod removes a container, there is the possibility that
removal will not fully succeed. The most notable problems are
storage issues, where the container cannot be removed from
c/storage.
When this occurs, we were faced with a choice. We can keep the
container in the state, appearing in `podman ps` and available for
other API operations, but likely unable to do any of them as it's
been partially removed. Or we can remove it very early and clean
up after it's already gone. We have, until now, used the second
approach.
The problem that arises is intermittent problems removing
storage. We end up removing a container, failing to remove its
storage, and ending up with a container permanently stuck in
c/storage that we can't remove with the normal Podman CLI, can't
use the name of, and generally can't interact with. A notable
cause is when Podman is hit by a SIGKILL midway through removal,
which can consistently cause `podman rm` to fail to remove
storage.
We now add a new state for containers that are in the process of
being removed, ContainerStateRemoving. We set this at the
beginning of the removal process. It notifies Podman that the
container cannot be used anymore, but preserves it in the DB
until it is fully removed. This will allow Remove to be run on
these containers again, which should successfully remove storage
if it fails.
Fixes#3906
Signed-off-by: Matthew Heon <mheon@redhat.com>
There were many situations that made exec act funky with input. pipes didn't work as expected, as well as sending input before the shell opened.
Thinking about it, it seemed as though the issues were because of how os.Stdin buffers (it doesn't). Dropping this input had some weird consequences.
Instead, read from os.Stdin as bufio.Reader, allowing the input to buffer before passing it to the container.
Signed-off-by: Peter Hunt <pehunt@redhat.com>
We have a lot of checks for container state scattered throughout
libpod. Many of these need to ensure the container is in one of a
given set of states so an operation may safely proceed.
Previously there was no set way of doing this, so we'd use unique
boolean logic for each one. Introduce a helper to standardize
state checks.
Note that this is only intended to replace checks for multiple
states. A simple check for one state (ContainerStateRunning, for
example) should remain a straight equality, and not use this new
helper.
Signed-off-by: Matthew Heon <mheon@redhat.com>
When a container is created with a given OCI runtime, but then it
is uninstalled or removed from the configuration file, Libpod
presently reacts very poorly. The EvictContainer code can
potentially remove these containers, but we still can't see them
in `podman ps` (aside from the massive logrus.Errorf messages
they create).
Providing a minimal OCI runtime implementation for missing
runtimes allows us to behave better. We'll be able to retrieve
containers from the database, though we still pop up an error for
each missing runtime. For containers which are stopped, we can
remove them as normal.
Signed-off-by: Matthew Heon <matthew.heon@pm.me>
For future work, we need multiple implementations of the OCI
runtime, not just a Conmon-wrapped runtime matching the runc CLI.
As part of this, do some refactoring on the interface for exec
(move to a struct, not a massive list of arguments). Also, add
'all' support to Kill and Stop (supported by runc and used a bit
internally for removing containers).
Signed-off-by: Matthew Heon <matthew.heon@pm.me>
This change matches what is happening on the podman local side
and should eliminate a race condition.
Also exit commands on the server side should start to return to client.
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
We have leaked the exit number codess all over the code, this patch
removes the numbers to constants.
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
use the inotify backend to be notified on the container exit instead
of polling continuosly the runtime. Polling the runtime slowns
significantly down the podman execution time for short lived
processes:
$ time bin/podman run --rm -ti fedora true
real 0m0.324s
user 0m0.088s
sys 0m0.064s
from:
$ time podman run --rm -ti fedora true
real 0m4.199s
user 0m5.339s
sys 0m0.344s
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Before, if the container was run with a specified user that wasn't root, exec would fail because it always set to root unless respecified by user.
instead, inherit the user from the container start.
Signed-off-by: Peter Hunt <pehunt@redhat.com>
If a container is restored multiple times from an exported checkpoint
with the help of '--import --name', the restore will fail if during
'podman run' a static container IP was set with '--ip'. The user can
tell the restore process to ignore the static IP with
'--ignore-static-ip'.
Signed-off-by: Adrian Reber <areber@redhat.com>
When forcibly removing a container, we are initiating an explicit
stop of the container, which is not reflected in 'podman events'.
Swap to using our standard 'stop()' function instead of a custom
one for force-remove, and move the event into the internal stop
function (so internal calls also register it).
This does add one more database save() to `podman remove`. This
should not be a terribly serious performance hit, and does have
the desirable side effect of making things generally safer.
Signed-off-by: Matthew Heon <matthew.heon@pm.me>
There's no way to get the error if we successfully get an exit code (as it's just printed to stderr instead).
instead of relying on the error to be passed to podman, and edit based on the error code, process it on the varlink side instead
Also move error codes to define package
Signed-off-by: Peter Hunt <pehunt@redhat.com>
This includes:
Implement exec -i and fix some typos in description of -i docs
pass failed runtime status to caller
Add resize handling for a terminal connection
Customize exec systemd-cgroup slice
fix healthcheck
fix top
add --detach-keys
Implement podman-remote exec (jhonce)
* Cleanup some orphaned code (jhonce)
adapt remote exec for conmon exec (pehunt)
Fix healthcheck and exec to match docs
Introduce two new OCIRuntime errors to more comprehensively describe situations in which the runtime can error
Use these different errors in branching for exit code in healthcheck and exec
Set conmon to use new api version
Signed-off-by: Jhon Honce <jhonce@redhat.com>
Signed-off-by: Peter Hunt <pehunt@redhat.com>
We have another patch running to do the same for exit files, with
a much more in-depth explanation of why it's necessary. Suffice
to say that persistent files in tmpfs tied to container CGroups
lead to significant memory allocations that last for the lifetime
of the file.
Based on a patch by Andrea Arcangeli (aarcange@redhat.com).
Signed-off-by: Matthew Heon <mheon@redhat.com>
The newly added functionality to include the container's root
file-system changes into the checkpoint archive can now be explicitly
disabled. Either during checkpoint or during restore.
If a container changes a lot of files during its runtime it might be
more effective to migrated the root file-system changes in some other
way and to not needlessly increase the size of the checkpoint archive.
If a checkpoint archive does not contain the root file-system changes
information it will automatically be skipped. If the root file-system
changes are part of the checkpoint archive it is also possible to tell
Podman to ignore these changes.
Signed-off-by: Adrian Reber <areber@redhat.com>
When a container is attached upon start, the WaitGroup counter may
never be decremented if an error is raised before start, causing
the caller to hang.
Synchronize with the start & attach goroutine using a channel, to be
able to detect failures before start.
Signed-off-by: Marco Vedovati <mvedovati@suse.com>
the compilation demands of having libpod in main is a burden for the
remote client compilations. to combat this, we should move the use of
libpod structs, vars, constants, and functions into the adapter code
where it will only be compiled by the local client.
this should result in cleaner code organization and smaller binaries. it
should also help if we ever need to compile the remote client on
non-Linux operating systems natively (not cross-compiled).
Signed-off-by: baude <bbaude@redhat.com>
We weren't properly populating the container's OCI Runtime in
Batch(), causing segfaults on attempting to access it. Add a test
to make sure we actually catch cases like this in the future.
Fixes#3411
Signed-off-by: Matthew Heon <matthew.heon@pm.me>
Allow Podman containers to request to use a specific OCI runtime
if multiple runtimes are configured. This is the first step to
properly supporting containers in a multi-runtime environment.
The biggest changes are that all OCI runtimes are now initialized
when Podman creates its runtime, and containers now use the
runtime requested in their configuration (instead of always the
default runtime).
Signed-off-by: Matthew Heon <matthew.heon@pm.me>
Extend kill's error message to include the container's ID and state.
This address cases where error messages caused by other containers
may confuse users.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
This way a tool can determine if the container exists or not, but is in the
wrong state.
Since 126 is documeted as:
**_126_** if the **_contained command_** cannot be invoked
It makes sense that the container would exit with this state.
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
Paul Holzinger
Sascha Grunert
Valentin Rothberg
openshift-ci[bot]
Toshiki Sonoda
OpenShift Merge Robot
Jhon Honce
Matthew Heon
Matthew Heon
Radostin Stoyanov
Valentin Rothberg
Adrian Reber
Giuseppe Scrivano
Daniel J Walsh
Mehul Arora
Matej Vasek
baude
unknown
Radostin Stoyanov
Josh Soref
Peter Hunt
Jakub Filak
Marco Vedovati