Merge lp://qastaging/~wallyworld/juju-core/ec2-root-disk-constraint into lp://qastaging/~go-bot/juju-core/trunk

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",
   },
  }

Is this intended to be in the 1.18 series as well?

Otherwise, LGTM.

John
=:->

On Mon, Jun 2, 2014 at 9:57 AM, Ian Booth <email address hidden> 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
> === m...