Add unit tests for SummaryAllocation and fix bug in Add

pkg/kubecost/allocation.go

@@ -1273,7 +1273,7 @@ func (as *AllocationSet) AggregateBy(aggregateBy []string, options *AllocationAg
 		for _, alloc := range aggSet.allocations {
 			for _, sharedAlloc := range shareSet.allocations {
 				if _, ok := shareCoefficients[alloc.Name]; !ok {
-					if !alloc.IsIdle() {
+					if !alloc.IsIdle() && !alloc.IsUnmounted() {
 						log.Warningf("AllocationSet.AggregateBy: error getting share coefficienct for '%s'", alloc.Name)
 					}
 					continue

pkg/kubecost/summaryallocation.go

@@ -90,13 +90,9 @@ func (sa *SummaryAllocation) Add(that *SummaryAllocation) error {
 		return errors.New("cannot Add a nil SummaryAllocation")
 	}
 
-	if sa.Properties == nil {
-		return errors.New("cannot Add a SummaryAllocation without Properties")
-	}
-
-	// Once Added, a SummaryAllocation has no Properties, preventing it from
-	// being Added a second time. This saves us from having to compute the
-	// intersection of two sets of Properties, which is expensive.
+	// Once Added, a SummaryAllocation has no Properties. This saves us from
+	// having to compute the intersection of two sets of Properties, which is
+	// expensive.
 	sa.Properties = nil
 
 	// Sum non-cumulative fields by turning them into cumulative, adding them,
@@ -148,6 +144,28 @@ func (sa *SummaryAllocation) Add(that *SummaryAllocation) error {
 	return nil
 }
 
+// Clone copies the SummaryAllocation and returns the copy
+func (sa *SummaryAllocation) Clone() *SummaryAllocation {
+	return &SummaryAllocation{
+		Name:                   sa.Name,
+		Properties:             sa.Properties.Clone(),
+		Start:                  sa.Start,
+		End:                    sa.End,
+		CPUCoreRequestAverage:  sa.CPUCoreRequestAverage,
+		CPUCoreUsageAverage:    sa.CPUCoreUsageAverage,
+		CPUCost:                sa.CPUCost,
+		GPUCost:                sa.GPUCost,
+		NetworkCost:            sa.NetworkCost,
+		LoadBalancerCost:       sa.LoadBalancerCost,
+		PVCost:                 sa.PVCost,
+		RAMBytesRequestAverage: sa.RAMBytesRequestAverage,
+		RAMBytesUsageAverage:   sa.RAMBytesUsageAverage,
+		RAMCost:                sa.RAMCost,
+		SharedCost:             sa.SharedCost,
+		ExternalCost:           sa.ExternalCost,
+	}
+}
+
 // CPUEfficiency is the ratio of usage to request. If there is no request and
 // no usage or cost, then efficiency is zero. If there is no request, but there
 // is usage or cost, then efficiency is 100%.

pkg/kubecost/summaryallocation_test.go

@@ -0,0 +1,212 @@
+package kubecost
+
+import (
+	"testing"
+	"time"
+
+	"github.com/kubecost/cost-model/pkg/util"
+)
+
+func TestSummaryAllocation_Add(t *testing.T) {
+	window, _ := ParseWindowUTC("yesterday")
+
+	var sa1, sa2, osa1, osa2, nilsa *SummaryAllocation
+	var err error
+
+	sa1Start := *window.Start()
+
+	sa1End := *window.End()
+
+	sa1 = &SummaryAllocation{
+		Name: "cluster1/namespace1/pod1/container1",
+		Properties: &AllocationProperties{
+			Cluster:   "cluster1",
+			Namespace: "namespace1",
+			Pod:       "pod1",
+			Container: "container1",
+		},
+		Start:                  sa1Start,
+		End:                    sa1End,
+		CPUCoreRequestAverage:  0.5,
+		CPUCoreUsageAverage:    0.1,
+		CPUCost:                0.2,
+		GPUCost:                1.0,
+		NetworkCost:            0.1,
+		LoadBalancerCost:       0.6,
+		PVCost:                 0.005,
+		RAMBytesRequestAverage: 50.0 * 1024.0 * 1024.0,
+		RAMBytesUsageAverage:   10.0 * 1024.0 * 1024.0,
+		RAMCost:                0.05,
+		SharedCost:             1.0,
+		ExternalCost:           1.0,
+	}
+	osa1 = sa1.Clone()
+
+	// sa2 is just as expensive, with twice as much usage and request, and half
+	// the time compared to sa1
+
+	sa2Start := *window.Start()
+	sa2Start = sa2Start.Add(6 * time.Hour)
+
+	sa2End := *window.End()
+	sa2End = sa2End.Add(-6 * time.Hour)
+
+	sa2 = &SummaryAllocation{
+		Name: "cluster1/namespace1/pod2/container2",
+		Properties: &AllocationProperties{
+			Cluster:   "cluster1",
+			Namespace: "namespace1",
+			Pod:       "pod2",
+			Container: "container2",
+		},
+		Start:                  sa2Start,
+		End:                    sa2End,
+		CPUCoreRequestAverage:  sa1.CPUCoreRequestAverage * 2.0,
+		CPUCoreUsageAverage:    sa1.CPUCoreUsageAverage * 2.0,
+		CPUCost:                sa1.CPUCost,
+		GPUCost:                sa1.GPUCost,
+		NetworkCost:            sa1.NetworkCost,
+		LoadBalancerCost:       sa1.LoadBalancerCost,
+		PVCost:                 sa1.PVCost,
+		RAMBytesRequestAverage: sa1.RAMBytesRequestAverage * 2.0,
+		RAMBytesUsageAverage:   sa1.RAMBytesUsageAverage * 2.0,
+		RAMCost:                sa1.RAMCost,
+		SharedCost:             sa1.SharedCost,
+		ExternalCost:           sa1.ExternalCost,
+	}
+	osa2 = sa2.Clone()
+
+	// add nil to nil, expect and error
+	t.Run("nil.Add(nil)", func(t *testing.T) {
+		err = nilsa.Add(nilsa)
+		if err == nil {
+			t.Fatalf("expected error: cannot add nil SummaryAllocations")
+		}
+	})
+
+	// reset
+	sa1 = osa1.Clone()
+	sa2 = osa2.Clone()
+
+	// add sa1 to nil, expect and error
+	t.Run("nil.Add(sa1)", func(t *testing.T) {
+		err = nilsa.Add(sa1)
+		if err == nil {
+			t.Fatalf("expected error: cannot add nil SummaryAllocations")
+		}
+	})
+
+	// reset
+	sa1 = osa1.Clone()
+	sa2 = osa2.Clone()
+
+	// add nil to sa1, expect and error
+	t.Run("sa1.Add(nil)", func(t *testing.T) {
+		err = sa1.Add(nilsa)
+		if err == nil {
+			t.Fatalf("expected error: cannot add nil SummaryAllocations")
+		}
+	})
+
+	// reset
+	sa1 = osa1.Clone()
+	sa2 = osa2.Clone()
+
+	// add sa1 to sa2 and expect same averages, but double costs
+	t.Run("sa2.Add(sa1)", func(t *testing.T) {
+		err = sa2.Add(sa1)
+		if err != nil {
+			t.Fatalf("unexpected error: %s", err)
+		}
+		if sa2.Properties != nil {
+			t.Fatalf("expected properties to be nil; actual: %s", sa1.Properties)
+		}
+		if !util.IsApproximately(sa2.CPUCoreRequestAverage, (0.5*osa2.CPUCoreRequestAverage)+osa1.CPUCoreRequestAverage) {
+			t.Fatalf("incorrect CPUCoreRequestAverage: expected %.5f; actual %.5f", (0.5*osa2.CPUCoreRequestAverage)+osa1.CPUCoreRequestAverage, sa2.CPUCoreRequestAverage)
+		}
+		if !util.IsApproximately(sa2.CPUCoreUsageAverage, (0.5*osa2.CPUCoreUsageAverage)+osa1.CPUCoreUsageAverage) {
+			t.Fatalf("incorrect CPUCoreUsageAverage: expected %.5f; actual %.5f", (0.5*osa2.CPUCoreUsageAverage)+osa1.CPUCoreRequestAverage, sa2.CPUCoreUsageAverage)
+		}
+		if !util.IsApproximately(sa2.RAMBytesRequestAverage, (0.5*osa2.RAMBytesRequestAverage)+osa1.RAMBytesRequestAverage) {
+			t.Fatalf("incorrect RAMBytesRequestAverage: expected %.5f; actual %.5f", (0.5*osa2.RAMBytesRequestAverage)+osa1.RAMBytesRequestAverage, sa2.RAMBytesRequestAverage)
+		}
+		if !util.IsApproximately(sa2.RAMBytesUsageAverage, (0.5*osa2.RAMBytesUsageAverage)+osa1.RAMBytesUsageAverage) {
+			t.Fatalf("incorrect RAMBytesUsageAverage: expected %.5f; actual %.5f", (0.5*osa2.RAMBytesUsageAverage)+osa1.RAMBytesRequestAverage, sa2.RAMBytesUsageAverage)
+		}
+		if !util.IsApproximately(sa2.CPUCost, osa2.CPUCost+osa1.CPUCost) {
+			t.Fatalf("incorrect CPUCost: expected %.5f; actual %.5f", osa2.CPUCost+osa1.CPUCost, sa2.CPUCost)
+		}
+		if !util.IsApproximately(sa2.GPUCost, osa2.GPUCost+osa1.GPUCost) {
+			t.Fatalf("incorrect GPUCost: expected %.5f; actual %.5f", osa2.GPUCost+osa1.GPUCost, sa2.GPUCost)
+		}
+		if !util.IsApproximately(sa2.NetworkCost, osa2.NetworkCost+osa1.NetworkCost) {
+			t.Fatalf("incorrect NetworkCost: expected %.5f; actual %.5f", osa2.NetworkCost+osa1.NetworkCost, sa2.NetworkCost)
+		}
+		if !util.IsApproximately(sa2.LoadBalancerCost, osa2.LoadBalancerCost+osa1.LoadBalancerCost) {
+			t.Fatalf("incorrect LoadBalancerCost: expected %.5f; actual %.5f", osa2.LoadBalancerCost+osa1.LoadBalancerCost, sa2.LoadBalancerCost)
+		}
+		if !util.IsApproximately(sa2.PVCost, osa2.PVCost+osa1.PVCost) {
+			t.Fatalf("incorrect PVCost: expected %.5f; actual %.5f", osa2.PVCost+osa1.PVCost, sa2.PVCost)
+		}
+		if !util.IsApproximately(sa2.RAMCost, osa2.RAMCost+osa1.RAMCost) {
+			t.Fatalf("incorrect RAMCost: expected %.5f; actual %.5f", osa2.RAMCost+osa1.RAMCost, sa2.RAMCost)
+		}
+		if !util.IsApproximately(sa2.SharedCost, osa2.SharedCost+osa1.SharedCost) {
+			t.Fatalf("incorrect SharedCost: expected %.5f; actual %.5f", osa2.SharedCost+osa1.SharedCost, sa2.SharedCost)
+		}
+		if !util.IsApproximately(sa2.ExternalCost, osa2.ExternalCost+osa1.ExternalCost) {
+			t.Fatalf("incorrect ExternalCost: expected %.5f; actual %.5f", osa2.ExternalCost+osa1.ExternalCost, sa2.ExternalCost)
+		}
+	})
+
+	// reset
+	sa1 = osa1.Clone()
+	sa2 = osa2.Clone()
+
+	// add sa2 to sa1 and expect same averages, but double costs
+	t.Run("sa1.Add(sa2)", func(t *testing.T) {
+		err = sa1.Add(sa2)
+		if err != nil {
+			t.Fatalf("unexpected error: %s", err)
+		}
+		if sa1.Properties != nil {
+			t.Fatalf("expected properties to be nil; actual: %s", sa1.Properties)
+		}
+		if !util.IsApproximately(sa1.CPUCoreRequestAverage, (0.5*osa2.CPUCoreRequestAverage)+osa1.CPUCoreRequestAverage) {
+			t.Fatalf("incorrect CPUCoreRequestAverage: expected %.5f; actual %.5f", (0.5*osa2.CPUCoreRequestAverage)+osa1.CPUCoreRequestAverage, sa2.CPUCoreRequestAverage)
+		}
+		if !util.IsApproximately(sa1.CPUCoreUsageAverage, (0.5*osa2.CPUCoreUsageAverage)+osa1.CPUCoreUsageAverage) {
+			t.Fatalf("incorrect CPUCoreUsageAverage: expected %.5f; actual %.5f", (0.5*osa2.CPUCoreUsageAverage)+osa1.CPUCoreRequestAverage, sa2.CPUCoreUsageAverage)
+		}
+		if !util.IsApproximately(sa1.RAMBytesRequestAverage, (0.5*osa2.RAMBytesRequestAverage)+osa1.RAMBytesRequestAverage) {
+			t.Fatalf("incorrect RAMBytesRequestAverage: expected %.5f; actual %.5f", (0.5*osa2.RAMBytesRequestAverage)+osa1.RAMBytesRequestAverage, sa2.RAMBytesRequestAverage)
+		}
+		if !util.IsApproximately(sa1.RAMBytesUsageAverage, (0.5*osa2.RAMBytesUsageAverage)+osa1.RAMBytesUsageAverage) {
+			t.Fatalf("incorrect RAMBytesUsageAverage: expected %.5f; actual %.5f", (0.5*osa2.RAMBytesUsageAverage)+osa1.RAMBytesRequestAverage, sa2.RAMBytesUsageAverage)
+		}
+		if !util.IsApproximately(sa1.CPUCost, osa2.CPUCost+osa1.CPUCost) {
+			t.Fatalf("incorrect CPUCost: expected %.5f; actual %.5f", osa2.CPUCost+osa1.CPUCost, sa2.CPUCost)
+		}
+		if !util.IsApproximately(sa1.GPUCost, osa2.GPUCost+osa1.GPUCost) {
+			t.Fatalf("incorrect GPUCost: expected %.5f; actual %.5f", osa2.GPUCost+osa1.GPUCost, sa2.GPUCost)
+		}
+		if !util.IsApproximately(sa1.NetworkCost, osa2.NetworkCost+osa1.NetworkCost) {
+			t.Fatalf("incorrect NetworkCost: expected %.5f; actual %.5f", osa2.NetworkCost+osa1.NetworkCost, sa2.NetworkCost)
+		}
+		if !util.IsApproximately(sa1.LoadBalancerCost, osa2.LoadBalancerCost+osa1.LoadBalancerCost) {
+			t.Fatalf("incorrect LoadBalancerCost: expected %.5f; actual %.5f", osa2.LoadBalancerCost+osa1.LoadBalancerCost, sa2.LoadBalancerCost)
+		}
+		if !util.IsApproximately(sa1.PVCost, osa2.PVCost+osa1.PVCost) {
+			t.Fatalf("incorrect PVCost: expected %.5f; actual %.5f", osa2.PVCost+osa1.PVCost, sa2.PVCost)
+		}
+		if !util.IsApproximately(sa1.RAMCost, osa2.RAMCost+osa1.RAMCost) {
+			t.Fatalf("incorrect RAMCost: expected %.5f; actual %.5f", osa2.RAMCost+osa1.RAMCost, sa2.RAMCost)
+		}
+		if !util.IsApproximately(sa1.SharedCost, osa2.SharedCost+osa1.SharedCost) {
+			t.Fatalf("incorrect SharedCost: expected %.5f; actual %.5f", osa2.SharedCost+osa1.SharedCost, sa2.SharedCost)
+		}
+		if !util.IsApproximately(sa1.ExternalCost, osa2.ExternalCost+osa1.ExternalCost) {
+			t.Fatalf("incorrect ExternalCost: expected %.5f; actual %.5f", osa2.ExternalCost+osa1.ExternalCost, sa2.ExternalCost)
+		}
+	})
+}

pkg/kubecost/totals.go

@@ -18,8 +18,8 @@ import (
 // on-the-fly would be expensive; e.g. idle allocation; sharing coefficients
 // for idle or shared resources, etc.
 type AllocationTotals struct {
-	Start                          time.Time `json:"end"`
-	End                            time.Time `json:"start"`
+	Start                          time.Time `json:"start"`
+	End                            time.Time `json:"end"`
 	Cluster                        string    `json:"cluster"`
 	Node                           string    `json:"node"`
 	Count                          int       `json:"count"`
@@ -125,8 +125,8 @@ func ComputeAllocationTotals(as *AllocationSet, prop string) map[string]*Allocat
 // knowledge is required to carry out a task, but computing totals on-the-fly
 // would be expensive; e.g. idle allocation, shared tenancy costs
 type AssetTotals struct {
-	Start                 time.Time `json:"end"`
-	End                   time.Time `json:"start"`
+	Start                 time.Time `json:"start"`
+	End                   time.Time `json:"end"`
 	Cluster               string    `json:"cluster"`
 	Node                  string    `json:"node"`
 	Count                 int       `json:"count"`
@@ -254,7 +254,6 @@ func ComputeAssetTotals(as *AssetSet, prop AssetProperty) map[string]*AssetTotal
 			arts[key].GPUCost += gpuCost
 			arts[key].GPUCostAdjustment += gpuCostAdjustment
 		} else if disk, ok := asset.(*Disk); ok {
-
 			key := fmt.Sprintf("%s/%s", disk.Properties().Cluster, disk.Properties().Name)
 			disks[key] = disk
 		} else if cm, ok := asset.(*ClusterManagement); ok && prop == AssetClusterProp {
@@ -282,7 +281,7 @@ func ComputeAssetTotals(as *AssetSet, prop AssetProperty) map[string]*AssetTotal
 			// cluster/node. But if we're aggregating by cluster only, then
 			// reset the key to just the cluster.
 			key := name
-			if prop != AssetClusterProp {
+			if prop == AssetClusterProp {
 				key = disk.Properties().Cluster
 			}