Merge lp:~pedronis/ubuntu-push/foreign-listener into lp:ubuntu-push

[Samuele Pedroni]

let the device listener setup code also take a prebuilt listener

let the device listener setup code also take a prebuilt listener

[John Lenton (chipaca)]

A very very very very small nit: can you eliminate the orphan "If" in DevicesRunner's doc? (either by taking it down to the next line, or pulling up the next word).

That is all I have. So you know it's good =)

review: Approve

[lp:~pedronis/ubuntu-push/foreign-listener]

80. By Samuele Pedroni on 2014-03-12

move If

=== modified file 'server/dev/server.go'
--- server/dev/server.go	2014-02-24 15:54:37 +0000
+++ server/dev/server.go	2014-03-12 12:33:02 +0000
@@ -64,7 +64,7 @@
 	handler := api.PanicTo500Handler(mux, logger)
 	go server.HTTPServeRunner(handler, &cfg.HTTPServeParsedConfig)()
 	// listen for device connections
-	server.DevicesRunner(func(conn net.Conn) error {
+	server.DevicesRunner(nil, func(conn net.Conn) error {
 		track := session.NewTracker(logger)
 		return session.Session(conn, broker, cfg, track)
 	}, logger, &cfg.DevicesParsedConfig)()

=== modified file 'server/listener/listener.go'
--- server/listener/listener.go	2014-02-21 16:04:44 +0000
+++ server/listener/listener.go	2014-03-12 12:33:02 +0000
@@ -41,8 +41,17 @@
 	net.Listener
 }
 
-// DeviceListen creates a DeviceListener for device connections based on config.
-func DeviceListen(cfg DeviceListenerConfig) (*DeviceListener, error) {
+// DeviceListen creates a DeviceListener for device connections based
+// on config.  If lst is not nil DeviceListen just wraps it with a TLS
+// layer instead of starting creating a new listener.
+func DeviceListen(lst net.Listener, cfg DeviceListenerConfig) (*DeviceListener, error) {
+	if lst == nil {
+		var err error
+		lst, err = net.Listen("tcp", cfg.Addr())
+		if err != nil {
+			return nil, err
+		}
+	}
 	cert, err := tls.X509KeyPair(cfg.CertPEMBlock(), cfg.KeyPEMBlock())
 	if err != nil {
 		return nil, err
@@ -51,11 +60,7 @@
 		Certificates:           []tls.Certificate{cert},
 		SessionTicketsDisabled: true,
 	}
-	lst, err := tls.Listen("tcp", cfg.Addr(), tlsCfg)
-	if err != nil {
-		return nil, err
-	}
-	return &DeviceListener{lst}, err
+	return &DeviceListener{tls.NewListener(lst, tlsCfg)}, err
 }
 
 // handleTemporary checks and handles if the error is just a temporary network

=== modified file 'server/listener/listener_test.go'
--- server/listener/listener_test.go	2014-02-10 23:19:08 +0000
+++ server/listener/listener_test.go	2014-03-12 12:33:02 +0000
@@ -74,7 +74,7 @@
 }
 
 func (s *listenerSuite) TestDeviceListen(c *C) {
-	lst, err := DeviceListen(&testDevListenerCfg{"127.0.0.1:0"})
+	lst, err := DeviceListen(nil, &testDevListenerCfg{"127.0.0.1:0"})
 	c.Check(err, IsNil)
 	defer lst.Close()
 	c.Check(lst.Addr().String(), Matches, `127.0.0.1:\d{5}`)
@@ -82,7 +82,7 @@
 
 func (s *listenerSuite) TestDeviceListenError(c *C) {
 	// assume tests are not running as root
-	_, err := DeviceListen(&testDevListenerCfg{"127.0.0.1:99"})
+	_, err := DeviceListen(nil, &testDevListenerCfg{"127.0.0.1:99"})
 	c.Check(err, ErrorMatches, ".*permission denied.*")
 }
 
@@ -142,7 +142,7 @@
 }
 
 func (s *listenerSuite) TestDeviceAcceptLoop(c *C) {
-	lst, err := DeviceListen(&testDevListenerCfg{"127.0.0.1:0"})
+	lst, err := DeviceListen(nil, &testDevListenerCfg{"127.0.0.1:0"})
 	c.Check(err, IsNil)
 	defer lst.Close()
 	errCh := make(chan error)
@@ -169,7 +169,7 @@
 	// ENFILE is not the temp network error we want to handle this way
 	// but is relatively easy to generate in a controlled way
 	var err error
-	lst, err := DeviceListen(&testDevListenerCfg{"127.0.0.1:0"})
+	lst, err := DeviceListen(nil, &testDevListenerCfg{"127.0.0.1:0"})
 	c.Check(err, IsNil)
 	defer lst.Close()
 	errCh := make(chan error)
@@ -202,7 +202,7 @@
 }
 
 func (s *listenerSuite) TestDeviceAcceptLoopPanic(c *C) {
-	lst, err := DeviceListen(&testDevListenerCfg{"127.0.0.1:0"})
+	lst, err := DeviceListen(nil, &testDevListenerCfg{"127.0.0.1:0"})
 	c.Check(err, IsNil)
 	defer lst.Close()
 	errCh := make(chan error)
@@ -219,3 +219,25 @@
 	c.Check(<-errCh, ErrorMatches, ".*use of closed.*")
 	c.Check(s.testlog.Captured(), Matches, "(?s)ERROR\\(PANIC\\) terminating device connection on: session crash:.*AcceptLoop.*")
 }
+
+func (s *listenerSuite) TestForeignListener(c *C) {
+	foreignLst, err := net.Listen("tcp", "127.0.0.1:0")
+	c.Check(err, IsNil)
+	lst, err := DeviceListen(foreignLst, &testDevListenerCfg{"127.0.0.1:0"})
+	c.Check(err, IsNil)
+	defer lst.Close()
+	errCh := make(chan error)
+	go func() {
+		errCh <- lst.AcceptLoop(testSession, s.testlog)
+	}()
+	listenerAddr := lst.Addr().String()
+	c.Check(listenerAddr, Equals, foreignLst.Addr().String())
+	conn1, err := testTlsDial(c, listenerAddr)
+	c.Assert(err, IsNil)
+	defer conn1.Close()
+	testWriteByte(c, conn1, '1')
+	testReadByte(c, conn1, '1')
+	lst.Close()
+	c.Check(<-errCh, ErrorMatches, ".*use of closed.*")
+	c.Check(s.testlog.Captured(), Equals, "")
+}

=== modified file 'server/runner_devices.go'
--- server/runner_devices.go	2014-02-10 23:19:08 +0000
+++ server/runner_devices.go	2014-03-12 12:33:02 +0000
@@ -87,7 +87,9 @@
 }
 
 // DevicesRunner returns a function to accept device connections.
