Просмотр исходного кода

Add and test isFilterEmpty helper for AggBy logic

Signed-off-by: Michael Dresser <michaelmdresser@gmail.com>
Michael Dresser 2 лет назад
Родитель
Сommit
2841ca4d3c
2 измененных файлов с 33 добавлено и 5 удалено
  1. 8 5
      pkg/kubecost/allocation.go
  2. 25 0
      pkg/kubecost/allocation_test.go

+ 8 - 5
pkg/kubecost/allocation.go

@@ -1059,6 +1059,13 @@ type AllocationAggregationOptions struct {
 	IncludeAggregatedMetadata             bool
 }
 
+func isFilterEmpty(filter AllocationMatcher) bool {
+	if _, isAllPass := filter.(*matcher.AllPass[*Allocation]); isAllPass {
+		return true
+	}
+	return false
+}
+
 // AggregateBy aggregates the Allocations in the given AllocationSet by the given
 // AllocationProperty. This will only be legal if the AllocationSet is divisible by the
 // given AllocationProperty; e.g. Containers can be divided by Namespace, but not vice-a-versa.
@@ -1147,11 +1154,7 @@ func (as *AllocationSet) AggregateBy(aggregateBy []string, options *AllocationAg
 	// an empty slice implies that we should aggregate everything. See
 	// generateKey for why that makes sense.
 	shouldAggregate := aggregateBy != nil
-	shouldFilter := true
-	// TODO: Make sure this works as expected
-	if _, isAllPass := filter.(*matcher.AllPass[*Allocation]); isAllPass {
-		shouldFilter = false
-	}
+	shouldFilter := !isFilterEmpty(filter)
 	shouldShare := len(options.SharedHourlyCosts) > 0 || len(options.ShareFuncs) > 0
 	if !shouldAggregate && !shouldFilter && !shouldShare && options.ShareIdle == ShareNone && !options.IncludeProportionalAssetResourceCosts {
 		// There is nothing for AggregateBy to do, so simply return nil

+ 25 - 0
pkg/kubecost/allocation_test.go

@@ -3436,3 +3436,28 @@ func Test_DetermineSharingName(t *testing.T) {
 		t.Fatalf("determineSharingName: expected \"unknown\"; actual \"%s\"", name)
 	}
 }
+
+func TestIsFilterEmptyTrue(t *testing.T) {
+	compiler := NewAllocationMatchCompiler()
+	matcher, err := compiler.Compile(nil)
+	if err != nil {
+		t.Fatalf("compiling nil filter: %s", err)
+	}
+
+	result := isFilterEmpty(matcher)
+	if !result {
+		t.Errorf("matcher '%+v' should be reported empty but wasn't", matcher)
+	}
+}
+
+func TestIsFilterEmptyFalse(t *testing.T) {
+	compiler := NewAllocationMatchCompiler()
+	matcher, err := compiler.Compile(ops.Eq(allocfilter.AllocationFieldClusterID, "test"))
+	if err != nil {
+		t.Fatalf("compiling nil filter: %s", err)
+	}
+	result := isFilterEmpty(matcher)
+	if result {
+		t.Errorf("matcher '%+v' should be not be reported empty but was", matcher)
+	}
+}