Merge pull request #1523 from avrodrigues5/avr/fix-total-eff-in-sas

Ignoring Idle Summary allocation in total efficiency computation of Summary allocation set

pkg/kubecost/summaryallocation.go

@@ -1183,6 +1183,9 @@ func (sas *SummaryAllocationSet) RAMEfficiency() float64 {
 	totalRAMBytesRequest := 0.0
 	totalRAMCost := 0.0
 	for _, sa := range sas.SummaryAllocations {
+		if sa.IsIdle() {
+			continue
+		}
 		totalRAMBytesUsage += sa.RAMBytesUsageAverage
 		totalRAMBytesRequest += sa.RAMBytesRequestAverage
 		totalRAMCost += sa.RAMCost
@@ -1212,6 +1215,9 @@ func (sas *SummaryAllocationSet) CPUEfficiency() float64 {
 	totalCPUCoreRequest := 0.0
 	totalCPUCost := 0.0
 	for _, sa := range sas.SummaryAllocations {
+		if sa.IsIdle() {
+			continue
+		}
 		totalCPUCoreUsage += sa.CPUCoreUsageAverage
 		totalCPUCoreRequest += sa.CPUCoreRequestAverage
 		totalCPUCost += sa.CPUCost
@@ -1237,19 +1243,18 @@ func (sas *SummaryAllocationSet) TotalEfficiency() float64 {
 	sas.RLock()
 	defer sas.RUnlock()
 
-	totalRAMCostEff := 0.0
-	totalCPUCostEff := 0.0
 	totalRAMCost := 0.0
 	totalCPUCost := 0.0
 	for _, sa := range sas.SummaryAllocations {
-		totalRAMCostEff += sa.RAMEfficiency() * sa.RAMCost
-		totalCPUCostEff += sa.CPUEfficiency() * sa.CPUCost
+		if sa.IsIdle() {
+			continue
+		}
 		totalRAMCost += sa.RAMCost
 		totalCPUCost += sa.CPUCost
 	}
 
 	if totalRAMCost+totalCPUCost > 0 {
-		return (totalRAMCostEff + totalCPUCostEff) / (totalRAMCost + totalCPUCost)
+		return (totalRAMCost*sas.RAMEfficiency() + totalCPUCost*sas.CPUEfficiency()) / (totalRAMCost + totalCPUCost)
 	}
 
 	return 0.0

pkg/kubecost/summaryallocation_test.go

@@ -213,10 +213,10 @@ func TestSummaryAllocation_Add(t *testing.T) {
 
 func TestSummaryAllocationSet_RAMEfficiency(t *testing.T) {
 	// Generating 6 sample summary allocations for testing
-	var sa1, sa2, sa3, sa4, sa5, sa6 *SummaryAllocation
+	var sa1, sa2, sa3, sa4, sa5, sa6, idlesa *SummaryAllocation
 
 	// Generating accumulated summary allocation sets for testing
-	var sas1, sas2, sas3, sas4, sas5 *SummaryAllocationSet
+	var sas1, sas2, sas3, sas4, sas5, sas6 *SummaryAllocationSet
 
 	window, _ := ParseWindowUTC("7d")
 
@@ -314,6 +314,20 @@ func TestSummaryAllocationSet_RAMEfficiency(t *testing.T) {
 		RAMCost:                0.10,
 	}
 
+	idlesa = &SummaryAllocation{
+		Name: IdleSuffix,
+		Properties: &AllocationProperties{
+			Cluster:   "cluster1",
+			Namespace: "namespace1",
+			Pod:       "pod1",
+			Container: "container7",
+		},
+		Start:   saStart,
+		End:     saEnd,
+		CPUCost: 1.0,
+		RAMCost: 1.0,
+	}
+
 	testcase1Map := map[string]*SummaryAllocation{
 		"cluster1/namespace1/pod1/container1": sa1,
 		"cluster1/namespace1/pod1/container2": sa2,
@@ -340,6 +354,12 @@ func TestSummaryAllocationSet_RAMEfficiency(t *testing.T) {
 		"cluster1/namespace1/pod1/container6": sa6,
 	}
 
+	testcase6Map := map[string]*SummaryAllocation{
+		"cluster1/namespace1/pod1/container1": sa1,
+		"cluster1/namespace1/pod1/container2": sa2,
+		"cluster1/__idle__":                   idlesa,
+	}
+
 	sas1 = &SummaryAllocationSet{
 		SummaryAllocations: testcase1Map,
 		Window:             window,
@@ -365,6 +385,11 @@ func TestSummaryAllocationSet_RAMEfficiency(t *testing.T) {
 		Window:             window,
 	}
 
+	sas6 = &SummaryAllocationSet{
+		SummaryAllocations: testcase6Map,
+		Window:             window,
+	}
+
 	cases := []struct {
 		name               string
 		testsas            *SummaryAllocationSet
@@ -395,6 +420,11 @@ func TestSummaryAllocationSet_RAMEfficiency(t *testing.T) {
 			testsas:            sas5,
 			expectedEfficiency: 0.65,
 		},
+		{
+			name:               "Check RAMEfficiency in presense of n idle allocation",
+			testsas:            sas6,
+			expectedEfficiency: 0.25,
+		},
 	}
 
 	for _, c := range cases {
@@ -410,10 +440,10 @@ func TestSummaryAllocationSet_RAMEfficiency(t *testing.T) {
 
 func TestSummaryAllocationSet_CPUEfficiency(t *testing.T) {
 	// Generating 6 sample summary allocations for testing
-	var sa1, sa2, sa3, sa4, sa5, sa6 *SummaryAllocation
+	var sa1, sa2, sa3, sa4, sa5, sa6, idlesa *SummaryAllocation
 
 	// Generating accumulated summary allocation sets for testing
-	var sas1, sas2, sas3, sas4, sas5 *SummaryAllocationSet
+	var sas1, sas2, sas3, sas4, sas5, sas6 *SummaryAllocationSet
 
 	window, _ := ParseWindowUTC("7d")
 
@@ -511,6 +541,20 @@ func TestSummaryAllocationSet_CPUEfficiency(t *testing.T) {
 		CPUCost:               0.2,
 	}
 
+	idlesa = &SummaryAllocation{
+		Name: IdleSuffix,
+		Properties: &AllocationProperties{
+			Cluster:   "cluster1",
+			Namespace: "namespace1",
+			Pod:       "pod1",
+			Container: "container7",
+		},
+		Start:   saStart,
+		End:     saEnd,
+		CPUCost: 1.0,
+		RAMCost: 1.0,
+	}
+
 	testcase1Map := map[string]*SummaryAllocation{
 		"cluster1/namespace1/pod1/container1": sa1,
 		"cluster1/namespace1/pod1/container2": sa2,
@@ -537,6 +581,12 @@ func TestSummaryAllocationSet_CPUEfficiency(t *testing.T) {
 		"cluster1/namespace1/pod1/container6": sa6,
 	}
 
+	testcase6Map := map[string]*SummaryAllocation{
+		"cluster1/namespace1/pod1/container1": sa1,
+		"cluster1/namespace1/pod1/container2": sa2,
+		"cluster1/__idle__":                   idlesa,
+	}
+
 	sas1 = &SummaryAllocationSet{
 		SummaryAllocations: testcase1Map,
 		Window:             window,
@@ -562,6 +612,11 @@ func TestSummaryAllocationSet_CPUEfficiency(t *testing.T) {
 		Window:             window,
 	}
 
+	sas6 = &SummaryAllocationSet{
+		SummaryAllocations: testcase6Map,
+		Window:             window,
+	}
+
 	cases := []struct {
 		name               string
 		testsas            *SummaryAllocationSet
@@ -592,6 +647,11 @@ func TestSummaryAllocationSet_CPUEfficiency(t *testing.T) {
 			testsas:            sas5,
 			expectedEfficiency: 0.50,
 		},
+		{
+			name:               "Check CPUEfficiency in presence of idle allocation",
+			testsas:            sas6,
+			expectedEfficiency: 0.30,
+		},
 	}
 
 	for _, c := range cases {
@@ -803,7 +863,7 @@ func TestSummaryAllocationSet_TotalEfficiency(t *testing.T) {
 		{
 			name:               "Check TotalEfficiency with idle cost",
 			testsas:            sas4,
-			expectedEfficiency: 0.20,
+			expectedEfficiency: 0.30,
 		},
 	}