Merge lp:~wallyworld/juju-core/amd64plz into lp:~go-bot/juju-core/trunk

[Ian Booth]

Always pick amd64 if it's an option

This branch takes lp:~natefinch/juju-core/045-amd64plz
and fixes some tests.
See https://codereview.appspot.com/89900043/

https://codereview.appspot.com/90720043/

Always pick amd64 if it's an option

This branch takes lp:~natefinch/juju-core/045-amd64plz
and fixes some tests.
See https://codereview.appspot.com/89900043/

https://codereview.appspot.com/90720043/

[Ian Booth (wallyworld)]

Reviewers: mp+216977_code.launchpad.net,

Message:
Please take a look.

Description:
Always pick amd64 if it's an option

This branch takes lp:~natefinch/juju-core/045-amd64plz
and fixes some tests.
See https://codereview.appspot.com/89900043/

https://code.launchpad.net/~wallyworld/juju-core/amd64plz/+merge/216977

(do not edit description out of merge proposal)

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

Affected files (+67, -2 lines):
   A [revision details]
   M environs/instances/image.go
   M environs/instances/image_test.go
   M provider/ec2/export_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-20140424033650-9am3xzxfcelms2x8
+New revision: <email address hidden>

Index: environs/instances/image.go
=== modified file 'environs/instances/image.go'
--- environs/instances/image.go 2014-04-17 05:44:57 +0000
+++ environs/instances/image.go 2014-04-24 04:56:36 +0000
@@ -8,6 +8,7 @@

   "launchpad.net/juju-core/constraints"
   "launchpad.net/juju-core/environs/imagemetadata"
+ "launchpad.net/juju-core/juju/arch"
  )

  // InstanceConstraint constrains the possible instances that may be
@@ -78,14 +79,19 @@
    return nil, fmt.Errorf("no instance types found matching
constraint: %s", ic)
   }

+ specs := []*InstanceSpec{}
   for _, itype := range matchingTypes {
    for _, image := range possibleImages {
     if image.match(itype) {
- return &InstanceSpec{itype, image}, nil
+ specs = append(specs, &InstanceSpec{itype, image})
     }
    }
   }

+ if spec := preferredSpec(specs); spec != nil {
+ return spec, nil
+ }
+
   names := make([]string, len(matchingTypes))
   for i, itype := range matchingTypes {
    names[i] = itype.Name
@@ -93,6 +99,23 @@
   return nil, fmt.Errorf("no %q images in %s matching instance types %v",
ic.Series, ic.Region, names)
  }

+// preferredSpec will if possible return a spec with arch matching that
+// of the host machine.
+func preferredSpec(specs []*InstanceSpec) *InstanceSpec {
+ if len(specs) > 1 {
+ hostArch := arch.HostArch()
+ for _, spec := range specs {
+ if spec.Image.Arch == hostArch {
+ return spec
+ }
+ }
+ }
+ if len(specs) > 0 {
+ return specs[0]
+ }
+ return nil
+}
+
  // Image holds the attributes that vary amongst relevant images for
  // a given series in a given region.
  type Image struct {

Index: environs/instances/image_test.go
=== modified file 'environs/instances/image_test.go'
--- environs/instances/image_test.go 2014-04-18 13:58:13 +0000
+++ environs/instances/image_test.go 2014-04-23 21:04:24 +0000
@@ -11,6 +11,7 @@
   "launchpad.net/juju-core/constraints"
   "launchpad.net/juju-core/environs/imagemetadata"
   "launchpad.net/juju-core/environs/simplestreams"
+ "launchpad.net/juju-core/juju/arch"
   "launchpad.net/juju-core/testing/testbase"
   "launchpad.net/juju-core/utils"
  )
@@ -306,6 +307,44 @@
   }
  }

+func (s *imageSuite) TestPreferredSpec(c *gc.C) {
+ type prefTest struct {
+ desc string
+ specs []*Instan...

[John A Meinel (jameinel)]

LGTM

https://codereview.appspot.com/90720043/

=== modified file 'environs/instances/image.go'
--- environs/instances/image.go	2014-04-17 05:44:57 +0000
+++ environs/instances/image.go	2014-04-24 05:00:30 +0000
@@ -8,6 +8,7 @@
 
 	"launchpad.net/juju-core/constraints"
 	"launchpad.net/juju-core/environs/imagemetadata"
+	"launchpad.net/juju-core/juju/arch"
 )
 
 // InstanceConstraint constrains the possible instances that may be
@@ -78,14 +79,19 @@
 		return nil, fmt.Errorf("no instance types found matching constraint: %s", ic)
 	}
 
