Merge lp:~julian-edwards/gwacl/augment-rolesizes into lp:gwacl

Proposed by Julian Edwards on 2013-07-22
Status: Merged
Approved by: Julian Edwards on 2013-07-23
Approved revision: 196
Merged at revision: 198
Proposed branch: lp:~julian-edwards/gwacl/augment-rolesizes
Merge into: lp:gwacl
Diff against target: 141 lines (+60/-24)
2 files modified
rolesizes.go (+44/-24)
rolesizes_test.go (+16/-0)
To merge this branch: bzr merge lp:~julian-edwards/gwacl/augment-rolesizes
Reviewer Review Type Date Requested Status
Jeroen T. Vermeulen (community) 2013-07-22 Approve on 2013-07-23
Review via email: mp+176108@code.launchpad.net

Commit message

Add more comments, better units and an extra map of role names to role sizes, with thanks to jtv for his ideas.

Description of the change

Taking inspiration from https://code.launchpad.net/~jtv/gwacl/machine-types/+merge/175736 which unfortunately was done at the same time as something I did for this, I've enhanced what I already landed with jtv's ideas.

To post a comment you must log in.
Jeroen T. Vermeulen (jtv) wrote :

Thanks for integrating this. It may seem strange that I used thousandths of US dollars in my branch, but that's also what Juju's EC2 provider does. EC2's pricing structure makes use of the extra precision, and Microsoft may find they'll want to tweak their pricing more.

I think it's still worth saying in so many words that we make no guarantees with these cost indications and that we'll happily let them go out of date if no updates are required for our purposes. Otherwise, your statement that the information is correct (do we even really know that?) as of a given date may reasonably be construed as a promise to keep the list updated.

review: Approve
Julian Edwards (julian-edwards) wrote :

On 23/07/13 12:44, Jeroen T. Vermeulen wrote:
> Review: Approve
>
> Thanks for integrating this. It may seem strange that I used thousandths of US dollars in my branch, but that's also what Juju's EC2 provider does. EC2's pricing structure makes use of the extra precision, and Microsoft may find they'll want to tweak their pricing more.
>
> I think it's still worth saying in so many words that we make no guarantees with these cost indications and that we'll happily let them go out of date if no updates are required for our purposes. Otherwise, your statement that the information is correct (do we even really know that?) as of a given date may reasonably be construed as a promise to keep the list updated.
>

Normally saying something like that means you know it's correct as of a
certain date but you make no guarantees it's correct at a later date.
It just means you checked it on that date.

