Fix matching on memory

christellejulias · Feb 1, 2014 · 28f76b7 · 28f76b7
1 parent ab1469b
commit 28f76b7
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 32 deletions.
diff --git a/environs/instances/instancetype.go b/environs/instances/instancetype.go
@@ -98,7 +98,7 @@ func getMatchingInstanceTypes(ic *InstanceConstraint, allInstanceTypes []Instanc
 	//   (previously an instance type with largest available memory was returned even if it didn't
 	//    match on other constraints like cpu-cores and this is wrong).
 	// - if no mem constraint specified, try opinionated default with enough mem to run a server.
-	// - if no matches, try again without any memory constraint and return the instance
+	// - if no matches and no mem constraint specified, try again and return any matching instance
 	//   with the largest memory
 	minMemCons := ic.Constraints
 	minMem := uint64(minMemoryHeuristic)
@@ -109,16 +109,13 @@ func getMatchingInstanceTypes(ic *InstanceConstraint, allInstanceTypes []Instanc
 	}
 	itypes = matchingTypesForConstraint(allInstanceTypes, cons)
 
-	if len(itypes) == 0 {
-		cons := ic.Constraints
-		cons.Mem = nil
-		itypes = matchingTypesForConstraint(allInstanceTypes, cons)
-		sort.Sort(byMemory(itypes))
-		for i, itype := range itypes {
-			if (ic.Constraints.Mem == nil && itype.Mem >= minMemoryHeuristic) || i == len(itypes)-1 {
-				itypes = []InstanceType{itype}
-				break
-			}
+	// No matches using opinionated default, so if no mem constraint specified,
+	// look for matching instance with largest memory.
+	if len(itypes) == 0 && ic.Constraints.Mem == nil {
+		itypes = matchingTypesForConstraint(allInstanceTypes, ic.Constraints)
+		if len(itypes) > 0 {
+			sort.Sort(byMemory(itypes))
+			itypes = []InstanceType{itypes[len(itypes)-1]}
 		}
 	}
 	// If we have matching instance types, we can return those, sorted by cost.

diff --git a/environs/instances/instancetype_test.go b/environs/instances/instancetype_test.go
@@ -143,6 +143,7 @@ var getInstanceTypesTest = []struct {
 		about: "enough memory for mongodb if mem not specified",
 		cons:  "cpu-cores=4",
 		itypesToUse: []InstanceType{
+			{Id: "5", Name: "it-5", Arches: []string{"amd64"}, Mem: 1024, CpuCores: 2},
 			{Id: "4", Name: "it-4", Arches: []string{"amd64"}, Mem: 2048, CpuCores: 4},
 			{Id: "3", Name: "it-3", Arches: []string{"amd64"}, Mem: 1024, CpuCores: 4},
 			{Id: "2", Name: "it-2", Arches: []string{"amd64"}, Mem: 256, CpuCores: 4},
@@ -161,8 +162,8 @@ var getInstanceTypesTest = []struct {
 		expectedItypes: []string{"it-1", "it-3"},
 	},
 	{
-		about: "mem specified but no mem matches, choose largest memory type with enough memory for mongdb",
-		cons:  "mem=8G arch=amd64",
+		about: "mem specified and match found",
+		cons:  "mem=4G arch=amd64",
 		itypesToUse: []InstanceType{
 			{Id: "4", Name: "it-4", Arches: []string{"arm"}, Mem: 8096},
 			{Id: "3", Name: "it-3", Arches: []string{"amd64"}, Mem: 4096},
@@ -171,25 +172,6 @@ var getInstanceTypesTest = []struct {
 		},
 		expectedItypes: []string{"it-3"},
 	},
-	{
-		about: "mem specified but no mem matches, choose largest memory type",
-		cons:  "mem=4G",
-		itypesToUse: []InstanceType{
-			{Id: "2", Name: "it-2", Arches: []string{"amd64"}, Mem: 256},
-			{Id: "1", Name: "it-1", Arches: []string{"amd64"}, Mem: 512},
-		},
-		expectedItypes: []string{"it-1"},
-	},
-	{
-		about: "mem specified but no mem matches, choose largest memory type matching other constraints",
-		cons:  "mem=4G arch=amd64",
-		itypesToUse: []InstanceType{
-			{Id: "3", Name: "it-3", Arches: []string{"arm"}, Mem: 1024},
-			{Id: "2", Name: "it-2", Arches: []string{"amd64"}, Mem: 256},
-			{Id: "1", Name: "it-1", Arches: []string{"amd64"}, Mem: 512},
-		},
-		expectedItypes: []string{"it-1"},
-	},
 	{
 		about: "largest mem available matching other constraints if mem not specified",
 		cons:  "cpu-cores=4",
@@ -240,6 +222,9 @@ func (s *instanceTypeSuite) TestGetMatchingInstanceTypesErrors(c *gc.C) {
 
 	_, err = getMatchingInstanceTypes(constraint("test", "cpu-cores=9000"), instanceTypes)
 	c.Check(err, gc.ErrorMatches, `no instance types in test matching constraints "cpu-cores=9000"`)
+
+	_, err = getMatchingInstanceTypes(constraint("test", "mem=90000M"), instanceTypes)
+	c.Check(err, gc.ErrorMatches, `no instance types in test matching constraints "mem=90000M"`)
 }
 
 var instanceTypeMatchTests = []struct {