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.