Merge pull request #858 from binary132/action-set

Action set

action-set permits action hooks to define an arbitrary map of params
which will be returned to the state server at the end of the Action.

This is a reopen of #415.  415 failed to
land due to too many pages of comments.

cmd/juju/helptool.go

@@ -1,4 +1,4 @@
-// Copyright 2013 Canonical Ltd.
+// Copyright 2013, 2014 Canonical Ltd.
 // Licensed under the AGPLv3, see LICENCE file for details.
 
 package main

worker/uniter/context.go

@@ -216,6 +216,18 @@ func (ctx *HookContext) SetActionFailed(message string) error {
 	return nil
 }
 
+// UpdateActionResults inserts new values for use with action-set and
+// action-fail.  The results struct will be delivered to the state server
+// upon completion of the Action.  It returns an error if not called on an
+// Action-containing HookContext.
+func (ctx *HookContext) UpdateActionResults(keys []string, value string) error {
+	if ctx.actionData == nil {
+		return fmt.Errorf("not running an action")
+	}
+	addValueToMap(keys, value, ctx.actionData.ResultsMap)
+	return nil
+}
+
 func (ctx *HookContext) HookRelation() (jujuc.ContextRelation, bool) {
 	return ctx.Relation(ctx.relationId)
 }
@@ -393,8 +405,8 @@ func (ctx *HookContext) finalizeContext(process string, ctxErr error) (err error
 // finalizeAction passes back the final status of an Action hook to state.
 // It wraps any errors which occurred in normal behavior of the Action run;
 // only errors passed in unhandledErr will be returned.
-// TODO (binary132): synchronize with gsamfira's reboot logic
 func (ctx *HookContext) finalizeAction(err, unhandledErr error) error {
+	// TODO (binary132): synchronize with gsamfira's reboot logic
 	message := ctx.actionData.ResultsMessage
 	results := ctx.actionData.ResultsMap
 	tag := ctx.actionData.ActionTag
@@ -740,3 +752,47 @@ func newActionData(tag *names.ActionTag, params map[string]interface{}) *actionD
 		ResultsMap:   map[string]interface{}{},
 	}
 }
+
+// actionStatus messages define the possible states of a completed Action.
+const (
+	actionStatusInit   = "init"
+	actionStatusFailed = "fail"
+)
+
+// addValueToMap adds the given value to the map on which the method is run.
+// This allows us to merge maps such as {foo: {bar: baz}} and {foo: {baz: faz}}
+// into {foo: {bar: baz, baz: faz}}.
+func addValueToMap(keys []string, value string, target map[string]interface{}) {
+	next := target
+
+	for i := range keys {
+		// if we are on last key set the value.
+		// shouldn't be a problem.  overwrites existing vals.
+		if i == len(keys)-1 {
+			next[keys[i]] = value
+			break
+		}
+
+		if iface, ok := next[keys[i]]; ok {
+			switch typed := iface.(type) {
+			case map[string]interface{}:
+				// If we already had a map inside, keep
+				// stepping through.
+				next = typed
+			default:
+				// If we didn't, then overwrite value
+				// with a map and iterate with that.
+				m := map[string]interface{}{}
+				next[keys[i]] = m
+				next = m
+			}
+			continue
+		}
+
+		// Otherwise, it wasn't present, so make it and step
+		// into.
+		m := map[string]interface{}{}
+		next[keys[i]] = m
+		next = m
+	}
+}

worker/uniter/context_test.go

@@ -773,16 +773,6 @@ func (s *HookContextSuite) AddUnit(c *gc.C, svc *state.Service) *state.Unit {
 	return unit
 }
 
