The list of vm filters is made provider aware (#137)

* vm filters list made provider aware * ntw performance filter added in case of ec2 and gce only
banzaicloud · Jul 27, 2018 · 2628af9 · 2628af9
1 parent 646366f
commit 2628af9
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 19 deletions.
diff --git a/pkg/recommender/engine.go b/pkg/recommender/engine.go
@@ -197,7 +197,7 @@ func (e *Engine) RecommendCluster(provider string, region string, req ClusterRec
 		}
 		log.Debugf("recommended values for [%s]: count:[%d] , values: [%#v./te]", attr, len(values), values)
 
-		vmFilters, _ := e.filtersForAttr(attr)
+		vmFilters, _ := e.filtersForAttr(attr, provider)
 
 		filteredVms, err := e.RecommendVms(provider, region, attr, values, vmFilters, req)
 		if err != nil {
@@ -466,15 +466,30 @@ func (e *Engine) RecommendAttrValues(provider string, region string, attr string
 }
 
 // filtersForAttr returns the slice for
-func (e *Engine) filtersForAttr(attr string) ([]vmFilter, error) {
+func (e *Engine) filtersForAttr(attr string, provider string) ([]vmFilter, error) {
+	var
+	// generic filters - not depending on providers and attributes
+	filters []vmFilter = []vmFilter{e.includesFilter, e.excludesFilter}
+
+	// provider specific filters
+	switch provider {
+	case "ec2":
+		filters = append(filters, e.currentGenFilter, e.burstFilter, e.ntwPerformanceFilter)
+	case "gce":
+		filters = append(filters, e.ntwPerformanceFilter)
+	}
+
+	// attribute specific filters
 	switch attr {
 	case Cpu:
-		return []vmFilter{e.currentGenFilter, e.ntwPerformanceFilter, e.minMemRatioFilter, e.burstFilter, e.includesFilter, e.excludesFilter}, nil
+		filters = append(filters, e.minMemRatioFilter)
 	case Memory:
-		return []vmFilter{e.currentGenFilter, e.ntwPerformanceFilter, e.minCpuRatioFilter, e.burstFilter, e.includesFilter, e.excludesFilter}, nil
+		filters = append(filters, e.minCpuRatioFilter)
 	default:
 		return nil, fmt.Errorf("unsupported attribute: [%s]", attr)
 	}
+
+	return filters, nil
 }
 
 // sortByAttrValue returns the slice for

diff --git a/pkg/recommender/engine_filters_test.go b/pkg/recommender/engine_filters_test.go
@@ -13,21 +13,23 @@ var (
 
 func TestEngine_filtersApply(t *testing.T) {
 	tests := []struct {
-		name   string
-		engine Engine
-		vm     VirtualMachine
-		req    ClusterRecommendationReq
-		attr   string
-		check  func(filtersApply bool)
+		name     string
+		engine   Engine
+		vm       VirtualMachine
+		req      ClusterRecommendationReq
+		attr     string
+		provider string
+		check    func(filtersApply bool)
 	}{
 		{
 			name:   "filter applies for cpu/mem and burst allowed",
 			engine: Engine{},
 			// minRatio = SumCpu/SumMem = 0.5
 			req: ClusterRecommendationReq{SumCpu: 4, SumMem: float64(8), AllowBurst: &trueVal},
 			// ratio = Cpus/Mem = 1
-			vm:   VirtualMachine{Cpus: 4, Mem: float64(4), Burst: true, CurrentGen: true},
-			attr: Memory,
+			vm:       VirtualMachine{Cpus: 4, Mem: float64(4), Burst: true, CurrentGen: true},
+			attr:     Memory,
+			provider: "ec2",
 			check: func(filtersApply bool) {
 				assert.Equal(t, true, filtersApply, "vm should pass all filters")
 			},
@@ -38,8 +40,9 @@ func TestEngine_filtersApply(t *testing.T) {
 			// minRatio = SumCpu/SumMem = 0.5
 			req: ClusterRecommendationReq{SumCpu: 4, SumMem: float64(8), AllowBurst: &falseVal},
 			// ratio = Cpus/Mem = 1
-			vm:   VirtualMachine{Cpus: 4, Mem: float64(4), Burst: true, CurrentGen: true},
-			attr: Cpu,
+			vm:       VirtualMachine{Cpus: 4, Mem: float64(4), Burst: true, CurrentGen: true},
+			attr:     Cpu,
+			provider: "ec2",
 			check: func(filtersApply bool) {
 				assert.Equal(t, false, filtersApply, "vm should not pass all filters")
 			},
@@ -50,8 +53,9 @@ func TestEngine_filtersApply(t *testing.T) {
 			// minRatio = AumMem/SumCpu = 2
 			req: ClusterRecommendationReq{SumMem: float64(8), SumCpu: 4, AllowBurst: &trueVal},
 			// ratio = Mem/Cpus = 1
-			vm:   VirtualMachine{Mem: float64(20), Cpus: 4, Burst: true, CurrentGen: true},
-			attr: Cpu,
+			vm:       VirtualMachine{Mem: float64(20), Cpus: 4, Burst: true, CurrentGen: true},
+			attr:     Cpu,
+			provider: "ec2",
 			check: func(filtersApply bool) {
 				assert.Equal(t, true, filtersApply, "vm should pass all filters")
 			},
@@ -62,16 +66,17 @@ func TestEngine_filtersApply(t *testing.T) {
 			// minRatio = AumMem/SumCpu = 2
 			req: ClusterRecommendationReq{SumMem: float64(8), SumCpu: 4, AllowBurst: &falseVal},
 			// ratio = Mem/Cpus = 1
-			vm:   VirtualMachine{Mem: float64(20), Cpus: 4, Burst: true, CurrentGen: true},
-			attr: Memory,
+			vm:       VirtualMachine{Mem: float64(20), Cpus: 4, Burst: true, CurrentGen: true},
+			attr:     Memory,
+			provider: "ec2",
 			check: func(filtersApply bool) {
 				assert.Equal(t, false, filtersApply, "vm should not pass all filters")
 			},
 		},
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			filters, err := test.engine.filtersForAttr(test.attr)
+			filters, err := test.engine.filtersForAttr(test.attr, test.provider)
 			assert.Nil(t, err, "should get filters for attribute")
 			test.check(test.engine.filtersApply(test.vm, filters, test.req))
 		})