Merge lp:~pedronis/ubuntu-push/delivery-hosts-details into lp:ubuntu-push/automatic

[Samuele Pedroni]

various sanity details, especially don't get stuck in infinite retries if somehow no hosts are considered (shouldn't happen)

small sanity details:

* panic if there are no hosts specified instead of getting stuck in infinite retrying

* check that host bit of the config isn't empty

* tweak test with -race problems differently after some discussion in the golang bug tracker

[John Lenton (chipaca)]

review: Approve

[Ubuntu One Auto Pilot (otto-pilot)]

The attempt to merge lp:~pedronis/ubuntu-push/delivery-hosts-details into lp:ubuntu-push/automatic failed. Below is the output from the failed tests.

mkdir -p /mnt/tarmac/cache/ubuntu-push-automatic/go-ws/bin
mkdir -p /mnt/tarmac/cache/ubuntu-push-automatic/go-ws/pkg
go get -u launchpad.net/godeps
go get -d -u launchpad.net/gocheck launchpad.net/go-dbus/v1 launchpad.net/go-xdg/v0 code.google.com/p/gosqlite/sqlite3
/mnt/tarmac/cache/ubuntu-push-automatic/go-ws/bin/godeps -u dependencies.tsv
"/mnt/tarmac/cache/ubuntu-push-automatic/go-ws/src/launchpad.net/gocheck" now at <email address hidden>
go install launchpad.net/gocheck launchpad.net/go-dbus/v1 launchpad.net/go-xdg/v0 code.google.com/p/gosqlite/sqlite3
go test launchpad.net/ubuntu-push/...
? launchpad.net/ubuntu-push [no test files]
ok launchpad.net/ubuntu-push/bus 0.009s
ok launchpad.net/ubuntu-push/bus/connectivity 1.143s
ok launchpad.net/ubuntu-push/bus/networkmanager 0.032s
ok launchpad.net/ubuntu-push/bus/notifications 0.018s
ok launchpad.net/ubuntu-push/bus/testing 0.024s
ok launchpad.net/ubuntu-push/bus/urldispatcher 0.012s
ok launchpad.net/ubuntu-push/client 0.113s
ok launchpad.net/ubuntu-push/client/gethosts 0.717s
ok launchpad.net/ubuntu-push/client/session 0.125s
ok launchpad.net/ubuntu-push/client/session/levelmap 0.155s
ok launchpad.net/ubuntu-push/config 0.009s
ok launchpad.net/ubuntu-push/external/murmur3 0.004s
--- FAIL: TestGoroutinesRunning (0.00 seconds)
 z_last_test.go:57: num goroutines = 1
 z_last_test.go:59: Too many goroutines.
 z_last_test.go:61: 1 instances of:
  syscall.Syscall(0x3, 0x9, 0x0, 0x0, 0x0, ...)
   /usr/lib/go/src/pkg/syscall/asm_linux_amd64.s:38 +0x64
  syscall.Close(0x9, 0x0, 0x0)
   /usr/lib/go/src/pkg/syscall/zsyscall_linux_amd64.go:267 +0x54
  net.closesocket(0x9, 0x2afbbc4cc3d8, 0x9)
   /usr/lib/go/src/pkg/net/fd_unix.go:474 +0x27
  net.(*netFD).destroy(0xc2110cfc40)
   /usr/lib/go/src/pkg/net/fd_unix.go:97 +0x45
  net.(*netFD).decref(0xc2110cfc40)
   /usr/lib/go/src/pkg/net/fd_unix.go:115 +0x47
  net.(*netFD).Close(0xc2110cfc40, 0x2afbbf7fcf30, 0x0)
   /usr/lib/go/src/pkg/net/fd_unix.go:164 +0xa8
  net.(*conn).Close(0xc210077ec0, 0xc21116ade0, 0xc210077ec0)
   /usr/lib/go/src/pkg/net/net.go:138 +0x95
  launchpad.net/ubuntu-push/http13client_test.func·083()
   /mnt/tarmac/cache/ubuntu-push-automatic/go-ws/src/launchpad.net/ubuntu-push/http13client/transport_test.go:1797 +0x13c
  created by launchpad.net/ubuntu-push/http13client_test.TestTransportTLSHandshakeTimeout
   /mnt/tarmac/cache/ubuntu-push-automatic/go-ws/src/launchpad.net/ubuntu-push/http13client/transport_test.go:1798 +0x22c
FAIL
FAIL launchpad.net/ubuntu-push/http13client 19.809s
ok launchpad.net/ubuntu-push/logger 0.026s
ok launchpad.net/ubuntu-push/protocol 0.018s
ok launchpad.net/ubuntu-push/server 0.073s
ok launchpad.net/ubuntu-push/server/acceptance 0.009s
? launchpad.net/ubuntu-push/server/acceptance/cmd [no test files]
? launchpad.net/ubuntu-push/server/acceptance/suites [no test files]
ok launchpad.net/ubuntu-push/server/api 0.031s
ok launchpad.net/ubuntu-push/server/broker 0.031s
ok launchpad.net/...