-func (s *HookContextSuite) TestNonActionCallsToActionMethodsFail(c *gc.C) {
-	ctx := uniter.HookContext{}
-	_, err := ctx.ActionParams()
-	c.Check(err, gc.ErrorMatches, "not running an action")
-	err = ctx.SetActionFailed("oops")
-	c.Check(err, gc.ErrorMatches, "not running an action")
-	err = ctx.RunAction("asdf", "fdsa", "qwerty", "uiop")
-	c.Check(err, gc.ErrorMatches, "not running an action")
-}
-
 func (s *HookContextSuite) AddContextRelation(c *gc.C, name string) {
 	s.AddTestingService(c, name, s.relch)
 	eps, err := s.State.InferEndpoints("u", name)
@@ -817,6 +807,69 @@ func (s *HookContextSuite) getHookContext(c *gc.C, uuid string, relid int,
 	return context
 }
 
+// TestNonActionCallsToActionMethodsFail does exactly what its name says:
+// it simply makes sure that Action-related calls to HookContexts with a nil
+// actionData member error out correctly.
+func (s *HookContextSuite) TestNonActionCallsToActionMethodsFail(c *gc.C) {
+	ctx := uniter.HookContext{}
+	_, err := ctx.ActionParams()
+	c.Check(err, gc.ErrorMatches, "not running an action")
+	err = ctx.SetActionFailed("oops")
+	c.Check(err, gc.ErrorMatches, "not running an action")
+	err = ctx.RunAction("asdf", "fdsa", "qwerty", "uiop")
+	c.Check(err, gc.ErrorMatches, "not running an action")
+	err = ctx.UpdateActionResults([]string{"1", "2", "3"}, "value")
+	c.Check(err, gc.ErrorMatches, "not running an action")
+}
+
+// TestUpdateActionResults demonstrates that UpdateActionResults functions
+// as expected.
+func (s *HookContextSuite) TestUpdateActionResults(c *gc.C) {
+	tests := []struct {
+		initial  map[string]interface{}
+		keys     []string
+		value    string
+		expected map[string]interface{}
+	}{{
+		initial: map[string]interface{}{},
+		keys:    []string{"foo"},
+		value:   "bar",
+		expected: map[string]interface{}{
+			"foo": "bar",
+		},
+	}, {
+		initial: map[string]interface{}{
+			"foo": "bar",
+		},
+		keys:  []string{"foo", "bar"},
+		value: "baz",
+		expected: map[string]interface{}{
+			"foo": map[string]interface{}{
+				"bar": "baz",
+			},
+		},
+	}, {
+		initial: map[string]interface{}{
+			"foo": map[string]interface{}{
+				"bar": "baz",
+			},
+		},
+		keys:  []string{"foo"},
+		value: "bar",
+		expected: map[string]interface{}{
+			"foo": "bar",
+		},
+	}}
+
+	for i, t := range tests {
+		c.Logf("UpdateActionResults test %d: %#v: %#v", i, t.keys, t.value)
+		hctx := uniter.GetStubActionContext(t.initial)
+		err := hctx.UpdateActionResults(t.keys, t.value)
+		c.Assert(err, gc.IsNil)
+		c.Check(hctx.ActionResultsMap(), jc.DeepEquals, t.expected)
+	}
+}
+
 func convertSettings(settings params.RelationSettings) map[string]interface{} {
 	result := make(map[string]interface{})
 	for k, v := range settings {

worker/uniter/export_test.go

@@ -1,4 +1,4 @@
-// Copyright 2013 Canonical Ltd.
+// Copyright 2013, 2014 Canonical Ltd.
 // Licensed under the AGPLv3, see LICENCE file for details.
 
 package uniter
@@ -17,6 +17,35 @@ func (u *Uniter) GetProxyValues() proxy.Settings {
 	return u.proxy
 }
 
+func (c *HookContext) ActionResultsMap() map[string]interface{} {
+	if c.actionData == nil {
+		panic("context not running an action")
+	}
+	return c.actionData.ResultsMap
+}
+
+func (c *HookContext) ActionFailed() bool {
+	if c.actionData == nil {
+		panic("context not running an action")
+	}
+	return c.actionData.ActionFailed
+}
+
+func (c *HookContext) ActionMessage() string {
+	if c.actionData == nil {
+		panic("context not running an action")
+	}
+	return c.actionData.ResultsMessage
+}
+
+func GetStubActionContext(in map[string]interface{}) *HookContext {
+	return &HookContext{
+		actionData: &actionData{
+			ResultsMap: in,
+		},
+	}
+}
+
 var MergeEnvironment = mergeEnvironment
 
 var SearchHook = searchHook

worker/uniter/jujuc/action-get_test.go

@@ -240,20 +240,18 @@ func (s *ActionGetSuite) TestActionGet(c *gc.C) {
 
 	for i, t := range actionGetTests {
 		c.Logf("test %d: %s\n args: %#v", i, t.summary, t.args)
-		hctx := s.GetHookContext(c, -1, "")
+		hctx := &Context{}
 		hctx.actionParams = t.actionParams
-
 		com, err := jujuc.NewCommand(hctx, cmdString("action-get"))
 		c.Assert(err, gc.IsNil)
 		ctx := testing.Context(c)
 		code := cmd.Main(com, ctx, t.args)
 		c.Check(code, gc.Equals, t.code)
+		c.Check(bufferString(ctx.Stdout), gc.Equals, t.out)
 		if code == 0 {
 			c.Check(bufferString(ctx.Stderr), gc.Equals, "")
-			c.Check(bufferString(ctx.Stdout), gc.Equals, t.out)
 		} else {
-			c.Check(bufferString(ctx.Stdout), gc.Equals, "")
-			expect := fmt.Sprintf(`(.|\n)*error: %s\n`, t.errMsg)
+			expect := fmt.Sprintf(`(\n)*error: %s\n`, t.errMsg)
 			c.Check(bufferString(ctx.Stderr), gc.Matches, expect)
 		}
 	}

worker/uniter/jujuc/action-set.go

@@ -0,0 +1,100 @@
+// Copyright 2014 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package jujuc
+
+import (
+	"fmt"
+	"regexp"
+	"strings"
+
+	"github.com/juju/cmd"
+	"launchpad.net/gnuflag"
+)
+
+var keyRule = regexp.MustCompile("^[a-z0-9](?:[a-z0-9-]*[a-z0-9])?$")
+
+// ActionSetCommand implements the action-set command.
+type ActionSetCommand struct {
+	cmd.CommandBase
+	ctx  Context
+	args [][]string
+}
+
+// NewActionSetCommand returns a new ActionSetCommand with the given context.
+func NewActionSetCommand(ctx Context) cmd.Command {
+	return &ActionSetCommand{ctx: ctx}
+}
+
+// Info returns the content for --help.
+func (c *ActionSetCommand) Info() *cmd.Info {
+	doc := `
+action-set adds the given values to the results map of the Action.  This map
+is returned to the user after the completion of the Action.
+
+Example usage:
+ action-set outfile.size=10G
+ action-set foo.bar=2
+ action-set foo.baz.val=3
+ action-set foo.bar.zab=4
+ action-set foo.baz=1
+
+ will yield:
+
+ outfile:
+   size: "10G"
+ foo:
+   bar:
+     zab: "4"
+   baz: "1"
+`
+	return &cmd.Info{
+		Name:    "action-set",
+		Args:    "<key>=<value> [<key>=<value> ...]",
+		Purpose: "set action results",
+		Doc:     doc,
+	}
+}
+
+// SetFlags handles known option flags.
+func (c *ActionSetCommand) SetFlags(f *gnuflag.FlagSet) {
+	// TODO(binary132): add cmd.Input type as in cmd.Output for YAML piping.
+}
+
+// Init accepts maps in the form of key=value, key.key2.keyN....=value
+func (c *ActionSetCommand) Init(args []string) error {
+	c.args = make([][]string, 0)
+	for _, arg := range args {
+		thisArg := strings.SplitN(arg, "=", 2)
+		if len(thisArg) != 2 {
+			return fmt.Errorf("argument %q must be of the form key...=value", arg)
+		}
+		keySlice := strings.Split(thisArg[0], ".")
+		// check each key for validity
+		for _, key := range keySlice {
+			if valid := keyRule.MatchString(key); !valid {
+				return fmt.Errorf("key %q must start and end with lowercase alphanumeric, and contain only lowercase alphanumeric and hyphens", key)
+			}
+		}
+		// [key, key, key, key, value]
+		c.args = append(c.args, append(keySlice, thisArg[1]))
+	}
+
+	return nil
+}
+
+// Run adds the given <key list>/<value> pairs, such as foo.bar=baz to the
+// existing map of results for the Action.
+func (c *ActionSetCommand) Run(ctx *cmd.Context) error {
+	for _, argSlice := range c.args {
+		valueIndex := len(argSlice) - 1
+		keys := argSlice[:valueIndex]
+		value := argSlice[valueIndex]
+		err := c.ctx.UpdateActionResults(keys, value)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}