Merge lp:~thumper/juju-core/uniter-context-run-commands into lp:~go-bot/juju-core/trunk

[Tim Penhey]

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://codereview.appspot.com/40370047/

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://codereview.appspot.com/40370047/

[Tim Penhey (thumper)]

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/co...

[Ian Booth (wallyworld)]

LGTM with a couple of fixes

https://codereview.appspot.com/40370047/diff/1/worker/uniter/context.go
File worker/uniter/context.go (right):

https://codereview.appspot.com/40370047/diff/1/worker/uniter/context.go#newcode214
worker/uniter/context.go:214: // into ctx to execute jujuc tools.
Doc string is out of date

https://codereview.appspot.com/40370047/diff/1/worker/uniter/context_test.go
File worker/uniter/context_test.go (right):

https://codereview.appspot.com/40370047/diff/1/worker/uniter/context_test.go#newcode733
worker/uniter/context_test.go:733: func (s *RunCommandSuite)
GetHookContext(c *gc.C) *uniter.HookContext {
this doesn't need to be exported

https://codereview.appspot.com/40370047/

[Tim Penhey (thumper)]

Please take a look.

https://codereview.appspot.com/40370047/diff/1/worker/uniter/context.go
File worker/uniter/context.go (right):

https://codereview.appspot.com/40370047/diff/1/worker/uniter/context.go#newcode214
worker/uniter/context.go:214: // into ctx to execute jujuc tools.
On 2013/12/13 00:32:03, wallyworld wrote:
> Doc string is out of date

Done.

https://codereview.appspot.com/40370047/diff/1/worker/uniter/context_test.go
File worker/uniter/context_test.go (right):

https://codereview.appspot.com/40370047/diff/1/worker/uniter/context_test.go#newcode733
worker/uniter/context_test.go:733: func (s *RunCommandSuite)
GetHookContext(c *gc.C) *uniter.HookContext {
On 2013/12/13 00:32:03, wallyworld wrote:
> this doesn't need to be exported

The height of pedentry :P

Unexported.

https://codereview.appspot.com/40370047/

=== modified file 'worker/uniter/context.go'
--- worker/uniter/context.go	2013-12-13 02:58:09 +0000
+++ worker/uniter/context.go	2013-12-13 02:58:09 +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"
@@ -207,8 +210,16 @@
 	return err
 }
 
+// RunCommands executes the commands in an environment which allows it to to
+// call back into the hook context 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.
+// into the hook context 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{}
 
=== modified file 'worker/uniter/context_test.go'
--- worker/uniter/context_test.go	2013-12-13 02:58:09 +0000
+++ worker/uniter/context_test.go	2013-12-13 02:58:09 +0000
@@ -208,7 +208,7 @@
 	c.Assert(err, gc.IsNil)
 	for i, t := range runHookTests {
 		c.Logf("\ntest %d: %s; perm %v", i, t.summary, t.spec.perm)
-		ctx := s.GetHookContext(c, uuid.String(), t.relid, t.remote)
+		ctx := s.getHookContext(c, uuid.String(), t.relid, t.remote)
 		var charmDir, outPath string
 		var hookExists bool
 		if t.spec.perm == 0 {
@@ -260,7 +260,7 @@
 	// Create a charm with a breaking hook.
 	uuid, err := utils.NewUUID()
 	c.Assert(err, gc.IsNil)
-	ctx := s.GetHookContext(c, uuid.String(), -1, "")
+	ctx := s.getHookContext(c, uuid.String(), -1, "")
 	charmDir, _ := makeCharm(c, hookSpec{
 		name: "something-happened",
 		perm: 0700,
@@ -551,7 +551,7 @@
 	remoteName string) jujuc.Context {
 	uuid, err := utils.NewUUID()
 	c.Assert(err, gc.IsNil)
-	return s.HookContextSuite.GetHookContext(c, uuid.String(), relId, remoteName)
+	return s.HookContextSuite.getHookContext(c, uuid.String(), relId, remoteName)
 }
 
 func (s *InterfaceSuite) TestUtils(c *gc.C) {
@@ -696,7 +696,7 @@
 	s.relctxs[rel.Id()] = uniter.NewContextRelation(apiRelUnit, nil)
 }
 
-func (s *HookContextSuite) GetHookContext(c *gc.C, uuid string, relid int,
+func (s *HookContextSuite) getHookContext(c *gc.C, uuid string, relid int,
 	remote string) *uniter.HookContext {
 	if relid != -1 {
 		_, found := s.relctxs[relid]
@@ -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")
+}