While writing a hook injector unit test for NRI I noticed that the OCI Prestart hook isn't getting appended as expected to &rspec.Hooks{}
The OCI runtime spec includes a Prestart hook that is deprecated, but is still currently expected to work
From discussing with @mikebrow it seems some changes were made in a previous commit on this repo so that the Prestart hook could be handled as CreateRuntime. But this removed the append() into &rspec.Hooks.Prestart which seems to have led to the mentioned issue in my test
For example, in the following, snippet, hooks ends up being populated with CreateRuntime instead of Prestart:
func testCreateContainerWithHooks(t *testing.T) {
t.Helper()
tempDir := t.TempDir()
hookJSON := []byte(`{
"version": "1.0.0",
"hook": {
"path": "/bin/echo",
"args": ["echo", "testing from hook"]
},
"when": {
"always": true
},
"stages": ["prestart"]
}`)
hookPath := filepath.Join(tempDir, "test-hook.json")
err := os.WriteFile(hookPath, hookJSON, 0644)
assert.NoError(t, err)
mgr, err := hooks.New(context.Background(), []string{tempDir}, []string{})
assert.NoError(t, err)
p := &plugin{mgr: mgr}
pod, container := createTestPodAndContainer()
adjust, updates, err := p.CreateContainer(context.Background(), pod, container)
assert.NoError(t, err)
assert.NotNil(t, adjust)
assert.Nil(t, updates)
hooks := adjust.Hooks
t.Log("Hooks:", hooks)
assert.NotNil(t, hooks.Hooks())
assert.NotEmpty(t, hooks.Prestart, "expected prestart hooks to be injected") // fails here because hooks has CreateRuntime instead of Prestart
[...]% go test -v .
=== RUN TestHookInjector
[...]
=== RUN TestHookInjector/hooks_injected_correctly
time="2026-01-27T10:48:56-05:00" level=info msg="test-pod-hook-injector/test-container-hook-injector: OCI hooks injected"
hook-injector_test.go:40: Hooks: create_runtime:{path:"/bin/echo" args:"echo" args:"testing from hook"}
hook-injector_test.go:40:
Error Trace: [...]/nri/plugins/hook-injector/hook-injector_test.go:101
[...]/nri/plugins/hook-injector/hook-injector_test.go:40
Error: Should NOT be empty, but was []
Test: TestHookInjector/hooks_injected_correctly
Messages: expected prestart hooks to be injected
hook-injector_test.go:40:
Error Trace: [...]/nri/plugins/hook-injector/hook-injector_test.go:110cc @giuseppe @mikebrow @haircommander - would it be possible to add an append() in the switch case statement so the Prestart hook gets added as well? i.e. add config.Hooks.Prestart = append(config.Hooks.Prestart, namedHook.hook.Hook) in here:
|
case "createRuntime", "prestart": |
|
config.Hooks.CreateRuntime = append(config.Hooks.CreateRuntime, namedHook.hook.Hook) |
While writing a hook injector unit test for NRI I noticed that the OCI Prestart hook isn't getting appended as expected to
&rspec.Hooks{}The OCI runtime spec includes a Prestart hook that is deprecated, but is still currently expected to work
From discussing with @mikebrow it seems some changes were made in a previous commit on this repo so that the
Prestarthook could be handled asCreateRuntime. But this removed theappend()into&rspec.Hooks.Prestartwhich seems to have led to the mentioned issue in my testFor example, in the following, snippet,
hooksends up being populated withCreateRuntimeinstead ofPrestart:cc @giuseppe @mikebrow @haircommander - would it be possible to add an
append()in the switch case statement so thePrestarthook gets added as well? i.e. addconfig.Hooks.Prestart = append(config.Hooks.Prestart, namedHook.hook.Hook)in here:common/pkg/hooks/hooks.go
Lines 125 to 126 in dc0f263