support json value for hooks

As discussed in opencontainers#470, there is not suitable separators for hooks. Because of path may contains them. So, decide to support json value for hooks. Signed-off-by: Ma Shimiao <[email protected]>
Mashimiao · Nov 28, 2017 · 6e78ff2 · 6e78ff2
1 parent 8e3ecf4
commit 6e78ff2
Show file tree

Hide file tree

Showing 4 changed files with 86 additions and 244 deletions.
diff --git a/cmd/oci-runtime-tool/generate.go b/cmd/oci-runtime-tool/generate.go
@@ -20,15 +20,12 @@ var generateFlags = []cli.Flag{
 	cli.StringSliceFlag{Name: "args", Usage: "command to run in the container"},
 	cli.StringSliceFlag{Name: "env", Usage: "add environment variable e.g. key=value"},
 	cli.StringSliceFlag{Name: "env-file", Usage: "read in a file of environment variables"},
-	cli.StringSliceFlag{Name: "hooks-poststart", Usage: "set command to run in poststart hooks"},
-	cli.StringSliceFlag{Name: "hooks-poststart-env", Usage: "set environment variables for commands to run in poststart hooks"},
-	cli.StringSliceFlag{Name: "hooks-poststart-timeout", Usage: "set timeout for commands to run in poststart hooks"},
-	cli.StringSliceFlag{Name: "hooks-poststop", Usage: "set command to run in poststop hooks"},
-	cli.StringSliceFlag{Name: "hooks-poststop-env", Usage: "set environment variables for commands to run in poststop hooks"},
-	cli.StringSliceFlag{Name: "hooks-poststop-timeout", Usage: "set timeout for commands to run in poststop hooks"},
-	cli.StringSliceFlag{Name: "hooks-prestart", Usage: "set command to run in prestart hooks"},
-	cli.StringSliceFlag{Name: "hooks-prestart-env", Usage: "set environment variables for commands to run in prestart hooks"},
-	cli.StringSliceFlag{Name: "hooks-prestart-timeout", Usage: "set timeout for commands to run in prestart hooks"},
+	cli.StringSliceFlag{Name: "hooks-poststart-add", Usage: "set command to run in poststart hooks"},
+	cli.BoolFlag{Name: "hooks-poststart-remove-all", Usage: "remove all poststart hooks"},
+	cli.StringSliceFlag{Name: "hooks-poststop-add", Usage: "set command to run in poststop hooks"},
+	cli.BoolFlag{Name: "hooks-poststop-remove-all", Usage: "remove all poststop hooks"},
+	cli.StringSliceFlag{Name: "hooks-prestart-add", Usage: "set command to run in prestart hooks"},
+	cli.BoolFlag{Name: "hooks-prestart-remove-all", Usage: "remove all prestart hooks"},
 	cli.StringFlag{Name: "hostname", Usage: "hostname value for the container"},
 	cli.StringSliceFlag{Name: "label", Usage: "add annotations to the configuration e.g. key=value"},
 	cli.StringFlag{Name: "linux-apparmor", Usage: "specifies the the apparmor profile for the container"},
@@ -442,102 +439,45 @@ func setupSpec(g *generate.Generator, context *cli.Context) error {
 		}
 	}
 