Thanks for the review!

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'rolesizes.go'
2--- rolesizes.go 2013-07-19 05:37:56 +0000
3+++ rolesizes.go 2013-07-22 03:52:27 +0000
4@@ -10,72 +10,92 @@
5 // http://msdn.microsoft.com/en-us/library/windowsazure/dn197896.aspx and
6 // pricing from here:
7 // http://www.windowsazure.com/en-us/pricing/details/virtual-machines/
8+// and is correct as of 2013-07-22.
9+//
10+// The Disk Space values are only the maxumim permitted; actual space is
11+// determined by the OS image being used.
12 type RoleSize struct {
13 Name string
14 CpuCores uint64
15 Mem uint64 // In MB
16- OSDiskSpaceCloud uint64 // In GB
17- OSDiskSpaceVirt uint64 // In GB
18+ OSDiskSpaceCloud uint64 // In MB
19+ OSDiskSpaceVirt uint64 // In MB
20 MaxDataDisks uint64 // 1TB each
21- Cost uint64 // US ยข (cents) per hour
22+ Cost uint64 // USD cents per hour
23 }
24
25+const (
26+ // MB is the unit in which we specify sizes, so it's 1.
27+ // But please include it anyway, so that units are always explicit.
28+ MB = 1
29+ GB = 1024*MB
30+ TB = 1024*GB
31+)
32+
33 var RoleSizes = []RoleSize{
34 {
35 Name: "ExtraSmall",
36 CpuCores: 1, // shared
37- Mem: 768,
38- OSDiskSpaceCloud: 19,
39- OSDiskSpaceVirt: 20,
40+ Mem: 768*MB,
41+ OSDiskSpaceCloud: 19*GB,
42+ OSDiskSpaceVirt: 20*GB,
43 MaxDataDisks: 1,
44 Cost: 2,
45 }, {
46 Name: "Small",
47 CpuCores: 1,
48- Mem: 1792,
49- OSDiskSpaceCloud: 224,
50- OSDiskSpaceVirt: 70,
51+ Mem: 1.75*GB,
52+ OSDiskSpaceCloud: 224*GB,
53+ OSDiskSpaceVirt: 70*GB,
54 MaxDataDisks: 2,
55 Cost: 6,
56 }, {
57 Name: "Medium",
58 CpuCores: 2,
59- Mem: 3584,
60- OSDiskSpaceCloud: 489,
61- OSDiskSpaceVirt: 135,
62+ Mem: 3.5*GB,
63+ OSDiskSpaceCloud: 489*GB,
64+ OSDiskSpaceVirt: 135*GB,
65 MaxDataDisks: 4,
66 Cost: 12,
67 }, {
68 Name: "Large",
69 CpuCores: 4,
70- Mem: 7168,
71- OSDiskSpaceCloud: 999,
72- OSDiskSpaceVirt: 285,
73+ Mem: 7*GB,
74+ OSDiskSpaceCloud: 999*GB,
75+ OSDiskSpaceVirt: 285*GB,
76 MaxDataDisks: 8,
77 Cost: 24,
78 }, {
79 Name: "ExtraLarge",
80 CpuCores: 8,
81- Mem: 14336,
82- OSDiskSpaceCloud: 2039,
83- OSDiskSpaceVirt: 65,
84+ Mem: 14*GB,
85+ OSDiskSpaceCloud: 2039*GB,
86+ OSDiskSpaceVirt: 65*GB,
87 MaxDataDisks: 16,
88 Cost: 48,
89 }, {
90 Name: "A6",
91 CpuCores: 4,
92- Mem: 28672,
93- OSDiskSpaceCloud: 999,
94- OSDiskSpaceVirt: 285,
95+ Mem: 28*GB,
96+ OSDiskSpaceCloud: 999*GB,
97+ OSDiskSpaceVirt: 285*GB,
98 MaxDataDisks: 8,
99 Cost: 82,
100 }, {
101 Name: "A7",
102 CpuCores: 8,
103- Mem: 57344,
104- OSDiskSpaceCloud: 2039,
105- OSDiskSpaceVirt: 65,
106+ Mem: 56*GB,
107+ OSDiskSpaceCloud: 2039*GB,
108+ OSDiskSpaceVirt: 65*GB,
109 MaxDataDisks: 16,
110 Cost: 164,
111 },
112 }
113+
114+var RoleNameMap map[string]RoleSize = make(map[string]RoleSize)
115+
116+func init() {
117+ for _, rolesize := range RoleSizes {
118+ RoleNameMap[rolesize.Name] = rolesize
119+ }
120+}
121
122=== added file 'rolesizes_test.go'
123--- rolesizes_test.go 1970-01-01 00:00:00 +0000
124+++ rolesizes_test.go 2013-07-22 03:52:27 +0000
125@@ -0,0 +1,16 @@
126+// Copyright 2013 Canonical Ltd. This software is licensed under the
127+// GNU Lesser General Public License version 3 (see the file COPYING).
128+
129+package gwacl
130+
131+import (
132+ . "launchpad.net/gocheck"
133+)
134+
135+type rolesizeSuite struct{}
136+
137+var _ = Suite(&rolesizeSuite{})
138+
139+func (suite *rolesizeSuite) TestMapIsCreated(c *C) {
140+ c.Check(RoleNameMap, HasLen, len(RoleSizes))
141+}

Subscribers

People subscribed via source and target branches

to all changes: