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

Proposed by Andrew Wilkins
Status: Merged
Merged at revision: 44
Proposed branch: lp:~axwalk/goamz/ec2test-empty-dnsname
Merge into: lp:goamz
Diff against target: 34 lines (+11/-2)
1 file modified
ec2/ec2test/server.go (+11/-2)
To merge this branch: bzr merge lp:~axwalk/goamz/ec2test-empty-dnsname
Reviewer Review Type Date Requested Status
goamz maintainers Pending
Review via email: mp+199466@code.launchpad.net

Commit message

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/

Description of the change

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/

To post a comment you must log in.
Revision history for this message
Andrew Wilkins (axwalk) wrote :

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),

Revision history for this message
Roger Peppe (rogpeppe) 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.

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

Revision history for this message
Andrew Wilkins (axwalk) wrote :

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/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'ec2/ec2test/server.go'
2--- ec2/ec2test/server.go 2013-10-03 12:10:33 +0000
3+++ ec2/ec2test/server.go 2013-12-18 14:30:43 +0000
4@@ -66,7 +66,8 @@
5
6 // instance holds a simulated ec2 instance
7 type Instance struct {
8- seq int
9+ seq int
10+ dnsNameSet bool
11 // UserData holds the data that was passed to the RunInstances request
12 // when the instance was started.
13 UserData []byte
14@@ -550,11 +551,19 @@
15
16 func (inst *Instance) ec2instance() ec2.Instance {
17 id := inst.id()
18+ // The first time the instance is returned, its DNSName
19+ // will be empty. The client should then refresh the instance.
20+ var dnsName string
21+ if inst.dnsNameSet {
22+ dnsName = fmt.Sprintf("%s.testing.invalid", id)
23+ } else {
24+ inst.dnsNameSet = true
25+ }
26 return ec2.Instance{
27 InstanceId: id,
28 InstanceType: inst.instType,
29 ImageId: inst.imageId,
30- DNSName: fmt.Sprintf("%s.testing.invalid", id),
31+ DNSName: dnsName,
32 PrivateDNSName: fmt.Sprintf("%s.internal.invalid", id),
33 IPAddress: fmt.Sprintf("8.0.0.%d", inst.seq%256),
34 PrivateIPAddress: fmt.Sprintf("127.0.0.%d", inst.seq%256),

Subscribers

People subscribed via source and target branches