فهرست منبع

Support label aliases (e.g. owner, product, etc.) as first-class aggregation options

Niko Kovacevic 4 سال پیش
والد
کامیت
ade44a131c
4فایلهای تغییر یافته به همراه174 افزوده شده و 66 حذف شده
  1. 100 49
      pkg/kubecost/allocation.go
  2. 43 17
      pkg/kubecost/allocation_test.go
  3. 28 0
      pkg/kubecost/allocationprops.go
  4. 3 0
      pkg/kubecost/config_test.go

+ 100 - 49
pkg/kubecost/allocation.go

@@ -3,7 +3,6 @@ package kubecost
 import (
 	"bytes"
 	"fmt"
-	"sort"
 	"strings"
 	"sync"
 	"time"
@@ -715,13 +714,14 @@ func NewAllocationSet(start, end time.Time, allocs ...*Allocation) *AllocationSe
 // simple flag for sharing idle resources.
 type AllocationAggregationOptions struct {
 	FilterFuncs       []AllocationMatchFunc
-	SplitIdle         bool
 	IdleByNode        bool
+	LabelConfig       *LabelConfig
 	MergeUnallocated  bool
+	SharedHourlyCosts map[string]float64
 	ShareFuncs        []AllocationMatchFunc
 	ShareIdle         string
 	ShareSplit        string
-	SharedHourlyCosts map[string]float64
+	SplitIdle         bool
 }
 
 // AggregateBy aggregates the Allocations in the given AllocationSet by the given
@@ -754,11 +754,26 @@ func (as *AllocationSet) AggregateBy(aggregateBy []string, options *AllocationAg
 	// 10. If the merge idle option is enabled, merge any remaining idle
 	//     allocations into a single idle allocation
 
+	if as.IsEmpty() {
+		return nil
+	}
+
 	if options == nil {
 		options = &AllocationAggregationOptions{}
 	}
 
-	if as.IsEmpty() {
+	if options.LabelConfig == nil {
+		options.LabelConfig = NewLabelConfig()
+	}
+
+	// If aggregateBy is nil, we don't aggregate anything. On the other hand,
+	// an empty slice implies that we should aggregate everything. See
+	// generateKey for why that makes sense.
+	shouldAggregate := aggregateBy != nil
+	shouldFilter := len(options.FilterFuncs) > 0
+	shouldShare := len(options.SharedHourlyCosts) > 0 || len(options.ShareFuncs) > 0
+	if !shouldAggregate && !shouldFilter && !shouldShare {
+		// There is nothing for AggregateBy to do, so simply return nil
 		return nil
 	}
 
@@ -1022,7 +1037,7 @@ func (as *AllocationSet) AggregateBy(aggregateBy []string, options *AllocationAg
 		}
 
 		// (5) generate key to use for aggregation-by-key and allocation name
-		key := alloc.generateKey(aggregateBy)
+		key := alloc.generateKey(aggregateBy, options.LabelConfig)
 
 		alloc.Name = key
 		if options.MergeUnallocated && alloc.IsUnallocated() {
@@ -1153,7 +1168,7 @@ func (as *AllocationSet) AggregateBy(aggregateBy []string, options *AllocationAg
 			}
 		}
 		if !skip {
-			key := alloc.generateKey(aggregateBy)
+			key := alloc.generateKey(aggregateBy, options.LabelConfig)
 
 			alloc.Name = key
 			aggSet.Insert(alloc)
@@ -1198,7 +1213,7 @@ func computeShareCoeffs(aggregateBy []string, options *AllocationAggregationOpti
 
 		// Determine the post-aggregation key under which the allocation will
 		// be shared.
-		name := alloc.generateKey(aggregateBy)
+		name := alloc.generateKey(aggregateBy, options.LabelConfig)
 
 		// If the current allocation will be filtered out in step 3, contribute
 		// its share of the shared coefficient to a "__filtered__" bin, which
@@ -1383,11 +1398,15 @@ func (a *Allocation) getIdleId(options *AllocationAggregationOptions) (string, e
 	return idleId, nil
 }
 
-func (a *Allocation) generateKey(aggregateBy []string) string {
+func (a *Allocation) generateKey(aggregateBy []string, labelConfig *LabelConfig) string {
 	if a == nil {
 		return ""
 	}
 
+	if labelConfig == nil {
+		labelConfig = NewLabelConfig()
+	}
+
 	// Names will ultimately be joined into a single name, which uniquely
 	// identifies allocations.
 	names := []string{}
@@ -1442,61 +1461,93 @@ func (a *Allocation) generateKey(aggregateBy []string) string {
 		case strings.HasPrefix(agg, "label:"):
 			labels := a.Properties.Labels
 			if labels == nil {
-				// Indicate that allocation has no labels
 				names = append(names, UnallocatedSuffix)
 			} else {
-				labelNames := []string{}
-				aggLabels := strings.Split(strings.TrimPrefix(agg, "label:"), ";")
-				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)
+				labelName := strings.TrimPrefix(agg, "label:")
+				if labelValue, ok := labels[labelName]; ok {
+					names = append(names, fmt.Sprintf("%s=%s", labelName, labelValue))
+				} else {
+					names = append(names, UnallocatedSuffix)
 				}
-				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...)
 			}
 		case strings.HasPrefix(agg, "annotation:"):
 			annotations := a.Properties.Annotations
 			if annotations == nil {
-				// Indicate that allocation has no annotations
 				names = append(names, UnallocatedSuffix)
 			} else {
-				annotationNames := []string{}
-				aggAnnotations := strings.Split(strings.TrimPrefix(agg, "annotation:"), ";")
-				for _, annotationName := range aggAnnotations {
-					if val, ok := annotations[annotationName]; ok {
-						annotationNames = append(annotationNames, fmt.Sprintf("%s=%s", annotationName, val))
-					} else if indexOf(UnallocatedSuffix, annotationNames) == -1 { // if UnallocatedSuffix not already in names
-						annotationNames = append(annotationNames, UnallocatedSuffix)
-					}
+				annotationName := strings.TrimPrefix(agg, "annotation:")
+				if annotationValue, ok := annotations[annotationName]; ok {
+					names = append(names, fmt.Sprintf("%s=%s", annotationName, annotationValue))
+				} else {
+					names = append(names, UnallocatedSuffix)
+				}
+			}
+		case agg == AllocationDepartmentProp:
+			labels := a.Properties.Labels
+			if labels == nil {
+				names = append(names, UnallocatedSuffix)
+			} else {
+				labelName := labelConfig.DepartmentLabel
+				if labelValue, ok := labels[labelName]; ok {
+					names = append(names, labelValue)
+				} else {
+					names = append(names, UnallocatedSuffix)
 				}
-				// resolve arbitrary ordering. e.g., app=app0/env=env0 is the same agg as env=env0/app=app0
-				if len(annotationNames) > 1 {
-					sort.Strings(annotationNames)
+			}
+		case agg == AllocationEnvironmentProp:
+			labels := a.Properties.Labels
+			if labels == nil {
+				names = append(names, UnallocatedSuffix)
+			} else {
+				labelName := labelConfig.EnvironmentLabel
+				if labelValue, ok := labels[labelName]; ok {
+					names = append(names, labelValue)
+				} else {
+					names = append(names, UnallocatedSuffix)
 				}
-				unallocatedSuffixIndex := indexOf(UnallocatedSuffix, annotationNames)
-				// suffix should be at index 0 if it exists b/c of underscores
-				if unallocatedSuffixIndex != -1 {
-					annotationNames = append(annotationNames[:unallocatedSuffixIndex], annotationNames[unallocatedSuffixIndex+1:]...)
-					annotationNames = append(annotationNames, UnallocatedSuffix) // append to end
+			}
+		case agg == AllocationOwnerProp:
+			labels := a.Properties.Labels
+			if labels == nil {
+				names = append(names, UnallocatedSuffix)
+			} else {
+				labelName := labelConfig.OwnerLabel
+				if labelValue, ok := labels[labelName]; ok {
+					names = append(names, labelValue)
+				} else {
+					names = append(names, UnallocatedSuffix)
+				}
+			}
+		case agg == AllocationProductProp:
+			labels := a.Properties.Labels
+			if labels == nil {
+				names = append(names, UnallocatedSuffix)
+			} else {
+				labelName := labelConfig.ProductLabel
+				if labelValue, ok := labels[labelName]; ok {
+					names = append(names, labelValue)
+				} else {
+					names = append(names, UnallocatedSuffix)
+				}
+			}
+		case agg == AllocationTeamProp:
+			labels := a.Properties.Labels
+			if labels == nil {
+				names = append(names, UnallocatedSuffix)
+			} else {
+				labelName := labelConfig.TeamLabel
+				if labelValue, ok := labels[labelName]; ok {
+					names = append(names, labelValue)
+				} else {
+					names = append(names, UnallocatedSuffix)
 				}
-
-				names = append(names, annotationNames...)
 			}
+		default:
+			// This case should never be reached, as input up until this point
+			// should be checked and rejected if invalid. But if we do get a
+			// value we don't recognize, log a warning.
+			log.Warningf("AggregateBy: illegal aggregation parameter: %s", agg)
 		}
-
 	}
 
 	return strings.Join(names, "/")

+ 43 - 17
pkg/kubecost/allocation_test.go

@@ -435,7 +435,7 @@ func TestAllocationSet_generateKey(t *testing.T) {
 		AllocationClusterProp,
 	}
 
-	key = alloc.generateKey(props)
+	key = alloc.generateKey(props, nil)
 	if key != "" {
 		t.Fatalf("generateKey: expected \"\"; actual \"%s\"", key)
 	}
@@ -449,7 +449,7 @@ func TestAllocationSet_generateKey(t *testing.T) {
 		},
 	}
 
-	key = alloc.generateKey(props)
+	key = alloc.generateKey(props, nil)
 	if key != "cluster1" {
 		t.Fatalf("generateKey: expected \"cluster1\"; actual \"%s\"", key)
 	}
@@ -460,7 +460,7 @@ func TestAllocationSet_generateKey(t *testing.T) {
 		"label:app",
 	}
 
-	key = alloc.generateKey(props)
+	key = alloc.generateKey(props, nil)
 	if key != "cluster1//app=app1" {
 		t.Fatalf("generateKey: expected \"cluster1//app=app1\"; actual \"%s\"", key)
 	}
@@ -473,10 +473,36 @@ func TestAllocationSet_generateKey(t *testing.T) {
 			"env": "env1",
 		},
 	}
-	key = alloc.generateKey(props)
+	key = alloc.generateKey(props, nil)
 	if key != "cluster1/namespace1/app=app1" {
 		t.Fatalf("generateKey: expected \"cluster1/namespace1/app=app1\"; actual \"%s\"", key)
 	}
+
+	props = []string{
+		AllocationDepartmentProp,
+		AllocationEnvironmentProp,
+		AllocationOwnerProp,
+		AllocationProductProp,
+		AllocationTeamProp,
+	}
+
+	labelConfig := NewLabelConfig()
+
+	alloc.Properties = &AllocationProperties{
+		Cluster:   "cluster1",
+		Namespace: "namespace1",
+		Labels: map[string]string{
+			labelConfig.DepartmentLabel:  "dept1",
+			labelConfig.EnvironmentLabel: "envt1",
+			labelConfig.OwnerLabel:       "ownr1",
+			labelConfig.ProductLabel:     "prod1",
+			labelConfig.TeamLabel:        "team1",
+		},
+	}
+	key = alloc.generateKey(props, nil)
+	if key != "dept1/envt1/ownr1/prod1/team1" {
+		t.Fatalf("generateKey: expected \"dept1/envt1/ownr1/prod1/team1\"; actual \"%s\"", key)
+	}
 }
 
 func TestNewAllocationSet(t *testing.T) {
@@ -920,17 +946,17 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 		// 2d AggregationProperties=(Label:app, Label:environment)
 		"2d": {
 			start:      start,
-			aggBy:      []string{"label:app;env"},
+			aggBy:      []string{"label:app", "label:env"},
 			aggOpts:    nil,
 			numResults: 3 + numIdle + numUnallocated,
 			totalCost:  activeTotalCost + idleTotalCost,
 			// sets should be {idle, unallocated, app1/env1, app2/env2, app2/unallocated}
 			results: map[string]float64{
-				"app=app1/env=env1":             16.00,
-				"app=app2/env=env2":             12.00,
-				"app=app2/" + UnallocatedSuffix: 12.00,
-				IdleSuffix:                      30.00,
-				UnallocatedSuffix:               42.00,
+				"app=app1/env=env1":                         16.00,
+				"app=app2/env=env2":                         12.00,
+				"app=app2/" + UnallocatedSuffix:             12.00,
+				IdleSuffix:                                  30.00,
+				UnallocatedSuffix + "/" + UnallocatedSuffix: 42.00,
 			},
 			windowStart: startYesterday,
 			windowEnd:   endYesterday,
@@ -939,17 +965,17 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 		// 2e AggregationProperties=(Cluster, Label:app, Label:environment)
 		"2e": {
 			start:      start,
-			aggBy:      []string{AllocationClusterProp, "label:app;env"},
+			aggBy:      []string{AllocationClusterProp, "label:app", "label:env"},
 			aggOpts:    nil,
 			numResults: 6,
 			totalCost:  activeTotalCost + idleTotalCost,
 			results: map[string]float64{
-				"cluster1/app=app2/env=env2":             12.00,
-				"__idle__":                               30.00,
-				"cluster1/app=app1/env=env1":             16.00,
-				"cluster1/" + UnallocatedSuffix:          18.00,
-				"cluster2/app=app2/" + UnallocatedSuffix: 12.00,
-				"cluster2/" + UnallocatedSuffix:          24.00,
+				"cluster1/app=app2/env=env2": 12.00,
+				"__idle__":                   30.00,
+				"cluster1/app=app1/env=env1": 16.00,
+				"cluster1/" + UnallocatedSuffix + "/" + UnallocatedSuffix: 18.00,
+				"cluster2/app=app2/" + UnallocatedSuffix:                  12.00,
+				"cluster2/" + UnallocatedSuffix + "/" + UnallocatedSuffix: 24.00,
 			},
 			windowStart: startYesterday,
 			windowEnd:   endYesterday,

+ 28 - 0
pkg/kubecost/allocationprops.go

@@ -4,6 +4,8 @@ import (
 	"fmt"
 	"sort"
 	"strings"
+
+	"github.com/kubecost/cost-model/pkg/prom"
 )
 
 const (
@@ -23,6 +25,11 @@ const (
 	AllocationStatefulSetProp    string = "statefulset"
 	AllocationDaemonSetProp      string = "daemonset"
 	AllocationJobProp            string = "job"
+	AllocationDepartmentProp     string = "department"
+	AllocationEnvironmentProp    string = "environment"
+	AllocationOwnerProp          string = "owner"
+	AllocationProductProp        string = "product"
+	AllocationTeamProp           string = "team"
 )
 
 func ParseProperty(text string) (string, error) {
@@ -57,7 +64,28 @@ func ParseProperty(text string) (string, error) {
 		return AllocationStatefulSetProp, nil
 	case "job":
 		return AllocationJobProp, nil
+	case "department":
+		return AllocationDepartmentProp, nil
+	case "environment":
+		return AllocationEnvironmentProp, nil
+	case "owner":
+		return AllocationOwnerProp, nil
+	case "product":
+		return AllocationProductProp, nil
+	case "team":
+		return AllocationTeamProp, nil
+	}
+
+	if strings.HasPrefix(text, "label:") {
+		label := prom.SanitizeLabelName(strings.TrimSpace(strings.TrimPrefix(text, "label:")))
+		return fmt.Sprintf("label:%s", label), nil
 	}
+
+	if strings.HasPrefix(text, "annotation:") {
+		annotation := prom.SanitizeLabelName(strings.TrimSpace(strings.TrimPrefix(text, "annotation:")))
+		return fmt.Sprintf("annotation:%s", annotation), nil
+	}
+
 	return AllocationNilProp, fmt.Errorf("invalid allocation property: %s", text)
 }
 

+ 3 - 0
pkg/kubecost/config_test.go

@@ -42,6 +42,7 @@ func TestLabelConfig_GetExternalAllocationName(t *testing.T) {
 
 	labels := map[string]string{
 		"kubens":                      "kubecost-staging",
+		"kubeowner":                   "kubecost-owner",
 		"env":                         "env1",
 		"app":                         "app1",
 		glueFormattedLabel:            "glue",
@@ -102,6 +103,7 @@ func TestLabelConfig_GetExternalAllocationName(t *testing.T) {
 	// Change the external label for namespace and confirm it still works
 	lc.NamespaceExternalLabel = "kubens"
 	lc.PodExternalLabel = "Non__GlueFormattedLabel"
+	lc.OwnerExternalLabel = "kubeowner"
 
 	// TODO how is e.g. OwnerExternalLabel supposed to work?
 
@@ -111,6 +113,7 @@ func TestLabelConfig_GetExternalAllocationName(t *testing.T) {
 	}{
 		{"namespace", "kubecost-staging"},
 		{"pod", "glue"},
+		// {"owner", "kubeowner"},
 	}
 	for _, tc := range testCases {
 		actual := lc.GetExternalAllocationName(labels, tc.aggBy)