Merge lp:~axwalk/juju-core/lp1167441-report-instance-state into lp:~go-bot/juju-core/trunk

[Andrew Wilkins]

Add Instance.Status method, use in juju status

Add Status() method to Instance interface, and
implementations for azure, ec2, openstack and lxc.
Local and MAAS always return "".

There will be a followup to bubble LXC status
up through the local provider.

Fixes bug #1167441

https://codereview.appspot.com/12333043/

Add Instance.Status method, use in juju status

Add Status() method to Instance interface, and
implementations for azure, ec2, openstack and lxc.
Local and MAAS always return "".

There will be a followup to bubble LXC status
up through the local provider.

Fixes bug #1167441

https://codereview.appspot.com/12333043/

[Andrew Wilkins (axwalk)]

Reviewers: mp+178222_code.launchpad.net,

Message:
Please take a look.

Description:
Add Instance.State method, use in juju status

Add State() method to Instance interface, and
implementations for azure, ec2, openstack and lxc.
Local and MAAS always return "".

https://code.launchpad.net/~axwalk/juju-core/lp1167441-report-instance-state/+merge/178222

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M cmd/juju/status.go
   M container/lxc/instance.go
   M container/lxc/lxc.go
   M container/lxc/lxc_test.go
   M environs/azure/instance.go
   M environs/azure/instance_test.go
   M environs/dummy/environs.go
   M environs/ec2/ec2.go
   M environs/ec2/local_test.go
   M environs/local/instance.go
   M environs/maas/instance.go
   M environs/openstack/local_test.go
   M environs/openstack/provider.go
   M environs/polling_test.go
   M instance/instance.go

[William Reade (fwereade)]

LGTM, this is great. I'd love to see a followup for the local provider,
that should be quick and easy.

https://codereview.appspot.com/12333043/diff/1/container/lxc/instance.go
File container/lxc/instance.go (right):

https://codereview.appspot.com/12333043/diff/1/container/lxc/instance.go#newcode30
container/lxc/instance.go:30: return string(state)
I see this, and it looks good, but I'm a bit confused as to why it
doesn't work with the local provider. (Well, I can see why, but I can't
understand the motivation -- can anyone think of a drawback to putting
the lxc.Instances into the local.Instances so that we can access this
method? I can understand differences in address handling, but I don't
think the same forces apply here.)

https://codereview.appspot.com/12333043/diff/1/environs/local/instance.go
File environs/local/instance.go (right):

https://codereview.appspot.com/12333043/diff/1/environs/local/instance.go#newcode27
environs/local/instance.go:27: return ""
I'd like to do what I suggested earlier and expose the lxc instance
state here. But please do it in a followup, and run it by thumper first,
because if there's a reason not to do it he'll know what it is.

https://codereview.appspot.com/12333043/diff/1/environs/maas/instance.go
File environs/maas/instance.go (right):

https://codereview.appspot.com/12333043/diff/1/environs/maas/instance.go#newcode26
environs/maas/instance.go:26: // TODO determine MAAS instance state
Add a bug in LP and note the number here please; and ping bigjools, or
someone else who knows maas, to find out what'll be needed to implement
it, and note that in the bug.

https://codereview.appspot.com/12333043/diff/1/environs/polling_test.go
File environs/polling_test.go (left):

https://codereview.appspot.com/12333043/diff/1/environs/polling_test.go#oldcode63
environs/polling_test.go:63: func (*dnsNameFakeInstance) Ports(string)
([]instance.Port, error) { return nil, nil }
Complete side note: would it be useful for dnsNameFakeInstance to just
embed an instance.Instance, and thus panic on use of unwanted methods?
Feels a bit saner than just returning nonsense without error...

https://codereview.appspot.com/12333043/

[Andrew Wilkins (axwalk)]

https://codereview.appspot.com/12333043/diff/1/container/lxc/instance.go
File container/lxc/instance.go (right):

https://codereview.appspot.com/12333043/diff/1/container/lxc/instance.go#newcode30
container/lxc/instance.go:30: return string(state)
On 2013/08/02 17:30:32, fwereade wrote:
> I see this, and it looks good, but I'm a bit confused as to why it
doesn't work
> with the local provider. (Well, I can see why, but I can't understand
the
> motivation -- can anyone think of a drawback to putting the
lxc.Instances into
> the local.Instances so that we can access this method? I can
understand
> differences in address handling, but I don't think the same forces
apply here.)

