agent/mongo: added MongoSelectPeerAddress

agent/mongo: added MongoSelectPeerAddress

Use this method to ensure that we get the same selection behavior in many
places. Including API calls, replicaset worker, and bootstrap state.

https://code.launchpad.net/~wwitzel3/juju-core/004-ha-agent-selectpeeraddr/+merge/213435

(do not edit description out of merge proposal)

[wwitzel, 2014-03-31 11:20:05]

Please take a look.

[wwitzel3, 2014-03-31 11:23:17]

https://codereview.appspot.com/82550043/diff/1/agent/mongo/mongo_test.go
File agent/mongo/mongo_test.go (right):

https://codereview.appspot.com/82550043/diff/1/agent/mongo/mongo_test.go#newc...
agent/mongo/mongo_test.go:144: func (s *MongoSuite) TestMongoPackageForSeries(c
*gc.C) {
This is unrelated I just noticed that the MongoPackageForSeries function didn't
have a test, it is trivial, so I added it.

[wwitzel, 2014-03-31 11:40:03]

Please take a look.

[rog, 2014-03-31 14:57:23]

LGTM with a few suggestions below.

https://codereview.appspot.com/82550043/diff/2/agent/mongo/mongo.go
File agent/mongo/mongo.go (right):

https://codereview.appspot.com/82550043/diff/2/agent/mongo/mongo.go#newcode35
agent/mongo/mongo.go:35: func MongoSelectPeerAddress(addrs []instance.Address)
string {
I know the "Mongo" is gonna go, but in the meantime, I think
SelectMongoPeerAddress reads a bit better.

https://codereview.appspot.com/82550043/diff/2/agent/mongo/mongo_test.go
File agent/mongo/mongo_test.go (right):

https://codereview.appspot.com/82550043/diff/2/agent/mongo/mongo_test.go#newc...
agent/mongo/mongo_test.go:147: {"10.0.0.1", instance.Ipv4Address, "cloud",
instance.NetworkCloudLocal},
I'd prefer to see these initialisers with tagged fields (I can never remember
which order the fields are on, and it makes it easier if the fields change in
the future).

https://codereview.appspot.com/82550043/diff/2/agent/mongo/mongo_test.go#newc...
agent/mongo/mongo_test.go:155: func (s *MongoSuite) TestMongoPackageForSeries(c
*gc.C) {
this doesn't seem to relate to the rest of the code.

[wwitzel3, 2014-03-31 15:19:39]

https://codereview.appspot.com/82550043/diff/2/agent/mongo/mongo.go
File agent/mongo/mongo.go (right):

https://codereview.appspot.com/82550043/diff/2/agent/mongo/mongo.go#newcode35
agent/mongo/mongo.go:35: func MongoSelectPeerAddress(addrs []instance.Address)
string {
On 2014/03/31 14:57:23, rog wrote:
> I know the "Mongo" is gonna go, but in the meantime, I think
> SelectMongoPeerAddress reads a bit better.

Done.

Since we will be renaming them very soon, I just went ahead and called it
SelectPeerAddress

https://codereview.appspot.com/82550043/diff/2/agent/mongo/mongo_test.go
File agent/mongo/mongo_test.go (right):

https://codereview.appspot.com/82550043/diff/2/agent/mongo/mongo_test.go#newc...
agent/mongo/mongo_test.go:147: {"10.0.0.1", instance.Ipv4Address, "cloud",
instance.NetworkCloudLocal},
On 2014/03/31 14:57:23, rog wrote:
> I'd prefer to see these initialisers with tagged fields (I can never remember
> which order the fields are on, and it makes it easier if the fields change in
> the future).

Done.

https://codereview.appspot.com/82550043/diff/2/agent/mongo/mongo_test.go#newc...
agent/mongo/mongo_test.go:155: func (s *MongoSuite) TestMongoPackageForSeries(c
*gc.C) {
On 2014/03/31 14:57:23, rog wrote:
> this doesn't seem to relate to the rest of the code.

It doesn't, I just noticed it was untested and I was editing the tests file
already, should I back it out and do it as a separate proposal?

[wwitzel, 2014-03-31 15:21:22]

Please take a look.

[rog, 2014-03-31 18:59:26]

still LGTM

https://codereview.appspot.com/82550043/diff/20001/agent/mongo/mongo_test.go
File agent/mongo/mongo_test.go (right):

https://codereview.appspot.com/82550043/diff/20001/agent/mongo/mongo_test.go#...
agent/mongo/mongo_test.go:147: {
if you put the { on the previous line,
you can lose an indentation level.

https://codereview.appspot.com/82550043/diff/20001/agent/mongo/mongo_test.go#...
agent/mongo/mongo_test.go:153: {
this would go on the previous line too.