Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

Commit

Permalink
hooks: Send the bundle path in the state that is sent with hooks
Browse files Browse the repository at this point in the history
We currently just send the pid in the state. While OCI specifies
a few other fields as well, this commit just adds the bundle path
and the container id to the state. This should fix the errors seen
with hooks that rely on the bundle path.

Other fields like running "state" string have been left out. As this
would need sending the strings that OCI recognises. Hooks have been
implemented in virtcontainers and sending the state string would
require calling into OCI specific code in virtcontainers.

The bundle path again is OCI specific, but this can be accessed
using annotations. Hooks really need to be moved to the cli as they
are OCI specific. This however needs network hotplug to be implemented
first so that the hooks can be called from the cli after the
VM has been created.

Fixes #271

Signed-off-by: Archana Shinde <[email protected]>
  • Loading branch information
amshinde committed Apr 27, 2018
1 parent 31eb51e commit a301a9e
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 27 deletions.
4 changes: 2 additions & 2 deletions virtcontainers/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ func startSandbox(p *Sandbox) (*Sandbox, error) {
}

// Execute poststart hooks.
if err := p.config.Hooks.postStartHooks(); err != nil {
if err := p.config.Hooks.postStartHooks(p); err != nil {
return nil, err
}

Expand Down Expand Up @@ -180,7 +180,7 @@ func StopSandbox(sandboxID string) (VCSandbox, error) {
}

// Execute poststop hooks.
if err := p.config.Hooks.postStopHooks(); err != nil {
if err := p.config.Hooks.postStopHooks(p); err != nil {
return nil, err
}

Expand Down
24 changes: 14 additions & 10 deletions virtcontainers/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"syscall"
"time"

vcAnnotations "github.com/kata-containers/runtime/virtcontainers/pkg/annotations"
specs "github.com/opencontainers/runtime-spec/specs-go"
"github.com/sirupsen/logrus"
)
Expand All @@ -38,14 +39,17 @@ func (h *Hooks) Logger() *logrus.Entry {
return virtLog.WithField("subsystem", "hooks")
}

func buildHookState(processID int) specs.State {
func buildHookState(processID int, s *Sandbox) specs.State {
annotations := s.GetAnnotations()
return specs.State{
Pid: processID,
Pid: processID,
Bundle: annotations[vcAnnotations.BundlePathKey],
ID: s.id,
}
}

func (h *Hook) runHook() error {
state := buildHookState(os.Getpid())
func (h *Hook) runHook(s *Sandbox) error {
state := buildHookState(os.Getpid(), s)
stateJSON, err := json.Marshal(state)
if err != nil {
return err
Expand Down Expand Up @@ -95,13 +99,13 @@ func (h *Hook) runHook() error {
return nil
}

func (h *Hooks) preStartHooks() error {
func (h *Hooks) preStartHooks(s *Sandbox) error {
if len(h.PreStartHooks) == 0 {
return nil
}

for _, hook := range h.PreStartHooks {
err := hook.runHook()
err := hook.runHook(s)
if err != nil {
h.Logger().WithFields(logrus.Fields{
"hook-type": "pre-start",
Expand All @@ -115,13 +119,13 @@ func (h *Hooks) preStartHooks() error {
return nil
}

func (h *Hooks) postStartHooks() error {
func (h *Hooks) postStartHooks(s *Sandbox) error {
if len(h.PostStartHooks) == 0 {
return nil
}

for _, hook := range h.PostStartHooks {
err := hook.runHook()
err := hook.runHook(s)
if err != nil {
// In case of post start hook, the error is not fatal,
// just need to be logged.
Expand All @@ -135,13 +139,13 @@ func (h *Hooks) postStartHooks() error {
return nil
}

func (h *Hooks) postStopHooks() error {
func (h *Hooks) postStopHooks(s *Sandbox) error {
if len(h.PostStopHooks) == 0 {
return nil
}

for _, hook := range h.PostStopHooks {
err := hook.runHook()
err := hook.runHook(s)
if err != nil {
// In case of post stop hook, the error is not fatal,
// just need to be logged.
Expand Down
69 changes: 55 additions & 14 deletions virtcontainers/hook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ import (
"os"
"path/filepath"
"reflect"
"sync"
"testing"

vcAnnotations "github.com/kata-containers/runtime/virtcontainers/pkg/annotations"
. "github.com/kata-containers/runtime/virtcontainers/pkg/mock"
specs "github.com/opencontainers/runtime-spec/specs-go"
)
Expand All @@ -22,6 +24,7 @@ var testContainerIDHook = "test-container-id"
var testControllerIDHook = "test-controller-id"
var testProcessIDHook = 12345
var testBinHookPath = "/usr/bin/virtcontainers/bin/test/hook"
var testBundlePath = "/test/bundle"

func getMockHookBinPath() string {
if DefaultMockHookBinPath == "" {
Expand All @@ -32,15 +35,32 @@ func getMockHookBinPath() string {
}

func TestBuildHookState(t *testing.T) {
t.Skip()
expected := specs.State{
Pid: testProcessIDHook,
}

hookState := buildHookState(testProcessIDHook)
s := &Sandbox{}

hookState := buildHookState(testProcessIDHook, s)

if reflect.DeepEqual(hookState, expected) == false {
t.Fatal()
}

s = createTestSandbox()
hookState = buildHookState(testProcessIDHook, s)

expected = specs.State{
Pid: testProcessIDHook,
Bundle: testBundlePath,
ID: testSandboxID,
}

if reflect.DeepEqual(hookState, expected) == false {
t.Fatal()
}

}

func createHook(timeout int) *Hook {
Expand All @@ -60,10 +80,24 @@ func createWrongHook() *Hook {
}
}

func createTestSandbox() *Sandbox {
c := &SandboxConfig{
Annotations: map[string]string{
vcAnnotations.BundlePathKey: testBundlePath,
},
}
return &Sandbox{
annotationsLock: &sync.RWMutex{},
config: c,
id: testSandboxID,
}
}

func testRunHookFull(t *testing.T, timeout int, expectFail bool) {
hook := createHook(timeout)

err := hook.runHook()
s := createTestSandbox()
err := hook.runHook(s)
if expectFail {
if err == nil {
t.Fatal("unexpected success")
Expand Down Expand Up @@ -91,8 +125,9 @@ func TestRunHookTimeout(t *testing.T) {

func TestRunHookExitFailure(t *testing.T) {
hook := createWrongHook()
s := createTestSandbox()

err := hook.runHook()
err := hook.runHook(s)
if err == nil {
t.Fatal()
}
Expand All @@ -103,7 +138,9 @@ func TestRunHookTimeoutFailure(t *testing.T) {

hook.Args = append(hook.Args, "2")

err := hook.runHook()
s := createTestSandbox()

err := hook.runHook(s)
if err == nil {
t.Fatal()
}
Expand All @@ -113,8 +150,9 @@ func TestRunHookWaitFailure(t *testing.T) {
hook := createHook(60)

hook.Args = append(hook.Args, "1", "panic")
s := createTestSandbox()

err := hook.runHook()
err := hook.runHook(s)
if err == nil {
t.Fatal()
}
Expand Down Expand Up @@ -156,18 +194,19 @@ func testHooks(t *testing.T, hook *Hook) {
PostStartHooks: []Hook{*hook},
PostStopHooks: []Hook{*hook},
}
s := createTestSandbox()

err := hooks.preStartHooks()
err := hooks.preStartHooks(s)
if err != nil {
t.Fatal(err)
}

err = hooks.postStartHooks()
err = hooks.postStartHooks(s)
if err != nil {
t.Fatal(err)
}

err = hooks.postStopHooks()
err = hooks.postStopHooks(s)
if err != nil {
t.Fatal(err)
}
Expand All @@ -179,18 +218,19 @@ func testFailingHooks(t *testing.T, hook *Hook) {
PostStartHooks: []Hook{*hook},
PostStopHooks: []Hook{*hook},
}
s := createTestSandbox()

err := hooks.preStartHooks()
err := hooks.preStartHooks(s)
if err == nil {
t.Fatal(err)
}

err = hooks.postStartHooks()
err = hooks.postStartHooks(s)
if err != nil {
t.Fatal(err)
}

err = hooks.postStopHooks()
err = hooks.postStopHooks(s)
if err != nil {
t.Fatal(err)
}
Expand All @@ -210,18 +250,19 @@ func TestFailingHooks(t *testing.T) {

func TestEmptyHooks(t *testing.T) {
hooks := &Hooks{}
s := createTestSandbox()

err := hooks.preStartHooks()
err := hooks.preStartHooks(s)
if err != nil {
t.Fatal(err)
}

err = hooks.postStartHooks()
err = hooks.postStartHooks(s)
if err != nil {
t.Fatal(err)
}

err = hooks.postStopHooks()
err = hooks.postStopHooks(s)
if err != nil {
t.Fatal(err)
}
Expand Down
2 changes: 1 addition & 1 deletion virtcontainers/sandbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -820,7 +820,7 @@ func (s *Sandbox) createNetwork() error {

// Execute prestart hooks inside netns
if err := s.network.run(netNsPath, func() error {
return s.config.Hooks.preStartHooks()
return s.config.Hooks.preStartHooks(s)
}); err != nil {
return err
}
Expand Down

0 comments on commit a301a9e

Please sign in to comment.