Reviewers: mp+189847_code.launchpad.net,

Message:
Please take a look.

Description:
provider/maas: Handle ip_addresses not existing

Older MAAS versions do not return the ip_addresses field in
their instance json. Update the provider to not treat this
as an error, but instead equivalent to having no further
addresses beyond the seperately exposed DNS name.

https://code.launchpad.net/~gz/juju-core/maas_missing_ip_addresses_1236734/+merge/189847

(do not edit description out of merge proposal)


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

Affected files (+59, -10 lines):
   A [revision details]
   M provider/maas/instance.go
   M provider/maas/instance_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-20131008120312-vr5sryx6voukx4o8
+New revision: <email address hidden>

Index: provider/maas/instance.go
=== modified file 'provider/maas/instance.go'
--- provider/maas/instance.go	2013-09-30 19:40:06 +0000
+++ provider/maas/instance.go	2013-10-08 13:29:41 +0000
@@ -76,7 +76,12 @@

  func (mi *maasInstance) ipAddresses() ([]string, error) {
  	// we have to do this the hard way, since maasObject doesn't have this  
built-in yet
-	objs, err := mi.maasObject.GetMap()["ip_addresses"].GetArray()
+	addressArray := mi.maasObject.GetMap()["ip_addresses"]
+	if addressArray.IsNil() {
+		// Older MAAS versions do not return ip_addresses.
+		return nil, nil
+	}
+	objs, err := addressArray.GetArray()
  	if err != nil {
  		return nil, err
  	}


Index: provider/maas/instance_test.go
=== modified file 'provider/maas/instance_test.go'
--- provider/maas/instance_test.go	2013-09-09 07:02:57 +0000
+++ provider/maas/instance_test.go	2013-10-08 13:29:41 +0000
@@ -9,7 +9,6 @@
  	gc "launchpad.net/gocheck"

  	"launchpad.net/juju-core/instance"
-	jc "launchpad.net/juju-core/testing/checkers"
  )

  type instanceTest struct {
@@ -88,6 +87,12 @@

  	expected := []instance.Address{
  		{
+			"DNS name",
+			instance.HostName,
+			"",
+			instance.NetworkPublic,
+		},
+		{
  			"1.2.3.4",
  			instance.Ipv4Address,
  			"",
@@ -99,16 +104,53 @@
  			"",
  			instance.NetworkUnknown,
  		},
-		{
-			"DNS name",
-			instance.HostName,
-			"",
-			instance.NetworkPublic,
-		},
  	}

  	addr, err := inst.Addresses()

-	c.Check(err, gc.IsNil)
-	c.Check(addr, jc.SameContents, expected)
+	c.Assert(err, gc.IsNil)
+	c.Check(addr, gc.DeepEquals, expected)
+}
+
+func (s *instanceTest) TestAddressesMissing(c *gc.C) {
+	// Older MAAS versions do not have ip_addresses returned, for these
+	// just the DNS name should be returned without error.
+	jsonValue := `{
+		"hostname": "testing.invalid",
+		"system_id": "system_id"
+		}`
+	obj := s.testMAASObject.TestServer.NewNode(jsonValue)
+	inst := maasInstance{&obj, s.environ}
+
+	addr, err := inst.Addresses()
+	c.Assert(err, gc.IsNil)
+	c.Check(addr, gc.DeepEquals, []instance.Address{
+		{"testing.invalid", instance.HostName, "", instance.NetworkPublic},
+	})
+}
+
+func (s *instanceTest) TestAddressesInvalid(c *gc.C) {
+	jsonValue := `{
+		"hostname": "testing.invalid",
+		"system_id": "system_id",
+		"ip_addresses": "incompatible"
+		}`
+	obj := s.testMAASObject.TestServer.NewNode(jsonValue)
+	inst := maasInstance{&obj, s.environ}
+
+	_, err := inst.Addresses()
+	c.Assert(err, gc.NotNil)
+}
+
+func (s *instanceTest) TestAddressesInvalidContents(c *gc.C) {
+	jsonValue := `{
+		"hostname": "testing.invalid",
+		"system_id": "system_id",
+		"ip_addresses": [42]
+		}`
+	obj := s.testMAASObject.TestServer.NewNode(jsonValue)
+	inst := maasInstance{&obj, s.environ}
+
+	_, err := inst.Addresses()
+	c.Assert(err, gc.NotNil)
  }