Code review comment for lp://staging/~wallyworld/juju-core/ec2-root-disk-constraint

Revision history for this message
Ian Booth (wallyworld) wrote :

Reviewers: mp+221664_code.launchpad.net,

Message:
Please take a look.

Description:
Fix root-disk consraints on ec2

EC2 instance types do not include a root
disk size as this is set on instance
creation. This was causing the contraints
matching to fail when root disk was specified.
The fix is to ignore 0 root disk values when
matching constraints.

https://code.launchpad.net/~wallyworld/juju-core/ec2-root-disk-constraint/+merge/221664

(do not edit description out of merge proposal)

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

Affected files (+9, -5 lines):
   A [revision details]
   M environs/instances/instancetype.go
   M environs/instances/instancetype_test.go
   M provider/ec2/image_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: tarmac-20140530163551-b1m9k0br0iaxwqim
+New revision: <email address hidden>

Index: environs/instances/instancetype.go
=== modified file 'environs/instances/instancetype.go'
--- environs/instances/instancetype.go 2014-03-20 05:03:57 +0000
+++ environs/instances/instancetype.go 2014-06-02 05:50:33 +0000
@@ -49,7 +49,7 @@
   if cons.Mem != nil && itype.Mem < *cons.Mem {
    return nothing, false
   }
- if cons.RootDisk != nil && itype.RootDisk < *cons.RootDisk {
+ if cons.RootDisk != nil && itype.RootDisk > 0 && itype.RootDisk <
*cons.RootDisk {
    return nothing, false
   }
   if cons.Tags != nil && len(*cons.Tags) > 0 && !tagsMatch(*cons.Tags,
itype.Tags) {

Index: environs/instances/instancetype_test.go
=== modified file 'environs/instances/instancetype_test.go'
--- environs/instances/instancetype_test.go 2014-05-20 04:27:02 +0000
+++ environs/instances/instancetype_test.go 2014-06-02 05:50:33 +0000
@@ -54,7 +54,6 @@
    CpuPower: CpuPower(800),
    Mem: 15360,
    Cost: 480,
- RootDisk: 65536,
   },
   {
    Name: "t1.micro",
@@ -80,7 +79,6 @@
    CpuPower: CpuPower(2000),
    Mem: 7168,
    Cost: 580,
- RootDisk: 32768,
   },
   {
    Name: "cc1.4xlarge",
@@ -89,7 +87,6 @@
    CpuPower: CpuPower(3350),
    Mem: 23552,
    Cost: 1300,
- RootDisk: 32768,
    VirtType: &hvm,
   }, {
    Name: "cc2.8xlarge",
@@ -98,7 +95,6 @@
    CpuPower: CpuPower(8800),
    Mem: 61952,
    Cost: 2400,
- RootDisk: 131072,
    VirtType: &hvm,
   },
  }

Index: provider/ec2/image_test.go
=== modified file 'provider/ec2/image_test.go'
--- provider/ec2/image_test.go 2014-05-19 09:06:00 +0000
+++ provider/ec2/image_test.go 2014-06-02 05:50:33 +0000
@@ -117,6 +117,12 @@
    cons: "instance-type=c1.medium",
    itype: "c1.medium",
    image: "ami-00000034",
+ }, {
+ series: "precise",
+ arches: both,
+ cons: "mem=4G root-disk=16384M",
+ itype: "m1.large",
+ image: "ami-00000033",
   },
  }

« Back to merge proposal