Code review comment for lp:~axwalk/juju-core/testing-mgo-addrinuse

Reviewers: mp+219456_code.launchpad.net,

Message:
Please take a look.

Description:
testing: reattempt MgoInstance.run if addr in use

Mongod does not support listening on port 0 to
acquire an ephemeral port, so our tests must
choose a port which may then be allocated to
another process. This CL adds code to reattempt
the starting of mongod if the port is in use,
up to a maximum of 5 attempts.

https://code.launchpad.net/~axwalk/juju-core/testing-mgo-addrinuse/+merge/219456

(do not edit description out of merge proposal)

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

Affected files (+39, -13 lines):
   A [revision details]
   M testing/mgo.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: tarmac-20140513233806-k5sipwwqoqnt8gzr
+New revision: <email address hidden>

Index: testing/mgo.go
=== modified file 'testing/mgo.go'
--- testing/mgo.go 2014-05-12 07:59:38 +0000
+++ testing/mgo.go 2014-05-14 05:26:43 +0000
@@ -38,6 +38,11 @@
   waitingForConnectionsRe = regexp.MustCompile(".*initandlisten.*waiting
for connections.*")
  )

+const (
+ // maximum number of times to attempt starting mongod
+ maxStartMongodAttempts = 5
+)
+
  type MgoInstance struct {
   // addr holds the address of the MongoDB server
   addr string
@@ -107,18 +112,29 @@
   if err != nil {
    return fmt.Errorf("cannot write cert/key PEM: %v", err)
   }
- inst.port = FindTCPPort()
- inst.addr = fmt.Sprintf("localhost:%d", inst.port)
- inst.dir = dbdir
- inst.ssl = ssl
- if err := inst.run(); err != nil {
- inst.addr = ""
- inst.port = 0
- os.RemoveAll(inst.dir)
- inst.dir = ""
- logger.Warningf("failed to start mongo: %v", err)
- } else {
- logger.Debugf("started mongod pid %d in %s on port %d",
inst.server.Process.Pid, dbdir, inst.port)
+
+ // Attempt to start mongo up to maxStartMongodAttempts times,
+ // as the port we choose may be taken from us in the mean time.
+ for i := 0; i < maxStartMongodAttempts; i++ {
+ inst.port = FindTCPPort()
+ inst.addr = fmt.Sprintf("localhost:%d", inst.port)
+ inst.dir = dbdir
+ inst.ssl = ssl
+ err = inst.run()
+ switch err.(type) {
+ case addrAlreadyInUseError:
+ logger.Debugf("failed to start mongo: %v, trying another port", err)
+ continue
+ case nil:
+ logger.Debugf("started mongod pid %d in %s on port %d",
inst.server.Process.Pid, dbdir, inst.port)
+ default:
+ inst.addr = ""
+ inst.port = 0
+ os.RemoveAll(inst.dir)
+ inst.dir = ""
+ logger.Warningf("failed to start mongo: %v", err)
+ }
+ break
   }
   return err
  }
@@ -186,7 +202,11 @@
    if readUntilMatching(prefix, io.TeeReader(out, &buf),
waitingForConnectionsRe) {
     listening <- nil
    } else {
- listening <- fmt.Errorf("mongod failed to listen on port %v", mgoport)
+ err := fmt.Errorf("mongod failed to listen on port %v", mgoport)
+ if strings.Contains(buf.String(), "addr already in use") {
+ err = addrAlreadyInUseError{err}
+ }
+ listening <- err
    }
    // Capture the last 20 lines of output from mongod, to log
    // in the event of unclean exit.
@@ -528,3 +548,7 @@
   l.Close()
   return l.Addr().(*net.TCPAddr).Port
  }
+
+type addrAlreadyInUseError struct {
+ error
+}