Merge upstream, resolve conflicts

christellejulias · May 17, 2013 · e03fafc · e03fafc
2 parents ac01630 + 3d3af2c
commit e03fafc
Show file tree

Hide file tree

Showing 13 changed files with 142 additions and 251 deletions.
diff --git a/environs/ec2/config.go b/environs/ec2/config.go
@@ -12,23 +12,21 @@ import (
 
 var configChecker = schema.StrictFieldMap(
 	schema.Fields{
-		"access-key":            schema.String(),
-		"secret-key":            schema.String(),
-		"region":                schema.String(),
-		"control-bucket":        schema.String(),
-		"public-bucket":         schema.String(),
-		"public-bucket-region":  schema.String(),
-		"default-image-id":      schema.String(),
-		"default-instance-type": schema.String(),
+		"access-key":           schema.String(),
+		"secret-key":           schema.String(),
+		"region":               schema.String(),
+		"control-bucket":       schema.String(),
+		"public-bucket":        schema.String(),
+		"public-bucket-region": schema.String(),
+		"default-image-id":     schema.String(),
 	},
 	schema.Defaults{
-		"access-key":            "",
-		"secret-key":            "",
-		"region":                "us-east-1",
-		"public-bucket":         "juju-dist",
-		"public-bucket-region":  "us-east-1",
-		"default-image-id":      "",
-		"default-instance-type": "",
+		"access-key":           "",
+		"secret-key":           "",
+		"region":               "us-east-1",
+		"public-bucket":        "juju-dist",
+		"public-bucket-region": "us-east-1",
+		"default-image-id":     "",
 	},
 )
 
@@ -65,10 +63,6 @@ func (c *environConfig) defaultImageId() string {
 	return c.attrs["default-image-id"].(string)
 }
 
-func (c *environConfig) defaultInstanceType() string {
-	return c.attrs["default-instance-type"].(string)
-}
-
 func (p environProvider) newConfig(cfg *config.Config) (*environConfig, error) {
 	valid, err := p.Validate(cfg, nil)
 	if err != nil {

diff --git a/environs/ec2/config_test.go b/environs/ec2/config_test.go
@@ -38,18 +38,17 @@ var testAuth = aws.Auth{"gopher", "long teeth"}
 // when mutated by the mutate function, or that the parse matches the
 // given error.
 type configTest struct {
-	config              attrs
-	change              attrs
-	region              string
-	cbucket             string
-	pbucket             string
-	pbucketRegion       string
-	accessKey           string
-	secretKey           string
-	defaultImageId      string
-	defaultInstanceType string
-	firewallMode        config.FirewallMode
-	err                 string
+	config         attrs
+	change         attrs
+	region         string
+	cbucket        string
+	pbucket        string
+	pbucketRegion  string
+	accessKey      string
+	secretKey      string
+	defaultImageId string
+	firewallMode   config.FirewallMode
+	err            string
 }
 
 type attrs map[string]interface{}
@@ -130,9 +129,6 @@ func (t configTest) check(c *C) {
 	if t.defaultImageId != "" {
 		c.Assert(ecfg.defaultImageId(), Equals, t.defaultImageId)
 	}
-	if t.defaultInstanceType != "" {
-		c.Assert(ecfg.defaultInstanceType(), Equals, t.defaultInstanceType)
-	}
 
 	// check storage buckets are configured correctly
 	env := e.(*environ)
@@ -258,11 +254,6 @@ var configTests = []configTest{
 			"default-image-id": "image-id",
 		},
 		defaultImageId: "image-id",
-	}, {
-		config: attrs{
-			"default-instance-type": "instance-type",
-		},
-		defaultInstanceType: "instance-type",
 	}, {
 		config:       attrs{},
 		firewallMode: config.FwInstance,

diff --git a/environs/ec2/ec2.go b/environs/ec2/ec2.go
@@ -427,13 +427,12 @@ func (e *environ) startInstance(scfg *startInstanceParams) (environs.Instance, e
 		return nil, err
 	}
 	spec, err := findInstanceSpec(baseURLs, &instances.InstanceConstraint{
-		Region:              e.ecfg().region(),
-		Series:              scfg.series,
-		Arches:              arches,
-		Constraints:         scfg.constraints,
-		DefaultInstanceType: e.ecfg().defaultInstanceType(),
-		DefaultImageId:      e.ecfg().defaultImageId(),
-		Storage:             &storage,
+		Region:         e.ecfg().region(),
+		Series:         scfg.series,
+		Arches:         arches,
+		Constraints:    scfg.constraints,
+		DefaultImageId: e.ecfg().defaultImageId(),
+		Storage:        &storage,
 	})
 	if err != nil {
 		return nil, err

diff --git a/environs/ec2/image_test.go b/environs/ec2/image_test.go
@@ -50,7 +50,6 @@ var findInstanceSpecTests = []struct {
 	series       string
 	arches       []string
 	cons         string
-	defaultItype string
 	defaultImage string
 	itype        string
 	image        string
@@ -119,12 +118,6 @@ var findInstanceSpecTests = []struct {
 		cons:   "arch=amd64",
 		itype:  "cc1.4xlarge",
 		image:  "ami-01000035",
-	}, {
-		series:       "precise",
-		arches:       both,
-		defaultItype: "m1.medium",
-		itype:        "m1.medium",
-		image:        "ami-00000033",
 	}, {
 		series:       "raring",
 		arches:       both,
@@ -139,13 +132,12 @@ func (s *specSuite) TestFindInstanceSpec(c *C) {
 		c.Logf("test %d", i)
 		storage := ebsStorage
 		spec, err := findInstanceSpec([]string{"test:"}, &instances.InstanceConstraint{
-			Region:              "test",
-			Series:              t.series,
-			Arches:              t.arches,
-			Constraints:         constraints.MustParse(t.cons),
-			DefaultInstanceType: t.defaultItype,
-			DefaultImageId:      t.defaultImage,
-			Storage:             &storage,
+			Region:         "test",
+			Series:         t.series,
+			Arches:         t.arches,
+			Constraints:    constraints.MustParse(t.cons),
+			DefaultImageId: t.defaultImage,
+			Storage:        &storage,
 		})
 		c.Assert(err, IsNil)
 		c.Check(spec.InstanceTypeName, Equals, t.itype)
@@ -154,12 +146,11 @@ func (s *specSuite) TestFindInstanceSpec(c *C) {
 }
 
 var findInstanceSpecErrorTests = []struct {
-	series              string
-	arches              []string
-	cons                string
-	defaultInstanceType string
-	defaultImageId      string
-	err                 string
+	series         string
+	arches         []string
+	cons           string
+	defaultImageId string
+	err            string
 }{
 	{
 		series: "bad",
@@ -168,12 +159,7 @@ var findInstanceSpecErrorTests = []struct {
 	}, {
 		series: "precise",
 		arches: []string{"arm"},
-		err:    `no "precise" images in test with arches \[arm\], and no override specified`,
-	}, {
-		series:              "precise",
-		arches:              both,
-		defaultInstanceType: "bad.type",
-		err:                 `invalid default instance type name "bad.type"`,
+		err:    `no "precise" images in test with arches \[arm\], and no default specified`,
 	}, {
 		series:         "precise",
 		arches:         both,
@@ -191,12 +177,11 @@ func (s *specSuite) TestFindInstanceSpecErrors(c *C) {
 	for i, t := range findInstanceSpecErrorTests {
 		c.Logf("test %d", i)
 		_, err := findInstanceSpec([]string{"test:"}, &instances.InstanceConstraint{
-			Region:              "test",
-			Series:              t.series,
-			Arches:              t.arches,
-			Constraints:         constraints.MustParse(t.cons),
-			DefaultInstanceType: t.defaultInstanceType,
-			DefaultImageId:      t.defaultImageId,
+			Region:         "test",
+			Series:         t.series,
+			Arches:         t.arches,
+			Constraints:    constraints.MustParse(t.cons),
+			DefaultImageId: t.defaultImageId,
 		})
 		c.Check(err, ErrorMatches, t.err)
 	}

diff --git a/environs/instances/image.go b/environs/instances/image.go
@@ -6,19 +6,16 @@ package instances
 import (
 	"fmt"
 	"launchpad.net/juju-core/constraints"
-	"sort"
 )
 
 // InstanceConstraint constrains the possible instances that may be
 // chosen by the environment provider.
 type InstanceConstraint struct {
-	Region              string
-	Series              string
-	Arches              []string
-	Constraints         constraints.Value
-	DefaultInstanceType string // the instance type to use if multiple matching ones are found
-	DefaultImageId      string // the image to use if multiple matching ones are found
-	OverrideImageId     string // ignore any known, published images, use this image
+	Region         string
+	Series         string
+	Arches         []string
+	Constraints    constraints.Value
+	DefaultImageId string // the image to use if multiple matching ones are found
 	// Optional filtering criteria not supported by all providers. These attributes are not specified
 	// by the user as a constraint but rather passed in by the provider implementation to restrict the
 	// choice of available images.
@@ -43,7 +40,6 @@ func FindInstanceSpec(possibleImages []Image, ic *InstanceConstraint, allInstanc
 		return nil, err
 	}
 
-	sort.Sort(byArch(possibleImages))
 	for _, itype := range matchingTypes {
 		typeMatch := false
 		for _, image := range possibleImages {
@@ -58,18 +54,18 @@ func FindInstanceSpec(possibleImages []Image, ic *InstanceConstraint, allInstanc
 			return nil, fmt.Errorf("invalid default image id %q", ic.DefaultImageId)
 		}
 	}
-	// if no matching image is found for whatever reason, use the override if one is specified.
-	if ic.OverrideImageId != "" && len(matchingTypes) > 0 {
+	// if no matching image is found for whatever reason, use the default if one is specified.
+	if ic.DefaultImageId != "" && len(matchingTypes) > 0 {
 		spec := &InstanceSpec{
 			InstanceTypeId:   matchingTypes[0].Id,
 			InstanceTypeName: matchingTypes[0].Name,
-			Image:            Image{Id: ic.OverrideImageId, Arch: ic.Arches[0]},
+			Image:            Image{Id: ic.DefaultImageId, Arch: ic.Arches[0]},
 		}
 		return spec, nil
 	}
 
 	if len(possibleImages) == 0 || len(matchingTypes) == 0 {
-		return nil, fmt.Errorf("no %q images in %s with arches %s, and no override specified",
+		return nil, fmt.Errorf("no %q images in %s with arches %s, and no default specified",
 			ic.Series, ic.Region, ic.Arches)
 	}
 
@@ -102,13 +98,3 @@ func (image Image) match(itype InstanceType) bool {
 	}
 	return false
 }
-
-// byArch is used to sort a slice of images by architecture preference, such
-// that amd64 images come earlier than i386 ones.
-type byArch []Image
-
-func (ba byArch) Len() int      { return len(ba) }
-func (ba byArch) Swap(i, j int) { ba[i], ba[j] = ba[j], ba[i] }
-func (ba byArch) Less(i, j int) bool {
-	return ba[i].Arch == "amd64" && ba[j].Arch != "amd64"
-}
diff --git a/environs/instances/image_test.go b/environs/instances/image_test.go
@@ -137,7 +137,6 @@ type instanceSpecTestParams struct {
 	constraints         string
 	instanceTypes       []InstanceType
 	defaultImageId      string
-	overrideImageId     string
 	defaultInstanceType string
 	imageId             string
 	instanceTypeId      string
@@ -159,16 +158,24 @@ func (p *instanceSpecTestParams) init() {
 var pv = "pv"
 var findInstanceSpecTests = []instanceSpecTestParams{
 	{
-		desc:            "image exists in metadata, don't use override",
-		region:          "test",
-		overrideImageId: "1234",
-		imageId:         "ami-00000033",
+		desc:    "image exists in metadata",
+		region:  "test",
+		imageId: "ami-00000033",
 		instanceTypes: []InstanceType{
 			{Id: "1", Name: "it-1", Arches: []string{"amd64"}, VType: &pv, Mem: 512},
 		},
 		instanceTypeId:   "1",
 		instanceTypeName: "it-1",
 	},
+	{
+		desc:           "images exist, invalid default image id",
+		region:         "test",
+		defaultImageId: "1234",
+		instanceTypes: []InstanceType{
+			{Id: "1", Name: "it-1", Arches: []string{"arm"}, VType: &pv, Mem: 512},
+		},
+		err: `invalid default image id "1234"`,
+	},
 	{
 		desc:           "multiple images exists in metadata, use default",
 		region:         "test",
@@ -190,15 +197,10 @@ var findInstanceSpecTests = []instanceSpecTestParams{
 		err: `invalid default image id "1234"`,
 	},
 	{
-		desc:            "no image exists in metadata, use supplied override",
-		region:          "invalid-region",
-		overrideImageId: "1234",
-		imageId:         "1234",
-	},
-	{
-		desc:   "no image exists in metadata, no override supplied",
-		region: "invalid-region",
-		err:    `no "precise" images in invalid-region with arches \[amd64 arm\], and no override specified`,
+		desc:    "no image exists in metadata, no default supplied",
+		region:  "invalid-region",
+		imageId: "1234",
+		err:     `no "precise" images in invalid-region with arches \[amd64 arm\], and no default specified`,
 	},
 	{
 		desc:          "no valid instance types",
@@ -235,12 +237,11 @@ func (s *imageSuite) TestFindInstanceSpec(c *C) {
 			})
 		}
 		spec, err := FindInstanceSpec(images, &InstanceConstraint{
-			Series:          "precise",
-			Region:          t.region,
-			Arches:          t.arches,
-			Constraints:     constraints.MustParse(t.constraints),
-			DefaultImageId:  t.defaultImageId,
-			OverrideImageId: t.overrideImageId,
+			Series:         "precise",
+			Region:         t.region,
+			Arches:         t.arches,
+			Constraints:    constraints.MustParse(t.constraints),
+			DefaultImageId: t.defaultImageId,
 		}, t.instanceTypes)
 		if t.err != "" {
 			c.Check(err, ErrorMatches, t.err)

diff --git a/environs/instances/instancetype.go b/environs/instances/instancetype.go
@@ -70,16 +70,16 @@ const minMemoryHeuristic = 1024
 func getMatchingInstanceTypes(ic *InstanceConstraint, allInstanceTypes []InstanceType) ([]InstanceType, error) {
 	cons := ic.Constraints
 	region := ic.Region
-	defaultInstanceTypeName := ic.DefaultInstanceType
+	//	defaultInstanceTypeName := ic.DefaultInstanceType
 	var itypes []InstanceType
 	var defaultInstanceType *InstanceType
 
 	// Iterate over allInstanceTypes, finding matching ones and recording the default if any.
 	for _, itype := range allInstanceTypes {
-		if itype.Name == defaultInstanceTypeName {
-			itcopy := itype
-			defaultInstanceType = &itcopy
-		}
+		//		if itype.Name == defaultInstanceTypeName {
+		//			itcopy := itype
+		//			defaultInstanceType = &itcopy
+		//		}
 		itype, ok := itype.match(cons)
 		if !ok {
 			continue
@@ -90,9 +90,9 @@ func getMatchingInstanceTypes(ic *InstanceConstraint, allInstanceTypes []Instanc
 	// If there is more than one instance type matching the constraints, use the specified
 	// default instance type, if any.
 	if len(itypes) > 0 {
-		if defaultInstanceTypeName != "" && defaultInstanceType == nil {
-			return nil, fmt.Errorf("invalid default instance type name %q", defaultInstanceTypeName)
-		}
+		//		if defaultInstanceTypeName != "" && defaultInstanceType == nil {
+		//			return nil, fmt.Errorf("invalid default instance type name %q", defaultInstanceTypeName)
+		//		}
 		if defaultInstanceType != nil {
 			itypes = []InstanceType{*defaultInstanceType}
 		}