Are you referring to the lxc.Instance being converted to local.Instance?
I don't understand the reason behind this. I think I'll have to have a
chat with thumper after IOM to understand this better.

https://codereview.appspot.com/12333043/diff/1/environs/maas/instance.go
File environs/maas/instance.go (right):

https://codereview.appspot.com/12333043/diff/1/environs/maas/instance.go#newcode26
environs/maas/instance.go:26: // TODO determine MAAS instance state
On 2013/08/02 17:30:32, fwereade wrote:
> Add a bug in LP and note the number here please; and ping bigjools, or
someone
> else who knows maas, to find out what'll be needed to implement it,
and note
> that in the bug.

I had a chat with bigjools on IRC about this. He tells me that the MAAS
server does not track the status of nodes after they're allocated, and
so any node that juju knows about will always show up as "allocated".

The TODO should probably be replaced with a note explaining that status
isn't available. Sound reasonable?

https://codereview.appspot.com/12333043/diff/1/environs/polling_test.go
File environs/polling_test.go (left):

https://codereview.appspot.com/12333043/diff/1/environs/polling_test.go#oldcode63
environs/polling_test.go:63: func (*dnsNameFakeInstance) Ports(string)
([]instance.Port, error) { return nil, nil }
On 2013/08/02 17:30:32, fwereade wrote:
> Complete side note: would it be useful for dnsNameFakeInstance to just
embed an
> instance.Instance, and thus panic on use of unwanted methods? Feels a
bit saner
> than just returning nonsense without error...

SGTM, done

https://codereview.appspot.com/12333043/

[Dimiter Naydenov (dimitern)]

LGTM with a couple of suggestions.

https://codereview.appspot.com/12333043/diff/1/container/lxc/lxc_test.go
File container/lxc/lxc_test.go (right):

https://codereview.appspot.com/12333043/diff/1/container/lxc/lxc_test.go#newcode130
container/lxc/lxc_test.go:130: c.Assert(instance.State(), gc.Equals,
string(golxc.StateUnknown))
This is a bit confusing.. should it be "stopped" or something instead?

https://codereview.appspot.com/12333043/diff/1/environs/azure/instance_test.go
File environs/azure/instance_test.go (right):