-	if context.IsSet("hooks-poststart") {
-		postStartHooks := context.StringSlice("hooks-poststart")
-		for _, hook := range postStartHooks {
-			path, args, err := parseHook(hook)
-			if err != nil {
-				return err
-			}
-			g.AddPostStartHook(path, args)
-		}
+	if context.IsSet("hooks-poststart-remove-all") {
+		g.ClearPostStartHooks()
 	}
 
-	if context.IsSet("hooks-poststart-env") {
-		postStartEnvs := context.StringSlice("hooks-poststart-env")
-		for _, postStartEnv := range postStartEnvs {
-			path, env, err := parseHookEnv(postStartEnv)
+	if context.IsSet("hooks-poststart-add") {
+		postStartHooks := context.StringSlice("hooks-poststart-add")
+		for _, hook := range postStartHooks {
+			err := g.AddPostStartHook(hook)
 			if err != nil {
 				return err
 			}
-			g.AddPostStartHookEnv(path, env)
 		}
 	}
 
-	if context.IsSet("hooks-poststart-timeout") {
-		postStartTimeouts := context.StringSlice("hooks-poststart-timeout")
-		for _, postStartTimeout := range postStartTimeouts {
-			path, timeout, err := parseHookTimeout(postStartTimeout)
-			if err != nil {
-				return err
-			}
-			g.AddPostStartHookTimeout(path, timeout)
-		}
+	if context.IsSet("hooks-poststop-remove-all") {
+		g.ClearPostStopHooks()
 	}
 
-	if context.IsSet("hooks-poststop") {
-		postStopHooks := context.StringSlice("hooks-poststop")
+	if context.IsSet("hooks-poststop-add") {
+		postStopHooks := context.StringSlice("hooks-poststop-add")
 		for _, hook := range postStopHooks {
-			path, args, err := parseHook(hook)
-			if err != nil {
-				return err
-			}
-			g.AddPostStopHook(path, args)
-		}
-	}
-
-	if context.IsSet("hooks-poststop-env") {
-		postStopEnvs := context.StringSlice("hooks-poststop-env")
-		for _, postStopEnv := range postStopEnvs {
-			path, env, err := parseHookEnv(postStopEnv)
+			err := g.AddPostStopHook(hook)
 			if err != nil {
 				return err
 			}
-			g.AddPostStopHookEnv(path, env)
 		}
 	}
 
-	if context.IsSet("hooks-poststop-timeout") {
-		postStopTimeouts := context.StringSlice("hooks-poststop-timeout")
-		for _, postStopTimeout := range postStopTimeouts {
-			path, timeout, err := parseHookTimeout(postStopTimeout)
-			if err != nil {
-				return err
-			}
-			g.AddPostStopHookTimeout(path, timeout)
-		}
+	if context.IsSet("hooks-prestart-remove-all") {
+		g.ClearPreStartHooks()
 	}
 
-	if context.IsSet("hooks-prestart") {
-		preStartHooks := context.StringSlice("hooks-prestart")
+	if context.IsSet("hooks-prestart-add") {
+		preStartHooks := context.StringSlice("hooks-prestart-add")
 		for _, hook := range preStartHooks {
-			path, args, err := parseHook(hook)
-			if err != nil {
-				return err
-			}
-			g.AddPreStartHook(path, args)
-		}
-	}
-
-	if context.IsSet("hooks-prestart-env") {
-		preStartEnvs := context.StringSlice("hooks-prestart-env")
-		for _, preStartEnv := range preStartEnvs {
-			path, env, err := parseHookEnv(preStartEnv)
+			err := g.AddPreStartHook(hook)
 			if err != nil {
 				return err
 			}
-			g.AddPreStartHookEnv(path, env)
-		}
-	}
-
-	if context.IsSet("hooks-prestart-timeout") {
-		preStartTimeouts := context.StringSlice("hooks-prestart-timeout")
-		for _, preStartTimeout := range preStartTimeouts {
-			path, timeout, err := parseHookTimeout(preStartTimeout)
-			if err != nil {
-				return err
-			}
-			g.AddPreStartHookTimeout(path, timeout)
 		}
 	}
 
@@ -804,44 +744,6 @@ func parseHugepageLimit(pageLimit string) (string, uint64, error) {
 	return pl[0], uint64(limit), nil
 }
 
-func parseHook(s string) (string, []string, error) {
-	args := []string{}
-	parts := strings.Split(s, ":")
-	if len(parts) > 1 && parts[0] == "" {
-		return "", args, fmt.Errorf("invalid hook value: %s", s)
-	}
-	path := parts[0]
-	if len(parts) > 1 {
-		args = parts[1:]
-	}
-	return path, args, nil
-}
-
-func parseHookEnv(s string) (string, []string, error) {
-	parts := strings.Split(s, ":")
-	envs := []string{}
-	if len(parts) < 2 {
-		return "", envs, fmt.Errorf("invalid format: %s", s)
-	}
-	envs = parts[1:]
-
-	return parts[0], envs, nil
-}
-
-func parseHookTimeout(s string) (string, int, error) {
-	parts := strings.Split(s, ":")
-	if len(parts) != 2 {
-		return "", 0, fmt.Errorf("invalid format: %s", s)
-	}
-
-	timeout, err := strconv.Atoi(parts[1])
-	if err != nil {
-		return "", 0, err
-	}
-
-	return parts[0], timeout, nil
-}
-
 func parseNetworkPriority(np string) (string, int32, error) {
 	var err error
 

diff --git a/completions/bash/oci-runtime-tool b/completions/bash/oci-runtime-tool
@@ -305,15 +305,9 @@ _oci-runtime-tool_generate() {
 		--args
 		--env
 		--env-file
-		--hooks-poststart
-		--hooks-poststart-env
-		--hooks-poststart-timeout
-		--hooks-poststop
-		--hooks-poststop-env
-		--hooks-poststop-timeout
-		--hooks-prestart
-		--hooks-prestart-env
-		--hooks-prestart-timeout
+		--hooks-poststart-add
+		--hooks-poststop-add
+		--hooks-prestart-add
 		--hostname
 		--label
 		--linux-apparmor
@@ -387,6 +381,9 @@ _oci-runtime-tool_generate() {
 
 	local boolean_options="
 		--help -h
+		--hooks-poststart-remove-all
+		--hooks-poststop-remove-all
+		--hooks-prestart-remove-all
 		--linux-device-remove-all
 		--linux-disable-oom-kill
 		--linux-namespace-remove-all
@@ -415,7 +412,7 @@ _oci-runtime-tool_generate() {
 			return
 			;;
 
-		--hooks-poststart|--hooks-poststop|--hooks-prestart)
+		--hooks-poststart-add|--hooks-poststop-add|--hooks-prestart-add)
 			COMPREPLY=( $( compgen -W "$( __oci-runtime-tool_hooks )" -- "$cur" ) )
 			__oci-runtime-tool_nospace
 			return

diff --git a/generate/generate.go b/generate/generate.go
@@ -743,42 +743,21 @@ func (g *Generator) ClearPreStartHooks() {
 }
 
 // AddPreStartHook add a prestart hook into g.spec.Hooks.Prestart.
-func (g *Generator) AddPreStartHook(path string, args []string) {
+func (g *Generator) AddPreStartHook(hookObject string) error {
 	g.initSpecHooks()
-	hook := rspec.Hook{Path: path, Args: args}
-	for i, hook := range g.spec.Hooks.Prestart {
-		if hook.Path == path {
-			g.spec.Hooks.Prestart[i] = hook
-			return
-		}
+	tmpHook := rspec.Hook{}
+	err := json.Unmarshal([]byte(hookObject), &tmpHook)
+	if err != nil {
+		return err
 	}
-	g.spec.Hooks.Prestart = append(g.spec.Hooks.Prestart, hook)
-}
-
-// AddPreStartHookEnv adds envs of a prestart hook into g.spec.Hooks.Prestart.
-func (g *Generator) AddPreStartHookEnv(path string, envs []string) {
-	g.initSpecHooks()
 	for i, hook := range g.spec.Hooks.Prestart {
-		if hook.Path == path {
-			g.spec.Hooks.Prestart[i].Env = envs
-			return
-		}
-	}
-	hook := rspec.Hook{Path: path, Env: envs}
-	g.spec.Hooks.Prestart = append(g.spec.Hooks.Prestart, hook)
-}
-
-// AddPreStartHookTimeout adds timeout of a prestart hook into g.spec.Hooks.Prestart.
-func (g *Generator) AddPreStartHookTimeout(path string, timeout int) {
-	g.initSpecHooks()
-	for i, hook := range g.spec.Hooks.Prestart {
-		if hook.Path == path {
-			g.spec.Hooks.Prestart[i].Timeout = &timeout
-			return
+		if hook.Path == tmpHook.Path {
+			g.spec.Hooks.Prestart[i] = tmpHook
+			return nil
 		}
 	}
-	hook := rspec.Hook{Path: path, Timeout: &timeout}
-	g.spec.Hooks.Prestart = append(g.spec.Hooks.Prestart, hook)
+	g.spec.Hooks.Prestart = append(g.spec.Hooks.Prestart, tmpHook)
+	return nil
 }
 
 // ClearPostStopHooks clear g.spec.Hooks.Poststop.
@@ -790,42 +769,21 @@ func (g *Generator) ClearPostStopHooks() {
 }
 
 // AddPostStopHook adds a poststop hook into g.spec.Hooks.Poststop.
-func (g *Generator) AddPostStopHook(path string, args []string) {
+func (g *Generator) AddPostStopHook(hookObject string) error {
 	g.initSpecHooks()
-	hook := rspec.Hook{Path: path, Args: args}
-	for i, hook := range g.spec.Hooks.Poststop {
-		if hook.Path == path {
-			g.spec.Hooks.Poststop[i] = hook
-			return
-		}
-	}
-	g.spec.Hooks.Poststop = append(g.spec.Hooks.Poststop, hook)
-}
-
-// AddPostStopHookEnv adds envs of a poststop hook into g.spec.Hooks.Poststop.
-func (g *Generator) AddPostStopHookEnv(path string, envs []string) {
-	g.initSpecHooks()
-	for i, hook := range g.spec.Hooks.Poststop {
-		if hook.Path == path {
-			g.spec.Hooks.Poststop[i].Env = envs
-			return
-		}
+	tmpHook := rspec.Hook{}
+	err := json.Unmarshal([]byte(hookObject), &tmpHook)
+	if err != nil {
+		return err
 	}
-	hook := rspec.Hook{Path: path, Env: envs}
-	g.spec.Hooks.Poststop = append(g.spec.Hooks.Poststop, hook)
-}
-
-// AddPostStopHookTimeout adds timeout of a poststop hook into g.spec.Hooks.Poststop.
-func (g *Generator) AddPostStopHookTimeout(path string, timeout int) {
-	g.initSpecHooks()
 	for i, hook := range g.spec.Hooks.Poststop {
-		if hook.Path == path {
-			g.spec.Hooks.Poststop[i].Timeout = &timeout
-			return
+		if hook.Path == tmpHook.Path {
+			g.spec.Hooks.Poststop[i] = tmpHook
+			return nil
 		}
 	}
-	hook := rspec.Hook{Path: path, Timeout: &timeout}
-	g.spec.Hooks.Poststop = append(g.spec.Hooks.Poststop, hook)
+	g.spec.Hooks.Poststop = append(g.spec.Hooks.Poststop, tmpHook)
+	return nil
 }
 
 // ClearPostStartHooks clear g.spec.Hooks.Poststart.
@@ -837,42 +795,21 @@ func (g *Generator) ClearPostStartHooks() {
 }
 
 // AddPostStartHook adds a poststart hook into g.spec.Hooks.Poststart.
-func (g *Generator) AddPostStartHook(path string, args []string) {
+func (g *Generator) AddPostStartHook(hookObject string) error {
 	g.initSpecHooks()
-	hook := rspec.Hook{Path: path, Args: args}
-	for i, hook := range g.spec.Hooks.Poststart {
-		if hook.Path == path {
-			g.spec.Hooks.Poststart[i] = hook
-			return
-		}
-	}
-	g.spec.Hooks.Poststart = append(g.spec.Hooks.Poststart, hook)
-}
-
-// AddPostStartHookEnv adds envs of a poststart hook into g.spec.Hooks.Poststart.
-func (g *Generator) AddPostStartHookEnv(path string, envs []string) {
-	g.initSpecHooks()
-	for i, hook := range g.spec.Hooks.Poststart {
-		if hook.Path == path {
-			g.spec.Hooks.Poststart[i].Env = envs
-			return
-		}
+	tmpHook := rspec.Hook{}
+	err := json.Unmarshal([]byte(hookObject), &tmpHook)
+	if err != nil {
+		return err
 	}
-	hook := rspec.Hook{Path: path, Env: envs}
-	g.spec.Hooks.Poststart = append(g.spec.Hooks.Poststart, hook)
-}
-
-// AddPostStartHookTimeout adds timeout of a poststart hook into g.spec.Hooks.Poststart.
-func (g *Generator) AddPostStartHookTimeout(path string, timeout int) {
-	g.initSpecHooks()
 	for i, hook := range g.spec.Hooks.Poststart {
-		if hook.Path == path {
-			g.spec.Hooks.Poststart[i].Timeout = &timeout
-			return
+		if hook.Path == tmpHook.Path {
+			g.spec.Hooks.Poststart[i] = tmpHook
+			return nil
 		}
 	}
-	hook := rspec.Hook{Path: path, Timeout: &timeout}
-	g.spec.Hooks.Poststart = append(g.spec.Hooks.Poststart, hook)
+	g.spec.Hooks.Poststart = append(g.spec.Hooks.Poststart, tmpHook)
+	return nil
 }
 
 // AddTmpfsMount adds a tmpfs mount into g.spec.Mounts.