hooks: Add package support for extension stages

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
This commit is contained in:
W. Trevor King
2018-05-11 13:03:28 -07:00
committed by Atomic Bot
parent 69a6cb255c
commit 45838b9561
9 changed files with 137 additions and 53 deletions

View File

@ -1333,7 +1333,7 @@ func (c *Container) setupOCIHooks(ctx context.Context, g *generate.Generator) er
} }
} }
manager, err := hooks.New(ctx, []string{c.runtime.config.HooksDir}, lang) manager, err := hooks.New(ctx, []string{c.runtime.config.HooksDir}, []string{}, lang)
if err != nil { if err != nil {
if c.runtime.config.HooksDirNotExistFatal || !os.IsNotExist(err) { if c.runtime.config.HooksDirNotExistFatal || !os.IsNotExist(err) {
return err return err
@ -1342,5 +1342,6 @@ func (c *Container) setupOCIHooks(ctx context.Context, g *generate.Generator) er
return nil return nil
} }
return manager.Hooks(g.Spec(), c.Spec().Annotations, len(c.config.UserVolumes) > 0) _, err = manager.Hooks(g.Spec(), c.Spec().Annotations, len(c.config.UserVolumes) > 0)
return err
} }

View File

@ -31,7 +31,7 @@ func Read(content []byte) (hook *Hook, err error) {
} }
// Validate performs load-time hook validation. // Validate performs load-time hook validation.
func (hook *Hook) Validate() (err error) { func (hook *Hook) Validate(extensionStages []string) (err error) {
if hook == nil { if hook == nil {
return errors.New("nil hook") return errors.New("nil hook")
} }
@ -68,6 +68,10 @@ func (hook *Hook) Validate() (err error) {
} }
validStages := map[string]bool{"prestart": true, "poststart": true, "poststop": true} validStages := map[string]bool{"prestart": true, "poststart": true, "poststop": true}
for _, stage := range extensionStages {
validStages[stage] = true
}
for _, stage := range hook.Stages { for _, stage := range hook.Stages {
if !validStages[stage] { if !validStages[stage] {
return fmt.Errorf("unknown stage %q", stage) return fmt.Errorf("unknown stage %q", stage)

View File

@ -51,7 +51,7 @@ func TestGoodValidate(t *testing.T) {
}, },
Stages: []string{"prestart"}, Stages: []string{"prestart"},
} }
err := hook.Validate() err := hook.Validate([]string{})
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@ -59,7 +59,7 @@ func TestGoodValidate(t *testing.T) {
func TestNilValidation(t *testing.T) { func TestNilValidation(t *testing.T) {
var hook *Hook var hook *Hook
err := hook.Validate() err := hook.Validate([]string{})
if err == nil { if err == nil {
t.Fatal("unexpected success") t.Fatal("unexpected success")
} }
@ -68,7 +68,7 @@ func TestNilValidation(t *testing.T) {
func TestWrongVersion(t *testing.T) { func TestWrongVersion(t *testing.T) {
hook := Hook{Version: "0.1.0"} hook := Hook{Version: "0.1.0"}
err := hook.Validate() err := hook.Validate([]string{})
if err == nil { if err == nil {
t.Fatal("unexpected success") t.Fatal("unexpected success")
} }
@ -80,7 +80,7 @@ func TestNoHookPath(t *testing.T) {
Version: "1.0.0", Version: "1.0.0",
Hook: rspec.Hook{}, Hook: rspec.Hook{},
} }
err := hook.Validate() err := hook.Validate([]string{})
if err == nil { if err == nil {
t.Fatal("unexpected success") t.Fatal("unexpected success")
} }
@ -94,7 +94,7 @@ func TestUnknownHookPath(t *testing.T) {
Path: filepath.Join("does", "not", "exist"), Path: filepath.Join("does", "not", "exist"),
}, },
} }
err := hook.Validate() err := hook.Validate([]string{})
if err == nil { if err == nil {
t.Fatal("unexpected success") t.Fatal("unexpected success")
} }
@ -111,7 +111,7 @@ func TestNoStages(t *testing.T) {
Path: path, Path: path,
}, },
} }
err := hook.Validate() err := hook.Validate([]string{})
if err == nil { if err == nil {
t.Fatal("unexpected success") t.Fatal("unexpected success")
} }
@ -126,13 +126,27 @@ func TestInvalidStage(t *testing.T) {
}, },
Stages: []string{"does-not-exist"}, Stages: []string{"does-not-exist"},
} }
err := hook.Validate() err := hook.Validate([]string{})
if err == nil { if err == nil {
t.Fatal("unexpected success") t.Fatal("unexpected success")
} }
assert.Regexp(t, "^unknown stage \"does-not-exist\"$", err.Error()) assert.Regexp(t, "^unknown stage \"does-not-exist\"$", err.Error())
} }
func TestExtensionStage(t *testing.T) {
hook := Hook{
Version: "1.0.0",
Hook: rspec.Hook{
Path: path,
},
Stages: []string{"prestart", "b"},
}
err := hook.Validate([]string{"a", "b", "c"})
if err != nil {
t.Fatal(err)
}
}
func TestInvalidAnnotationKey(t *testing.T) { func TestInvalidAnnotationKey(t *testing.T) {
hook := Hook{ hook := Hook{
Version: "1.0.0", Version: "1.0.0",
@ -146,7 +160,7 @@ func TestInvalidAnnotationKey(t *testing.T) {
}, },
Stages: []string{"prestart"}, Stages: []string{"prestart"},
} }
err := hook.Validate() err := hook.Validate([]string{})
if err == nil { if err == nil {
t.Fatal("unexpected success") t.Fatal("unexpected success")
} }
@ -166,7 +180,7 @@ func TestInvalidAnnotationValue(t *testing.T) {
}, },
Stages: []string{"prestart"}, Stages: []string{"prestart"},
} }
err := hook.Validate() err := hook.Validate([]string{})
if err == nil { if err == nil {
t.Fatal("unexpected success") t.Fatal("unexpected success")
} }
@ -184,7 +198,7 @@ func TestInvalidCommand(t *testing.T) {
}, },
Stages: []string{"prestart"}, Stages: []string{"prestart"},
} }
err := hook.Validate() err := hook.Validate([]string{})
if err == nil { if err == nil {
t.Fatal("unexpected success") t.Fatal("unexpected success")
} }