https://codereview.appspot.com/12333043/diff/1/environs/azure/instance_test.go#newcode28
environs/azure/instance_test.go:28: func (*InstanceSuite) TestId(c *C) {
Thanks for fixing this. Maybe while you're at it, make it instanceState
instead - no need to be exported.

https://codereview.appspot.com/12333043/diff/1/environs/maas/instance.go
File environs/maas/instance.go (right):

https://codereview.appspot.com/12333043/diff/1/environs/maas/instance.go#newcode26
environs/maas/instance.go:26: // TODO determine MAAS instance state
On 2013/08/05 07:09:06, axw1 wrote:
> On 2013/08/02 17:30:32, fwereade wrote:
> > Add a bug in LP and note the number here please; and ping bigjools,
or someone
> > else who knows maas, to find out what'll be needed to implement it,
and note
> > that in the bug.

> I had a chat with bigjools on IRC about this. He tells me that the
MAAS server
> does not track the status of nodes after they're allocated, and so any
node that
> juju knows about will always show up as "allocated".

> The TODO should probably be replaced with a note explaining that
status isn't
> available. Sound reasonable?

+1

https://codereview.appspot.com/12333043/

[John A Meinel (jameinel)]

Is it possible to call the function "Instance.Status()"? We have a bit
of an overload of State things, and it would be one case less.

I think the reasoning might be that the status command outputs 'state:
pending' but avoiding the word State() in an interface seems useful to
me.

https://codereview.appspot.com/12333043/

[Roger Peppe (rogpeppe)]

+1 to Status, good call.

On 8 August 2013 18:27, John A Meinel <email address hidden> wrote:
> Is it possible to call the function "Instance.Status()"? We have a bit
> of an overload of State things, and it would be one case less.
>
> I think the reasoning might be that the status command outputs 'state:
> pending' but avoiding the word State() in an interface seems useful to
> me.
>
>
> https://codereview.appspot.com/12333043/
>
> --
> https://code.launchpad.net/~axwalk/juju-core/lp1167441-report-instance-state/+merge/178222
> Your team juju hackers is requested to review the proposed merge of lp:~axwalk/juju-core/lp1167441-report-instance-state into lp:juju-core.

[John A Meinel (jameinel)]

Another small thing, if you can mention "bug #1167441" somewhere in your
proposal, it makes it obvious that there is a bug thread on Launchpad
that can be read to find more context.

It is true that the LP merge proposal does give the link to related
bugs, but it is a helpful reminder when reading the proposal (CL) that
there is more context available.

https://codereview.appspot.com/12333043/

[Andrew Wilkins (axwalk)]

On 2013/08/08 17:27:01, jameinel wrote:
> Is it possible to call the function "Instance.Status()"? We have a bit
of an
> overload of State things, and it would be one case less.

> I think the reasoning might be that the status command outputs 'state:
pending'
> but avoiding the word State() in an interface seems useful to me.

Hah, I had it as Status originally, and changed it to State to match
"instance-state". I'll change it back.

https://codereview.appspot.com/12333043/

[Andrew Wilkins (axwalk)]

Please take a look.

https://codereview.appspot.com/12333043/

[Go Bot (go-bot)]

The attempt to merge lp:~axwalk/juju-core/lp1167441-report-instance-state into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core/agent 1.667s
ok launchpad.net/juju-core/agent/tools 26.411s
ok launchpad.net/juju-core/bzr 6.795s
ok launchpad.net/juju-core/cert 2.645s
ok launchpad.net/juju-core/charm 0.518s
? launchpad.net/juju-core/charm/hooks [no test files]
ok launchpad.net/juju-core/cloudinit 0.019s
ok launchpad.net/juju-core/cmd 0.248s
? launchpad.net/juju-core/cmd/builddb [no test files]
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/juju 105.948s
ok launchpad.net/juju-core/cmd/jujud 51.162s
ok launchpad.net/juju-core/cmd/plugins/juju-metadata 0.859s
ok launchpad.net/juju-core/constraints 0.024s
ok launchpad.net/juju-core/container/lxc 0.433s
? launchpad.net/juju-core/container/lxc/mock [no test files]
ok launchpad.net/juju-core/downloader 5.308s
ok launchpad.net/juju-core/environs 1.305s
? launchpad.net/juju-core/environs/all [no test files]
ok launchpad.net/juju-core/environs/azure 4.000s
ok launchpad.net/juju-core/environs/cloudinit 0.443s
ok launchpad.net/juju-core/environs/config 0.745s
ok launchpad.net/juju-core/environs/dummy 16.007s
2013-08-09 02:50:55 WARNING juju.environs.config config.go:429 unknown config field "future"
2013-08-09 02:50:55 WARNING juju.environs.config config.go:429 unknown config field "future"
2013-08-09 02:50:55 WARNING juju.environs.config config.go:429 unknown config field "future"

----------------------------------------------------------------------
FAIL: local_test.go:306: localServerSuite.TestInstanceStatus

[LOG] 68.93735 INFO juju environs/testing: uploading FAKE tools 1.13.1-precise-amd64
[LOG] 68.95787 INFO juju environs/ec2: opening environment "sample"
[LOG] 73.78884 INFO juju environs/ec2: bootstrapping environment "sample"
[LOG] 73.78886 INFO juju environs: reading tools with major version 1
[LOG] 73.78888 DEBUG juju.agent.tools reading v1.* tools
[LOG] 73.78932 INFO juju environs: falling back to public bucket
[LOG] 73.78933 DEBUG juju.agent.tools reading v1.* tools
[LOG] 73.78992 DEBUG juju.agent.tools found 1.13.1-precise-amd64
[LOG] 73.78998 INFO juju environs: filtering tools by series: precise
[LOG] 73.79000 INFO juju environs: picked newest version: 1.13.1
[LOG] 73.82766 WARNING juju.environs.imagemetadata cannot load index "test:"/"streams/v1/index.sjson": cannot find URL "test:/streams/v1/index.sjson" not found
[LOG] 73.82778 DEBUG juju.environs.imagemetadata candidate matches for products ["com.ubuntu.cloud:server:12.04:amd64"] are [0xc20021da00]
[LOG] 73.99677 DEBUG juju environs/ec2: ec2 user data; 7410 bytes
[LOG] 74.00157 INFO juju environs/ec2: started instance "i-3"
[LOG] 74.00251 DEBUG juju waiting for DNS name(s) of state server instances [i-3]
[LOG] 74.00499 INFO juju environs: reading tools with major version 1
[LOG] 74.00500 DEBUG juju.agent.tools reading v1.* tools
[LOG] 74.00549 INFO juju environs: falling back to public bucket
[LOG] 74.00550 DEBUG juju.agent.tools reading v1.* tools
[L...

[Andrew Wilkins (axwalk)]

Bot needs an updated goamz. I take it that it's not using dependencies.tsv yet...

[Tim Penhey (thumper)]

https://codereview.appspot.com/12333043/diff/1/container/lxc/instance.go
File container/lxc/instance.go (right):

https://codereview.appspot.com/12333043/diff/1/container/lxc/instance.go#newcode30
container/lxc/instance.go:30: return string(state)
On 2013/08/05 07:09:06, axw1 wrote:
> On 2013/08/02 17:30:32, fwereade wrote:
> > I see this, and it looks good, but I'm a bit confused as to why it
doesn't
> work
> > with the local provider. (Well, I can see why, but I can't
understand the
> > motivation -- can anyone think of a drawback to putting the
lxc.Instances into
> > the local.Instances so that we can access this method? I can
understand
> > differences in address handling, but I don't think the same forces
apply
> here.)

> Are you referring to the lxc.Instance being converted to
local.Instance? I don't
> understand the reason behind this. I think I'll have to have a chat
with thumper
> after IOM to understand this better.

I think the comment is more to have local.Instance use lxc.Instance not
the other way around. I've not looked at the differences yet, so for
now would prefer to keep them separate.

Perhaps, we have local.Instance be composed of an lxc.Instance so it can
defer to the lxc commands and just override what is different. Take
this with a grain of salt as I haven't looked at the code right now.

https://codereview.appspot.com/12333043/

=== modified file 'cmd/juju/status.go'
--- cmd/juju/status.go	2013-08-08 15:34:49 +0000
+++ cmd/juju/status.go	2013-08-09 02:41:46 +0000
@@ -357,6 +357,7 @@
 		inst, ok := context.instances[instid]
 		if ok {
 			status.DNSName, _ = inst.DNSName()
+			status.InstanceState = inst.Status()
 		} else {
 			// Double plus ungood.  There is an instance id recorded
 			// for this machine in the state, yet the environ cannot

=== modified file 'container/lxc/instance.go'
--- container/lxc/instance.go	2013-07-19 01:18:26 +0000
+++ container/lxc/instance.go	2013-08-09 02:41:46 +0000
@@ -6,10 +6,13 @@
 import (
 	"fmt"
 
+	"launchpad.net/golxc"
+
 	"launchpad.net/juju-core/instance"
 )
 
 type lxcInstance struct {
+	golxc.Container
 	id string
 }
 
@@ -20,6 +23,13 @@
 	return instance.Id(lxc.id)
 }
 
+// Status implements instance.Instance.Status.
+func (lxc *lxcInstance) Status() string {
+	// On error, the state will be "unknown".
+	state, _, _ := lxc.Info()
+	return string(state)
+}
+
 func (lxc *lxcInstance) Addresses() ([]instance.Address, error) {
 	logger.Errorf("lxcInstance.Addresses not implemented")
 	return nil, nil

=== modified file 'container/lxc/lxc.go'
--- container/lxc/lxc.go	2013-08-06 07:40:45 +0000
+++ container/lxc/lxc.go	2013-08-09 02:41:46 +0000
@@ -188,7 +188,7 @@
 		return nil, err
 	}
 	logger.Tracef("container started")
-	return &lxcInstance{name}, nil
+	return &lxcInstance{container, name}, nil
 }
 
 func (manager *containerManager) StopContainer(instance instance.Instance) error {
@@ -240,7 +240,7 @@
 			continue
 		}
 		if container.IsRunning() {
-			result = append(result, &lxcInstance{name})
+			result = append(result, &lxcInstance{container, name})
 		}
 	}
 	return

