Fix the rest of test

pkg/kubecost/allocation.go

@@ -1155,11 +1155,11 @@ func (a *Allocation) generateKey(properties AllocationProperties) string {
 			// Indicate that allocation has no controller
 			controllerKind = UnallocatedSuffix
 		}
-		// TODO figure out if the is functional, refactor breaks this
-		//if prop := properties.ControllerKind; prop != "" && prop != controllerKind {
-		//	// The allocation does not have the specified controller kind
-		//	controllerKind = UnallocatedSuffix
-		//}
+		// TODO find better way to pass controller kind filter
+		if prop := properties.ControllerKind; prop != "true" && prop != controllerKind {
+			// The allocation does not have the specified controller kind
+			controllerKind = UnallocatedSuffix
+		}
 		names = append(names, controllerKind)
 	}
 
@@ -1195,16 +1195,18 @@ func (a *Allocation) generateKey(properties AllocationProperties) string {
 		if services == nil {
 			// Indicate that allocation has no services
 			names = append(names, UnallocatedSuffix)
-		}
-		if services != nil && len(services) > 0 {
-			for _, service := range services {
-				names = append(names, service)
-				break
-			}
 		} else {
-			// Indicate that allocation has no services
-			names = append(names, UnallocatedSuffix)
+			if len(services) > 0 {
+				for _, service := range services {
+					names = append(names, service)
+					break
+				}
+			} else {
+				// Indicate that allocation has no services
+				names = append(names, UnallocatedSuffix)
+			}
 		}
+
 	}
 
 	if properties.Annotations != nil   {
@@ -1212,7 +1214,7 @@ func (a *Allocation) generateKey(properties AllocationProperties) string {
 		if annotations == nil {
 			// Indicate that allocation has no annotations
 			names = append(names, UnallocatedSuffix)
-		}
+		} else {
 			annotationNames := []string{}
 			aggAnnotations := properties.Annotations
 			for annotationName := range aggAnnotations {
@@ -1234,7 +1236,7 @@ func (a *Allocation) generateKey(properties AllocationProperties) string {
 			}
 
 			names = append(names, annotationNames...)
-
+		}
 	}
 
 	if properties.Labels != nil {
@@ -1242,28 +1244,29 @@ func (a *Allocation) generateKey(properties AllocationProperties) string {
 		if labels == nil {
 			// Indicate that allocation has no labels
 			names = append(names, UnallocatedSuffix)
-		}
-		labelNames := []string{}
-		aggLabels:= properties.Labels
-		for labelName := range aggLabels {
-			if val, ok := labels[labelName]; ok {
-				labelNames = append(labelNames, fmt.Sprintf("%s=%s", labelName, val))
-			} else if indexOf(UnallocatedSuffix, labelNames) == -1 { // if UnallocatedSuffix not already in names
-				labelNames = append(labelNames, UnallocatedSuffix)
+		} else {
+			labelNames := []string{}
+			aggLabels := properties.Labels
+			for labelName := range aggLabels {
+				if val, ok := labels[labelName]; ok {
+					labelNames = append(labelNames, fmt.Sprintf("%s=%s", labelName, val))
+				} else if indexOf(UnallocatedSuffix, labelNames) == -1 { // if UnallocatedSuffix not already in names
+					labelNames = append(labelNames, UnallocatedSuffix)
+				}
+			}
+			// resolve arbitrary ordering. e.g., app=app0/env=env0 is the same agg as env=env0/app=app0
+			if len(labelNames) > 1 {
+				sort.Strings(labelNames)
+			}
+			unallocatedSuffixIndex := indexOf(UnallocatedSuffix, labelNames)
+			// suffix should be at index 0 if it exists b/c of underscores
+			if unallocatedSuffixIndex != -1 {
+				labelNames = append(labelNames[:unallocatedSuffixIndex], labelNames[unallocatedSuffixIndex+1:]...)
+				labelNames = append(labelNames, UnallocatedSuffix) // append to end
 			}
-		}
-		// resolve arbitrary ordering. e.g., app=app0/env=env0 is the same agg as env=env0/app=app0
-		if len(labelNames) > 1 {
-			sort.Strings(labelNames)
-		}
-		unallocatedSuffixIndex := indexOf(UnallocatedSuffix, labelNames)
-		// suffix should be at index 0 if it exists b/c of underscores
-		if unallocatedSuffixIndex != -1 {
-			labelNames = append(labelNames[:unallocatedSuffixIndex], labelNames[unallocatedSuffixIndex+1:]...)
-			labelNames = append(labelNames, UnallocatedSuffix) // append to end
-		}
 
-		names = append(names, labelNames...)
+			names = append(names, labelNames...)
+		}
 	}
 
 	return strings.Join(names, "/")

pkg/kubecost/allocation_test.go

@@ -1154,7 +1154,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 
 	// 5a Filter by cluster with separate idle
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Cluster: ""}, &AllocationAggregationOptions{
+	err = as.AggregateBy(AllocationProperties{Cluster: "true"}, &AllocationAggregationOptions{
 		FilterFuncs: []AllocationMatchFunc{isCluster("cluster1")},
 		ShareIdle:   ShareNone,
 	})
@@ -1167,7 +1167,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 
 	// 5b Filter by cluster with shared idle
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Cluster: ""}, &AllocationAggregationOptions{
+	err = as.AggregateBy(AllocationProperties{Cluster: "true"}, &AllocationAggregationOptions{
 		FilterFuncs: []AllocationMatchFunc{isCluster("cluster1")},
 		ShareIdle:   ShareWeighted,
 	})
@@ -1179,7 +1179,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 
 	// 5c Filter by cluster, agg by namespace, with separate idle
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Namespace: ""}, &AllocationAggregationOptions{
+	err = as.AggregateBy(AllocationProperties{Namespace: "true"}, &AllocationAggregationOptions{
 		FilterFuncs: []AllocationMatchFunc{isCluster("cluster1")},
 		ShareIdle:   ShareNone,
 	})
@@ -1193,7 +1193,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 
 	// 5d Filter by namespace, agg by cluster, with separate idle
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Cluster: ""}, &AllocationAggregationOptions{
+	err = as.AggregateBy(AllocationProperties{Cluster: "true"}, &AllocationAggregationOptions{
 		FilterFuncs: []AllocationMatchFunc{isNamespace("namespace2")},
 		ShareIdle:   ShareNone,
 	})
@@ -1209,7 +1209,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 
 	// 6a SplitIdle
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Namespace: ""}, &AllocationAggregationOptions{SplitIdle: true})
+	err = as.AggregateBy(AllocationProperties{Namespace: "true"}, &AllocationAggregationOptions{SplitIdle: true})
 	assertAllocationSetTotals(t, as, "6a", err, numNamespaces+numSplitIdle, activeTotalCost+idleTotalCost)
 	assertAllocationTotals(t, as, "6a", map[string]float64{
 		"namespace1":                           28.00,
@@ -1224,7 +1224,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 	// Should match values from unfiltered aggregation (3a)
 	// namespace2: 46.3125 = 36.00 + 5.0*(3.0/6.0) + 15.0*(3.0/16.0) + 5.0*(3.0/6.0) + 5.0*(3.0/6.0)
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Namespace: ""}, &AllocationAggregationOptions{
+	err = as.AggregateBy(AllocationProperties{Namespace: "true"}, &AllocationAggregationOptions{
 		FilterFuncs: []AllocationMatchFunc{isNamespace("namespace2")},
 		ShareIdle:   ShareWeighted,
 	})
@@ -1238,7 +1238,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 	// Should match values from unfiltered aggregation (3b)
 	// namespace2: 51.0000 = 36.00 + 5.0*(1.0/2.0) + 15.0*(1.0/2.0) + 5.0*(1.0/2.0) + 5.0*(1.0/2.0)
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Namespace: ""}, &AllocationAggregationOptions{
+	err = as.AggregateBy(AllocationProperties{Namespace: "true"}, &AllocationAggregationOptions{
 		FilterFuncs: []AllocationMatchFunc{isNamespace("namespace2")},
 		ShareIdle:   ShareEven,
 	})
@@ -1255,7 +1255,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 	// idle:       30.0000
 	// Then namespace 2 is filtered.
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Namespace: ""}, &AllocationAggregationOptions{
+	err = as.AggregateBy(AllocationProperties{Namespace: "true"}, &AllocationAggregationOptions{
 		FilterFuncs:       []AllocationMatchFunc{isNamespace("namespace2")},
 		SharedHourlyCosts: map[string]float64{"total": sharedOverheadHourlyCost},
 		ShareSplit:        ShareWeighted,
@@ -1276,7 +1276,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 	// namespace2: 54.667 = 36.00 + (28.00)*(36.00/54.00)
 	// idle:       30.0000
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Namespace: ""}, &AllocationAggregationOptions{
+	err = as.AggregateBy(AllocationProperties{Namespace: "true"}, &AllocationAggregationOptions{
 		FilterFuncs: []AllocationMatchFunc{isNamespace("namespace2")},
 		ShareFuncs:  []AllocationMatchFunc{isNamespace("namespace1")},
 		ShareSplit:  ShareWeighted,
@@ -1324,7 +1324,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 	//   shared cost    14.2292 = (42.6875)*(18.0/54.0)
 	//
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Namespace: ""}, &AllocationAggregationOptions{
+	err = as.AggregateBy(AllocationProperties{Namespace: "true"}, &AllocationAggregationOptions{
 		ShareFuncs: []AllocationMatchFunc{isNamespace("namespace1")},
 		ShareSplit: ShareWeighted,
 		ShareIdle:  ShareWeighted,
@@ -1374,7 +1374,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 	// Then, filter for namespace2: 74.7708
 	//
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Namespace: ""}, &AllocationAggregationOptions{
+	err = as.AggregateBy(AllocationProperties{Namespace: "true"}, &AllocationAggregationOptions{
 		FilterFuncs: []AllocationMatchFunc{isNamespace("namespace2")},
 		ShareFuncs:  []AllocationMatchFunc{isNamespace("namespace1")},
 		ShareSplit:  ShareWeighted,
@@ -1415,7 +1415,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 	//
 	// Then namespace 2 is filtered.
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Namespace: ""}, &AllocationAggregationOptions{
+	err = as.AggregateBy(AllocationProperties{Namespace: "true"}, &AllocationAggregationOptions{
 		FilterFuncs:       []AllocationMatchFunc{isNamespace("namespace2")},
 		ShareSplit:        ShareWeighted,
 		ShareIdle:         ShareWeighted,

pkg/kubecost/allocationprops.go

@@ -300,7 +300,7 @@ func (p *AllocationProperties) AggregationStrings() []string {
 	return aggStrs
 }
 
-func (p *AllocationProperties) isEmpty() bool {
+func (p *AllocationProperties) IsEmpty() bool {
 	if p == nil {
 		return true
 	}