diff options
author | W. Trevor King <wking@tremily.us> | 2018-04-27 10:35:32 -0700 |
---|---|---|
committer | Atomic Bot <atomic-devel@projectatomic.io> | 2018-05-11 16:26:35 +0000 |
commit | 68eb128fb0ad261f48f960547742542584ff1f11 (patch) | |
tree | 024e42956545ae7ef59731bebd8cd1f3392039b9 /libpod | |
parent | 9657cd6c15372f8c655ec13ce4f0310ff5d6a7a1 (diff) | |
download | podman-68eb128fb0ad261f48f960547742542584ff1f11.tar.gz podman-68eb128fb0ad261f48f960547742542584ff1f11.tar.bz2 podman-68eb128fb0ad261f48f960547742542584ff1f11.zip |
pkg/hooks: Version the hook structure and add 1.0.0 hooks
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
Diffstat (limited to 'libpod')
-rw-r--r-- | libpod/container_internal.go | 62 | ||||
-rw-r--r-- | libpod/runtime.go | 2 |
2 files changed, 10 insertions, 54 deletions
diff --git a/libpod/container_internal.go b/libpod/container_internal.go index cbf2a61b8..0015bed66 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -9,7 +9,6 @@ import ( "os" "path" "path/filepath" - "regexp" "strings" "syscall" "time" @@ -1064,7 +1063,7 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) { } } - if err := c.setupOCIHooks(&g); err != nil { + if err := c.setupOCIHooks(ctx, &g); err != nil { return nil, errors.Wrapf(err, "error setting up OCI Hooks") } // Bind builtin image volumes @@ -1283,58 +1282,15 @@ func (c *Container) saveSpec(spec *spec.Spec) error { return nil } -// Add OCI hooks to a container's spec -func (c *Container) setupOCIHooks(g *generate.Generator) error { - addedHooks := map[string]struct{}{} - ocihooks, err := hooks.SetupHooks(c.runtime.config.HooksDir) - if err != nil { - return err - } - addHook := func(hook hooks.HookParams) error { - // Only add a hook once - if _, ok := addedHooks[hook.Hook]; !ok { - if err := hooks.AddOCIHook(g, hook); err != nil { - return err - } - addedHooks[hook.Hook] = struct{}{} - } +func (c *Container) setupOCIHooks(ctx context.Context, g *generate.Generator) error { + if c.runtime.config.HooksDir == "" { return nil } - for _, hook := range ocihooks { - logrus.Debugf("SetupOCIHooks", hook) - if hook.HasBindMounts && len(c.config.UserVolumes) > 0 { - if err := addHook(hook); err != nil { - return err - } - continue - } - for _, cmd := range hook.Cmds { - match, err := regexp.MatchString(cmd, c.config.Spec.Process.Args[0]) - if err != nil { - logrus.Errorf("Invalid regex %q:%q", cmd, err) - continue - } - if match { - if err := addHook(hook); err != nil { - return err - } - } - } - annotations := c.Spec().Annotations - for _, annotationRegex := range hook.Annotations { - for _, annotation := range 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 - } - } - } - } + + manager, err := hooks.New(ctx, []string{c.runtime.config.HooksDir}) + if err != nil { + return err } - return nil + + return manager.Hooks(g.Spec(), c.Spec().Annotations, len(c.config.UserVolumes) > 0) } diff --git a/libpod/runtime.go b/libpod/runtime.go index bf484429e..e9d41a991 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -159,7 +159,7 @@ var ( "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", }, CgroupManager: CgroupfsCgroupsManager, - HooksDir: hooks.DefaultHooksDir, + HooksDir: hooks.DefaultDir, StaticDir: filepath.Join(storage.DefaultStoreOptions.GraphRoot, "libpod"), TmpDir: "/var/run/libpod", MaxLogSize: -1, |