=== modified file 'container/lxc/lxc_test.go'
--- container/lxc/lxc_test.go	2013-08-06 07:40:45 +0000
+++ container/lxc/lxc_test.go	2013-08-09 02:41:46 +0000
@@ -11,6 +11,7 @@
 	stdtesting "testing"
 
 	gc "launchpad.net/gocheck"
+	"launchpad.net/golxc"
 	"launchpad.net/goyaml"
 	"launchpad.net/loggo"
 
@@ -142,6 +143,20 @@
 	c.Assert(location, gc.Equals, expectedTarget)
 }
 
+func (s *LxcSuite) TestContainerState(c *gc.C) {
+	manager := lxc.NewContainerManager(lxc.ManagerConfig{})
+	instance := StartContainer(c, manager, "1/lxc/0")
+
+	// The mock container will be immediately "running".
+	c.Assert(instance.Status(), gc.Equals, string(golxc.StateRunning))
+
+	// StopContainer stops and then destroys the container, putting it
+	// into "unknown" state.
+	err := manager.StopContainer(instance)
+	c.Assert(err, gc.IsNil)
+	c.Assert(instance.Status(), gc.Equals, string(golxc.StateUnknown))
+}
+
 func (s *LxcSuite) TestStopContainer(c *gc.C) {
 	manager := lxc.NewContainerManager(lxc.ManagerConfig{})
 	instance := StartContainer(c, manager, "1/lxc/0")

=== modified file 'environs/azure/instance.go'
--- environs/azure/instance.go	2013-08-07 10:23:50 +0000
+++ environs/azure/instance.go	2013-08-09 02:41:46 +0000
@@ -29,6 +29,11 @@
 	return instance.Id(azInstance.ServiceName)
 }
 
