packer/rpc: Get rid of the heavy server stuff

packer/rpc/build.go

@@ -30,12 +30,9 @@ func (b *Build) Prepare() {
 func (b *Build) Run(ui packer.Ui) {
 	// Create and start the server for the UI
 	// TODO: Error handling
-	server := NewServer()
-	server.RegisterUi(ui)
-	server.Start()
-	defer server.Stop()
-
-	args := &BuildRunArgs{server.Address()}
+	server := rpc.NewServer()
+	RegisterUi(server, ui)
+	args := &BuildRunArgs{serveSingleConn(server)}
 	b.client.Call("Build.Run", args, new(interface{}))
 }
 

packer/rpc/builder.go

@@ -32,13 +32,11 @@ func (b *Builder) Prepare(config interface{}) {
 func (b *Builder) Run(build packer.Build, ui packer.Ui) {
 	// Create and start the server for the Build and UI
 	// TODO: Error handling
-	server := NewServer()
-	server.RegisterBuild(build)
-	server.RegisterUi(ui)
-	server.Start()
-	defer server.Stop()
+	server := rpc.NewServer()
+	RegisterBuild(server, build)
+	RegisterUi(server, ui)
 
-	args := &BuilderRunArgs{server.Address()}
+	args := &BuilderRunArgs{serveSingleConn(server)}
 	b.client.Call("Builder.Run", args, new(interface{}))
 }
 

packer/rpc/builder_test.go

@@ -33,13 +33,12 @@ func TestBuilderRPC(t *testing.T) {
 	b := new(testBuilder)
 
 	// Start the server
-	server := NewServer()
-	server.RegisterBuilder(b)
-	server.Start()
-	defer server.Stop()
+	server := rpc.NewServer()
+	RegisterBuilder(server, b)
+	address := serveSingleConn(server)
 
 	// Create the client over RPC and run some methods to verify it works
-	client, err := rpc.Dial("tcp", server.Address())
+	client, err := rpc.Dial("tcp", address)
 	assert.Nil(err, "should be able to connect")
 
 	// Test Prepare

packer/rpc/environment.go

@@ -49,11 +49,10 @@ func (e *EnvironmentServer) Builder(name *string, reply *string) error {
 	builder := e.env.Builder(*name)
 
 	// Wrap it
-	server := NewServer()
-	server.RegisterBuilder(builder)
-	server.StartSingle()
+	server := rpc.NewServer()
+	RegisterBuilder(server, builder)
 
-	*reply = server.Address()
+	*reply = serveSingleConn(server)
 	return nil
 }
 
@@ -66,10 +65,9 @@ func (e *EnvironmentServer) Ui(args *interface{}, reply *string) error {
 	ui := e.env.Ui()
 
 	// Wrap it
-	server := NewServer()
-	server.RegisterUi(ui)
-	server.StartSingle()
+	server := rpc.NewServer()
+	RegisterUi(server, ui)
 
-	*reply = server.Address()
+	*reply = serveSingleConn(server)
 	return nil
 }

packer/rpc/environment_test.go

@@ -42,13 +42,12 @@ func TestEnvironmentRPC(t *testing.T) {
 	e := &testEnvironment{}
 
 	// Start the server
-	server := NewServer()
-	server.RegisterEnvironment(e)
-	server.Start()
-	defer server.Stop()
+	server := rpc.NewServer()
+	RegisterEnvironment(server, e)
+	address := serveSingleConn(server)
 
 	// Create the client over RPC and run some methods to verify it works
-	client, err := rpc.Dial("tcp", server.Address())
+	client, err := rpc.Dial("tcp", address)
 	assert.Nil(err, "should be able to connect")
 	eClient := &Environment{client}
 

packer/rpc/port_test.go

@@ -16,16 +16,18 @@ func Test_netListenerInRange(t *testing.T) {
 	assert := asserts.NewTestingAsserts(t, true)
 
 	// Open up port 10000 so that we take up a port
-	L1000, err := net.Listen("tcp", ":10000")
+	L1000, err := net.Listen("tcp", ":11000")
 	defer L1000.Close()
 	assert.Nil(err, "should be able to bind to port 10000")
 
-	// Verify it selects an open port
-	L := netListenerInRange(10000, 10005)
-	assert.NotNil(L, "should have a listener")
-	assert.Equal(addrPort(L.Addr()), "10001", "should bind to open port")
+	if err == nil {
+		// Verify it selects an open port
+		L := netListenerInRange(11000, 11005)
+		assert.NotNil(L, "should have a listener")
+		assert.Equal(addrPort(L.Addr()), "11001", "should bind to open port")
 
-	// Returns nil if there are no open ports
-	L = netListenerInRange(10000, 10000)
-	assert.Nil(L, "should not get a listener")
+		// Returns nil if there are no open ports
+		L = netListenerInRange(11000, 11000)
+		assert.Nil(L, "should not get a listener")
+	}
 }

packer/rpc/server.go

 package rpc
 
 import (
-	"errors"
 	"github.com/mitchellh/packer/packer"
-	"net"
 	"net/rpc"
 )
 
-func RegisterCommand(s *rpc.Server, c packer.Command) {
-	s.RegisterName("Command", &ServerCommand{c})
-}
-
-// A Server is a Golang RPC server that has helper methods for automatically
-// setting up the endpoints for Packer interfaces.
-type Server struct {
-	listener net.Listener
-	server *rpc.Server
-}
-
-// Creates and returns a new Server.
-func NewServer() *Server {
-	return &Server{
-		server: rpc.NewServer(),
-	}
+// Registers the appropriate endpoint on an RPC server to serve a
+// Packer Build.
+func RegisterBuild(s *rpc.Server, b packer.Build) {
+	s.RegisterName("Build", &BuildServer{b})
 }
 
-func (s *Server) Address() string {
-	if s.listener == nil {
-		panic("Server not listening.")
-	}
-
-	return s.listener.Addr().String()
-}
-
-func (s *Server) RegisterBuild(b packer.Build) {
-	s.server.RegisterName("Build", &BuildServer{b})
+// Registers the appropriate endpoint on an RPC server to serve a
+// Packer Builder.
+func RegisterBuilder(s *rpc.Server, b packer.Builder) {
+	s.RegisterName("Builder", &BuilderServer{b})
 }
 
-func (s *Server) RegisterBuilder(b packer.Builder) {
-	s.server.RegisterName("Builder", &BuilderServer{b})
-}
-
-func (s *Server) RegisterCommand(c packer.Command) {
-	s.server.RegisterName("Command", &ServerCommand{c})
-}
-
-func (s *Server) RegisterEnvironment(e packer.Environment) {
-	s.server.RegisterName("Environment", &EnvironmentServer{e})
-}
-
-func (s *Server) RegisterUi(ui packer.Ui) {
-	s.server.RegisterName("Ui", &UiServer{ui})
-}
-
-func (s *Server) Start() error {
-	return s.start(false)
+// Registers the appropriate endpoint on an RPC server to serve a
+// Packer Command.
+func RegisterCommand(s *rpc.Server, c packer.Command) {
+	s.RegisterName("Command", &ServerCommand{c})
 }
 
-func (s *Server) StartSingle() error {
-	return s.start(true)
+// Registers the appropriate endpoint on an RPC server to serve a
+// Packer Environment
+func RegisterEnvironment(s *rpc.Server, e packer.Environment) {
+	s.RegisterName("Environment", &EnvironmentServer{e})
 }
 
-func (s *Server) Stop() {
-	if s.listener != nil {
-		s.listener.Close()
-		s.listener = nil
-	}
+// Registers the appropriate endpoint on an RPC server to serve a
+// Packer UI
+func RegisterUi(s *rpc.Server, ui packer.Ui) {
+	s.RegisterName("Ui", &UiServer{ui})
 }
 
-func (s *Server) start(singleConn bool) error {
-	if s.listener != nil {
-		return errors.New("Server already started.")
-	}
-
-	// Start the TCP listener and a goroutine responsible for cleaning up the
-	// listener.
-	s.listener = netListenerInRange(portRangeMin, portRangeMax)
-	if s.listener == nil {
-		return errors.New("Could not open a port ot listen on.")
-	}
-
-	// Start accepting connections
-	go func(l net.Listener) {
-		for {
-			conn, err := l.Accept()
-			if err != nil {
-				break
-			}
+func serveSingleConn(s *rpc.Server) string {
+	l := netListenerInRange(portRangeMin, portRangeMax)
 
-			go s.server.ServeConn(conn)
-
-			// If we're only accepting a single connection then
-			// stop.
-			if singleConn {
-				s.Stop()
-				break
-			}
+	// Accept a single connection in a goroutine and then exit
+	go func() {
+		defer l.Close()
+		conn, err := l.Accept()
+		if err != nil {
+			return
 		}
-	}(s.listener)
 
-	return nil
+		s.ServeConn(conn)
+	}()
+
+	return l.Addr().String()
 }

packer/rpc/server_test.go

 package rpc
 
-import (
-	"cgl.tideland.biz/asserts"
-	"net/rpc"
-	"testing"
-)
-
-func TestServer_Address_PanicIfNotStarted(t *testing.T) {
-	assert := asserts.NewTestingAsserts(t, true)
-
-	defer func() {
-		p := recover()
-		assert.NotNil(p, "should panic")
-		assert.Equal(p.(string), "Server not listening.", "right panic")
-	}()
-
-	NewServer().Address()
-}
-
-func TestServer_Start(t *testing.T) {
-	assert := asserts.NewTestingAsserts(t, true)
-
-	s := NewServer()
-
-	// Verify it can start
-	err := s.Start()
-	assert.Nil(err, "should start without err")
-	addr := s.Address()
-
-	// Verify we can connect to it!
-	_, err = rpc.Dial("tcp", addr)
-	assert.Nil(err, "should be able to connect to RPC")
-
-	// Verify it stops
-	s.Stop()
-	_, err = rpc.Dial("tcp", addr)
-	assert.NotNil(err, "should NOT be able to connect to RPC")
-}
-
-func TestServer_RegisterUi(t *testing.T) {
-	assert := asserts.NewTestingAsserts(t, true)
-
-	ui := &testUi{}
-
-	// Start the server with a UI
-	s := NewServer()
-	s.RegisterUi(ui)
-	assert.Nil(s.Start(), "should start properly")
-	defer s.Stop()
-
-	// Verify it works
-	client, err := rpc.Dial("tcp", s.Address())
-	assert.Nil(err, "should connect via RPC")
-
-	uiClient := &Ui{client}
-	uiClient.Say("format")
-
-	assert.Equal(ui.sayFormat, "format", "format should be correct")
-}

packer/rpc/ui_test.go

@@ -25,13 +25,12 @@ func TestUiRPC(t *testing.T) {
 	ui := new(testUi)
 
 	// Start the RPC server
-	server := NewServer()
-	server.RegisterUi(ui)
-	server.Start()
-	defer server.Stop()
+	server := rpc.NewServer()
+	RegisterUi(server, ui)
+	address := serveSingleConn(server)
 
 	// Create the client over RPC and run some methods to verify it works
-	client, err := rpc.Dial("tcp", server.Address())
+	client, err := rpc.Dial("tcp", address)
 	if err != nil {
 		panic(err)
 	}