-func DevicesRunner(session func(net.Conn) error, logger logger.Logger, parsedCfg *DevicesParsedConfig) func() {
+// If adoptLst is not nil it will be used as the underlying listener, instead
+// of creating one, wrapped in a TLS layer.
+func DevicesRunner(adoptLst net.Listener, session func(net.Conn) error, logger logger.Logger, parsedCfg *DevicesParsedConfig) func() {
 	BootLogger.Debugf("PingInterval: %s, ExchangeTimeout %s", parsedCfg.PingInterval(), parsedCfg.ExchangeTimeout())
 	var rlim syscall.Rlimit
 	err := syscall.Getrlimit(syscall.RLIMIT_NOFILE, &rlim)
@@ -95,7 +97,7 @@
 		BootLogFatalf("getrlimit failed: %v", err)
 	}
 	BootLogger.Debugf("nofile soft: %d hard: %d", rlim.Cur, rlim.Max)
-	lst, err := listener.DeviceListen(parsedCfg)
+	lst, err := listener.DeviceListen(adoptLst, parsedCfg)
 	if err != nil {
 		BootLogFatalf("start device listening: %v", err)
 	}

=== modified file 'server/runner_test.go'
--- server/runner_test.go	2014-02-10 23:19:08 +0000
+++ server/runner_test.go	2014-03-12 12:33:02 +0000
@@ -108,9 +108,25 @@
 	defer func() {
 		BootLogger = prevBootLogger
 	}()
-	runner := DevicesRunner(func(conn net.Conn) error { return nil }, BootLogger, &testDevicesParsedConfig)
+	runner := DevicesRunner(nil, func(conn net.Conn) error { return nil }, BootLogger, &testDevicesParsedConfig)
 	c.Assert(s.lst, Not(IsNil))
 	s.lst.Close()
 	c.Check(s.kind, Equals, "devices")
 	c.Check(runner, PanicMatches, "accepting device connections:.*closed.*")
 }
+
+func (s *runnerSuite) TestDevicesRunnerAdoptListener(c *C) {
+	prevBootLogger := BootLogger
+	testlog := helpers.NewTestLogger(c, "debug")
+	BootLogger = testlog
+	defer func() {
+		BootLogger = prevBootLogger
+	}()
+	lst0, err := net.Listen("tcp", "127.0.0.1:0")
+	c.Assert(err, IsNil)
+	defer lst0.Close()
+	DevicesRunner(lst0, func(conn net.Conn) error { return nil }, BootLogger, &testDevicesParsedConfig)
+	c.Assert(s.lst, Not(IsNil))
+	c.Check(s.lst.Addr().String(), Equals, lst0.Addr().String())
+	s.lst.Close()
+}