View File

@ -44,7 +44,7 @@ Each JSON file should contain an object with the following properties:
Entries MUST be [POSIX extended regular expressions][POSIX-ERE]. Entries MUST be [POSIX extended regular expressions][POSIX-ERE].
* **`hasBindMounts`** (OPTIONAL, boolean) If `hasBindMounts` is true and the caller requested host-to-container bind mounts (beyond those that CRI-O or libpod use by default), this condition matches. * **`hasBindMounts`** (OPTIONAL, boolean) If `hasBindMounts` is true and the caller requested host-to-container bind mounts (beyond those that CRI-O or libpod use by default), this condition matches.
* **`stages`** (REQUIRED, array of strings) Stages when the hook MUST be injected. * **`stages`** (REQUIRED, array of strings) Stages when the hook MUST be injected.
Entries MUST be chosen from the 1.0.1 OCI Runtime Specification [hook stages][spec-hooks]. Entries MUST be chosen from the 1.0.1 OCI Runtime Specification [hook stages][spec-hooks] or from extention stages supported by the package consumer.
If *all* of the conditions set in `when` match, then the `hook` MUST be injected for the stages set in `stages`. If *all* of the conditions set in `when` match, then the `hook` MUST be injected for the stages set in `stages`.
@ -114,10 +114,7 @@ Previous versions of CRI-O and libpod supported the 0.1.0 hook schema:
The injected hook's [`args`][spec-hooks] is `hook` with `arguments` appended. The injected hook's [`args`][spec-hooks] is `hook` with `arguments` appended.
* **`stages`** (REQUIRED, array of strings) Stages when the hook MUST be injected. * **`stages`** (REQUIRED, array of strings) Stages when the hook MUST be injected.
`stage` is an allowed synonym for this property, but you MUST NOT set both `stages` and `stage`. `stage` is an allowed synonym for this property, but you MUST NOT set both `stages` and `stage`.
Entries MUST be chosen from: Entries MUST be chosen from the 1.0.1 OCI Runtime Specification [hook stages][spec-hooks] or from extention stages supported by the package consumer.
* **`prestart`**, to inject [pre-start][].
* **`poststart`**, to inject [post-start][].
* **`poststop`**, to inject [post-stop][].
* **`cmds`** (OPTIONAL, array of strings) The hook MUST be injected if the configured [`process.args[0]`][spec-process] matches an entry. * **`cmds`** (OPTIONAL, array of strings) The hook MUST be injected if the configured [`process.args[0]`][spec-process] matches an entry.
`cmd` is an allowed synonym for this property, but you MUST NOT set both `cmds` and `cmd`. `cmd` is an allowed synonym for this property, but you MUST NOT set both `cmds` and `cmd`.
Entries MUST be [POSIX extended regular expressions][POSIX-ERE]. Entries MUST be [POSIX extended regular expressions][POSIX-ERE].

