Code review comment for lp:~axwalk/juju-core/agent-setapihostports

Reviewers: mp+213409_code.launchpad.net,

Message:
Please take a look.

Description:
agent: add ConfigSetterOnly.SetAPIHostPorts

We still only store addresses in the config,
so we lose some metadata. We make use of the
scope metadata before storing, though, to
prune the addresses to the internal ones.

https://code.launchpad.net/~axwalk/juju-core/agent-setapihostports/+merge/213409

(do not edit description out of merge proposal)

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

Affected files (+54, -0 lines):
   A [revision details]
   M agent/agent.go
   M agent/agent_test.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-20140330221944-tt65zj7hl6967aad
+New revision: <email address hidden>

Index: agent/agent.go
=== modified file 'agent/agent.go'
--- agent/agent.go 2014-03-28 08:55:45 +0000
+++ agent/agent.go 2014-03-31 06:30:43 +0000
@@ -16,6 +16,7 @@
   "github.com/errgo/errgo"
   "github.com/juju/loggo"

+ "launchpad.net/juju-core/instance"
   "launchpad.net/juju-core/state"
   "launchpad.net/juju-core/state/api"
   "launchpad.net/juju-core/state/api/params"
@@ -133,6 +134,9 @@
   // the agent has successfully upgraded to.
   SetUpgradedToVersion(newVersion version.Number)

+ // SetAPIHostPorts sets the API host/port addresses to connect to.
+ SetAPIHostPorts(servers [][]instance.HostPort)
+
   // Migrate takes an existing agent config and applies the given
   // parameters to change it.
   //
@@ -423,6 +427,20 @@
   c.upgradedToVersion = newVersion
  }

+func (c *configInternal) SetAPIHostPorts(servers [][]instance.HostPort) {
+ if c.apiDetails == nil {
+ return
+ }
+ var addrs []string
+ for _, serverHostPorts := range servers {
+ addr := instance.SelectInternalHostPort(serverHostPorts, false)
+ if addr != "" {
+ addrs = append(addrs, addr)
+ }
+ }
+ c.apiDetails.addresses = addrs
+}
+
  func (c *configInternal) SetValue(key, value string) {
   if value == "" {
    delete(c.values, key)

Index: agent/agent_test.go
=== modified file 'agent/agent_test.go'
--- agent/agent_test.go 2014-03-28 08:28:04 +0000
+++ agent/agent_test.go 2014-03-31 06:30:43 +0000
@@ -10,6 +10,7 @@
   gc "launchpad.net/gocheck"

   "launchpad.net/juju-core/agent"
+ "launchpad.net/juju-core/instance"
   "launchpad.net/juju-core/state"
   "launchpad.net/juju-core/state/api"
   "launchpad.net/juju-core/state/api/params"
@@ -430,3 +431,36 @@
   conf.SetUpgradedToVersion(expectVers)
   c.Assert(conf.UpgradedToVersion(), gc.Equals, expectVers)
  }
+
+func (*suite) TestSetAPIHostPorts(c *gc.C) {
+ conf, err := agent.NewAgentConfig(attributeParams)
+ c.Assert(err, gc.IsNil)
+
+ addrs, err := conf.APIAddresses()
+ c.Assert(err, gc.IsNil)
+ c.Assert(addrs, gc.DeepEquals, attributeParams.APIAddresses)
+
+ // The first cloud-local address for each server is used,
+ // else if there are none then the first public- or unknown-
+ // scope address.
+ //
+ // If a server has only machine-local addresses, or none
+ // at all, then it will be excluded.
+ server1 := instance.NewAddresses("0.1.2.3", "0.1.2.4", "zeroonetwothree")
+ server1[0].NetworkScope = instance.NetworkCloudLocal
+ server1[1].NetworkScope = instance.NetworkCloudLocal
+ server1[2].NetworkScope = instance.NetworkPublic
+ server2 := instance.NewAddresses("127.0.0.1")
+ server2[0].NetworkScope = instance.NetworkMachineLocal
+ server3 := instance.NewAddresses("0.1.2.5", "zeroonetwofive")
+ server3[0].NetworkScope = instance.NetworkUnknown
+ server3[1].NetworkScope = instance.NetworkUnknown
+ conf.SetAPIHostPorts([][]instance.HostPort{
+ instance.AddressesWithPort(server1, 123),
+ instance.AddressesWithPort(server2, 124),
+ instance.AddressesWithPort(server3, 125),
+ })
+ addrs, err = conf.APIAddresses()
+ c.Assert(err, gc.IsNil)
+ c.Assert(addrs, gc.DeepEquals, []string{"0.1.2.3:123", "0.1.2.5:125"})
+}