Merge lp:~fwereade/pyjuju/go-cmd-server-package into lp:pyjuju/go

[William Reade]

Add cmd/server package to enable hosted Command execution via RPC

prereq: lp:~fwereade/juju/go-tweak-supercommand

This is all new code, and shouldn't impact anything else, so it should be
adequately explained by the inline comments (and if it isn't, those should
be fixed). Nonetheless, the general idea is as follows:

1) Hook commands need to run the "meat" of their code on the server.

2) The python implementation includes an awful lot of plumbing to support a
   mechanism whereby each individual command marshals relevant params to a
   server and gets back results which are then formatted for stdout; in
   juju.hooks, the cli, commands, and protocol modules all contain a lot of
   code purely to support this approach.

3) By implementing a single server method that takes a command line, a
   working directory, and a context ID; and returns an exit code and the
   command's output: at a stroke, we eliminate the vast majority of the
   marshalling busywork, and become able to implement each hook tool as a
   single Command, which can have direct access to the hook context in which
   it will execute.

https://codereview.appspot.com/5754052/

[William Reade (fwereade)]

Reviewers: mp+96140_code.launchpad.net,

Message:
Please take a look.

Description:
prereq: lp:~fwereade/juju/go-tweak-supercommand

This is all new code, and shouldn't impact anything else, so it should
be
adequately explained by the inline comments (and if it isn't, those
should
be fixed).

https://code.launchpad.net/~fwereade/juju/go-cmd-server-package/+merge/96140

(do not edit description out of merge proposal)

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

Affected files:
   M cmd/command.go
   A cmd/context.go
   A cmd/context_test.go
   M cmd/juju/bootstrap.go
   M cmd/juju/main_test.go
   M cmd/jujud/initzk.go
   M cmd/jujud/machine.go
   M cmd/jujud/main_test.go
   M cmd/jujud/provisioning.go
   M cmd/jujud/unit.go
   A cmd/server/server.go
   A cmd/server/server_test.go
   M cmd/supercommand.go
   M cmd/supercommand_test.go

[William Reade (fwereade)]

Please take a look.

https://codereview.appspot.com/5754052/

[lp:~fwereade/pyjuju/go-cmd-server-package]

84. By William Reade on 2012-03-06

client id became context id

[William Reade (fwereade)]

Please take a look.

https://codereview.appspot.com/5754052/

[lp:~fwereade/pyjuju/go-cmd-server-package]

85. By William Reade on 2012-03-06

fix param names

86. By William Reade on 2012-03-12

merge parent

87. By William Reade on 2012-03-13

merge parent

88. By William Reade on 2012-03-17

merge parent

Unmerged revisions

88. By William Reade on 2012-03-17

merge parent

87. By William Reade on 2012-03-13

merge parent

86. By William Reade on 2012-03-12

merge parent

85. By William Reade on 2012-03-06

fix param names

84. By William Reade on 2012-03-06

client id became context id

83. By William Reade on 2012-03-06

add cmd/server package

82. By William Reade on 2012-03-06

anticipatory SuperCommand tweaks

