goamz

Merge lp:~niemeyer/goamz/run-with-devices into lp:goamz

  1. run-with-devices
  2. Merge into trunk
Proposed by Gustavo Niemeyer
Status: Merged
Merged at revision: 43
Proposed branch: lp:~niemeyer/goamz/run-with-devices
Merge into: lp:goamz
Diff against target: 85 lines (+42/-1)
2 files modified
ec2/ec2.go (+26/-1)
ec2/ec2_test.go (+16/-0)
To merge this branch: bzr merge lp:~niemeyer/goamz/run-with-devices
Reviewer Review Type Date Requested Status
goamz maintainers Pending
Review via email: mp+166428@code.launchpad.net

Description of the change

ec2: support RunInstances with BlockDeviceMappings

https://codereview.appspot.com/9860044/

To post a comment you must log in.
Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :
Download full text (4.0 KiB)

Reviewers: mp+166428_code.launchpad.net,

Message:
Please take a look.

Description:
ec2: support RunInstances with BlockDeviceMappings

https://code.launchpad.net/~niemeyer/goamz/run-with-devices/+merge/166428

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M ec2/ec2.go
   M ec2/ec2_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: <email address hidden>
+New revision: <email address hidden>

Index: ec2/ec2.go
=== modified file 'ec2/ec2.go'
--- ec2/ec2.go 2013-05-23 01:06:45 +0000
+++ ec2/ec2.go 2013-05-30 03:05:27 +0000
@@ -209,6 +209,7 @@
   DisableAPITermination bool
   ShutdownBehavior string
   PrivateIPAddress string
+ BlockDeviceMappings []BlockDeviceMapping
  }

  // Response to a RunInstances request.
@@ -275,6 +276,30 @@
     j++
    }
   }
+ for i, b := range options.BlockDeviceMappings {
+ n := strconv.Itoa(i + 1)
+ if b.DeviceName != "" {
+ params["BlockDeviceMapping."+n+".DeviceName"] = b.DeviceName
+ }
+ if b.VirtualName != "" {
+ params["BlockDeviceMapping."+n+".VirtualName"] = b.VirtualName
+ }
+ if b.SnapshotId != "" {
+ params["BlockDeviceMapping."+n+".Ebs.SnapshotId"] = b.SnapshotId
+ }
+ if b.VolumeType != "" {
+ params["BlockDeviceMapping."+n+".Ebs.VolumeType"] = b.VolumeType
+ }
+ if b.VolumeSize > 0 {
+ params["BlockDeviceMapping."+n+".Ebs.VolumeSize"] =
strconv.FormatInt(b.VolumeSize, 10)
+ }
+ if b.IOPS > 0 {
+ params["BlockDeviceMapping."+n+".Ebs.Iops"] = strconv.FormatInt(b.IOPS,
10)
+ }
+ if b.DeleteOnTermination {
+ params["BlockDeviceMapping."+n+".Ebs.DeleteOnTermination"] = "true"
+ }
+ }
   token, err := clientToken()
   if err != nil {
    return nil, err
@@ -429,7 +454,7 @@
   VirtualName string `xml:"virtualName"`
   SnapshotId string `xml:"ebs>snapshotId"`
   VolumeType string `xml:"ebs>volumeType"`
- VolumeSize int64 `xml:"ebs>volumeSize"`
+ VolumeSize int64 `xml:"ebs>volumeSize"` // Size is given in GB
   DeleteOnTermination bool `xml:"ebs>deleteOnTermination"`

   // The number of I/O operations per second (IOPS) that the volume
supports.

Index: ec2/ec2_test.go
=== modified file 'ec2/ec2_test.go'
--- ec2/ec2_test.go 2013-05-23 02:56:41 +0000
+++ ec2/ec2_test.go 2013-05-30 03:05:27 +0000
@@ -96,6 +96,15 @@
    DisableAPITermination: true,
    ShutdownBehavior: "terminate",
    PrivateIPAddress: "10.0.0.25",
+ BlockDeviceMappings: []ec2.BlockDeviceMapping{{
+ DeviceName: "device-name",
+ VirtualName: "virtual-name",
+ SnapshotId: "snapshot-id",
+ VolumeType: "volume-type",
+ VolumeSize: 10,
+ DeleteOnTermination: true,
+ IOPS: 1000,
+ }},
   }
   resp, err := s.ec2.RunInstances(&options)

