Merge lp:~axwalk/goamz/ec2test-empty-dnsname into lp:goamz

[Andrew Wilkins]

ec2/ec2test: set empty DNSName initially

Initially set empty DNSName, causing the
client to re-request the DNSName. This
exercises the address refreshing logic
in juju's ec2 provider.

https://codereview.appspot.com/43650045/

ec2/ec2test: set empty DNSName initially

Initially set empty DNSName, causing the
client to re-request the DNSName. This
exercises the address refreshing logic
in juju's ec2 provider.

https://codereview.appspot.com/43650045/

[Andrew Wilkins (axwalk)]

Reviewers: mp+199466_code.launchpad.net,

Message:
Please take a look.

Description:
ec2/ec2test: set empty DNSName initially

Initially set empty DNSName, causing the
client to re-request the DNSName. This
exercises the address refreshing logic
in juju's ec2 provider.

https://code.launchpad.net/~axwalk/goamz/ec2test-empty-dnsname/+merge/199466

(do not edit description out of merge proposal)

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

Affected files (+13, -2 lines):
   A [revision details]
   M ec2/ec2test/server.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: <email address hidden>
+New revision: <email address hidden>

Index: ec2/ec2test/server.go
=== modified file 'ec2/ec2test/server.go'
--- ec2/ec2test/server.go 2013-10-03 12:10:33 +0000
+++ ec2/ec2test/server.go 2013-12-18 14:22:35 +0000
@@ -66,7 +66,8 @@

  // instance holds a simulated ec2 instance
  type Instance struct {
- seq int
+ seq int
+ dnsNameSet bool
   // UserData holds the data that was passed to the RunInstances request
   // when the instance was started.
   UserData []byte
@@ -550,11 +551,19 @@

  func (inst *Instance) ec2instance() ec2.Instance {
   id := inst.id()
+ // The first time the instance is returned, its DNSName
+ // will be empty. The client should then refresh the instance.
+ var dnsName string
+ if inst.dnsNameSet {
+ dnsName = fmt.Sprintf("%s.testing.invalid", id)
+ } else {
+ inst.dnsNameSet = true
+ }
   return ec2.Instance{
    InstanceId: id,
    InstanceType: inst.instType,
    ImageId: inst.imageId,
- DNSName: fmt.Sprintf("%s.testing.invalid", id),
+ DNSName: dnsName,
    PrivateDNSName: fmt.Sprintf("%s.internal.invalid", id),
    IPAddress: fmt.Sprintf("8.0.0.%d", inst.seq%256),
    PrivateIPAddress: fmt.Sprintf("127.0.0.%d", inst.seq%256),

[Roger Peppe (rogpeppe)]

LGTM with one thought below.

https://codereview.appspot.com/43650045/diff/1/ec2/ec2test/server.go
File ec2/ec2test/server.go (right):

https://codereview.appspot.com/43650045/diff/1/ec2/ec2test/server.go#newcode567
ec2/ec2test/server.go:567: PrivateDNSName:
fmt.Sprintf("%s.internal.invalid", id),
I wonder if we should do it for these addresses too.

https://codereview.appspot.com/43650045/

[Andrew Wilkins (axwalk)]

On 2013/12/18 14:39:38, rog wrote:
> LGTM with one thought below.

> https://codereview.appspot.com/43650045/diff/1/ec2/ec2test/server.go
> File ec2/ec2test/server.go (right):

https://codereview.appspot.com/43650045/diff/1/ec2/ec2test/server.go#newcode567
> ec2/ec2test/server.go:567: PrivateDNSName:
fmt.Sprintf("%s.internal.invalid",
> id),
> I wonder if we should do it for these addresses too.

I can see it could be useful, but I think only if you could control
which ones are empty, or if they're initially all empty and each one is
updated on successive calls.
That's a much bigger change, though, that I'd like to leave to when we
need it.

https://codereview.appspot.com/43650045/

=== modified file 'ec2/ec2test/server.go'
--- ec2/ec2test/server.go	2013-10-03 12:10:33 +0000
+++ ec2/ec2test/server.go	2013-12-18 14:30:43 +0000
@@ -66,7 +66,8 @@
 
 // instance holds a simulated ec2 instance
 type Instance struct {
-	seq         int
+	seq        int
+	dnsNameSet bool
 	// UserData holds the data that was passed to the RunInstances request
 	// when the instance was started.
 	UserData    []byte
@@ -550,11 +551,19 @@
 
 func (inst *Instance) ec2instance() ec2.Instance {
 	id := inst.id()
+	// The first time the instance is returned, its DNSName
+	// will be empty. The client should then refresh the instance.
+	var dnsName string
+	if inst.dnsNameSet {
+		dnsName = fmt.Sprintf("%s.testing.invalid", id)
+	} else {
+		inst.dnsNameSet = true
+	}
 	return ec2.Instance{
 		InstanceId:       id,
 		InstanceType:     inst.instType,
 		ImageId:          inst.imageId,
-		DNSName:          fmt.Sprintf("%s.testing.invalid", id),
+		DNSName:          dnsName,
 		PrivateDNSName:   fmt.Sprintf("%s.internal.invalid", id),
 		IPAddress:        fmt.Sprintf("8.0.0.%d", inst.seq%256),
 		PrivateIPAddress: fmt.Sprintf("127.0.0.%d", inst.seq%256),