=== added directory 'cmd/server'
=== added file 'cmd/server/server.go'
--- cmd/server/server.go	1970-01-01 00:00:00 +0000
+++ cmd/server/server.go	2012-03-17 01:16:19 +0000
@@ -0,0 +1,144 @@
+// The cmd/server package allows a process to expose an RPC interface that
+// allows client processes to delegate execution of cmd.Commands to a server
+// process (with exposed commands and their particular state amenable to
+// selection by client id). This will allow the unit agent to provide tailor-
+// made hook command sets for any hook execution context it creates.
+package server
+
+import (
+	"bytes"
+	"fmt"
+	"launchpad.net/juju/go/cmd"
+	"launchpad.net/tomb"
+	"net"
+	"net/rpc"
+	"os"
+	"path/filepath"
+)
+
+// Request contains the information necessary to run a Command whose view of the
+// world is filtered according to its ContextId.
+type Request struct {
+	ContextId string
+	Dir       string
+	Args      []string
+}
+
+// Response contains the return code and output generated by a Request.
+type Response struct {
+	Code   int
+	Stdout string
+	Stderr string
+}
+
+// CmdFactory returns the Commands that should be exposed in the context
+// identified by contextId.
+type CmdFactory func(contextId string) ([]cmd.Command, error)
+
+// Server wraps net.rpc.Server so as to allow Commands to be executed in one
+// process on behalf of another.
+type Server struct {
+	sockPath string
+	server   *rpc.Server
+	*tomb.Tomb
+}
+
+// NewServer starts an RPC server which runs on sockPath and exposes Commands
+// chosen (at run time) by factory (according to the Request's ContextId).
+func NewServer(factory CmdFactory, sockPath string) (*Server, error) {
+	s := &Server{sockPath, rpc.NewServer(), tomb.New()}
+	if err := s.server.Register(&RPCcmd{factory}); err != nil {
+		return nil, err
+	}
+	listener, err := net.Listen("unix", sockPath)
+	if err != nil {
+		return nil, err
+	}
+	go s.serve(listener)
+	return s, nil
+}
+
+// serve starts the RPC server (serving one request at a time).
+func (s *Server) serve(listener net.Listener) {
+	defer s.Done()
+	go func() {
+		<-s.Dying
+		listener.Close()
+		<-s.Dead
+		os.Remove(s.sockPath)
+	}()
+	for {
+		conn, err := listener.Accept()
+		if err != nil {
+			if s.Err() != tomb.Stop {
+				s.Fatal(err)
+			}
+			return
+		}
+		s.server.ServeConn(conn)
+	}
+}
+
+// Close stops the RPC server.
+func (s *Server) Close() error {
+	s.Fatal(tomb.Stop)
+	return s.Wait()
+}
+
+// RPCcmd wraps cmd for RPC.
+type RPCcmd struct {
+	factory CmdFactory
+}
+
+var jujucDoc = `
+The jujuc command forwards invocations over RPC for execution by another
+process. It expects to be called via a symlink named for the desired remote
+command, and requires that JUJU_AGENT_SOCKET and JUJU_CONTEXT_ID be set in
+its environment.
+`
+
+// command returns a cmd.SuperCommand which is responsible for selecting one
+// of the Commands produced by calling rpcc.factory.
+func (rpcc *RPCcmd) command(contextId string) (cmd.Command, error) {
+	cmds, err := rpcc.factory(contextId)
+	if err != nil {
+		return nil, err
+	}
+	sc := cmd.NewSuperCommand("(-> jujuc)", jujucDoc)
+	sc.SetsLog = false
+	sc.Purpose = "invoke a command implemented by another process"
+	for _, c := range cmds {
+		sc.Register(c)
+	}
+	return sc, nil
+}
+
+// run invokes c with args and fills in resp.
+func run(c cmd.Command, dir string, args []string, resp *Response) error {
+	stdout := bytes.NewBuffer([]byte{})
+	stderr := bytes.NewBuffer([]byte{})
+	ctx := &cmd.Context{dir, stdout, stderr}
+	resp.Code = cmd.Main(c, ctx, args)
+	resp.Stdout = stdout.String()
+	resp.Stderr = stderr.String()
+	return nil
+}
+
+func badReq(format string, v ...interface{}) error {
+	return fmt.Errorf("bad request: "+format, v...)
+}
+
+// Main implements the equivalent of cmd.Main for RPC.
+func (rpcc *RPCcmd) Main(req Request, resp *Response) error {
+	if req.Args == nil || len(req.Args) < 1 {
+		return badReq("Args too short")
+	}
+	if !filepath.IsAbs(req.Dir) {
+		return badReq("Dir is not absolute")
+	}
+	c, err := rpcc.command(req.ContextId)
+	if err != nil {
+		return badReq("%s", err)
+	}
+	return run(c, req.Dir, req.Args, resp)
+}