=== modified file 'client/client.go'
--- client/client.go	2014-03-28 14:20:23 +0000
+++ client/client.go	2014-03-31 14:34:40 +0000
@@ -20,9 +20,11 @@
 
 import (
 	"encoding/pem"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"os"
+	"strings"
 
 	"launchpad.net/go-dbus/v1"
 
@@ -96,6 +98,12 @@
 	if err != nil {
 		return fmt.Errorf("reading config: %v", err)
 	}
+	// ignore spaces
+	client.config.Addr = strings.Replace(client.config.Addr, " ", "", -1)
+	if client.config.Addr == "" {
+		return errors.New("no hosts specified")
+	}
+
 	// later, we'll be specifying more logging options in the config file
 	client.log = logger.NewSimpleLogger(os.Stderr, client.config.LogLevel)
 

=== modified file 'client/client_test.go'
--- client/client_test.go	2014-03-28 14:20:23 +0000
+++ client/client_test.go	2014-03-31 14:34:40 +0000
@@ -88,6 +88,7 @@
 }
 
 func (cs *clientSuite) writeTestConfig(overrides map[string]interface{}) {
+	pem_file := helpers.SourceRelative("../server/acceptance/config/testing.cert")
 	cfgMap := map[string]interface{}{
 		"connect_timeout":        "7ms",
 		"exchange_timeout":       "10ms",
@@ -97,6 +98,7 @@
 		"connectivity_check_url": "",
 		"connectivity_check_md5": "",
 		"addr":            ":0",
+		"cert_pem_file":   pem_file,
 		"recheck_timeout": "3h",
 		"log_level":       "debug",
 	}
@@ -114,10 +116,8 @@
 	cs.log = helpers.NewTestLogger(c, "debug")
 	dir := c.MkDir()
 	cs.configPath = filepath.Join(dir, "config")
-	pem_file := helpers.SourceRelative("../server/acceptance/config/testing.cert")
-	cs.writeTestConfig(map[string]interface{}{
-		"cert_pem_file": pem_file,
-	})
+
+	cs.writeTestConfig(nil)
 }
 
 type sqlientSuite struct{ clientSuite }
@@ -215,6 +215,25 @@
 	c.Assert(err, ErrorMatches, "no PEM found.*")
 }
 
+func (cs *clientSuite) TestConfigureBailsOnNoHosts(c *C) {
+	cs.writeTestConfig(map[string]interface{}{
+		"addr": "  ",
+	})
+	cli := NewPushClient(cs.configPath, cs.leveldbPath)
+	err := cli.configure()
+	c.Assert(err, ErrorMatches, "no hosts specified")
+}
+
+func (cs *clientSuite) TestConfigureRemovesBlanksInAddr(c *C) {
+	cs.writeTestConfig(map[string]interface{}{
+		"addr": " foo: 443",
+	})
+	cli := NewPushClient(cs.configPath, cs.leveldbPath)
+	err := cli.configure()
+	c.Assert(err, IsNil)
+	c.Check(cli.config.Addr, Equals, "foo:443")
+}
+
 /*****************************************************************
     deriveSessionConfig tests
 ******************************************************************/

=== modified file 'client/gethosts/gethost_test.go'
--- client/gethosts/gethost_test.go	2014-03-24 15:32:29 +0000
+++ client/gethosts/gethost_test.go	2014-03-31 14:34:40 +0000
@@ -61,18 +61,16 @@
 }
 
 func (s *getHostsSuite) TestGetTimeout(c *C) {
-	finish := make(chan bool, 1)
+	started := make(chan bool, 1)
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		<-finish
+		started <- true
+		time.Sleep(700 * time.Millisecond)
 	}))
 	defer func() {
-		time.Sleep(100 * time.Millisecond) // work around -race issue
+		<-started
 		ts.Close()
 	}()
-	defer func() {
-		finish <- true
-	}()
-	gh := New("foobar", ts.URL, 1*time.Second)
+	gh := New("foobar", ts.URL, 500*time.Millisecond)
 	_, err := gh.Get()
 	c.Check(err, ErrorMatches, ".*closed.*")
 }

=== modified file 'client/session/session.go'
--- client/session/session.go	2014-03-28 14:49:56 +0000
+++ client/session/session.go	2014-03-31 14:34:40 +0000
@@ -197,6 +197,9 @@
 		sess.tryHost = 0
 	}
 	sess.leftToTry = len(sess.deliveryHosts)
+	if sess.leftToTry == 0 {
+		panic("should have got hosts from config or remote at this point")
+	}
 	sess.lastAttemptTimestamp = time.Now()
 }
 

=== modified file 'client/session/session_test.go'
--- client/session/session_test.go	2014-03-28 13:09:53 +0000
+++ client/session/session_test.go	2014-03-31 14:34:40 +0000
@@ -345,6 +345,19 @@
 	sess.tryHost = 2
 }
 
+func (cs *clientSessionSuite) TestStartConnectionAttemptNoHostsPanic(c *C) {
+	since := time.Since(time.Time{})
+	sess := &ClientSession{
+		ClientSessionConfig: ClientSessionConfig{
+			ExpectAllRepairedTime: 10 * time.Second,
+		},
+		timeSince: func(ts time.Time) time.Duration {
+			return since
+		},
+	}
+	c.Check(sess.startConnectionAttempt, PanicMatches, "should have got hosts from config or remote at this point")
+}
+
 func (cs *clientSessionSuite) TestNextHostToTry(c *C) {
 	sess := &ClientSession{
 		deliveryHosts: []string{"foo:443", "bar:443", "baz:443"},