+	specs := []*InstanceSpec{}
 	for _, itype := range matchingTypes {
 		for _, image := range possibleImages {
 			if image.match(itype) {
-				return &InstanceSpec{itype, image}, nil
+				specs = append(specs, &InstanceSpec{itype, image})
 			}
 		}
 	}
 
+	if spec := preferredSpec(specs); spec != nil {
+		return spec, nil
+	}
+
 	names := make([]string, len(matchingTypes))
 	for i, itype := range matchingTypes {
 		names[i] = itype.Name
@@ -93,6 +99,23 @@
 	return nil, fmt.Errorf("no %q images in %s matching instance types %v", ic.Series, ic.Region, names)
 }
 
+// preferredSpec will if possible return a spec with arch matching that
+// of the host machine.
+func preferredSpec(specs []*InstanceSpec) *InstanceSpec {
+	if len(specs) > 1 {
+		hostArch := arch.HostArch()
+		for _, spec := range specs {
+			if spec.Image.Arch == hostArch {
+				return spec
+			}
+		}
+	}
+	if len(specs) > 0 {
+		return specs[0]
+	}
+	return nil
+}
+
 // Image holds the attributes that vary amongst relevant images for
 // a given series in a given region.
 type Image struct {

=== modified file 'environs/instances/image_test.go'
--- environs/instances/image_test.go	2014-04-18 13:58:13 +0000
+++ environs/instances/image_test.go	2014-04-24 05:00:30 +0000
@@ -11,6 +11,7 @@
 	"launchpad.net/juju-core/constraints"
 	"launchpad.net/juju-core/environs/imagemetadata"
 	"launchpad.net/juju-core/environs/simplestreams"
+	"launchpad.net/juju-core/juju/arch"
 	"launchpad.net/juju-core/testing/testbase"
 	"launchpad.net/juju-core/utils"
 )
@@ -306,6 +307,44 @@
 	}
 }
 
+func (s *imageSuite) TestPreferredSpec(c *gc.C) {
+	type prefTest struct {
+		desc     string
+		specs    []*InstanceSpec
+		expected *InstanceSpec
+	}
+
+	s.PatchValue(&arch.HostArch, func() string { return arch.ARM64 })
+
+	amd64 := &InstanceSpec{Image: Image{Arch: arch.AMD64}}
+	i386 := &InstanceSpec{Image: Image{Arch: arch.I386}}
+	arm64 := &InstanceSpec{Image: Image{Arch: arch.ARM64}}
+
+	prefTests := []prefTest{
+		{
+			"choose hostarch (arm64) over other arches",
+			[]*InstanceSpec{i386, arm64, amd64},
+			arm64,
+		},
+		{
+			"choose first image if no arm64",
+			[]*InstanceSpec{i386, amd64},
+			i386,
+		},
+		{
+			"choose only image only one there",
+			[]*InstanceSpec{amd64},
+			amd64,
+		},
+	}
+
+	for n, test := range prefTests {
+		c.Logf("PreferredSpec test %d: %s", n, test.desc)
+		actual := preferredSpec(test.specs)
+		c.Assert(actual, gc.Equals, test.expected)
+	}
+}
+
 var imageMatchtests = []struct {
 	image Image
 	itype InstanceType

=== modified file 'provider/ec2/export_test.go'
--- provider/ec2/export_test.go	2014-04-02 11:35:49 +0000
+++ provider/ec2/export_test.go	2014-04-24 05:00:30 +0000
@@ -152,6 +152,7 @@
 			"com.ubuntu.cloud:server:12.04:i386",
 			"com.ubuntu.cloud:server:12.04:amd64",
 			"com.ubuntu.cloud:server:12.10:amd64",
+			"com.ubuntu.cloud:server:12.10:i386",
 			"com.ubuntu.cloud:server:13.04:i386"
 		   ],
 		   "path": "streams/v1/com.ubuntu.cloud:released:aws.js"

=== modified file 'provider/ec2/image_test.go'
--- provider/ec2/image_test.go	2014-04-17 05:44:57 +0000
+++ provider/ec2/image_test.go	2014-04-24 05:00:30 +0000
@@ -64,7 +64,7 @@
 		image:  "ami-00000033",
 	}, {
 		series: "quantal",
-		arches: both,
+		arches: []string{"i386"},
 		itype:  "m1.small",
 		image:  "ami-01000034",
 	}, {