+// Status is specified in the Instance interface.
+func (azInstance *azureInstance) Status() string {
+	return azInstance.HostedServiceDescriptor.Status
+}
+
 var AZURE_DOMAIN_NAME = "cloudapp.net"
 
 // Addresses is specified in the Instance interface.

=== modified file 'environs/azure/instance_test.go'
--- environs/azure/instance_test.go	2013-08-08 11:22:08 +0000
+++ environs/azure/instance_test.go	2013-08-09 02:41:46 +0000
@@ -32,6 +32,14 @@
 	c.Check(azInstance.Id(), Equals, instance.Id(serviceName))
 }
 
+func (*instanceSuite) TestStatus(c *C) {
+	serviceName := "test-name"
+	testService := makeHostedServiceDescriptor(serviceName)
+	testService.Status = "something"
+	azInstance := azureInstance{*testService, nil}
+	c.Check(azInstance.Status(), Equals, testService.Status)
+}
+
 func (*instanceSuite) TestDNSName(c *C) {
 	// An instance's DNS name is computed from its hosted-service name.
 	host := "hostname"

=== modified file 'environs/dummy/environs.go'
--- environs/dummy/environs.go	2013-07-30 16:39:37 +0000
+++ environs/dummy/environs.go	2013-08-09 02:41:46 +0000
@@ -726,6 +726,10 @@
 	return inst.id
 }
 
+func (inst *dummyInstance) Status() string {
+	return ""
+}
+
 func (inst *dummyInstance) DNSName() (string, error) {
 	defer delay()
 	return string(inst.id) + ".dns", nil

=== modified file 'environs/ec2/ec2.go'
--- environs/ec2/ec2.go	2013-07-30 23:46:30 +0000
+++ environs/ec2/ec2.go	2013-08-09 02:41:46 +0000
@@ -72,6 +72,10 @@
 	return instance.Id(inst.InstanceId)
 }
 
+func (inst *ec2Instance) Status() string {
+	return inst.State.Name
+}
+
 func (inst *ec2Instance) hardwareCharacteristics() *instance.HardwareCharacteristics {
 	hc := &instance.HardwareCharacteristics{Arch: inst.arch}
 	if inst.instType != nil {

=== modified file 'environs/ec2/local_test.go'
--- environs/ec2/local_test.go	2013-07-30 23:46:30 +0000
+++ environs/ec2/local_test.go	2013-08-09 02:41:46 +0000
@@ -303,6 +303,21 @@
 	c.Assert(err, NotNil)
 }
 
+func (t *localServerSuite) TestInstanceStatus(c *C) {
+	err := environs.Bootstrap(t.env, constraints.Value{})
+	c.Assert(err, IsNil)
+	series := t.env.Config().DefaultSeries()
+	info, apiInfo, err := t.env.StateInfo()
+	c.Assert(err, IsNil)
+	c.Assert(info, NotNil)
+	info.Tag = "machine-1"
+	apiInfo.Tag = "machine-1"
+	t.srv.ec2srv.SetInitialInstanceState(ec2test.Terminated)
+	inst, _, err := t.env.StartInstance("1", "fake_nonce", series, constraints.Value{}, info, apiInfo)
+	c.Assert(err, IsNil)
+	c.Assert(inst.Status(), Equals, "terminated")
+}
+
 func (t *localServerSuite) TestStartInstanceHardwareCharacteristics(c *C) {
 	err := environs.Bootstrap(t.env, constraints.Value{})
 	c.Assert(err, IsNil)

=== modified file 'environs/local/instance.go'
--- environs/local/instance.go	2013-07-19 01:18:26 +0000
+++ environs/local/instance.go	2013-08-09 02:41:46 +0000
@@ -22,6 +22,11 @@
 	return inst.id
 }
 
+// Status implements instance.Instance.Status.
+func (inst *localInstance) Status() string {
+	return ""
+}
+
 func (inst *localInstance) Addresses() ([]instance.Address, error) {
 	logger.Errorf("localInstance.Addresses not implemented")
 	return nil, nil

=== modified file 'environs/maas/instance.go'
--- environs/maas/instance.go	2013-07-19 01:18:26 +0000
+++ environs/maas/instance.go	2013-08-09 02:41:46 +0000
@@ -22,6 +22,15 @@
 	return instance.Id((*mi.maasObject).URI().String())
 }
 
+func (mi *maasInstance) Status() string {
+	// MAAS does not track node status once they're allocated.
+	// Since any instance that juju knows about will be an
+	// allocated one, it doesn't make sense to report any
+	// state unless we obtain it through some means other than
+	// through the MAAS API.
+	return ""
+}
+
 // refreshInstance refreshes the instance with the most up-to-date information
 // from the MAAS server.
 func (mi *maasInstance) refreshInstance() error {

=== modified file 'environs/openstack/local_test.go'
--- environs/openstack/local_test.go	2013-07-30 23:46:30 +0000
+++ environs/openstack/local_test.go	2013-08-09 02:41:46 +0000
@@ -330,6 +330,14 @@
 	},
 }
 
