libpod: Add HooksDirNotExistFatal

And add an argument to WithHooksDir to set it.

If the hook dir doesn't exist, the new hooks package considers that a
fatal error.  When a podman caller sets
--hooks-dir-path=/some/typoed/directory, a fatal error is more helpful
than silently not loading any hooks.  However, callers who call podman
without setting --hooks-dir-path may not need hooks at all.  We don't
want to pester those callers with not-exist errors.  With this commit,
we:

* Assume the caller knows what they're doing if they set
  --hooks-dir-path and set HooksDirNotExistFatal.

* If the caller does not explicitly set --hooks-dir-path, assume they
  won't mind if the hook directory is missing and set
  HooksDirNotExistFatal false.

We also considered checking for the directory's existence in the code
calling WithHooksDir or from within WithHooksDir, but checks there
would race with the underlying ioutil.ReadDir in the hooks package.
By pushing the warn/error decision down into libpod's implementation,
we avoid a racy "do we expect this to work once libpod gets to it?"
pre-check.

I've also added a check to error if WithHooksDir is called with an
empty-string argument, because we haven't defined the semantics of
that (is it clearing a previous value?  Is it effectively the same as
the current directory?).  I agree with Matthew that a separate
WithNoHooks, or a *string argument to WithHooks, or some such would be
a better API for clearing previous values [1].  But for now, I'm just
erroring out to fail early for callers who might otherwise be
surprised that libpod ignores empty-string HooksDir.

[1]: https://github.com/projectatomic/libpod/pull/686#issuecomment-385119370

Signed-off-by: W. Trevor King <wking@tremily.us>

Closes: #686
Approved by: mheon

3 files changed, 13 insertions, 2 deletions

diff --git a/libpod/container_internal.go b/libpod/container_internal.go
index 0015bed66..992c1d07b 100644
--- a/libpod/container_internal.go
+++ b/libpod/container_internal.go
@@ -1289,7 +1289,11 @@ func (c *Container) setupOCIHooks(ctx context.Context, g *generate.Generator) er
 
 	manager, err := hooks.New(ctx, []string{c.runtime.config.HooksDir})
 	if err != nil {
-		return err
+		if c.runtime.config.HooksDirNotExistFatal || !os.IsNotExist(err) {
+			return err
+		}
+		logrus.Warnf("failed to load hooks: {}", err)
+		return nil
 	}
 
 	return manager.Hooks(g.Spec(), c.Spec().Annotations, len(c.config.UserVolumes) > 0)
diff --git a/libpod/options.go b/libpod/options.go
index 90b376668..1af788e46 100644
--- a/libpod/options.go
+++ b/libpod/options.go
@@ -180,13 +180,18 @@ func WithStaticDir(dir string) RuntimeOption {
 // WithHooksDir sets the directory to look for OCI runtime hooks config.
 // Note we are not saving this in database, since this is really just for used
 // for testing.
-func WithHooksDir(hooksDir string) RuntimeOption {
+func WithHooksDir(hooksDir string, dirNotExistFatal bool) RuntimeOption {
 	return func(rt *Runtime) error {
 		if rt.valid {
 			return ErrRuntimeFinalized
 		}
 
+		if hooksDir == "" {
+			return errors.Wrap(ErrInvalidArg, "empty-string hook directories are not supported")
+		}
+
 		rt.config.HooksDir = hooksDir
+		rt.config.HooksDirNotExistFatal = dirNotExistFatal
 		return nil
 	}
 }
diff --git a/libpod/runtime.go b/libpod/runtime.go
index e9d41a991..29011be10 100644
--- a/libpod/runtime.go
+++ b/libpod/runtime.go
@@ -131,6 +131,8 @@ type RuntimeConfig struct {
 	CNIPluginDir []string `toml:"cni_plugin_dir"`
 	// HooksDir Path to the directory containing hooks configuration files
 	HooksDir string `toml:"hooks_dir"`
+	// HooksDirNotExistFatal switches between fatal errors and non-fatal warnings if the configured HooksDir does not exist.
+	HooksDirNotExistFatal bool `toml:"hooks_dir_not_exist_fatal"`
 	// DefaultMountsFile is the path to the default mounts file for testing purposes only
 	DefaultMountsFile string `toml:"-"`
 }