瀏覽代碼

Alloc.add handles changes in only controller name

The new AddDifferentController test failed before the change to
Allocation.add and outlines the situation. We want to be able
to handle a situation that the Kubernetes spec does not expect
but occurs in the wild: a pod whose controller's name changes
but nothing else does. We'd like the two _technically_ separate
allocations that this generates to be added as if they were the
same without empty-stringing the controller name field. We opted
to pick the alphabetically first string but another strategy
could be used instead.
Michael Dresser 5 年之前
父節點
當前提交
168a5939a2
共有 2 個文件被更改,包括 54 次插入 和 0 次删除
  1. 30 0
      pkg/kubecost/allocation.go
  2. 24 0
      pkg/kubecost/allocation_test.go

+ 30 - 0
pkg/kubecost/allocation.go

@@ -3,6 +3,7 @@ package kubecost
 import (
 	"bytes"
 	"fmt"
+	"sort"
 	"strings"
 	"sync"
 	"time"
@@ -601,9 +602,38 @@ func (a *Allocation) add(that *Allocation) {
 		log.Warningf("Allocation.AggregateBy: trying to add a nil receiver")
 		return
 	}
+
+	// Save the original properties so we can set a controller name in the event of
+	// equality in all fields except the controller name
+	leftProperties := a.Properties
+	rightProperties := that.Properties
+
 	// Preserve string properties that are matching between the two allocations
 	a.Properties = a.Properties.Intersection(that.Properties)
 
+	// Overwrite regular intersection logic for the controller name property in the
+	// case that all properties except controller name are the same.
+	if leftProperties != nil &&
+		rightProperties != nil &&
+		leftProperties.Controller != rightProperties.Controller &&
+		leftProperties.Controller != "" &&
+		rightProperties.Controller != "" {
+
+		lpNoController := leftProperties.Clone()
+		lpNoController.Controller = ""
+		rpNoController := rightProperties.Clone()
+		rpNoController.Controller = ""
+
+		if lpNoController.Equal(rpNoController) {
+			controllers := []string{
+				leftProperties.Controller,
+				rightProperties.Controller,
+			}
+			sort.Strings(controllers)
+			a.Properties.Controller = controllers[0]
+		}
+	}
+
 	// Expand the window to encompass both Allocations
 	a.Window = a.Window.Expand(that.Window)
 

+ 24 - 0
pkg/kubecost/allocation_test.go

@@ -355,6 +355,30 @@ func TestAllocation_Share(t *testing.T) {
 	}
 }
 
+func TestAllocation_AddDifferentController(t *testing.T) {
+	a1 := &Allocation{
+		Properties: &AllocationProperties{
+			Container:  "container",
+			Pod:        "pod",
+			Namespace:  "ns",
+			Cluster:    "cluster",
+			Controller: "controller 1",
+		},
+	}
+	a2 := a1.Clone()
+	a2.Properties.Controller = "controller 2"
+
+	result, err := a1.Add(a2)
+	if err != nil {
+		t.Fatalf("Allocation.Add: unexpected error: %s", err)
+	}
+
+	if result.Properties.Controller == "" {
+		t.Errorf("Adding allocations whose properties only differ in controller name should not result in an empty string controller name.")
+	}
+
+}
+
 func TestAllocation_MarshalJSON(t *testing.T) {
 	start := time.Date(2021, time.January, 1, 0, 0, 0, 0, time.UTC)
 	end := time.Date(2021, time.January, 2, 0, 0, 0, 0, time.UTC)