summaryrefslogtreecommitdiff
path: root/pkg/hooks
Commit message (Collapse)AuthorAge
* pkg/hooks/exec: Include failed command in hook errorsW. Trevor King2019-01-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | For example: $ cat /etc/containers/oci/hooks.d/test.json { "version": "1.0.0", "hook": { "path": "/bin/sh", "args": ["sh", "-c", "echo 'oh, noes!' >&2; exit 1"] }, "when": { "always": true }, "stages": ["precreate"] } $ podman run --rm docker.io/library/alpine echo 'successful container' error setting up OCI Hooks: executing [sh -c echo 'oh, noes!' >&2; exit 1]: exit status 1 The rendered command isn't in in the right syntax for copy/pasting into a shell, but it should be enough for the user to be able to locate the failing hook. They'll need to know their hook directories, but with the previous commits requiring explicit hook directories it's more likely that the caller is aware of them. And if they run at a debug level, they can see the lookups in the logs: $ podman --log-level=debug --hooks-dir=/etc/containers/oci/hooks.d run --rm docker.io/library/alpine echo 'successful container' 2>&1 | grep -i hook time="2018-12-02T22:15:16-08:00" level=debug msg="reading hooks from /etc/containers/oci/hooks.d" time="2018-12-02T22:15:16-08:00" level=debug msg="added hook /etc/containers/oci/hooks.d/test.json" time="2018-12-02T22:15:16-08:00" level=debug msg="hook test.json matched; adding to stages [precreate]" time="2018-12-02T22:15:16-08:00" level=warning msg="container 3695c6ba0cc961918bd3e4a769c52bd08b82afea5cd79e9749e9c7a63b5e7100: precreate hook: executing [sh -c echo 'oh, noes!' >&2; exit 1]: exit status 1" time="2018-12-02T22:15:16-08:00" level=error msg="error setting up OCI Hooks: executing [sh -c echo 'oh, noes!' >&2; exit 1]: exit status 1" Signed-off-by: W. Trevor King <wking@tremily.us>
* hooks/exec/runtimeconfigfilter: Log config changesW. Trevor King2019-01-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | To make it easier to notice and track down errors (or other surprising behavior) due to precreate hooks. With this commit, the logged messages look like: time="2018-11-19T13:35:18-08:00" level=debug msg="precreate hook 0 made configuration changes: --- Old +++ New @@ -18,3 +18,3 @@ Namespaces: ([]specs.LinuxNamespace) <nil>, - Devices: ([]specs.LinuxDevice) (len=1) { + Devices: ([]specs.LinuxDevice) (len=2) { (specs.LinuxDevice) { @@ -24,2 +24,11 @@ Minor: (int64) 229, + FileMode: (*os.FileMode)(-rw-------), + UID: (*uint32)(0), + GID: (*uint32)(0) + }, + (specs.LinuxDevice) { + Path: (string) (len=8) "/dev/sda", + Type: (string) (len=1) "b", + Major: (int64) 8, + Minor: (int64) 0, FileMode: (*os.FileMode)(-rw-------), " time="2018-11-19T13:35:18-08:00" level=debug msg="precreate hook 1 made configuration changes: --- Old +++ New @@ -29,3 +29,3 @@ (specs.LinuxDevice) { - Path: (string) (len=8) "/dev/sda", + Path: (string) (len=8) "/dev/sdb", Type: (string) (len=1) "b", " Ideally those logs would include the container ID, but we don't have access to that down at this level. I'm not sure if it's worth teaching RuntimeConfigFilter to accept a *logrus.Entry (so the caller could use WithFields [1]) or to use a generic logging interface (like go-log [2]). For now, I've left the container ID unlogged here. The spew/difflib implementation is based on stretchr/testify/assert, but I think the ~10 lines I'm borrowing are probably small enough to stay under the "all copies or substantial portions" condition in its MIT license. [1]: https://godoc.org/github.com/sirupsen/logrus#WithFields [2]: https://github.com/go-log/log Signed-off-by: W. Trevor King <wking@tremily.us>
* hooks: Add pre-create hooks for runtime-config manipulationW. Trevor King2019-01-08
| | | | | | | | | | | | | | | | | | | | | | | There's been a lot of discussion over in [1] about how to support the NVIDIA folks and others who want to be able to create devices (possibly after having loaded kernel modules) and bind userspace libraries into the container. Currently that's happening in the middle of runc's create-time mount handling before the container pivots to its new root directory with runc's incorrectly-timed prestart hook trigger [2]. With this commit, we extend hooks with a 'precreate' stage to allow trusted parties to manipulate the config JSON before calling the runtime's 'create'. I'm recycling the existing Hook schema from pkg/hooks for this, because we'll want Timeout for reliability and When to avoid the expense of fork/exec when a given hook does not need to make config changes [3]. [1]: https://github.com/opencontainers/runc/pull/1811 [2]: https://github.com/opencontainers/runc/issues/1710 [3]: https://github.com/containers/libpod/issues/1828#issuecomment-439888059 Signed-off-by: W. Trevor King <wking@tremily.us>
* Change references to cri-o to point at new repositoryDaniel J Walsh2018-09-07
| | | | | | | Signed-off-by: Daniel J Walsh <dwalsh@redhat.com> Closes: #1425 Approved by: mheon
* Fix a bug with hook ALWAYS matching with a processMatthew Heon2018-08-22
| | | | | | | | | | | | When a non-nil process was used and a hook was set to match always, this would not actually match. Fix this. Fixes: #1308 Signed-off-by: Matthew Heon <matthew.heon@gmail.com> Closes: #1311 Approved by: rhatdan
* switch projectatomic to containersDaniel J Walsh2018-08-16
| | | | | | | | | | Need to get some small changes into libpod to pull back into buildah to complete buildah transition. Signed-off-by: Daniel J Walsh <dwalsh@redhat.com> Closes: #1270 Approved by: mheon
* hooks: Add debug logging for initial hook loadingW. Trevor King2018-06-04
| | | | | | | | | | | | | We've had logrus logging in the monitor code since it landed in 68eb128f (pkg/hooks: Version the hook structure and add 1.0.0 hooks, 2018-04-27, #686). This commit adds similar logging to the initial hook.New() and Manager.Hooks() calls to make it easier to see if those are working as expected. Signed-off-by: W. Trevor King <wking@tremily.us> Closes: #887 Approved by: rhatdan
* hooks/docs: Fix 1.0.0 Nvidia example (adding version, etc.)W. Trevor King2018-06-04
| | | | | | | | | | | | | Reported by Gary Edwards [1]. Both typos are originally from 68eb128f (pkg/hooks: Version the hook structure and add 1.0.0 hooks, 2018-04-27, #686). [1]: https://github.com/projectatomic/libpod/issues/884#issuecomment-394174571 Signed-off-by: W. Trevor King <wking@tremily.us> Closes: #887 Approved by: rhatdan
* hooks/1.0.0/when_test: Fix "both, and" -> "both, or" name typoW. Trevor King2018-06-04
| | | | | | | | | | The typo is a copy/paste error from 68eb128f (pkg/hooks: Version the hook structure and add 1.0.0 hooks, 2018-04-27, #686). Signed-off-by: W. Trevor King <wking@tremily.us> Closes: #887 Approved by: rhatdan
* hooks/1.0.0: Fix 'annotation' -> 'annotations' in JSONW. Trevor King2018-06-04
| | | | | | | | | | | This typo from 68eb128f (pkg/hooks: Version the hook structure and add 1.0.0 hooks, 2018-04-27, #686) was causing any 'annotations' entries in hook JSON to be silently ignored. Signed-off-by: W. Trevor King <wking@tremily.us> Closes: #887 Approved by: rhatdan
* hooks: Fail ReadDir if a configured hook executable is missingW. Trevor King2018-06-04
| | | | | | | | | | | | | | | | The continue here is from 5676597f (hooks/read: Ignore IsNotExist for JSON files in ReadDir, 2018-04-27, #686), where it was intended to silently ignore missing JSON files. However, the old logic was also silently ignoring not-exist errors from the os.Stat(hook.Hook.Path) from 68eb128f (pkg/hooks: Version the hook structure and add 1.0.0 hooks, 2018-04-27, #686). This commit adjusts the check so JSON not-exist errors continue to be silently ignored while hook executable not-exist errors become fatal. Signed-off-by: W. Trevor King <wking@tremily.us> Closes: #887 Approved by: rhatdan
* hooks/exec: Allow successful reaps for 0s post-kill timeoutsW. Trevor King2018-06-01
| | | | | | | | | | | | | | | | | | | | | | | | | I'd been getting the failed-to-reap errors locally, but on an unrelated pull-request the FAH27 suite successfully reaped that hook [1]: --- FAIL: TestRunKillTimeout (0.50s) assertions.go:226: Error Trace: exec_test.go:210 Error: Expect "signal: killed" to match "^failed to reap process within 0s of the kill signal$" FAIL The successful-reap cases limit our coverage, but I don't think that's a big enough problem to be worth repeated polling or similar until we do get the failed-to-reap error. [1]: https://s3.amazonaws.com/aos-ci/ghprb/projectatomic/libpod/96c1535fdc11b2de24421863d7ad5d3b94338b37.0.1527811547665239762/output.log Signed-off-by: W. Trevor King <wking@tremily.us> Closes: #868 Approved by: rhatdan
* pkg/hooks/exec: Add a new package for local hook executionW. Trevor King2018-05-31
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This wraps os/exec to: * Clear the environment when the hook doesn't set 'env'. The runtime spec has [1]: > * env (array of strings, OPTIONAL) with the same semantics as IEEE > Std 1003.1-2008's environ. And running execle or similar with NULL env results in an empty environment: $ cat test.c #include <unistd.h> int main() { return execle("/usr/bin/env", "env", NULL, NULL); } $ cc -o test test.c $ ./test ...no output... Go's Cmd.Env, on the other hand, has [2]: > If Env is nil, the new process uses the current process's environment. This commit works around that by setting []string{} in those cases to avoid leaking the runtime environment into the hooks. * Roll the 'timeout' value (if set) into the passed context. There's no need for two separate ways to cancel hook execution. * Add a configurable timeout on abandoning a post-kill wait. The waiting goroutine will continue and eventually reap the process, but this avoids blocking the Run() call when that takes inordinately long (for example, if a GPU cleanup hook is stuck in I/O sleep [3]). The 'env' output format is specified in POSIX [4]. [1]: https://github.com/opencontainers/runtime-spec/blob/v1.0.1/config.md#posix-platform-hooks [2]: https://golang.org/pkg/os/exec/#Cmd [3]: https://github.com/projectatomic/libpod/pull/857#discussion_r192191002 [4]: http://pubs.opengroup.org/onlinepubs/9699919799/utilities/env.html Signed-off-by: W. Trevor King <wking@tremily.us> Closes: #857 Approved by: mheon
* hooks: Rename Hooks() output to extensionStageHooksW. Trevor King2018-05-31
| | | | | | | | | | To more clearly distinguish between the extensionStages input to New() (a slice of strings) and the map output from Hooks(). Signed-off-by: W. Trevor King <wking@tremily.us> Closes: #855 Approved by: rhatdan
* hooks: Allow local control of OCI stages via extensionStagesW. Trevor King2018-05-31
| | | | | | | | | | | | This allows callers to avoid delegating to OCI runtimes for cases where they feel that the runtime hook handling is unreliable [1]. [1]: https://github.com/projectatomic/libpod/issues/730#issuecomment-392959938 Signed-off-by: W. Trevor King <wking@tremily.us> Closes: #855 Approved by: rhatdan
* hooks/1.0.0: Error on empty process.args instead of panickingW. Trevor King2018-05-24
| | | | | | | | | | | | | | | | | | | | | | | The process property is optional [1], which this package already handled appropriately, although I've added a new test here to guard against regressions. The process.args entry is required when process is set [2], and it's also required to contain at least one entry [3]. The previous implementation here assumed that would always be satisfied, and panicked on empty process.args. With this commit, we avoid the panic and instead return an error message explaining why the input was invalid. [1]: https://github.com/opencontainers/runtime-spec/blame/v1.0.1/config.md#L145 [2]: https://github.com/opencontainers/runtime-spec/blame/v1.0.1/config.md#L157 [3]: https://github.com/opencontainers/runtime-spec/blame/v1.0.1/config.md#L158 Reported-by: Brent Baude <bbaude@redhat.com> Signed-off-by: W. Trevor King <wking@tremily.us> Closes: #829 Approved by: mheon
* hooks/README: Fix some Markdown typos (e.g. missing runc target)W. Trevor King2018-05-21
| | | | | | | | | | I'd accidentally introduced these typos in ea415610 (hooks/docs: Add oci-hooks.5 and per-package man page building, 2018-05-15, #772). Signed-off-by: W. Trevor King <wking@tremily.us> Closes: #810 Approved by: mheon
* oci-hooks.5: Discuss directory precedence and monitoringW. Trevor King2018-05-21
| | | | | | | | | | | We've had this functionality since 68eb128f (pkg/hooks: Version the hook structure and add 1.0.0 hooks, 2018-04-27, #686), but didn't have any user-facing docs for it. Signed-off-by: W. Trevor King <wking@tremily.us> Closes: #811 Approved by: mheon
* hooks: Fix monitoring of multiple directoriesW. Trevor King2018-05-17
| | | | | | | | | | | | | | | | | | | | | | | | | | This isn't an issue with podman, which will only ever use one directory. But CRI-O generally uses two directories, and we want to make sure that changes to the fallback directory are not clobbering hooks configured in the override directory. More background in [1]. I've split the handling into a single-directory block and a multiple-directory block so we don't waste time polling the filesystem for single-directory removals. I'm using the single-directory block for the the zero-directory case as well. Managers with zero directories should not be receiving fsnotify events, so I don't think it really matters which block handles them. If we want to handle this case robustly (because we're concerned about something in the hook package adjusted the private .directories property on the fly?), then we'll probably want to add an explicit zero-directory block in future work. [1]: https://github.com/kubernetes-incubator/cri-o/pull/1470 Signed-off-by: W. Trevor King <wking@tremily.us> Closes: #757 Approved by: rhatdan
* hooks/docs: Add oci-hooks.5 and per-package man page buildingW. Trevor King2018-05-17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This allows us to reference the hooks docs from podman(1) in a way that will survive system installation. The downside is that the GitHub rendered pages become less usable, now that we can no longer embed links as freely as we could before. I've followed the "Sections within a manual page" suggestions from [1]. locale(7) is [2], which is Linux-specific. Even section numbering is platform-dependent [3], so it's unlikely that these external man references are particularly portable. Platform packagers can adjust our local references to match their target system, but that leaves the GitHub rendering in an awkward place. For now, I think a Linux-centric GitHub rendering without clickable links may be the best we can do without moving away from go-md2man. As far as I can tell, there's not a nice way to get go-md2man to wrap the links in SEE ALSO without sometimes hyphenating a URL (which makes it harder for man-page readers to copy/paste those links into their browser). I've also fixed some "extention" -> "extension" typos. [1]: http://man7.org/linux/man-pages/man7/man-pages.7.html [2]: http://man7.org/linux/man-pages/man7/locale.7.html [3]: https://en.wikipedia.org/wiki/Man_page#Manual_sections Signed-off-by: W. Trevor King <wking@tremily.us> Closes: #772 Approved by: mheon
* hooks: Add package support for extension stagesW. Trevor King2018-05-14
| | | | | | | | | | | | | We aren't consuming this yet, but these pkg/hooks changes lay the groundwork for future libpod changes to support post-exit hooks [1,2]. [1]: https://github.com/projectatomic/libpod/issues/730 [2]: https://github.com/opencontainers/runc/issues/1797 Signed-off-by: W. Trevor King <wking@tremily.us> Closes: #758 Approved by: rhatdan
* hooks: Order injection by collated JSON filenameW. Trevor King2018-05-11
| | | | | | | | | | | | | | | | | | | | | | | We also considered ordering with sort.Strings, but Matthew rejected that because it uses a byte-by-byte UTF-8 comparison [1] which would fail many language-specific conventions [2]. There's some more discussion of the localeToLanguage mapping in [3]. Currently language.Parse does not handle either 'C' or 'POSIX', returning: und, language: tag is not well-formed for both. [1]: https://github.com/projectatomic/libpod/pull/686#issuecomment-387914358 [2]: https://en.wikipedia.org/wiki/Alphabetical_order#Language-specific_conventions [3]: https://github.com/golang/go/issues/25340 Signed-off-by: W. Trevor King <wking@tremily.us> Closes: #686 Approved by: mheon
* hooks/read: Ignore IsNotExist for JSON files in ReadDirW. Trevor King2018-05-11
| | | | | | | | | | | | | | | If a .json file existed when we called ioutil.ReadDir but that file has been removed by the time we get around to calling Read on it, silently ignore the file. Iterating through all the files in the directory shouldn't take particularly long, so this is an unlikely corner case. And when it happens, silently ignoring the file gives the same outcome as you'd have gotten if the parallel remove had happened slightly earlier before the ioutil.ReadDir call. Signed-off-by: W. Trevor King <wking@tremily.us> Closes: #686 Approved by: mheon
* pkg/hooks: Version the hook structure and add 1.0.0 hooksW. Trevor King2018-05-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This shifts the matching logic out of libpod/container_internal and into the hook package, where we can reuse it after vendoring into CRI-O. It also adds unit tests with almost-complete coverage. Now libpod is even more isolated from the hook internals, which makes it fairly straightforward to bump the hook config file to 1.0.0. I've dubbed the old format 0.1.0, although it doesn't specify an explicit version. Motivation for some of my changes with 1.0.0: * Add an explicit version field. This will make any future JSON structure migrations more straightforward by avoiding the need for version-guessing heuristics. * Collect the matching properties in a new When sub-structure. This makes the root Hook structure easier to understand, because you don't have to read over all the matching properties when wrapping your head around Hook. * Replace the old 'hook' and 'arguments' with a direct embedding of the runtime-spec's hook structure. This provides access to additional upstream properties (args[0], env, and timeout) and avoids the complication of a CRI-O-specific analog structure. * Add a 'when.always' property. You can usually accomplish this effect in another way (e.g. when.commands = [".*"]), but having a boolean explicitly for this use-case makes for easier reading and writing. * Replace the previous annotations array with an annotations map. The 0.1.0 approach matched only the values regardless of key, and that seems unreliable. * Replace 'cmds' with 'when.commands', because while there are a few ways to abbreviate "commands", there's only one way to write it out in full ;). This gives folks one less thing to remember when writing hook JSON. * Replace the old "inject if any specified condition matches" with "inject if all specified conditions match". This allows for more precise targeting. Users that need more generous targeting can recover the previous behavior by creating a separate 1.0.0 hook file for each specified 0.1.0 condition. I've added doc-compat support for the various pluralizations of the 0.1.0 properties. Previously, the docs and code were not in agreement. More on this particular facet in [1]. I've updated the docs to point out that the annotations being matched are the OCI config annotations. This differs from CRI-O, where the annotations used are the Kubernetes-supplied annotations [2,3]. For example, io.kubernetes.cri-o.Volumes [4] is part of CRI-O's runtime config annotations [5], but not part of the Kubernetes-supplied annotations CRI-O uses for matching hooks. The Monitor method supports the CRI-O use-case [6]. podman doesn't need it directly, but CRI-O will need it when we vendor this package there. I've used nvidia-container-runtime-hook for the annotation examples because Dan mentioned the Nvidia folks as the motivation behind annotation matching. The environment variables are documented in [7]. The 0.1.0 hook config, which does not allow for environment variables, only works because runc currently leaks the host environment into the hooks [8]. I haven't been able to find documentation for their usual annotation trigger or hook-install path, so I'm just guessing there. [1]: https://github.com/kubernetes-incubator/cri-o/pull/1235 [2]: https://github.com/kubernetes-incubator/cri-o/blob/v1.10.0/server/container_create.go#L760 [3]: https://github.com/kubernetes-incubator/cri-o/blob/v1.10.0/server/container_create.go#L772 [4]: https://github.com/kubernetes-incubator/cri-o/blob/v1.10.0/pkg/annotations/annotations.go#L97-L98 [5]: https://github.com/kubernetes-incubator/cri-o/blob/v1.10.0/server/container_create.go#L830-L834 [6]: https://github.com/kubernetes-incubator/cri-o/pull/1345/ [7]: https://github.com/NVIDIA/nvidia-container-runtime/tree/v1.3.0-1#environment-variables-oci-spec [8]: https://github.com/opencontainers/runc/pull/1738 Signed-off-by: W. Trevor King <wking@tremily.us> Closes: #686 Approved by: mheon
* Add hooks support to podmanDaniel J Walsh2018-04-05
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com> Closes: #155 Approved by: mheon
generated by cgit v1.2.3-54-g00ecf (git 2.39.0) at 2024-09-27 00:20:15 +0000