View File

@ -27,10 +27,11 @@ const (
// Manager provides an opaque interface for managing CRI-O hooks. // Manager provides an opaque interface for managing CRI-O hooks.
type Manager struct { type Manager struct {
hooks map[string]*current.Hook hooks map[string]*current.Hook
language language.Tag directories []string
directories []string extensionStages []string
lock sync.Mutex language language.Tag
lock sync.Mutex
} }
type namedHook struct { type namedHook struct {
@ -44,15 +45,16 @@ type namedHooks []*namedHook
// increasing preference (hook configurations in later directories // increasing preference (hook configurations in later directories
// override configurations with the same filename from earlier // override configurations with the same filename from earlier
// directories). // directories).
func New(ctx context.Context, directories []string, lang language.Tag) (manager *Manager, err error) { func New(ctx context.Context, directories []string, extensionStages []string, lang language.Tag) (manager *Manager, err error) {
manager = &Manager{ manager = &Manager{
hooks: map[string]*current.Hook{}, hooks: map[string]*current.Hook{},
directories: directories, directories: directories,
language: lang, extensionStages: extensionStages,
language: lang,
} }
for _, dir := range directories { for _, dir := range directories {
err = ReadDir(dir, manager.hooks) err = ReadDir(dir, manager.extensionStages, manager.hooks)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -80,14 +82,18 @@ func (m *Manager) namedHooks() (hooks []*namedHook) {
} }
// Hooks injects OCI runtime hooks for a given container configuration. // Hooks injects OCI runtime hooks for a given container configuration.
func (m *Manager) Hooks(config *rspec.Spec, annotations map[string]string, hasBindMounts bool) (err error) { func (m *Manager) Hooks(config *rspec.Spec, annotations map[string]string, hasBindMounts bool) (extensionStages map[string][]rspec.Hook, err error) {
hooks := m.namedHooks() hooks := m.namedHooks()
collator := collate.New(m.language, collate.IgnoreCase, collate.IgnoreWidth) collator := collate.New(m.language, collate.IgnoreCase, collate.IgnoreWidth)
collator.Sort(namedHooks(hooks)) collator.Sort(namedHooks(hooks))
validStages := map[string]bool{} // beyond the OCI stages
for _, stage := range m.extensionStages {
validStages[stage] = true
}
for _, namedHook := range hooks { for _, namedHook := range hooks {
match, err := namedHook.hook.When.Match(config, annotations, hasBindMounts) match, err := namedHook.hook.When.Match(config, annotations, hasBindMounts)
if err != nil { if err != nil {
return errors.Wrapf(err, "matching hook %q", namedHook.name) return extensionStages, errors.Wrapf(err, "matching hook %q", namedHook.name)
} }
if match { if match {
if config.Hooks == nil { if config.Hooks == nil {
@ -102,12 +108,19 @@ func (m *Manager) Hooks(config *rspec.Spec, annotations map[string]string, hasBi
case "poststop": case "poststop":
config.Hooks.Poststop = append(config.Hooks.Poststop, namedHook.hook.Hook) config.Hooks.Poststop = append(config.Hooks.Poststop, namedHook.hook.Hook)
default: default:
return fmt.Errorf("hook %q: unknown stage %q", namedHook.name, stage) if !validStages[stage] {
return extensionStages, fmt.Errorf("hook %q: unknown stage %q", namedHook.name, stage)
}
if extensionStages == nil {
extensionStages = map[string][]rspec.Hook{}
}
extensionStages[stage] = append(extensionStages[stage], namedHook.hook.Hook)
} }
} }
} }
} }
return nil
return extensionStages, nil
} }
// remove remove a hook by name. // remove remove a hook by name.
@ -125,7 +138,7 @@ func (m *Manager) remove(hook string) (ok bool) {
func (m *Manager) add(path string) (err error) { func (m *Manager) add(path string) (err error) {
m.lock.Lock() m.lock.Lock()
defer m.lock.Unlock() defer m.lock.Unlock()
hook, err := Read(path) hook, err := Read(path, m.extensionStages)
if err != nil { if err != nil {
return err return err
} }

View File

@ -48,13 +48,13 @@ func TestGoodNew(t *testing.T) {
t.Fatal(err) t.Fatal(err)
} }
manager, err := New(ctx, []string{dir}, lang) manager, err := New(ctx, []string{dir}, []string{}, lang)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
config := &rspec.Spec{} config := &rspec.Spec{}
err = manager.Hooks(config, map[string]string{}, false) extensionStages, err := manager.Hooks(config, map[string]string{}, false)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@ -90,6 +90,9 @@ func TestGoodNew(t *testing.T) {
}, },
}, },
}, config.Hooks) }, config.Hooks)
var nilExtensionStages map[string][]rspec.Hook
assert.Equal(t, nilExtensionStages, extensionStages)
} }
func TestBadNew(t *testing.T) { func TestBadNew(t *testing.T) {
@ -112,7 +115,7 @@ func TestBadNew(t *testing.T) {
t.Fatal(err) t.Fatal(err)
} }
_, err = New(ctx, []string{dir}, lang) _, err = New(ctx, []string{dir}, []string{}, lang)
if err == nil { if err == nil {
t.Fatal("unexpected success") t.Fatal("unexpected success")
} }
@ -139,11 +142,14 @@ func TestBrokenMatch(t *testing.T) {
Args: []string{"/bin/sh"}, Args: []string{"/bin/sh"},
}, },
} }
err := manager.Hooks(config, map[string]string{}, false) extensionStages, err := manager.Hooks(config, map[string]string{}, false)
if err == nil { if err == nil {
t.Fatal("unexpected success") t.Fatal("unexpected success")
} }
assert.Regexp(t, "^matching hook \"a\\.json\": command: error parsing regexp: .*", err.Error()) assert.Regexp(t, "^matching hook \"a\\.json\": command: error parsing regexp: .*", err.Error())
var nilExtensionStages map[string][]rspec.Hook
assert.Equal(t, nilExtensionStages, extensionStages)
} }
func TestInvalidStage(t *testing.T) { func TestInvalidStage(t *testing.T) {
@ -162,11 +168,60 @@ func TestInvalidStage(t *testing.T) {
}, },
}, },
} }
err := manager.Hooks(&rspec.Spec{}, map[string]string{}, false) extensionStages, err := manager.Hooks(&rspec.Spec{}, map[string]string{}, false)
if err == nil { if err == nil {
t.Fatal("unexpected success") t.Fatal("unexpected success")
} }
assert.Regexp(t, "^hook \"a\\.json\": unknown stage \"does-not-exist\"$", err.Error()) assert.Regexp(t, "^hook \"a\\.json\": unknown stage \"does-not-exist\"$", err.Error())
var nilExtensionStages map[string][]rspec.Hook
assert.Equal(t, nilExtensionStages, extensionStages)
}
func TestExtensionStage(t *testing.T) {
always := true
manager := Manager{
hooks: map[string]*current.Hook{
"a.json": {
Version: current.Version,
Hook: rspec.Hook{
Path: "/a/b/c",
},
When: current.When{
Always: &always,
},
Stages: []string{"prestart", "a", "b"},
},
},
extensionStages: []string{"a", "b", "c"},
}
config := &rspec.Spec{}
extensionStages, err := manager.Hooks(config, map[string]string{}, false)
if err != nil {
t.Fatal(err)
}
assert.Equal(t, &rspec.Hooks{
Prestart: []rspec.Hook{
{
Path: "/a/b/c",
},
},
}, config.Hooks)
assert.Equal(t, map[string][]rspec.Hook{
"a": {
{
Path: "/a/b/c",
},
},
"b": {
{
Path: "/a/b/c",
},
},
}, extensionStages)
} }
func init() { func init() {

View File

@ -27,7 +27,7 @@ func TestMonitorGood(t *testing.T) {
t.Fatal(err) t.Fatal(err)
} }
manager, err := New(ctx, []string{dir}, lang) manager, err := New(ctx, []string{dir}, []string{}, lang)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@ -50,7 +50,7 @@ func TestMonitorGood(t *testing.T) {
time.Sleep(100 * time.Millisecond) // wait for monitor to notice time.Sleep(100 * time.Millisecond) // wait for monitor to notice
config := &rspec.Spec{} config := &rspec.Spec{}
err = manager.Hooks(config, map[string]string{}, false) _, err = manager.Hooks(config, map[string]string{}, false)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@ -84,7 +84,7 @@ func TestMonitorGood(t *testing.T) {
config := &rspec.Spec{} config := &rspec.Spec{}
expected := config.Hooks expected := config.Hooks
err = manager.Hooks(config, map[string]string{}, false) _, err = manager.Hooks(config, map[string]string{}, false)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@ -101,7 +101,7 @@ func TestMonitorGood(t *testing.T) {
config := &rspec.Spec{} config := &rspec.Spec{}
expected := config.Hooks expected := config.Hooks
err = manager.Hooks(config, map[string]string{}, false) _, err = manager.Hooks(config, map[string]string{}, false)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@ -126,7 +126,7 @@ func TestMonitorBadWatcher(t *testing.T) {
t.Fatal(err) t.Fatal(err)
} }
manager, err := New(ctx, []string{}, lang) manager, err := New(ctx, []string{}, []string{}, lang)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }

View File

@ -25,7 +25,7 @@ var (
) )
// Read reads a hook JSON file, verifies it, and returns the hook configuration. // Read reads a hook JSON file, verifies it, and returns the hook configuration.
func Read(path string) (*current.Hook, error) { func Read(path string, extensionStages []string) (*current.Hook, error) {
if !strings.HasSuffix(path, ".json") { if !strings.HasSuffix(path, ".json") {
return nil, ErrNoJSONSuffix return nil, ErrNoJSONSuffix
} }
@ -37,7 +37,7 @@ func Read(path string) (*current.Hook, error) {
if err != nil { if err != nil {
return nil, errors.Wrapf(err, "parsing hook %q", path) return nil, errors.Wrapf(err, "parsing hook %q", path)
} }
err = hook.Validate() err = hook.Validate(extensionStages)
return hook, err return hook, err
} }
@ -60,14 +60,14 @@ func read(content []byte) (hook *current.Hook, err error) {
// ReadDir reads hook JSON files from a directory into the given map, // ReadDir reads hook JSON files from a directory into the given map,
// clobbering any previous entries with the same filenames. // clobbering any previous entries with the same filenames.
func ReadDir(path string, hooks map[string]*current.Hook) error { func ReadDir(path string, extensionStages []string, hooks map[string]*current.Hook) error {
files, err := ioutil.ReadDir(path) files, err := ioutil.ReadDir(path)
if err != nil { if err != nil {
return err return err
} }
for _, file := range files { for _, file := range files {
hook, err := Read(filepath.Join(path, file.Name())) hook, err := Read(filepath.Join(path, file.Name()), extensionStages)
if err != nil { if err != nil {
if err == ErrNoJSONSuffix { if err == ErrNoJSONSuffix {
continue continue

View File

@ -13,12 +13,12 @@ import (
) )
func TestNoJSONSuffix(t *testing.T) { func TestNoJSONSuffix(t *testing.T) {
_, err := Read("abc") _, err := Read("abc", []string{})
assert.Equal(t, err, ErrNoJSONSuffix) assert.Equal(t, err, ErrNoJSONSuffix)
} }
func TestUnknownPath(t *testing.T) { func TestUnknownPath(t *testing.T) {
_, err := Read(filepath.Join("does", "not", "exist.json")) _, err := Read(filepath.Join("does", "not", "exist.json"), []string{})
if err == nil { if err == nil {
t.Fatal("unexpected success") t.Fatal("unexpected success")
} }
@ -41,7 +41,7 @@ func TestGoodFile(t *testing.T) {
t.Fatal(err) t.Fatal(err)
} }
hook, err := Read(jsonPath) hook, err := Read(jsonPath, []string{})
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@ -71,7 +71,7 @@ func TestBadFile(t *testing.T) {
t.Fatal(err) t.Fatal(err)
} }
_, err = Read(path) _, err = Read(path, []string{})
if err == nil { if err == nil {
t.Fatal("unexpected success") t.Fatal("unexpected success")
} }
@ -139,7 +139,7 @@ func TestGoodDir(t *testing.T) {
} }
hooks := map[string]*current.Hook{} hooks := map[string]*current.Hook{}
err = ReadDir(dir, hooks) err = ReadDir(dir, []string{}, hooks)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@ -161,7 +161,7 @@ func TestGoodDir(t *testing.T) {
func TestUnknownDir(t *testing.T) { func TestUnknownDir(t *testing.T) {
hooks := map[string]*current.Hook{} hooks := map[string]*current.Hook{}
err := ReadDir(filepath.Join("does", "not", "exist"), hooks) err := ReadDir(filepath.Join("does", "not", "exist"), []string{}, hooks)
if err == nil { if err == nil {
t.Fatal("unexpected success") t.Fatal("unexpected success")
} }
@ -185,7 +185,7 @@ func TestBadDir(t *testing.T) {
} }
hooks := map[string]*current.Hook{} hooks := map[string]*current.Hook{}
err = ReadDir(dir, hooks) err = ReadDir(dir, []string{}, hooks)
if err == nil { if err == nil {
t.Fatal("unexpected success") t.Fatal("unexpected success")
} }