sync up HookState with OCI spec State

`HookState` struct should follow definition of `State` in runtime-spec: * modify json name of `version` to `ociVersion`. * Remove redundant `Rootfs` field as rootfs can be retrived from `bundlePath/config.json`. Signed-off-by: Zhang Wei <zhangwei555@huawei.com>
kinvolk · Dec 19, 2016 · a344b2d · a344b2d
1 parent 9a1e53e
commit a344b2d
Show file tree

Hide file tree

Showing 7 changed files with 73 additions and 36 deletions.
diff --git a/libcontainer/configs/config.go b/libcontainer/configs/config.go
@@ -8,6 +8,7 @@ import (
 	"time"
 
 	"github.com/Sirupsen/logrus"
+	"github.com/opencontainers/runtime-spec/specs-go"
 )
 
 type Rlimit struct {
@@ -243,13 +244,7 @@ func (hooks Hooks) MarshalJSON() ([]byte, error) {
 }
 
 // HookState is the payload provided to a hook on execution.
-type HookState struct {
-	Version    string `json:"ociVersion"`
-	ID         string `json:"id"`
-	Pid        int    `json:"pid"`
-	Root       string `json:"root"`
-	BundlePath string `json:"bundlePath"`
-}
+type HookState specs.State
 
 type Hook interface {
 	// Run executes the hook with the provided state.

diff --git a/libcontainer/configs/config_test.go b/libcontainer/configs/config_test.go
@@ -120,10 +120,10 @@ func TestMarshalHooksWithUnexpectedType(t *testing.T) {
 
 func TestFuncHookRun(t *testing.T) {
 	state := configs.HookState{
-		Version: "1",
-		ID:      "1",
-		Pid:     1,
-		Root:    "root",
+		Version:    "1",
+		ID:         "1",
+		Pid:        1,
+		BundlePath: "/bundle",
 	}
 
 	fHook := configs.NewFunctionHook(func(s configs.HookState) error {
@@ -138,10 +138,10 @@ func TestFuncHookRun(t *testing.T) {
 
 func TestCommandHookRun(t *testing.T) {
 	state := configs.HookState{
-		Version: "1",
-		ID:      "1",
-		Pid:     1,
-		Root:    "root",
+		Version:    "1",
+		ID:         "1",
+		Pid:        1,
+		BundlePath: "/bundle",
 	}
 	timeout := time.Second
 
@@ -161,10 +161,10 @@ func TestCommandHookRun(t *testing.T) {
 
 func TestCommandHookRunTimeout(t *testing.T) {
 	state := configs.HookState{
-		Version: "1",
-		ID:      "1",
-		Pid:     1,
-		Root:    "root",
+		Version:    "1",
+		ID:         "1",
+		Pid:        1,
+		BundlePath: "/bundle",
 	}
 	timeout := (10 * time.Millisecond)
 

diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go
@@ -266,7 +266,6 @@ func (c *linuxContainer) start(process *Process, isInit bool) error {
 				Version:    c.config.Version,
 				ID:         c.id,
 				Pid:        parent.pid(),
-				Root:       c.config.Rootfs,
 				BundlePath: utils.SearchLabels(c.config.Labels, "bundle"),
 			}
 			for i, hook := range c.config.Hooks.Poststart {
@@ -1038,10 +1037,10 @@ func (c *linuxContainer) criuNotifications(resp *criurpc.CriuResp, process *Proc
 	case notify.GetScript() == "setup-namespaces":
 		if c.config.Hooks != nil {
 			s := configs.HookState{
-				Version: c.config.Version,
-				ID:      c.id,
-				Pid:     int(notify.GetPid()),
-				Root:    c.config.Rootfs,
+				Version:    c.config.Version,
+				ID:         c.id,
+				Pid:        int(notify.GetPid()),
+				BundlePath: utils.SearchLabels(c.config.Labels, "bundle"),
 			}
 			for i, hook := range c.config.Hooks.Prestart {
 				if err := hook.Run(s); err != nil {

diff --git a/libcontainer/integration/exec_test.go b/libcontainer/integration/exec_test.go
@@ -2,6 +2,7 @@ package integration
 
 import (
 	"bytes"
+	"encoding/json"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -1048,25 +1049,44 @@ func TestHook(t *testing.T) {
 	if testing.Short() {
 		return
 	}
-	root, err := newTestRoot()
+
+	bundle, err := newTestBundle()
 	ok(t, err)
-	defer os.RemoveAll(root)
+	defer remove(bundle)
 
 	rootfs, err := newRootfs()
 	ok(t, err)
 	defer remove(rootfs)
 
 	config := newTemplateConfig(rootfs)
-	expectedBundlePath := "/path/to/bundle/path"
+	expectedBundlePath := bundle
 	config.Labels = append(config.Labels, fmt.Sprintf("bundle=%s", expectedBundlePath))
+
+	getRootfsFromBundle := func(bundle string) (string, error) {
+		f, err := os.Open(filepath.Join(bundle, "config.json"))
+		if err != nil {
+			return "", err
+		}
+
+		var config configs.Config
+		if err = json.NewDecoder(f).Decode(&config); err != nil {
+			return "", err
+		}
+		return config.Rootfs, nil
+	}
+
 	config.Hooks = &configs.Hooks{
 		Prestart: []configs.Hook{
 			configs.NewFunctionHook(func(s configs.HookState) error {
 				if s.BundlePath != expectedBundlePath {
 					t.Fatalf("Expected prestart hook bundlePath '%s'; got '%s'", expectedBundlePath, s.BundlePath)
 				}
 
-				f, err := os.Create(filepath.Join(s.Root, "test"))
+				root, err := getRootfsFromBundle(s.BundlePath)
+				if err != nil {
+					return err
+				}
+				f, err := os.Create(filepath.Join(root, "test"))
 				if err != nil {
 					return err
 				}
@@ -1079,7 +1099,11 @@ func TestHook(t *testing.T) {
 					t.Fatalf("Expected poststart hook bundlePath '%s'; got '%s'", expectedBundlePath, s.BundlePath)
 				}
 
-				return ioutil.WriteFile(filepath.Join(s.Root, "test"), []byte("hello world"), 0755)
+				root, err := getRootfsFromBundle(s.BundlePath)
+				if err != nil {
+					return err
+				}
+				return ioutil.WriteFile(filepath.Join(root, "test"), []byte("hello world"), 0755)
 			}),
 		},
 		Poststop: []configs.Hook{
@@ -1088,10 +1112,20 @@ func TestHook(t *testing.T) {
 					t.Fatalf("Expected poststop hook bundlePath '%s'; got '%s'", expectedBundlePath, s.BundlePath)
 				}
 
-				return os.RemoveAll(filepath.Join(s.Root, "test"))
+				root, err := getRootfsFromBundle(s.BundlePath)
+				if err != nil {
+					return err
+				}
+				return os.RemoveAll(filepath.Join(root, "test"))
 			}),
 		},
 	}
+
+	// write config of json format into config.json under bundle
+	f, err := os.OpenFile(filepath.Join(bundle, "config.json"), os.O_CREATE|os.O_RDWR, 0644)
+	ok(t, err)
+	ok(t, json.NewEncoder(f).Encode(config))
+
 	container, err := factory.Create("test", config)
 	ok(t, err)
 

diff --git a/libcontainer/integration/utils_test.go b/libcontainer/integration/utils_test.go
@@ -76,6 +76,17 @@ func newTestRoot() (string, error) {
 	return dir, nil
 }
 
+func newTestBundle() (string, error) {
+	dir, err := ioutil.TempDir("", "bundle")
+	if err != nil {
+		return "", err
+	}
+	if err := os.MkdirAll(dir, 0700); err != nil {
+		return "", err
+	}
+	return dir, nil
+}
+
 // newRootfs creates a new tmp directory and copies the busybox root filesystem
 func newRootfs() (string, error) {
 	dir, err := ioutil.TempDir("", "")

diff --git a/libcontainer/process_linux.go b/libcontainer/process_linux.go
@@ -339,10 +339,10 @@ func (p *initProcess) start() error {
 			if !p.config.Config.Namespaces.Contains(configs.NEWNS) {
 				if p.config.Config.Hooks != nil {
 					s := configs.HookState{
-						Version: p.container.config.Version,
-						ID:      p.container.id,
-						Pid:     p.pid(),
-						Root:    p.config.Config.Rootfs,
+						Version:    p.container.config.Version,
+						ID:         p.container.id,
+						Pid:        p.pid(),
+						BundlePath: utils.SearchLabels(p.config.Config.Labels, "bundle"),
 					}
 					for i, hook := range p.config.Config.Hooks.Prestart {
 						if err := hook.Run(s); err != nil {
@@ -362,7 +362,6 @@ func (p *initProcess) start() error {
 					Version:    p.container.config.Version,
 					ID:         p.container.id,
 					Pid:        p.pid(),
-					Root:       p.config.Config.Rootfs,
 					BundlePath: utils.SearchLabels(p.config.Config.Labels, "bundle"),
 				}
 				for i, hook := range p.config.Config.Hooks.Prestart {

diff --git a/libcontainer/state_linux.go b/libcontainer/state_linux.go
@@ -60,7 +60,6 @@ func runPoststopHooks(c *linuxContainer) error {
 		s := configs.HookState{
 			Version:    c.config.Version,
 			ID:         c.id,
-			Root:       c.config.Rootfs,
 			BundlePath: utils.SearchLabels(c.config.Labels, "bundle"),
 		}
 		for _, hook := range c.config.Hooks.Poststop {