=== added file 'cmd/server/server_test.go'
--- cmd/server/server_test.go	1970-01-01 00:00:00 +0000
+++ cmd/server/server_test.go	2012-03-17 01:16:19 +0000
@@ -0,0 +1,152 @@
+package server_test
+
+import (
+	"errors"
+	"io/ioutil"
+	"launchpad.net/gnuflag"
+	. "launchpad.net/gocheck"
+	"launchpad.net/juju/go/cmd"
+	"launchpad.net/juju/go/cmd/server"
+	"net/rpc"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+func Test(t *testing.T) { TestingT(t) }
+
+type RpcCommand struct {
+	Value string
+}
+
+func (c *RpcCommand) Info() *cmd.Info {
+	return &cmd.Info{"magic", "[options]", "do magic", "blah doc", true}
+}
+
+func (c *RpcCommand) InitFlagSet(f *gnuflag.FlagSet) {
+	f.StringVar(&c.Value, "value", "", "doc")
+}
+
+func (c *RpcCommand) ParsePositional(args []string) error {
+	return cmd.CheckEmpty(args)
+}
+
+func (c *RpcCommand) Run(ctx *cmd.Context) error {
+	if c.Value != "zyxxy" {
+		return errors.New("insufficiently magic")
+	}
+	ctx.Stdout.Write([]byte("eye of newt\n"))
+	ctx.Stderr.Write([]byte("toe of frog\n"))
+	return ioutil.WriteFile(ctx.AbsPath("local"), []byte{}, 0644)
+}
+
+func factory(contextId string) ([]cmd.Command, error) {
+	if contextId != "merlin" {
+		return nil, errors.New("unknown client")
+	}
+	return []cmd.Command{&RpcCommand{}}, nil
+}
+
+type ServerSuite struct {
+	server   *server.Server
+	sockPath string
+}
+
+var _ = Suite(&ServerSuite{})
+
+func (s *ServerSuite) SetUpTest(c *C) {
+	s.sockPath = filepath.Join(c.MkDir(), "test.sock")
+	srv, err := server.NewServer(factory, s.sockPath)
+	c.Assert(err, IsNil)
+	c.Assert(srv, NotNil)
+	s.server = srv
+}
+
+func (s *ServerSuite) TearDownTest(c *C) {
+	c.Assert(s.server.Close(), IsNil)
+	_, err := os.Open(s.sockPath)
+	c.Assert(os.IsNotExist(err), Equals, true)
+}
+
+func (s *ServerSuite) Call(c *C, req server.Request) (resp server.Response, err error) {
+	client, err := rpc.Dial("unix", s.sockPath)
+	c.Assert(err, IsNil)
+	defer client.Close()
+	err = client.Call("RPCcmd.Main", req, &resp)
+	return resp, err
+}
+
+func (s *ServerSuite) TestHappyPath(c *C) {
+	dir := c.MkDir()
+	resp, err := s.Call(c, server.Request{
+		"merlin", dir, []string{"magic", "--value", "zyxxy"}})
+	c.Assert(err, IsNil)
+	c.Assert(resp.Code, Equals, 0)
+	c.Assert(resp.Stdout, Equals, "eye of newt\n")
+	c.Assert(resp.Stderr, Equals, "toe of frog\n")
+	_, err = os.Stat(filepath.Join(dir, "local"))
+	c.Assert(err, IsNil)
+}
+
+func (s *ServerSuite) TestBadArgs(c *C) {
+	dir := c.MkDir()
+	for _, req := range []server.Request{
+		{"merlin", dir, nil},
+		{"merlin", dir, []string{}},
+		{"mordred", dir, nil},
+		{"mordred", dir, []string{}},
+	} {
+		_, err := s.Call(c, req)
+		c.Assert(err, ErrorMatches, "bad request: Args too short")
+	}
+}
+
+func (s *ServerSuite) TestBadDir(c *C) {
+	for _, req := range []server.Request{
+		{"merlin", "", []string{"cmd"}},
+		{"merlin", "foo/bar", []string{"cmd"}},
+	} {
+		_, err := s.Call(c, req)
+		c.Assert(err, ErrorMatches, "bad request: Dir is not absolute")
+	}
+}
+
+func (s *ServerSuite) TestBadContextId(c *C) {
+	_, err := s.Call(c, server.Request{"mordred", c.MkDir(), []string{"magic"}})
+	c.Assert(err, ErrorMatches, "bad request: unknown client")
+}
+
+func (s *ServerSuite) AssertBadCommand(c *C, args []string, code int) server.Response {
+	resp, err := s.Call(c, server.Request{"merlin", c.MkDir(), args})
+	c.Assert(err, IsNil)
+	c.Assert(resp.Code, Equals, code)
+	return resp
+}
+
+func lines(s string) []string {
+	return strings.Split(s, "\n")
+}
+
+func (s *ServerSuite) TestUnknownCommand(c *C) {
+	resp := s.AssertBadCommand(c, []string{"witchcraft"}, 2)
+	c.Assert(resp.Stdout, Equals, "")
+	usageStart := []string{
+		"unrecognised command: (-> jujuc) witchcraft",
+		"usage: (-> jujuc) <command> [options] ...",
+		"purpose: invoke a command implemented by another process",
+	}
+	c.Assert(lines(resp.Stderr)[:3], DeepEquals, usageStart)
+}
+
+func (s *ServerSuite) TestParseError(c *C) {
+	resp := s.AssertBadCommand(c, []string{"magic", "--cheese"}, 2)
+	c.Assert(resp.Stdout, Equals, "")
+	c.Assert(lines(resp.Stderr)[0], Equals, "flag provided but not defined: --cheese")
+}
+
+func (s *ServerSuite) TestBrokenCommand(c *C) {
+	resp := s.AssertBadCommand(c, []string{"magic"}, 1)
+	c.Assert(resp.Stdout, Equals, "")
+	c.Assert(lines(resp.Stderr)[0], Equals, "insufficiently magic")
+}