Reviewers: mp+214857_code.launchpad.net,

Message:
Please take a look.

Description:
Noisy instance poller

The instance poller was trying to poll instances which
are not yet provisioned, creating lots of log noise.
This fixes that.

https://code.launchpad.net/~wallyworld/juju-core/instance-poller-noise/+merge/214857

(do not edit description out of merge proposal)


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

Affected files (+25, -9 lines):
   A [revision details]
   M worker/instancepoller/machine_test.go
   M worker/instancepoller/updater.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-20140408170048-h58307lijtftb3su
+New revision: <email address hidden>

Index: worker/instancepoller/machine_test.go
=== modified file 'worker/instancepoller/machine_test.go'
--- worker/instancepoller/machine_test.go	2014-04-01 23:56:19 +0000
+++ worker/instancepoller/machine_test.go	2014-04-09 00:14:53 +0000
@@ -61,24 +61,31 @@
  func (s *machineSuite) TestShortPollIntervalWhenNoAddress(c *gc.C) {
  	s.PatchValue(&ShortPoll, 1*time.Millisecond)
  	s.PatchValue(&LongPoll, coretesting.LongWait)
-	count := countPolls(c, nil, "running", params.StatusStarted)
+	count := countPolls(c, nil, "i1234", "running", params.StatusStarted)
  	c.Assert(count, jc.GreaterThan, 2)
  }

  func (s *machineSuite) TestShortPollIntervalWhenNoStatus(c *gc.C) {
  	s.PatchValue(&ShortPoll, 1*time.Millisecond)
  	s.PatchValue(&LongPoll, coretesting.LongWait)
-	count := countPolls(c, testAddrs, "", params.StatusStarted)
+	count := countPolls(c, testAddrs, "i1234", "", params.StatusStarted)
  	c.Assert(count, jc.GreaterThan, 2)
  }

  func (s *machineSuite) TestShortPollIntervalWhenNotStarted(c *gc.C) {
  	s.PatchValue(&ShortPoll, 1*time.Millisecond)
  	s.PatchValue(&LongPoll, coretesting.LongWait)
-	count := countPolls(c, testAddrs, "pending", params.StatusPending)
+	count := countPolls(c, testAddrs, "i1234", "pending",  
params.StatusPending)
  	c.Assert(count, jc.GreaterThan, 2)
  }

+func (s *machineSuite) TestShortPollIntervalWhenNotProvisioned(c *gc.C) {
+	s.PatchValue(&ShortPoll, 1*time.Millisecond)
+	s.PatchValue(&LongPoll, coretesting.LongWait)
+	count := countPolls(c, testAddrs, "", "pending", params.StatusPending)
+	c.Assert(count, gc.Equals, 0)
+}
+
  func (s *machineSuite) TestShortPollIntervalExponent(c *gc.C) {
  	s.PatchValue(&ShortPoll, 1*time.Microsecond)
  	s.PatchValue(&LongPoll, coretesting.LongWait)
@@ -89,7 +96,7 @@
  	// ShortPollBackoff of ShortWait/ShortPoll, given that sleep will
  	// sleep for at least the requested interval.
  	maxCount :=  
int(math.Log(float64(coretesting.ShortWait)/float64(ShortPoll))/math.Log(ShortPollBackoff)  
+ 1)
-	count := countPolls(c, nil, "", params.StatusStarted)
+	count := countPolls(c, nil, "i1234", "", params.StatusStarted)
  	c.Assert(count, jc.GreaterThan, 2)
  	c.Assert(count, jc.LessThan, maxCount)
  	c.Logf("actual count: %v; max %v", count, maxCount)
@@ -98,7 +105,7 @@
  func (s *machineSuite) TestLongPollIntervalWhenHasAllInstanceInfo(c *gc.C)  
{
  	s.PatchValue(&ShortPoll, coretesting.LongWait)
  	s.PatchValue(&LongPoll, 1*time.Millisecond)
-	count := countPolls(c, testAddrs, "running", params.StatusStarted)
+	count := countPolls(c, testAddrs, "i1234", "running",  
params.StatusStarted)
  	c.Assert(count, jc.GreaterThan, 2)
  }

@@ -106,7 +113,7 @@
  // addresses and status to be returned from getInstanceInfo,
  // waits for coretesting.ShortWait, and returns the
  // number of times the instance is polled.
-func countPolls(c *gc.C, addrs []instance.Address, instStatus string,  
machineStatus params.Status) int {
+func countPolls(c *gc.C, addrs []instance.Address, instId, instStatus  
string, machineStatus params.Status) int {
  	count := int32(0)
  	getInstanceInfo := func(id instance.Id) (instanceInfo, error) {
  		c.Check(id, gc.Equals, instance.Id("i1234"))
@@ -122,7 +129,7 @@
  	}
  	m := &testMachine{
  		id:         "99",
-		instanceId: "i1234",
+		instanceId: instance.Id(instId),
  		refresh:    func() error { return nil },
  		addresses:  addrs,
  		life:       state.Alive,
@@ -342,6 +349,9 @@
  }

  func (m *testMachine) InstanceId() (instance.Id, error) {
+	if m.instanceId == "" {
+		return "", state.NotProvisionedError(m.Id())
+	}
  	return m.instanceId, m.instanceIdErr
  }



Index: worker/instancepoller/updater.go
=== modified file 'worker/instancepoller/updater.go'
--- worker/instancepoller/updater.go	2014-03-30 22:34:01 +0000
+++ worker/instancepoller/updater.go	2014-04-09 00:14:53 +0000
@@ -171,7 +171,7 @@
  	for {
  		if pollInstance {
  			instInfo, err := pollInstanceInfo(context, m)
-			if err != nil {
+			if err != nil && !state.IsNotProvisionedError(err) {
  				// If the provider doesn't implement Addresses/Status now,
  				// it never will until we're upgraded, so don't bother
  				// asking any more. We could use less resources
@@ -220,7 +220,11 @@
  func pollInstanceInfo(context machineContext, m machine) (instInfo  
instanceInfo, err error) {
  	instInfo = instanceInfo{}
  	instId, err := m.InstanceId()
-	if err != nil && !state.IsNotProvisionedError(err) {
+	// We can't ask the machine for its addresses if it isn't provisioned yet.
+	if state.IsNotProvisionedError(err) {
+		return instInfo, err
+	}
+	if err != nil {
  		return instInfo, fmt.Errorf("cannot get machine's instance id: %v", err)
  	}
  	instInfo, err = context.instanceInfo(instId)