+func (s *localServerSuite) TestInstanceStatus(c *C) {
+	// goose's test service always returns ACTIVE state.
+	inst, _ := testing.StartInstance(c, s.Env, "100")
+	c.Assert(inst.Status(), Equals, nova.StatusActive)
+	err := s.Env.StopInstances([]instance.Instance{inst})
+	c.Assert(err, IsNil)
+}
+
 func (s *localServerSuite) TestInstancesGathering(c *C) {
 	inst0, _ := testing.StartInstance(c, s.Env, "100")
 	id0 := inst0.Id()

=== modified file 'environs/openstack/provider.go'
--- environs/openstack/provider.go	2013-07-30 23:46:30 +0000
+++ environs/openstack/provider.go	2013-08-09 02:41:46 +0000
@@ -242,6 +242,10 @@
 	return instance.Id(inst.ServerDetail.Id)
 }
 
+func (inst *openstackInstance) Status() string {
+	return inst.ServerDetail.Status
+}
+
 func (inst *openstackInstance) hardwareCharacteristics() *instance.HardwareCharacteristics {
 	hc := &instance.HardwareCharacteristics{Arch: inst.arch}
 	if inst.instType != nil {

=== modified file 'environs/polling_test.go'
--- environs/polling_test.go	2013-07-24 14:37:17 +0000
+++ environs/polling_test.go	2013-08-09 02:41:46 +0000
@@ -42,6 +42,8 @@
 // DNSName returns whatever you tell it to, and WaitDNSName delegates to the
 // shared WaitDNSName implementation.  All the other methods are empty stubs.
 type dnsNameFakeInstance struct {
+	// embed a nil Instance to panic on unimplemented method
+	instance.Instance
 	name string
 	err  error
 }
@@ -56,11 +58,7 @@
 	return environs.WaitDNSName(inst)
 }
 
-func (*dnsNameFakeInstance) Addresses() ([]instance.Address, error)   { return nil, nil }
-func (*dnsNameFakeInstance) Id() instance.Id                          { return "" }
-func (*dnsNameFakeInstance) OpenPorts(string, []instance.Port) error  { return nil }
-func (*dnsNameFakeInstance) ClosePorts(string, []instance.Port) error { return nil }
-func (*dnsNameFakeInstance) Ports(string) ([]instance.Port, error)    { return nil, nil }
+func (*dnsNameFakeInstance) Id() instance.Id { return "" }
 
 func (pollingSuite) TestWaitDNSNameReturnsDNSNameIfAvailable(c *C) {
 	inst := dnsNameFakeInstance{name: "anansi"}

=== modified file 'instance/instance.go'
--- instance/instance.go	2013-07-24 13:07:02 +0000
+++ instance/instance.go	2013-08-09 02:41:46 +0000
@@ -32,6 +32,9 @@
 	// Id returns a provider-generated identifier for the Instance.
 	Id() Id
 
+	// Status returns the provider-specific status for the instance.
+	Status() string
+
 	// Addresses returns a list of hostnames or ip addresses
 	// associated with the instance. This will supercede DNSName
 	// which can be implemented by selecting a preferred address.