summaryrefslogtreecommitdiff
path: root/libpod/container_exec.go
Commit message (Collapse)AuthorAge
* golangci-lint: enable nolintlintPaul Holzinger2022-06-14
| | | | | | | | | | The nolintlint linter does not deny the use of `//nolint` Instead it allows us to enforce a common nolint style: - force that a linter name must be specified - do not add a space between `//` and `nolint` - make sure nolint is only used when there is actually a problem Signed-off-by: Paul Holzinger <pholzing@redhat.com>
* First batch of resolutions to FIXMEsMatthew Heon2022-05-25
| | | | | | | | | 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>
* enable gocritic linterPaul Holzinger2022-04-26
| | | | | | | | | | | | | | | | | | | | | | 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>
* replace golint with revive linterPaul Holzinger2022-04-22
| | | | | | | | | | | | golint, scopelint and interfacer are deprecated. golint is replaced by revive. This linter is better because it will also check for our error style: `error strings should not be capitalized or end with punctuation or a newline` scopelint is replaced by exportloopref (already endabled) interfacer has no replacement but I do not think this linter is important. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
* Fix a potential race around the exec cleanup processMatthew Heon2022-03-23
| | | | | | | | | | | | | | | | | | | | | Every exec session run attached will, on exit, do two things: it will signal the associated `podman exec` that it is finished (to allow Podman to collect the exit code and exit), and spawn a cleanup process to clean up the exec session (in case the `podman exec` process died, we still need to clean up). If an exec session is created that exits almost instantly, but generates a large amount of output (e.g. prints thousands of lines), the cleanup process can potentially execute before `podman exec` has a chance to read the exit code, resulting in errors. Handle this by detecting if the cleanup process has already removed the exec session before handling the error from reading the exec exit code. [NO NEW TESTS NEEDED] I have no idea how to test this in CI. Fixes #13227 Signed-off-by: Matthew Heon <mheon@redhat.com>
* exec: retry rm -rf on ENOTEMPTY and EBUSYGiuseppe Scrivano2022-01-24
| | | | | | | | | | | | | | | when running on NFS, a RemoveAll could cause EBUSY because of some unlinked files that are still kept open and "silly renamed" to .nfs$ID. This is only half of the fix, as conmon needs to be fixed too. Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2040379 Related: https://github.com/containers/conmon/pull/319 [NO NEW TESTS NEEDED] as it requires NFS as the underlying storage. Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
* bump go module to version 4Valentin Rothberg2022-01-18
| | | | | | | | | | | | | Automated for .go files via gomove [1]: `gomove github.com/containers/podman/v3 github.com/containers/podman/v4` Remaining files via vgrep [2]: `vgrep github.com/containers/podman/v3` [1] https://github.com/KSubedi/gomove [2] https://github.com/vrothberg/vgrep Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
* libpod: add execSessionNoCopyValentin Rothberg2021-09-29
| | | | | | | | | To avoid creating an expensive deep copy, create an internal function to access the exec session. [NO TESTS NEEDED] Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
* standardize logrus messages to upper caseDaniel J Walsh2021-09-22
| | | | | | | | Remove ERROR: Error stutter from logrus messages also. [ NO TESTS NEEDED] This is just code cleanup. Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
* Fix resize race with podman exec -itPaul Holzinger2021-06-16
| | | | | | | | | | | When starting a process with `podman exec -it` the terminal is resized after the process is started. To fix this allow exec start to accept the terminal height and width as parameter and let it resize right before the process is started. Fixes #10560 Signed-off-by: Paul Holzinger <pholzing@redhat.com>
* Add ExecDied event and use it to retrieve exit codesMatthew Heon2021-06-10
| | | | | | | | | | | | | | | | When making Exec Cleanup processes mandatory, I introduced a race wherein attached exec sessions could be cleaned up and removed by the cleanup process before the frontend had a chance to get their exit code. Fortunately, we've dealt with this issue before in containers, and the same solution can be applied here. I added an event for an exec session's process exiting, `exec_died` (Docker has an identical event, so this actually improves our compatibility there) that includes the exit code of the exec session. If the race happens and the exec session no longer exists when we go to remove it, pick up exit code from the event and exit cleanly. Signed-off-by: Matthew Heon <mheon@redhat.com>
* Always spawn a cleanup process with execMatthew Heon2021-06-10
| | | | | | | | | | | | We were previously only doing this for detached exec. I don't know why we did that, but I don't see any reason not to extend it to all exec sessions - it guarantees that we will always clean up exec sessions, even if the original `podman exec` process died. [NO TESTS NEEDED] because I don't really know how to test this one. Signed-off-by: Matthew Heon <mheon@redhat.com>
* oci: drop ExecContainerCleanupPeter Hunt2021-04-16
| | | | | | without the socketsDir, we no longer need to worry about cleaning up after an exec. Signed-off-by: Peter Hunt <pehunt@redhat.com>
* Ensure that `--userns=keep-id` sets user in configMatthew Heon2021-04-06
| | | | | | | | | | | | | | | | | | | | | | | One of the side-effects of the `--userns=keep-id` command is switching the default user of the container to the UID of the user running Podman (though this can still be overridden by the `--user` flag). However, it did this by setting the UID and GID in the OCI spec, and not by informing Libpod of its intention to switch users via the `WithUser()` option. Because of this, a lot of the code that should have triggered when the container ran with a non-root user was not triggering. In the case of the issue that this fixed, the code to remove capabilities from non-root users was not triggering. Adjust the keep-id code to properly inform Libpod of our intention to use a non-root user to fix this. Also, fix an annoying race around short-running exec sessions where Podman would always print a warning that the exec session had already stopped. Fixes #9919 Signed-off-by: Matthew Heon <matthew.heon@pm.me>
* prune remotecommand dependencybaude2021-02-25
| | | | | | | | | | | 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>
* container removal: handle already removed containersValentin Rothberg2021-02-23
| | | | | | | | | | | | Since commit d54478d8eaec, a container's lock is released before attempting to stop it via the OCI runtime. This opened the window for various kinds of race conditions. One of them led to #9479 where the removal+cleanup sequences of a `run --rm` session overlapped with `rm -af`. Make both execution paths more robust by handling the case of an already removed container. Fixes: #9479 Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
* bump go module to v3Valentin Rothberg2021-02-22
| | | | | | | | | 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>
* Enable golint linterPaul Holzinger2021-02-11
| | | | | | | | Use the golint linter and fix the reported problems. [NO TESTS NEEDED] Signed-off-by: Paul Holzinger <paul.holzinger@web.de>
* Enable stylecheck linterPaul Holzinger2021-02-11
| | | | | | | | Use the stylecheck linter and fix the reported problems. [NO TESTS NEEDED] Signed-off-by: Paul Holzinger <paul.holzinger@web.de>
* Handle podman exec capabilities correctlyDaniel J Walsh2021-01-07
| | | | Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
* Preserve groups in exec sessions in ctrs with --userMatthew Heon2020-09-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Podman wants to guarantee that exec sessions retain the groups of the container they are started in, unless explicitly overridden by the user. This guarantee was broken for containers where the `--user` flag was specified; this patch resolves that. Somewhere in the Exec rewrite for APIv2, I changed the location where the container's User is passed into the exec session (similar to groups, we also want to preserve user unless overridden). The lower-level Exec APIs already handled setting user and group appropriately if not specified when the exec session was created, but I added duplicate code to handle this higher in the stack - and that code only handled setting user, not supplemental groups, breaking support in that specific case. Two things conspired to make this one hard to track down: first, things were only broken if the container explicitly set a user; otherwise, the container user would still appear to be unset to the lower-level code, which would properly set supplemental groups (this tricked our existing test into passing). Also, the `crun` OCI runtime will add the groups without prompting, which further masked the problem there. I debated making `runc` do the same, but in the end it's better to fix this in Podman - it's better to be explicit about what we want done so we will work with all OCI runtimes. Signed-off-by: Matthew Heon <matthew.heon@pm.me>
* Fix up errors found by codespellDaniel J Walsh2020-09-11
| | | | Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
* Send HTTP Hijack headers after successful attachMatthew Heon2020-08-27
| | | | | | | | | | | | | | | | | | | | | | | | | 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>
* Ensure that exec errors write exit codes to the DBMatthew Heon2020-08-05
| | | | | | | | | | | | | | | | | | | 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 #6893 Signed-off-by: Matthew Heon <matthew.heon@pm.me>
* Switch all references to github.com/containers/libpod -> podmanDaniel J Walsh2020-07-28
| | | | Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
* Do not print an error message on non-0 exec exit codeMatthew Heon2020-07-21
| | | | | | | | | | | This was added with an earlier exec rework, and honestly is very confusing. Podman is printing an error message, but the error had nothing to do with Podman; it was the executable we ran inside the container that errored, and per `podman run` convention we should set the Podman exit code to the process's exit code and print no error. Signed-off-by: Matthew Heon <mheon@redhat.com>
* Remove all instances of named return "err" from LibpodMatthew Heon2020-07-09
| | | | | | | | | | | | | | | | | 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>
* move go module to v2Valentin Rothberg2020-07-06
| | | | | | | | | | | | | | | 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>
* Enable detached exec for remoteMatthew Heon2020-06-02
| | | | | | | | | | | | | | | | | | | | | | | The biggest obstacle here was cleanup - we needed a way to remove detached exec sessions after they exited, but there's no way to tell if an exec session will be attached or detached when it's created, and that's when we must add the exit command that would do the removal. The solution was adding a delay to the exit command (5 minutes), which gives sufficient time for attached exec sessions to retrieve the exit code of the session after it exits, but still guarantees that they will be removed, even for detached sessions. This requires Conmon 2.0.17, which has the new `--exit-delay` flag. As part of the exit command rework, we can drop the hack we were using to clean up exec sessions (remove them as part of inspect). This is a lot cleaner, and I'm a lot happier about it. Otherwise, this is just plumbing - we need a bindings call for detached exec, and that needed to be added to the tunnel mode backend for entities. Signed-off-by: Matthew Heon <matthew.heon@pm.me>
* Enable cleanup processes for detached execMatthew Heon2020-05-20
| | | | | | | | | | | | | | | | | | | | | | The cleanup command creation logic is made public as part of this and wired such that we can call it both within SpecGen (to make container exit commands) and from the ABI detached exec handler. Exit commands are presently only used for detached exec, but theoretically could be turned on for all exec sessions if we wanted (I'm declining to do this because of potential overhead). I also forgot to copy the exit command from the exec config into the ExecOptions struct used by the OCI runtime, so it was not being added. There are also two significant bugfixes for exec in here. One is for updating the status of running exec sessions - this was always failing as I had coded it to remove the exit file *before* reading it, instead of after (oops). The second was that removing a running exec session would always fail because I inverted the check to see if it was running. Signed-off-by: Matthew Heon <matthew.heon@pm.me>
* Add ability to clean up exec sessions with cleanupMatthew Heon2020-05-20
| | | | | | | | | | | | | | We need to be able to use cleanup processes to remove exec sessions as part of detached exec. This PR adds that ability. A new flag is added to `podman container cleanup`, `--exec`, to specify an exec session to be cleaned up. As part of this, ensure that `ExecCleanup` can clean up exec sessions that were running, but have since exited. This ensures that we can come back to an exec session that was running but has since stopped, and clean it up. Signed-off-by: Matthew Heon <matthew.heon@pm.me>
* Add backend code for detached execMatthew Heon2020-05-20
| | | | | | | | | | | | As part of the massive exec rework, I stubbed out a function for non-detached exec, which is implemented here. It's largely similar to the existing exec functions, but missing a few pieces. This also involves implemented a new OCI runtime call for detached exec. Again, very similar to the other functions, but with a few missing pieces. Signed-off-by: Matthew Heon <matthew.heon@pm.me>
* Add exit commands to exec sessionsMatthew Heon2020-05-20
| | | | | | | | | | | These are required for detached exec, where they will be used to clean up and remove exec sessions when they exit. As part of this, move all Exec related functionality for the Conmon OCI runtime into a separate file; the existing one was around 2000 lines. Signed-off-by: Matthew Heon <matthew.heon@pm.me>
* Prune stale exec sessions on inspectMatthew Heon2020-05-14
| | | | | | | | | | | | | | | | | | | | The usual flow for exec is going to be: - Create exec session - Start and attach to exec session - Exec session exits, attach session terminates - Client does an exec inspect to pick up exit code The safest point to remove the exec session, without doing any database changes to track stale sessions, is to remove during the last part of this - the single inspect after the exec session exits. This is definitely different from Docker (which would retain the exec session for up to 10 minutes after it exits, where we will immediately discard) but should be close enough to be not noticeable in regular usage. Signed-off-by: Matthew Heon <matthew.heon@pm.me>
* Don't fail when saving exec status fails on removed ctrMatthew Heon2020-05-14
| | | | | | | We can't save the exec session, but it's because the container is entirely gone, so no point erroring. Signed-off-by: Matthew Heon <mheon@redhat.com>
* Ensure that Streams are set to defaults for HTTP attachMatthew Heon2020-05-14
| | | | | | | | | | | If not overridden, we should use the attach configuration given when the exec session was first created. Also, setting streams should not conflict with a TTY - the two are allowed together with Attach and should be allowed together here. Signed-off-by: Matthew Heon <mheon@redhat.com>
* Add an initial implementation of HTTP-forwarded execMatthew Heon2020-05-14
| | | | | | | | | | | | | | | This is heavily based off the existing exec implementation, but does not presently share code with it, to try and ensure we don't break anything. Still to do: - Add code sharing with existing exec implementation - Wire in the frontend (exec HTTP endpoint) - Move all exec-related code in oci_conmon_linux.go into a new file - Investigate code sharing between HTTP attach and HTTP exec. Signed-off-by: Matthew Heon <mheon@redhat.com>
* v2podman attach and execBrent Baude2020-04-05
| | | | | | | | 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>
* Merge pull request #5573 from mheon/add_basic_exec_endpointsOpenShift Merge Robot2020-03-26
|\ | | | | Implement APIv2 Exec Create and Inspect Endpoints
| * Add bindings for Container Exec Create + InspectMatthew Heon2020-03-26
| | | | | | | | | | | | | | Also adds some basic tests for these two. More tests are needed but will have to wait for state to be finished. Signed-off-by: Matthew Heon <matthew.heon@pm.me>
| * Implement APIv2 Exec Create and Inspect EndpointsMatthew Heon2020-03-23
| | | | | | | | | | | | Start and Resize require further implementation work. Signed-off-by: Matthew Heon <matthew.heon@pm.me>
* | Ensure that exec sends resize eventsMatthew Heon2020-03-25
|/ | | | | | | | | | | | | | | We previously tried to send resize events only after the exec session successfully started, which makes sense (we might drop an event or two that came in before the exec session started otherwise). However, the start function blocks, so waiting actually means we send no resize events at all, which is obviously worse than losing a few.. Sending resizes before attach starts seems to work fine in my testing, so let's do that until we get bug reports that it doesn't work. Fixes #5584 Signed-off-by: Matthew Heon <mheon@redhat.com>
* Add inspect for exec sessionsMatthew Heon2020-03-18
| | | | | | | This produces detailed information about the configuration of an exec session in a format suitable for the new HTTP API. Signed-off-by: Matthew Heon <matthew.heon@pm.me>
* Add structure for new exec session tracking to DBMatthew Heon2020-03-18
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>
generated by cgit v1.2.3-54-g00ecf (git 2.39.0) at 2024-12-26 19:21:37 +0000