Include the unit tests (i.e., _test.go files) for linting to make the
tests more robust and enforce the linters' coding styles etc.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
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>
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>
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>
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]: 96c1535fdc.0.1527811547665239762/output.log
Signed-off-by: W. Trevor King <wking@tremily.us>
Closes: #868
Approved by: rhatdan
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