@@ -120,6 +129,13 @@
   c.Assert(req.Form["DisableApiTermination"], DeepEquals, []string{"true"})
   c.Assert(req.Form["InstanceInitiatedShutdownBehavior"], DeepEquals,
[]stri...

Read more...

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

ping

https://codereview.appspot.com/9860044/

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

https://codereview.appspot.com/9860044/diff/1/ec2/ec2.go
File ec2/ec2.go (right):

https://codereview.appspot.com/9860044/diff/1/ec2/ec2.go#newcode457
ec2/ec2.go:457: VolumeSize int64 `xml:"ebs>volumeSize"` //
Size is given in GB
On 2013/06/04 02:23:13, dfc wrote:
> could this be uin64 ? Does a negative size mean anything in ec2 land ?

I would actually prefer to see it as "int", were not for compatibility
reasons. For a while, mapping 2^31 **GB** won't be realistic.

https://codereview.appspot.com/9860044/diff/1/ec2/ec2.go#newcode461
ec2/ec2.go:461: IOPS int64 `xml:"ebs>iops"`
On 2013/06/04 02:23:13, dfc wrote:
> same, negative iops doesn't make a lot of sense.

Neither does negative indexes/length/capacity on slices, but they're
signed still. In general, unless there's a reason for things to be
unsigned, I prefer to keep them signed as well.

https://codereview.appspot.com/9860044/

lp:~niemeyer/goamz/run-with-devices updated
39. By Gustavo Niemeyer

Merged from trunk.

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

Please take a look.

https://codereview.appspot.com/9860044/

Revision history for this message
Nate Finch (natefinch) wrote :

On 2013/10/04 17:07:01, niemeyer wrote:
> Please take a look.

LGTM

https://codereview.appspot.com/9860044/

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

*** Submitted:

ec2: support RunInstances with BlockDeviceMappings

R=dfc, nate.finch
CC=
https://codereview.appspot.com/9860044

https://codereview.appspot.com/9860044/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'ec2/ec2.go'
2--- ec2/ec2.go 2013-10-02 19:10:36 +0000
3+++ ec2/ec2.go 2013-10-04 17:06:33 +0000
4@@ -209,6 +209,7 @@
5 DisableAPITermination bool
6 ShutdownBehavior string
7 PrivateIPAddress string
8+ BlockDeviceMappings []BlockDeviceMapping
9 }
10
11 // Response to a RunInstances request.
12@@ -278,6 +279,30 @@
13 j++
14 }
15 }
16+ for i, b := range options.BlockDeviceMappings {
17+ n := strconv.Itoa(i + 1)
18+ if b.DeviceName != "" {
19+ params["BlockDeviceMapping."+n+".DeviceName"] = b.DeviceName
20+ }
21+ if b.VirtualName != "" {
22+ params["BlockDeviceMapping."+n+".VirtualName"] = b.VirtualName
23+ }
24+ if b.SnapshotId != "" {
25+ params["BlockDeviceMapping."+n+".Ebs.SnapshotId"] = b.SnapshotId
26+ }
27+ if b.VolumeType != "" {
28+ params["BlockDeviceMapping."+n+".Ebs.VolumeType"] = b.VolumeType
29+ }
30+ if b.VolumeSize > 0 {
31+ params["BlockDeviceMapping."+n+".Ebs.VolumeSize"] = strconv.FormatInt(b.VolumeSize, 10)
32+ }
33+ if b.IOPS > 0 {
34+ params["BlockDeviceMapping."+n+".Ebs.Iops"] = strconv.FormatInt(b.IOPS, 10)
35+ }
36+ if b.DeleteOnTermination {
37+ params["BlockDeviceMapping."+n+".Ebs.DeleteOnTermination"] = "true"
38+ }
39+ }
40 token, err := clientToken()
41 if err != nil {
42 return nil, err
43@@ -432,7 +457,7 @@
44 VirtualName string `xml:"virtualName"`
45 SnapshotId string `xml:"ebs>snapshotId"`
46 VolumeType string `xml:"ebs>volumeType"`
47- VolumeSize int64 `xml:"ebs>volumeSize"`
48+ VolumeSize int64 `xml:"ebs>volumeSize"` // Size is given in GB
49 DeleteOnTermination bool `xml:"ebs>deleteOnTermination"`
50
51 // The number of I/O operations per second (IOPS) that the volume supports.
52
53=== modified file 'ec2/ec2_test.go'
54--- ec2/ec2_test.go 2013-10-02 19:10:36 +0000
55+++ ec2/ec2_test.go 2013-10-04 17:06:33 +0000
56@@ -96,6 +96,15 @@
57 DisableAPITermination: true,
58 ShutdownBehavior: "terminate",
59 PrivateIPAddress: "10.0.0.25",
60+ BlockDeviceMappings: []ec2.BlockDeviceMapping{{
61+ DeviceName: "device-name",
62+ VirtualName: "virtual-name",
63+ SnapshotId: "snapshot-id",
64+ VolumeType: "volume-type",
65+ VolumeSize: 10,
66+ DeleteOnTermination: true,
67+ IOPS: 1000,
68+ }},
69 }
70 resp, err := s.ec2.RunInstances(&options)
71
72@@ -120,6 +129,13 @@
73 c.Assert(req.Form["DisableApiTermination"], DeepEquals, []string{"true"})
74 c.Assert(req.Form["InstanceInitiatedShutdownBehavior"], DeepEquals, []string{"terminate"})
75 c.Assert(req.Form["PrivateIpAddress"], DeepEquals, []string{"10.0.0.25"})
76+ c.Assert(req.Form["BlockDeviceMapping.1.DeviceName"], DeepEquals, []string{"device-name"})
77+ c.Assert(req.Form["BlockDeviceMapping.1.VirtualName"], DeepEquals, []string{"virtual-name"})
78+ c.Assert(req.Form["BlockDeviceMapping.1.Ebs.SnapshotId"], DeepEquals, []string{"snapshot-id"})
79+ c.Assert(req.Form["BlockDeviceMapping.1.Ebs.VolumeType"], DeepEquals, []string{"volume-type"})
80+ c.Assert(req.Form["BlockDeviceMapping.1.Ebs.VolumeSize"], DeepEquals, []string{"10"})
81+ c.Assert(req.Form["BlockDeviceMapping.1.Ebs.Iops"], DeepEquals, []string{"1000"})
82+ c.Assert(req.Form["BlockDeviceMapping.1.Ebs.DeleteOnTermination"], DeepEquals, []string{"true"})
83
84 c.Assert(err, IsNil)
85 c.Assert(resp.RequestId, Equals, "59dbff89-35bd-4eac-99ed-be587EXAMPLE")

Subscribers

People subscribed via source and target branches