From 139d0841e89b880ec5aca07998965c905782ae4b Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Sat, 12 Aug 2017 10:29:22 +0000 Subject: [PATCH] Add support for oci-hooks to libkpod Add new directory /etc/crio/hooks.d, where packagers can drop a json config file to specify a hook. The json must specify a valid executable to run. The json must also specify which stage(s) to run the hook: prestart, poststart, poststop The json must specify under which criteria the hook should be launched If the container HasBindMounts If the container cmd matches a list of regular expressions If the containers annotations matches a list of regular expressions. If any of these match the the hook will be launched. Signed-off-by: Daniel J Walsh --- README.md | 3 + cmd/crio/config.go | 3 + cmd/crio/main.go | 9 ++ contrib/test/crio-integration-playbook.yaml | 6 ++ hooks.md | 90 +++++++++++++++++++ libkpod/config.go | 7 ++ libkpod/container_server.go | 22 +++++ libkpod/hooks.go | 98 +++++++++++++++++++++ server/container_create.go | 73 +++++++++++++++ test/helpers.bash | 8 +- test/hooks.bats | 38 ++++++++ test/hooks/checkhook.json | 5 ++ test/hooks/checkhook.sh | 4 + 13 files changed, 365 insertions(+), 1 deletion(-) create mode 100644 hooks.md create mode 100644 libkpod/hooks.go create mode 100644 test/hooks.bats create mode 100644 test/hooks/checkhook.json create mode 100755 test/hooks/checkhook.sh diff --git a/README.md b/README.md index 2153d2b1..6fe5e496 100644 --- a/README.md +++ b/README.md @@ -67,6 +67,9 @@ It is currently in active development in the Kubernetes community through the [d | ---------------------------------------------------- | ---------------------------------------------------------------------------------------------------- | | [crio.conf(5)](/docs/crio.conf.5.md) | CRI-O Configuation file | +## OCI Hooks Support + +[CRI-O configures OCI Hooks to run when launching a container](./hooks.md) ## cri-o Usage Transfer diff --git a/cmd/crio/config.go b/cmd/crio/config.go index 2bd22fc2..17ec67ca 100644 --- a/cmd/crio/config.go +++ b/cmd/crio/config.go @@ -102,6 +102,9 @@ apparmor_profile = "{{ .ApparmorProfile }}" # for the runtime. cgroup_manager = "{{ .CgroupManager }}" +# hooks_dir_path is the oci hooks directory for automatically executed hooks +hooks_dir_path = "{{ .HooksDirPath }}" + # pids_limit is the number of processes allowed in a container pids_limit = {{ .PidsLimit }} diff --git a/cmd/crio/main.go b/cmd/crio/main.go index f15b37c2..7c991ed3 100644 --- a/cmd/crio/main.go +++ b/cmd/crio/main.go @@ -111,6 +111,9 @@ func mergeConfig(config *server.Config, ctx *cli.Context) error { if ctx.GlobalIsSet("cgroup-manager") { config.CgroupManager = ctx.GlobalString("cgroup-manager") } + if ctx.GlobalIsSet("hooks-dir-path") { + config.HooksDirPath = ctx.GlobalString("hooks-dir-path") + } if ctx.GlobalIsSet("pids-limit") { config.PidsLimit = ctx.GlobalInt64("pids-limit") } @@ -277,6 +280,12 @@ func main() { Value: string(libkpod.ImageVolumesMkdir), Usage: "image volume handling ('mkdir' or 'ignore')", }, + cli.StringFlag{ + Name: "hooks-dir-path", + Usage: "set the OCI hooks directory path", + Value: libkpod.DefaultHooksDirPath, + Hidden: true, + }, cli.BoolFlag{ Name: "profile", Usage: "enable pprof remote profiler on localhost:6060", diff --git a/contrib/test/crio-integration-playbook.yaml b/contrib/test/crio-integration-playbook.yaml index 0ad67aa4..a51c93e5 100644 --- a/contrib/test/crio-integration-playbook.yaml +++ b/contrib/test/crio-integration-playbook.yaml @@ -46,6 +46,9 @@ - libgpg-error-devel - pkgconfig - skopeo-containers + - oci-systemd-hook + - oci-register-machine + - oci-umount async: 600 poll: 10 when: (ansible_distribution == 'RedHat' or ansible_distribution == 'CentOS') @@ -73,6 +76,9 @@ - libgpg-error-devel - pkgconfig - skopeo-containers + - oci-systemd-hook + - oci-register-machine + - oci-umount async: 600 poll: 10 when: ansible_distribution == 'Fedora' diff --git a/hooks.md b/hooks.md new file mode 100644 index 00000000..809dbdc2 --- /dev/null +++ b/hooks.md @@ -0,0 +1,90 @@ +# OCI Hooks Configuration + +[The OCI Runtime Specification defines POSIX-platform Hooks:]( +https://github.com/opencontainers/runtime-spec/blob/master/config.md#posix-platform-hooks) + +## POSIX-platform Hooks + +For POSIX platforms, the configuration structure supports hooks for configuring custom actions related to the life cycle of the container. + +hooks (object, OPTIONAL) MAY contain any of the following properties: + + * prestart (array of objects, OPTIONAL) is an array of pre-start hooks. Entries in the array contain the following properties: + * path (string, REQUIRED) with similar semantics to [IEEE Std 1003.1-2008 execv's path][ieee-1003.1-2008-functions-exec]. This specification extends the IEEE standard in that path MUST be absolute. + * args (array of strings, OPTIONAL) with the same semantics as [IEEE Std 1003.1-2008 execv's argv][ieee-1003.1-2008-functions-exec]. + * env (array of strings, OPTIONAL) with the same semantics as IEEE Std 1003.1-2008's environ. + * timeout (int, OPTIONAL) is the number of seconds before aborting the hook. If set, timeout MUST be greater than zero. + * poststart (array of objects, OPTIONAL) is an array of post-start hooks. Entries in the array have the same schema as pre-start entries. + * poststop (array of objects, OPTIONAL) is an array of post-stop hooks. Entries in the array have the same schema as pre-start entries. + +Hooks allow users to specify programs to run before or after various lifecycle events. Hooks MUST be called in the listed order. The state of the container MUST be passed to hooks over stdin so that they may do work appropriate to the current state of the container. + +### Prestart + +The Prestart hooks MUST be called after the start operation is called but before the user-specified program command is executed. On Linux, for example, they are called after the container namespaces are created, so they provide an opportunity to customize the container (e.g. the network namespace could be specified in this hook). + +### Poststart + +The post-start hooks MUST be called after the user-specified process is executed but before the start operation returns. For example, this hook can notify the user that the container process is spawned. + +### Poststop + +The post-stop hooks MUST be called after the container is deleted but before the delete operation returns. Cleanup or debugging functions are examples of such a hook. + +## CRI-O configuration files for automatically enabling Hooks + +The way you enable the hooks above is by editing the OCI Specification to add your hook before running the oci runtime, like runc. But this is what `CRI-O` and `Kpod create` do for you, so we wanted a way for developers to drop configuration files onto the system, so that their hooks would be able to be plugged in. + +One problem with hooks is that the runtime actually stalls execution of the container before running the hooks and stalls completion of the container, until all hooks complete. This can cause some performance issues. Also a lot of hooks just check if certain configuration is set and then exit early, without doing anything. For example the [oci-systemd-hook](https://github.com/projectatomic/oci-systemd-hook) only executes if the command is `init` or `systemd`, otherwise it just exits. This means if we automatically enable all hooks, every container will have to execute oci-systemd-hook, even if they don't run systemd inside of the container. Also since there are three stages, prestart, poststart, poststop each hook gets executed three times. + + + +### Json Definition + +We decided to add a json file for hook builders which allows them to tell CRI-O when to run the hook and in which stage. +CRI-O reads all json files in /usr/share/containers/oci/hooks.d/*.json and /etc/containers/oci/hooks.d and sets up the specified hooks to run. If the same name is in both directories, the one in /etc/containers/oci/hooks.d takes precedence. + +The json configuration looks like this in GO +``` +// HookParams is the structure returned from read the hooks configuration +type HookParams struct { + Hook string `json:"hook"` + Stage []string `json:"stages"` + Cmds []string `json:"cmds"` + Annotations []string `json:"annotations"` + HasBindMounts bool `json:"hasbindmounts"` +} +``` + +| Key | Description | Required/Optional | +| ------ |----------------------------------------------------------------------------------------------------------------------------------- | -------- | +| hook | Path to the hook | Required | +| stages | List of stages to run the hook in: Valid options are `prestart`, `poststart`, `poststop` | Required | +| cmds | List of regular expressions to match the command for running the container. If the command matches a regex, the hook will be run | Optional | +| annotations | List of regular expressions to match against the Annotations in the container runtime spec, if an Annotation matches the hook will be run|optional | +| hasbindmounts | Tells CRI-O to run the hook if the container has bind mounts from the host into the container | Optional | + +### Example + + +``` +cat /etc/containers/oci/hooks.d/oci-systemd-hook.json +{ + "cmds": [".*/init$" , ".*/systemd$" ], + "hook": "/usr/libexec/oci/hooks.d/oci-systemd-hook", + "stages": [ "prestart", "poststop" ] +} +``` + +In the above example CRI-O will only run the oci-systemd-hook in the prestart and poststop stage, if the command ends with /init or /systemd + + +``` +cat /etc/containers/oci/hooks.d/oci-systemd-hook.json +{ + "hasbindmounts": true, + "hook": "/usr/libexec/oci/hooks.d/oci-umount", + "stages": [ "prestart" ] +} +``` +In this example the oci-umount will only be run during the prestart phase if the container has volume/bind mounts from the host into the container. diff --git a/libkpod/config.go b/libkpod/config.go index c3e94043..4a3ade4f 100644 --- a/libkpod/config.go +++ b/libkpod/config.go @@ -135,6 +135,12 @@ type RuntimeConfig struct { // handle cgroups for containers. CgroupManager string `toml:"cgroup_manager"` + // HooksDirPath location of oci hooks config files + HooksDirPath string `toml:"hooks_dir_path"` + + // Hooks List of hooks to run with container + Hooks map[string]HookParams + // PidsLimit is the number of processes each container is restricted to // by the cgroup process number controller. PidsLimit int64 `toml:"pids_limit"` @@ -267,6 +273,7 @@ func DefaultConfig() *Config { CgroupManager: cgroupManager, PidsLimit: DefaultPidsLimit, ContainerExitsDir: containerExitsDir, + HooksDirPath: DefaultHooksDirPath, }, ImageConfig: ImageConfig{ DefaultTransport: defaultTransport, diff --git a/libkpod/container_server.go b/libkpod/container_server.go index c51ea4b5..0da8c5b1 100644 --- a/libkpod/container_server.go +++ b/libkpod/container_server.go @@ -36,6 +36,7 @@ type ContainerServer struct { ctrIDIndex *truncindex.TruncIndex podNameIndex *registrar.Registrar podIDIndex *truncindex.TruncIndex + hooks map[string]HookParams imageContext *types.SystemContext stateLock sync.Locker @@ -48,6 +49,11 @@ func (c *ContainerServer) Runtime() *oci.Runtime { return c.runtime } +// Hooks returns the oci hooks for the ContainerServer +func (c *ContainerServer) Hooks() map[string]HookParams { + return c.hooks +} + // Store returns the Store for the ContainerServer func (c *ContainerServer) Store() cstorage.Store { return c.store @@ -131,6 +137,21 @@ func New(config *Config) (*ContainerServer, error) { lock = new(sync.Mutex) } + hooks := make(map[string]HookParams) + // If hooks directory is set in config use it + if config.HooksDirPath != "" { + if err := readHooks(config.HooksDirPath, hooks); err != nil { + return nil, err + } + // If user overrode default hooks, this means it is in a test, so don't + // use OverrideHooksDirPath + if config.HooksDirPath == DefaultHooksDirPath { + if err := readHooks(OverrideHooksDirPath, hooks); err != nil { + return nil, err + } + } + } + return &ContainerServer{ runtime: runtime, store: store, @@ -141,6 +162,7 @@ func New(config *Config) (*ContainerServer, error) { podNameIndex: registrar.NewRegistrar(), podIDIndex: truncindex.NewTruncIndex([]string{}), imageContext: &types.SystemContext{SignaturePolicyPath: config.SignaturePolicyPath}, + hooks: hooks, stateLock: lock, state: &containerServerState{ containers: oci.NewMemoryStore(), diff --git a/libkpod/hooks.go b/libkpod/hooks.go new file mode 100644 index 00000000..f353cdcd --- /dev/null +++ b/libkpod/hooks.go @@ -0,0 +1,98 @@ +package libkpod + +import ( + "encoding/json" + "io/ioutil" + "os" + "path/filepath" + "regexp" + "strings" + "syscall" + + "github.com/pkg/errors" + "github.com/sirupsen/logrus" +) + +const ( + // DefaultHooksDirPath Default directory containing hooks config files + DefaultHooksDirPath = "/usr/share/containers/oci/hooks.d" + // OverrideHooksDirPath Directory where admin can override the default configuration + OverrideHooksDirPath = "/etc/containers/oci/hooks.d" +) + +// HookParams is the structure returned from read the hooks configuration +type HookParams struct { + Hook string `json:"hook"` + Stage []string `json:"stage"` + Cmds []string `json:"cmd"` + Annotations []string `json:"annotation"` + HasBindMounts bool `json:"hasbindmounts"` +} + +// readHook reads hooks json files, verifies it and returns the json config +func readHook(hookPath string) (HookParams, error) { + var hook HookParams + raw, err := ioutil.ReadFile(hookPath) + if err != nil { + return hook, errors.Wrapf(err, "error Reading hook %q", hookPath) + } + if err := json.Unmarshal(raw, &hook); err != nil { + return hook, errors.Wrapf(err, "error Unmarshalling JSON for %q", hookPath) + } + if _, err := os.Stat(hook.Hook); err != nil { + return hook, errors.Wrapf(err, "unable to stat hook %q in hook config %q", hook.Hook, hookPath) + } + validStage := map[string]bool{"prestart": true, "poststart": true, "poststop": true} + for _, cmd := range hook.Cmds { + if _, err = regexp.Compile(cmd); err != nil { + return hook, errors.Wrapf(err, "invalid cmd regular expression %q defined in hook config %q", cmd, hookPath) + } + } + for _, cmd := range hook.Annotations { + if _, err = regexp.Compile(cmd); err != nil { + return hook, errors.Wrapf(err, "invalid cmd regular expression %q defined in hook config %q", cmd, hookPath) + } + } + for _, stage := range hook.Stage { + if !validStage[stage] { + return hook, errors.Wrapf(err, "unknown stage %q defined in hook config %q", stage, hookPath) + } + } + return hook, nil +} + +// readHooks reads hooks json files in directory to setup OCI Hooks +// adding hooks to the passedin hooks map. +func readHooks(hooksPath string, hooks map[string]HookParams) error { + if _, err := os.Stat(hooksPath); err != nil { + if os.IsNotExist(err) { + logrus.Warnf("hooks path: %q does not exist", hooksPath) + return nil + } + return errors.Wrapf(err, "unable to stat hooks path %q", hooksPath) + } + + files, err := ioutil.ReadDir(hooksPath) + if err != nil { + return err + } + + for _, file := range files { + if !strings.HasSuffix(file.Name(), ".json") { + continue + } + hook, err := readHook(filepath.Join(hooksPath, file.Name())) + if err != nil { + return err + } + for key, h := range hooks { + // hook.Hook can only be defined in one hook file, unless it has the + // same name in the override path. + if hook.Hook == h.Hook && key != file.Name() { + return errors.Wrapf(syscall.EINVAL, "duplicate path, hook %q from %q already defined in %q", hook.Hook, hooksPath, key) + } + } + hooks[file.Name()] = hook + } + return nil +} diff --git a/server/container_create.go b/server/container_create.go index 55a58407..3e06275d 100644 --- a/server/container_create.go +++ b/server/container_create.go @@ -7,6 +7,7 @@ import ( "io" "os" "path/filepath" + "regexp" "strconv" "strings" "time" @@ -198,6 +199,24 @@ func buildOCIProcessArgs(containerKubeConfig *pb.ContainerConfig, imageOCIConfig return processArgs, nil } +// addOCIHook look for hooks programs installed in hooksDirPath and add them to spec +func addOCIHook(specgen *generate.Generator, hook libkpod.HookParams) error { + logrus.Debugf("AddOCIHook", hook) + for _, stage := range hook.Stage { + switch stage { + case "prestart": + specgen.AddPreStartHook(hook.Hook, []string{hook.Hook, "prestart"}) + + case "poststart": + specgen.AddPostStartHook(hook.Hook, []string{hook.Hook, "poststart"}) + + case "poststop": + specgen.AddPostStopHook(hook.Hook, []string{hook.Hook, "poststop"}) + } + } + return nil +} + // setupContainerUser sets the UID, GID and supplemental groups in OCI runtime config func setupContainerUser(specgen *generate.Generator, rootfs string, sc *pb.LinuxContainerSecurityContext, imageConfig *v1.Image) error { if sc != nil { @@ -355,6 +374,56 @@ func (s *Server) CreateContainer(ctx context.Context, req *pb.CreateContainerReq return resp, nil } +func (s *Server) setupOCIHooks(specgen *generate.Generator, sb *sandbox.Sandbox, containerConfig *pb.ContainerConfig, command string) error { + mounts := containerConfig.GetMounts() + addedHooks := map[string]struct{}{} + addHook := func(hook libkpod.HookParams) error { + // Only add a hook once + if _, ok := addedHooks[hook.Hook]; !ok { + if err := addOCIHook(specgen, hook); err != nil { + return err + } + addedHooks[hook.Hook] = struct{}{} + } + return nil + } + for _, hook := range s.Hooks() { + logrus.Debugf("SetupOCIHooks", hook) + if hook.HasBindMounts && len(mounts) > 0 { + if err := addHook(hook); err != nil { + return err + } + continue + } + for _, cmd := range hook.Cmds { + match, err := regexp.MatchString(cmd, command) + if err != nil { + logrus.Errorf("Invalid regex %q:%q", cmd, err) + continue + } + if match { + if err := addHook(hook); err != nil { + return err + } + } + } + for _, annotationRegex := range hook.Annotations { + for _, annotation := range sb.Annotations() { + match, err := regexp.MatchString(annotationRegex, annotation) + if err != nil { + logrus.Errorf("Invalid regex %q:%q", annotationRegex, err) + continue + } + if match { + if err := addHook(hook); err != nil { + return err + } + } + } + } + } + return nil +} func (s *Server) createSandboxContainer(ctx context.Context, containerID string, containerName string, sb *sandbox.Sandbox, SandboxConfig *pb.PodSandboxConfig, containerConfig *pb.ContainerConfig) (*oci.Container, error) { if sb == nil { return nil, errors.New("createSandboxContainer needs a sandbox") @@ -762,6 +831,10 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID string, } specgen.SetProcessCwd(containerCwd) + if err := s.setupOCIHooks(&specgen, sb, containerConfig, processArgs[0]); err != nil { + return nil, err + } + // Setup user and groups if linux != nil { if err = setupContainerUser(&specgen, mountPoint, linux.GetSecurityContext(), containerImageConfig); err != nil { diff --git a/test/helpers.bash b/test/helpers.bash index e203283b..e4e721a4 100644 --- a/test/helpers.bash +++ b/test/helpers.bash @@ -59,6 +59,11 @@ PIDS_LIMIT=${PIDS_LIMIT:-1024} TESTDIR=$(mktemp -d) +# Setup default hooks dir +HOOKSDIR=$TESTDIR/hooks +mkdir ${HOOKSDIR} +HOOKS_OPTS="--hooks-dir-path=$HOOKSDIR" + # We may need to set some default storage options. case "$(stat -f -c %T ${TESTDIR})" in aufs) @@ -223,7 +228,7 @@ function start_crio() { "$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTS --runroot "$TESTDIR/crio-run" --image-name=mrunalp/image-volume-test --import-from=dir:"$ARTIFACTS_PATH"/image-volume-test-image --add-name=docker.io/library/mrunalp/image-volume-test --signature-policy="$INTEGRATION_ROOT"/policy.json "$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTS --runroot "$TESTDIR/crio-run" --image-name=busybox:latest --import-from=dir:"$ARTIFACTS_PATH"/busybox-image --add-name=docker.io/library/busybox:latest --signature-policy="$INTEGRATION_ROOT"/policy.json "$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTS --runroot "$TESTDIR/crio-run" --image-name=runcom/stderr-test:latest --import-from=dir:"$ARTIFACTS_PATH"/stderr-test --add-name=docker.io/runcom/stderr-test:latest --signature-policy="$INTEGRATION_ROOT"/policy.json - "$CRIO_BINARY" --conmon "$CONMON_BINARY" --listen "$CRIO_SOCKET" --cgroup-manager "$CGROUP_MANAGER" --registry "docker.io" --runtime "$RUNTIME_BINARY" --root "$TESTDIR/crio" --runroot "$TESTDIR/crio-run" $STORAGE_OPTS --seccomp-profile "$seccomp" --apparmor-profile "$apparmor" --cni-config-dir "$CRIO_CNI_CONFIG" --signature-policy "$INTEGRATION_ROOT"/policy.json --image-volumes "$IMAGE_VOLUMES" --pids-limit "$PIDS_LIMIT" --config /dev/null config >$CRIO_CONFIG + "$CRIO_BINARY" ${HOOKS_OPTS} --conmon "$CONMON_BINARY" --listen "$CRIO_SOCKET" --cgroup-manager "$CGROUP_MANAGER" --registry "docker.io" --runtime "$RUNTIME_BINARY" --root "$TESTDIR/crio" --runroot "$TESTDIR/crio-run" $STORAGE_OPTS --seccomp-profile "$seccomp" --apparmor-profile "$apparmor" --cni-config-dir "$CRIO_CNI_CONFIG" --signature-policy "$INTEGRATION_ROOT"/policy.json --image-volumes "$IMAGE_VOLUMES" --pids-limit "$PIDS_LIMIT" --config /dev/null config >$CRIO_CONFIG # Prepare the CNI configuration files, we're running with non host networking by default if [[ -n "$4" ]]; then @@ -291,6 +296,7 @@ function cleanup_ctrs() { done fi fi + rm -f /run/hookscheck } function cleanup_images() { diff --git a/test/hooks.bats b/test/hooks.bats new file mode 100644 index 00000000..0c1a51ea --- /dev/null +++ b/test/hooks.bats @@ -0,0 +1,38 @@ +#!/usr/bin/env bats + +load helpers + +function teardown() { + cleanup_test +} + +cp hooks/checkhook.sh ${HOOKSDIR} +sed "s|HOOKSDIR|${HOOKSDIR}|" hooks/checkhook.json > ${HOOKSDIR}/checkhook.json + +@test "pod test hooks" { + run rm -f /run/hookscheck + start_crio + run crioctl pod run --config "$TESTDATA"/sandbox_config.json + echo "$output" + [ "$status" -eq 0 ] + pod_id="$output" + run crioctl ctr create --config "$TESTDATA"/container_redis.json --pod "$pod_id" + echo "$output" + [ "$status" -eq 0 ] + ctr_id="$output" + run crioctl ctr start --id "$ctr_id" + echo "$output" + [ "$status" -eq 0 ] + run crioctl pod stop --id "$pod_id" + echo "$output" + [ "$status" -eq 0 ] + run crioctl pod remove --id "$pod_id" + echo "$output" + [ "$status" -eq 0 ] + run cat /run/hookscheck + echo "$output" + [ "$status" -eq 0 ] + cleanup_ctrs + cleanup_pods + stop_crio +} diff --git a/test/hooks/checkhook.json b/test/hooks/checkhook.json new file mode 100644 index 00000000..50ff2372 --- /dev/null +++ b/test/hooks/checkhook.json @@ -0,0 +1,5 @@ +{ + "cmd" : [".*"], + "hook" : "HOOKSDIR/checkhook.sh", + "stage" : [ "prestart" ] +} diff --git a/test/hooks/checkhook.sh b/test/hooks/checkhook.sh new file mode 100755 index 00000000..8b755cb4 --- /dev/null +++ b/test/hooks/checkhook.sh @@ -0,0 +1,4 @@ +#!/bin/sh +echo $@ >> /run/hookscheck +read line +echo $line >> /run/hookscheck