Code review comment for lp:~thumper/juju-core/uniter-context-run-commands

Reviewers: mp+198671_code.launchpad.net,

Message:
Please take a look.

Description:
Uniter context run commands.

The uniter.Context gains the ability to run arbitrary commands in a hook
context.

Execute the commands by passing as stdin to "/bin/bash -s".

https://code.launchpad.net/~thumper/juju-core/uniter-context-run-commands/+merge/198671

Requires:
https://code.launchpad.net/~thumper/juju-core/refactor-uniter-tests/+merge/198668

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/40370047/

Affected files (+99, -0 lines):
   A [revision details]
   M worker/uniter/context.go
   M worker/uniter/context_test.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: worker/uniter/context.go
=== modified file 'worker/uniter/context.go'
--- worker/uniter/context.go 2013-12-12 05:18:00 +0000
+++ worker/uniter/context.go 2013-12-12 06:14:41 +0000
@@ -5,6 +5,7 @@

  import (
   "bufio"
+ "bytes"
   "fmt"
   "io"
   "os"
@@ -13,9 +14,11 @@
   "sort"
   "strings"
   "sync"
+ "syscall"
   "time"

   "launchpad.net/juju-core/charm"
+ "launchpad.net/juju-core/cmd"
   "launchpad.net/juju-core/state/api/params"
   "launchpad.net/juju-core/state/api/uniter"
   unitdebug "launchpad.net/juju-core/worker/uniter/debug"
@@ -209,6 +212,14 @@

  // RunHook executes a hook in an environment which allows it to to call
back
  // into ctx to execute jujuc tools.
+func (ctx *HookContext) RunCommands(commands, charmDir, toolsDir,
socketPath string) (*cmd.RemoteResponse, error) {
+ env := ctx.hookVars(charmDir, toolsDir, socketPath)
+ result, err := runCommands(commands, charmDir, env)
+ return result, ctx.finalizeContext("run commands", err)
+}
+
+// RunHook executes a hook in an environment which allows it to to call
back
+// into ctx to execute jujuc tools.
  func (ctx *HookContext) RunHook(hookName, charmDir, toolsDir, socketPath
string) error {
   var err error
   env := ctx.hookVars(charmDir, toolsDir, socketPath)
@@ -253,6 +264,38 @@
   return err
  }

+func runCommands(commands, charmDir string, env []string)
(*cmd.RemoteResponse, error) {
+ ps := exec.Command("/bin/bash", "-s")
+ ps.Env = env
+ ps.Dir = charmDir
+ ps.Stdin = bytes.NewBufferString(commands)
+
+ stdout := &bytes.Buffer{}
+ stderr := &bytes.Buffer{}
+
+ ps.Stdout = stdout
+ ps.Stderr = stderr
+
+ err := ps.Start()
+ if err == nil {
+ err = ps.Wait()
+ }
+ result := &cmd.RemoteResponse{
+ Stdout: stdout.Bytes(),
+ Stderr: stderr.Bytes(),
+ }
+ if ee, ok := err.(*exec.ExitError); ok && err != nil {
+ status := ee.ProcessState.Sys().(syscall.WaitStatus)
+ if status.Exited() {
+ // A non-zero return code isn't considered an error here.
+ result.Code = status.ExitStatus()
+ err = nil
+ }
+ logger.Infof("run result: %v", ee)
+ }
+ return result, err
+}
+
  type hookLogger struct {
   r io.ReadCloser
   done chan struct{}

Index: worker/uniter/context_test.go
=== modified file 'worker/uniter/context_test.go'
--- worker/uniter/context_test.go 2013-12-12 05:18:00 +0000
+++ worker/uniter/context_test.go 2013-12-12 06:14:41 +0000
@@ -723,3 +723,57 @@
   }
   return result
  }
+
+type RunCommandSuite struct {
+ HookContextSuite
+}
+
+var _ = gc.Suite(&RunCommandSuite{})
+
+func (s *RunCommandSuite) GetHookContext(c *gc.C) *uniter.HookContext {
+ uuid, err := utils.NewUUID()
+ c.Assert(err, gc.IsNil)
+ return s.HookContextSuite.GetHookContext(c, uuid.String(), -1, "")
+}
+
+func (s *RunCommandSuite) TestRunCommandsHasEnvironSet(c *gc.C) {
+ context := s.GetHookContext(c)
+ charmDir := c.MkDir()
+ result, err := context.RunCommands("env | sort",
charmDir, "/path/to/tools", "/path/to/socket")
+ c.Assert(err, gc.IsNil)
+
+ executionEnvironment := map[string]string{}
+ for _, value := range strings.Split(string(result.Stdout), "\n") {
+ bits := strings.SplitN(value, "=", 2)
+ if len(bits) == 2 {
+ executionEnvironment[bits[0]] = bits[1]
+ }
+ }
+ expected := map[string]string{
+ "APT_LISTCHANGES_FRONTEND": "none",
+ "DEBIAN_FRONTEND": "noninteractive",
+ "CHARM_DIR": charmDir,
+ "JUJU_CONTEXT_ID": "TestCtx",
+ "JUJU_AGENT_SOCKET": "/path/to/socket",
+ "JUJU_UNIT_NAME": "u/0",
+ }
+ for key, value := range expected {
+ c.Check(executionEnvironment[key], gc.Equals, value)
+ }
+}
+
+func (s *RunCommandSuite) TestRunCommandsStdOutAndErrAndRC(c *gc.C) {
+ context := s.GetHookContext(c)
+ charmDir := c.MkDir()
+ commands := `
+echo this is standard out
+echo this is standard err >&2
+exit 42
+`
+ result, err := context.RunCommands(commands,
charmDir, "/path/to/tools", "/path/to/socket")
+ c.Assert(err, gc.IsNil)
+
+ c.Assert(result.Code, gc.Equals, 42)
+ c.Assert(string(result.Stdout), gc.Equals, "this is standard out\n")
+ c.Assert(string(result.Stderr), gc.Equals, "this is standard err\n")
+}