Convert Allocation.TotalCost to a function and refactor; fix Marshal/UnmarshalJSON and leave TODOs for Window JSON

pkg/cloud/provider.go

@@ -265,7 +265,7 @@ func CustomPricesEnabled(p Provider) bool {
 	if err != nil {
 		return false
 	}
-	// [TODO:CLEANUP] what is going on with this?
+	// TODO:CLEANUP what is going on with this?
 	if config.NegotiatedDiscount == "" {
 		config.NegotiatedDiscount = "0%"
 	}

pkg/costmodel/allocation.go

@@ -365,15 +365,6 @@ func (cm *CostModel) ComputeAllocation(start, end time.Time, resolution time.Dur
 				}
 			}
 
-			alloc.TotalCost = 0.0
-			alloc.TotalCost += alloc.CPUCost
-			alloc.TotalCost += alloc.RAMCost
-			alloc.TotalCost += alloc.GPUCost
-			alloc.TotalCost += alloc.PVCost
-			alloc.TotalCost += alloc.NetworkCost
-			alloc.TotalCost += alloc.SharedCost
-			alloc.TotalCost += alloc.ExternalCost
-
 			// Make sure that the name is correct (node may not be present at this
 			// point due to it missing from queryMinutes) then insert.
 			alloc.Name = fmt.Sprintf("%s/%s/%s/%s/%s", cluster, nodeName, namespace, pod, container)
@@ -1533,7 +1524,6 @@ func applyUnmountedPVs(window kubecost.Window, podMap map[podKey]*Pod, pvMap map
 		podMap[key].Allocations[container].Properties.SetContainer(container)
 		podMap[key].Allocations[container].PVByteHours = unmountedPVBytes[cluster] * window.Minutes() / 60.0
 		podMap[key].Allocations[container].PVCost = amount
-		podMap[key].Allocations[container].TotalCost = amount
 	}
 }
 
@@ -1577,7 +1567,6 @@ func applyUnmountedPVCs(window kubecost.Window, podMap map[podKey]*Pod, pvcMap m
 		podMap[podKey].Allocations[container].Properties.SetContainer(container)
 		podMap[podKey].Allocations[container].PVByteHours = unmountedPVCBytes[key] * window.Minutes() / 60.0
 		podMap[podKey].Allocations[container].PVCost = amount
-		podMap[podKey].Allocations[container].TotalCost = amount
 	}
 }
 
@@ -1609,7 +1598,7 @@ func (cm *CostModel) getNodePricing(nodeMap map[nodeKey]*NodePricing, nodeKey no
 
 	// If any of the values are NaN or zero, replace them with the custom
 	// values as default.
-	// [TODO:CLEANUP] can't we parse these custom prices once? why do we store
+	// TODO:CLEANUP can't we parse these custom prices once? why do we store
 	// them as strings like this?
 
 	if node.CostPerCPUHr == 0 || math.IsNaN(node.CostPerCPUHr) {
@@ -1625,7 +1614,7 @@ func (cm *CostModel) getNodePricing(nodeMap map[nodeKey]*NodePricing, nodeKey no
 		node.CostPerCPUHr = costPerCPUHr
 	}
 
-	if node.CostPerGPUHr == 0 || math.IsNaN(node.CostPerGPUHr) {
+	if math.IsNaN(node.CostPerGPUHr) {
 		log.Warningf("CostModel: node pricing has illegal CostPerGPUHr; replacing with custom pricing: %s", nodeKey)
 		gpuCostStr := customPricingConfig.GPU
 		if node.Preemptible {
@@ -1734,8 +1723,8 @@ func (p Pod) AppendContainer(container string) {
 }
 
 // PVC describes a PersistentVolumeClaim
-// TODO move to pkg/kubecost?  [TODO:CLEANUP]
-// TODO add PersistentVolumeClaims field to type Allocation?  [TODO:CLEANUP]
+// TODO:CLEANUP move to pkg/kubecost?
+// TODO:CLEANUP add PersistentVolumeClaims field to type Allocation?
 type PVC struct {
 	Bytes     float64   `json:"bytes"`
 	Count     int       `json:"count"`
@@ -1778,7 +1767,7 @@ func (pvc *PVC) String() string {
 }
 
 // PV describes a PersistentVolume
-// TODO move to pkg/kubecost? [TODO:CLEANUP]
+// TODO move to pkg/kubecost? TODO:CLEANUP
 type PV struct {
 	Bytes          float64 `json:"bytes"`
 	CostPerGiBHour float64 `json:"costPerGiBHour"` // TODO niko/computeallocation GiB or GB?

pkg/costmodel/router.go

@@ -244,7 +244,7 @@ func ParsePercentString(percentStr string) (float64, error) {
 }
 
 // parseDuration converts a Prometheus-style duration string into a Duration
-// [TODO:CLEANUP] delete this. do it now.
+// TODO:CLEANUP delete this. do it now.
 func ParseDuration(duration string) (*time.Duration, error) {
 	unitStr := duration[len(duration)-1:]
 	var unit time.Duration

pkg/kubecost/allocation.go

@@ -10,6 +10,7 @@ import (
 	"time"
 
 	"github.com/kubecost/cost-model/pkg/log"
+	"github.com/kubecost/cost-model/pkg/util"
 )
 
 // TODO Clean-up use of IsEmpty; nil checks should be separated for safety.
@@ -46,7 +47,7 @@ const ShareNone = "__none__"
 // Allocation is a unit of resource allocation and cost for a given window
 // of time and for a given kubernetes construct with its associated set of
 // properties.
-// TODO niko/computeallocation compute efficiency on the fly?
+// TODO:CLEANUP make TotalCost a function
 type Allocation struct {
 	Name                   string     `json:"name"`
 	Properties             Properties `json:"properties,omitempty"`
@@ -63,12 +64,12 @@ type Allocation struct {
 	PVByteHours            float64    `json:"pvByteHours"`
 	PVCost                 float64    `json:"pvCost"`
 	RAMByteHours           float64    `json:"ramByteHours"`
-	RAMBytesRequestAverage float64    `json:"ramBytesRequestAverage"`
-	RAMBytesUsageAverage   float64    `json:"ramBytesUsageAverage"`
+	RAMBytesRequestAverage float64    `json:"ramByteRequestAverage"`
+	RAMBytesUsageAverage   float64    `json:"ramByteUsageAverage"`
 	RAMCost                float64    `json:"ramCost"`
 	SharedCost             float64    `json:"sharedCost"`
 	ExternalCost           float64    `json:"externalCost"`
-	TotalCost              float64    `json:"totalCost"`
+	// TotalCost              float64    `json:"totalCost"`
 }
 
 // AllocationMatchFunc is a function that can be used to match Allocations by
@@ -121,7 +122,6 @@ func (a *Allocation) Clone() *Allocation {
 		RAMCost:                a.RAMCost,
 		SharedCost:             a.SharedCost,
 		ExternalCost:           a.ExternalCost,
-		TotalCost:              a.TotalCost,
 	}
 }
 
@@ -147,46 +147,48 @@ func (a *Allocation) Equal(that *Allocation) bool {
 	if !a.End.Equal(that.End) {
 		return false
 	}
-	if a.CPUCoreHours != that.CPUCoreHours {
+	if !util.IsApproximately(a.CPUCoreHours, that.CPUCoreHours) {
 		return false
 	}
-	if a.CPUCost != that.CPUCost {
+	if !util.IsApproximately(a.CPUCost, that.CPUCost) {
 		return false
 	}
-	if a.GPUHours != that.GPUHours {
+	if !util.IsApproximately(a.GPUHours, that.GPUHours) {
 		return false
 	}
-	if a.GPUCost != that.GPUCost {
+	if !util.IsApproximately(a.GPUCost, that.GPUCost) {
 		return false
 	}
-	if a.NetworkCost != that.NetworkCost {
+	if !util.IsApproximately(a.NetworkCost, that.NetworkCost) {
 		return false
 	}
-	if a.PVByteHours != that.PVByteHours {
+	if !util.IsApproximately(a.PVByteHours, that.PVByteHours) {
 		return false
 	}
-	if a.PVCost != that.PVCost {
+	if !util.IsApproximately(a.PVCost, that.PVCost) {
 		return false
 	}
-	if a.RAMByteHours != that.RAMByteHours {
+	if !util.IsApproximately(a.RAMByteHours, that.RAMByteHours) {
 		return false
 	}
-	if a.RAMCost != that.RAMCost {
+	if !util.IsApproximately(a.RAMCost, that.RAMCost) {
 		return false
 	}
-	if a.SharedCost != that.SharedCost {
+	if !util.IsApproximately(a.SharedCost, that.SharedCost) {
 		return false
 	}
-	if a.ExternalCost != that.ExternalCost {
-		return false
-	}
-	if a.TotalCost != that.TotalCost {
+	if !util.IsApproximately(a.ExternalCost, that.ExternalCost) {
 		return false
 	}
 
 	return true
 }
 
+// TotalCost is the total cost of the Allocation
+func (a *Allocation) TotalCost() float64 {
+	return a.CPUCost + a.GPUCost + a.RAMCost + a.PVCost + a.NetworkCost + a.SharedCost + a.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%.
@@ -259,8 +261,8 @@ func (a *Allocation) MarshalJSON() ([]byte, error) {
 	jsonEncodeString(buffer, "name", a.Name, ",")
 	jsonEncode(buffer, "properties", a.Properties, ",")
 	jsonEncode(buffer, "window", a.Window, ",")
-	jsonEncodeString(buffer, "start", a.Start.Format(timeFmt), ",")
-	jsonEncodeString(buffer, "end", a.End.Format(timeFmt), ",")
+	jsonEncodeString(buffer, "start", a.Start.Format(time.RFC3339), ",")
+	jsonEncodeString(buffer, "end", a.End.Format(time.RFC3339), ",")
 	jsonEncodeFloat64(buffer, "minutes", a.Minutes(), ",")
 	jsonEncodeFloat64(buffer, "cpuCores", a.CPUCores(), ",")
 	jsonEncodeFloat64(buffer, "cpuCoreRequestAverage", a.CPUCoreRequestAverage, ",")
@@ -281,7 +283,8 @@ func (a *Allocation) MarshalJSON() ([]byte, error) {
 	jsonEncodeFloat64(buffer, "ramCost", a.RAMCost, ",")
 	jsonEncodeFloat64(buffer, "ramEfficiency", a.RAMEfficiency(), ",")
 	jsonEncodeFloat64(buffer, "sharedCost", a.SharedCost, ",")
-	jsonEncodeFloat64(buffer, "totalCost", a.TotalCost, ",")
+	jsonEncodeFloat64(buffer, "externalCost", a.ExternalCost, ",")
+	jsonEncodeFloat64(buffer, "totalCost", a.TotalCost(), ",")
 	jsonEncodeFloat64(buffer, "totalEfficiency", a.TotalEfficiency(), "")
 	buffer.WriteString("}")
 	return buffer.Bytes(), nil
@@ -333,7 +336,7 @@ func (a *Allocation) Share(that *Allocation) (*Allocation, error) {
 	// Convert all costs of shared Allocation to SharedCost, zero out all
 	// non-shared costs, then add.
 	share := that.Clone()
-	share.SharedCost += share.TotalCost
+	share.SharedCost += share.TotalCost()
 	share.CPUCost = 0
 	share.CPUCoreHours = 0
 	share.RAMCost = 0
@@ -357,7 +360,7 @@ func (a *Allocation) Share(that *Allocation) (*Allocation, error) {
 
 // String represents the given Allocation as a string
 func (a *Allocation) String() string {
-	return fmt.Sprintf("%s%s=%.2f", a.Name, NewWindow(&a.Start, &a.End), a.TotalCost)
+	return fmt.Sprintf("%s%s=%.2f", a.Name, NewWindow(&a.Start, &a.End), a.TotalCost())
 }
 
 func (a *Allocation) add(that *Allocation) {
@@ -434,7 +437,6 @@ func (a *Allocation) add(that *Allocation) {
 	a.NetworkCost += that.NetworkCost
 	a.SharedCost += that.SharedCost
 	a.ExternalCost += that.ExternalCost
-	a.TotalCost += that.TotalCost
 }
 
 // AllocationSet stores a set of Allocations, each with a unique name, that share
@@ -559,7 +561,6 @@ func (as *AllocationSet) AggregateBy(properties Properties, options *AllocationA
 				Start:      as.Start(),
 				End:        as.End(),
 				SharedCost: totalSharedCost,
-				TotalCost:  totalSharedCost,
 			})
 		}
 	}
@@ -734,7 +735,6 @@ func (as *AllocationSet) AggregateBy(properties Properties, options *AllocationA
 				alloc.CPUCost += idleCPUCost
 				alloc.GPUCost += idleGPUCost
 				alloc.RAMCost += idleRAMCost
-				alloc.TotalCost += idleCPUCost + idleGPUCost + idleRAMCost
 			}
 		}
 
@@ -797,7 +797,6 @@ func (as *AllocationSet) AggregateBy(properties Properties, options *AllocationA
 				idleAlloc.CPUCoreHours *= resourceCoeffs["cpu"]
 				idleAlloc.RAMCost *= resourceCoeffs["ram"]
 				idleAlloc.RAMByteHours *= resourceCoeffs["ram"]
-				idleAlloc.TotalCost = idleAlloc.CPUCost + idleAlloc.RAMCost
 			}
 
 		}
@@ -826,8 +825,7 @@ func (as *AllocationSet) AggregateBy(properties Properties, options *AllocationA
 					continue
 				}
 
-				alloc.SharedCost += sharedAlloc.TotalCost * shareCoefficients[alloc.Name]
-				alloc.TotalCost += sharedAlloc.TotalCost * shareCoefficients[alloc.Name]
+				alloc.SharedCost += sharedAlloc.TotalCost() * shareCoefficients[alloc.Name]
 			}
 		}
 	}
@@ -895,8 +893,8 @@ func computeShareCoeffs(properties Properties, options *AllocationAggregationOpt
 			total += 1.0
 		} else {
 			// Both are additive for weighted distribution
-			coeffs[name] += alloc.TotalCost
-			total += alloc.TotalCost
+			coeffs[name] += alloc.TotalCost()
+			total += alloc.TotalCost()
 		}
 	}
 
@@ -1001,13 +999,13 @@ func computeIdleCoeffs(properties Properties, options *AllocationAggregationOpti
 	return coeffs, nil
 }
 
-func (alloc *Allocation) generateKey(properties Properties) (string, error) {
+func (a *Allocation) generateKey(properties Properties) (string, error) {
 	// Names will ultimately be joined into a single name, which uniquely
 	// identifies allocations.
 	names := []string{}
 
 	if properties.HasCluster() {
-		cluster, err := alloc.Properties.GetCluster()
+		cluster, err := a.Properties.GetCluster()
 		if err != nil {
 			return "", err
 		}
@@ -1015,7 +1013,7 @@ func (alloc *Allocation) generateKey(properties Properties) (string, error) {
 	}
 
 	if properties.HasNode() {
-		node, err := alloc.Properties.GetNode()
+		node, err := a.Properties.GetNode()
 		if err != nil {
 			return "", err
 		}
@@ -1023,7 +1021,7 @@ func (alloc *Allocation) generateKey(properties Properties) (string, error) {
 	}
 
 	if properties.HasNamespace() {
-		namespace, err := alloc.Properties.GetNamespace()
+		namespace, err := a.Properties.GetNamespace()
 		if err != nil {
 			return "", err
 		}
@@ -1031,7 +1029,7 @@ func (alloc *Allocation) generateKey(properties Properties) (string, error) {
 	}
 
 	if properties.HasControllerKind() {
-		controllerKind, err := alloc.Properties.GetControllerKind()
+		controllerKind, err := a.Properties.GetControllerKind()
 		if err != nil {
 			// Indicate that allocation has no controller
 			controllerKind = UnallocatedSuffix
@@ -1046,13 +1044,13 @@ func (alloc *Allocation) generateKey(properties Properties) (string, error) {
 
 	if properties.HasController() {
 		if !properties.HasControllerKind() {
-			controllerKind, err := alloc.Properties.GetControllerKind()
+			controllerKind, err := a.Properties.GetControllerKind()
 			if err == nil {
 				names = append(names, controllerKind)
 			}
 		}
 
-		controller, err := alloc.Properties.GetController()
+		controller, err := a.Properties.GetController()
 		if err != nil {
 			// Indicate that allocation has no controller
 			controller = UnallocatedSuffix
@@ -1062,7 +1060,7 @@ func (alloc *Allocation) generateKey(properties Properties) (string, error) {
 	}
 
 	if properties.HasPod() {
-		pod, err := alloc.Properties.GetPod()
+		pod, err := a.Properties.GetPod()
 		if err != nil {
 			return "", err
 		}
@@ -1071,7 +1069,7 @@ func (alloc *Allocation) generateKey(properties Properties) (string, error) {
 	}
 
 	if properties.HasContainer() {
-		container, err := alloc.Properties.GetContainer()
+		container, err := a.Properties.GetContainer()
 		if err != nil {
 			return "", err
 		}
@@ -1080,7 +1078,7 @@ func (alloc *Allocation) generateKey(properties Properties) (string, error) {
 	}
 
 	if properties.HasService() {
-		services, err := alloc.Properties.GetServices()
+		services, err := a.Properties.GetServices()
 		if err != nil {
 			// Indicate that allocation has no services
 			names = append(names, UnallocatedSuffix)
@@ -1099,7 +1097,7 @@ func (alloc *Allocation) generateKey(properties Properties) (string, error) {
 	}
 
 	if properties.HasAnnotations() {
-		annotations, err := alloc.Properties.GetAnnotations() // annotations that the individual allocation possesses
+		annotations, err := a.Properties.GetAnnotations() // annotations that the individual allocation possesses
 		if err != nil {
 			// Indicate that allocation has no annotations
 			names = append(names, UnallocatedSuffix)
@@ -1135,7 +1133,7 @@ func (alloc *Allocation) generateKey(properties Properties) (string, error) {
 	}
 
 	if properties.HasLabel() {
-		labels, err := alloc.Properties.GetLabels() // labels that the individual allocation possesses
+		labels, err := a.Properties.GetLabels() // labels that the individual allocation possesses
 		if err != nil {
 			// Indicate that allocation has no labels
 			names = append(names, UnallocatedSuffix)
@@ -1335,7 +1333,6 @@ func (as *AllocationSet) ComputeIdleAllocations(assetSet *AssetSet) (map[string]
 			GPUCost:    resources["gpu"],
 			RAMCost:    resources["ram"],
 		}
-		idleAlloc.TotalCost = idleAlloc.CPUCost + idleAlloc.GPUCost + idleAlloc.RAMCost
 
 		// Do not continue if multiple idle allocations are computed for a
 		// single cluster.
@@ -1611,7 +1608,7 @@ func (as *AllocationSet) TotalCost() float64 {
 
 	tc := 0.0
 	for _, a := range as.allocations {
-		tc += a.TotalCost
+		tc += a.TotalCost()
 	}
 	return tc
 }

pkg/kubecost/allocation_test.go

@@ -1,6 +1,7 @@
 package kubecost
 
 import (
+	"encoding/json"
 	"fmt"
 	"math"
 	"testing"
@@ -50,7 +51,6 @@ func NewUnitAllocation(name string, start time.Time, resolution time.Duration, p
 		RAMCost:                1,
 		RAMBytesRequestAverage: 1,
 		RAMBytesUsageAverage:   1,
-		TotalCost:              5,
 	}
 
 	// If idle allocation, remove non-idle costs, but maintain total cost
@@ -86,21 +86,75 @@ func TestAllocation_Add(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Allocation.Add unexpected error: %s", err)
 	}
-	if nilZeroSum == nil || nilZeroSum.TotalCost != 0.0 {
+	if nilZeroSum == nil || nilZeroSum.TotalCost() != 0.0 {
 		t.Fatalf("Allocation.Add failed; exp: 0.0; act: %s", nilZeroSum)
 	}
 
 	// TODO niko/etl more
 }
 
-// TODO niko/etl
-// func TestAllocation_Clone(t *testing.T) {}
+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)
+	hrs := 24.0
+
+	gib := 1024.0 * 1024.0 * 1024.0
+
+	cpuPrice := 0.02
+	gpuPrice := 2.00
+	ramPrice := 0.01
+	pvPrice := 0.00005
+
+	before := &Allocation{
+		Name: "cluster1/namespace1/node1/pod1/container1",
+		Properties: Properties{
+			ClusterProp:   "cluster1",
+			NodeProp:      "node1",
+			NamespaceProp: "namespace1",
+			PodProp:       "pod1",
+			ContainerProp: "container1",
+		},
+		Window:                 NewWindow(&start, &end),
+		Start:                  start,
+		End:                    end,
+		CPUCoreHours:           2.0 * hrs,
+		CPUCoreRequestAverage:  2.0,
+		CPUCoreUsageAverage:    1.0,
+		CPUCost:                2.0 * hrs * cpuPrice,
+		GPUHours:               1.0 * hrs,
+		GPUCost:                1.0 * hrs * gpuPrice,
+		NetworkCost:            0.05,
+		PVByteHours:            100.0 * gib * hrs,
+		PVCost:                 100.0 * hrs * pvPrice,
+		RAMByteHours:           8.0 * gib * hrs,
+		RAMBytesRequestAverage: 8.0 * gib,
+		RAMBytesUsageAverage:   4.0 * gib,
+		RAMCost:                8.0 * hrs * ramPrice,
+		SharedCost:             2.00,
+		ExternalCost:           1.00,
+	}
+
+	data, err := json.Marshal(before)
+	if err != nil {
+		t.Fatalf("Allocation.MarshalJSON: unexpected error: %s", err)
+	}
 
-// TODO niko/etl
-// func TestAllocation_IsIdle(t *testing.T) {}
+	after := &Allocation{}
+	err = json.Unmarshal(data, after)
+	if err != nil {
+		t.Fatalf("Allocation.UnmarshalJSON: unexpected error: %s", err)
+	}
 
-func TestAllocation_String(t *testing.T) {
-	// TODO niko/etl
+	// TODO:CLEANUP fix json marshaling of Window so that all of this works.
+	// In the meantime, just set the Window so that we can test the rest.
+	after.Window = before.Window.Clone()
+
+	fmt.Println(*before)
+	fmt.Println(*after)
+
+	if !after.Equal(before) {
+		t.Fatalf("Allocation.MarshalJSON: before and after are not equal")
+	}
 }
 
 func TestNewAllocationSet(t *testing.T) {
@@ -116,7 +170,6 @@ func generateAllocationSet(start time.Time) *AllocationSet {
 	a1i.CPUCost = 5.0
 	a1i.RAMCost = 15.0
 	a1i.GPUCost = 0.0
-	a1i.TotalCost = 20.0
 
 	a2i := NewUnitAllocation(fmt.Sprintf("cluster2/%s", IdleSuffix), start, day, &Properties{
 		ClusterProp: "cluster2",
@@ -124,7 +177,6 @@ func generateAllocationSet(start time.Time) *AllocationSet {
 	a2i.CPUCost = 5.0
 	a2i.RAMCost = 5.0
 	a2i.GPUCost = 0.0
-	a2i.TotalCost = 10.0
 
 	// Active allocations
 	a1111 := NewUnitAllocation("cluster1/namespace1/pod1/container1", start, day, &Properties{
@@ -134,7 +186,6 @@ func generateAllocationSet(start time.Time) *AllocationSet {
 		ContainerProp: "container1",
 	})
 	a1111.RAMCost = 11.00
-	a1111.TotalCost = 15.00
 
 	a11abc2 := NewUnitAllocation("cluster1/namespace1/pod-abc/container2", start, day, &Properties{
 		ClusterProp:   "cluster1",
@@ -289,8 +340,8 @@ func assertAllocationSetTotals(t *testing.T, as *AllocationSet, msg string, err
 func assertAllocationTotals(t *testing.T, as *AllocationSet, msg string, exps map[string]float64) {
 	as.Each(func(k string, a *Allocation) {
 		if exp, ok := exps[a.Name]; ok {
-			if math.Round(a.TotalCost*100) != math.Round(exp*100) {
-				t.Fatalf("AllocationSet.AggregateBy[%s]: expected total cost %.2f, actual %.2f", msg, exp, a.TotalCost)
+			if math.Round(a.TotalCost()*100) != math.Round(exp*100) {
+				t.Fatalf("AllocationSet.AggregateBy[%s]: expected total cost %.2f, actual %.2f", msg, exp, a.TotalCost())
 			}
 		} else {
 			t.Fatalf("AllocationSet.AggregateBy[%s]: unexpected allocation: %s", msg, a.Name)
@@ -964,8 +1015,8 @@ func TestAllocationSet_ComputeIdleAllocations(t *testing.T) {
 	if idle, ok := idles["cluster1"]; !ok {
 		t.Fatalf("expected idle cost for %s", "cluster1")
 	} else {
-		if !util.IsApproximately(idle.TotalCost, 72.0) {
-			t.Fatalf("%s idle: expected total cost %f; got total cost %f", "cluster1", 72.0, idle.TotalCost)
+		if !util.IsApproximately(idle.TotalCost(), 72.0) {
+			t.Fatalf("%s idle: expected total cost %f; got total cost %f", "cluster1", 72.0, idle.TotalCost())
 		}
 	}
 	if !util.IsApproximately(idles["cluster1"].CPUCost, 44.0) {
@@ -981,8 +1032,8 @@ func TestAllocationSet_ComputeIdleAllocations(t *testing.T) {
 	if idle, ok := idles["cluster2"]; !ok {
 		t.Fatalf("expected idle cost for %s", "cluster2")
 	} else {
-		if !util.IsApproximately(idle.TotalCost, 82.0) {
-			t.Fatalf("%s idle: expected total cost %f; got total cost %f", "cluster2", 82.0, idle.TotalCost)
+		if !util.IsApproximately(idle.TotalCost(), 82.0) {
+			t.Fatalf("%s idle: expected total cost %f; got total cost %f", "cluster2", 82.0, idle.TotalCost())
 		}
 	}
 
@@ -1051,8 +1102,8 @@ func TestAllocationSet_ComputeIdleAllocations(t *testing.T) {
 	if idle, ok := idles["cluster1"]; !ok {
 		t.Fatalf("expected idle cost for %s", "cluster1")
 	} else {
-		if !util.IsApproximately(idle.TotalCost, 72.0) {
-			t.Fatalf("%s idle: expected total cost %f; got total cost %f", "cluster1", 72.0, idle.TotalCost)
+		if !util.IsApproximately(idle.TotalCost(), 72.0) {
+			t.Fatalf("%s idle: expected total cost %f; got total cost %f", "cluster1", 72.0, idle.TotalCost())
 		}
 	}
 	if !util.IsApproximately(idles["cluster1"].CPUCost, 44.0) {
@@ -1068,8 +1119,8 @@ func TestAllocationSet_ComputeIdleAllocations(t *testing.T) {
 	if idle, ok := idles["cluster2"]; !ok {
 		t.Fatalf("expected idle cost for %s", "cluster2")
 	} else {
-		if !util.IsApproximately(idle.TotalCost, 82.0) {
-			t.Fatalf("%s idle: expected total cost %f; got total cost %f", "cluster2", 82.0, idle.TotalCost)
+		if !util.IsApproximately(idle.TotalCost(), 82.0) {
+			t.Fatalf("%s idle: expected total cost %f; got total cost %f", "cluster2", 82.0, idle.TotalCost())
 		}
 	}
 
@@ -1249,8 +1300,8 @@ func TestAllocationSetRange_Accumulate(t *testing.T) {
 	if alloc.RAMEfficiency() != 1.0 {
 		t.Fatalf("accumulating AllocationSetRange: expected 1.0; actual %f", alloc.RAMEfficiency())
 	}
-	if alloc.TotalCost != 10.0 {
-		t.Fatalf("accumulating AllocationSetRange: expected 10.0; actual %f", alloc.TotalCost)
+	if alloc.TotalCost() != 10.0 {
+		t.Fatalf("accumulating AllocationSetRange: expected 10.0; actual %f", alloc.TotalCost())
 	}
 	if alloc.TotalEfficiency() != 1.0 {
 		t.Fatalf("accumulating AllocationSetRange: expected 1.0; actual %f", alloc.TotalEfficiency())
@@ -1351,8 +1402,8 @@ func TestAllocationSetRange_InsertRange(t *testing.T) {
 			if !util.IsApproximately(a.NetworkCost, unit.NetworkCost) {
 				t.Fatalf("allocation %s: expected %f; got %f", k, unit.NetworkCost, a.NetworkCost)
 			}
-			if !util.IsApproximately(a.TotalCost, unit.TotalCost) {
-				t.Fatalf("allocation %s: expected %f; got %f", k, unit.TotalCost, a.TotalCost)
+			if !util.IsApproximately(a.TotalCost(), unit.TotalCost()) {
+				t.Fatalf("allocation %s: expected %f; got %f", k, unit.TotalCost(), a.TotalCost())
 			}
 		})
 	})
@@ -1399,8 +1450,8 @@ func TestAllocationSetRange_InsertRange(t *testing.T) {
 		if !util.IsApproximately(a.NetworkCost, 2*unit.NetworkCost) {
 			t.Fatalf("allocation %s: expected %f; got %f", k, unit.NetworkCost, a.NetworkCost)
 		}
-		if !util.IsApproximately(a.TotalCost, 2*unit.TotalCost) {
-			t.Fatalf("allocation %s: expected %f; got %f", k, unit.TotalCost, a.TotalCost)
+		if !util.IsApproximately(a.TotalCost(), 2*unit.TotalCost()) {
+			t.Fatalf("allocation %s: expected %f; got %f", k, unit.TotalCost(), a.TotalCost())
 		}
 	})
 	tAS, err := thisASR.Get(1)
@@ -1432,8 +1483,8 @@ func TestAllocationSetRange_InsertRange(t *testing.T) {
 		if !util.IsApproximately(a.NetworkCost, unit.NetworkCost) {
 			t.Fatalf("allocation %s: expected %f; got %f", k, unit.NetworkCost, a.NetworkCost)
 		}
-		if !util.IsApproximately(a.TotalCost, unit.TotalCost) {
-			t.Fatalf("allocation %s: expected %f; got %f", k, unit.TotalCost, a.TotalCost)
+		if !util.IsApproximately(a.TotalCost(), unit.TotalCost()) {
+			t.Fatalf("allocation %s: expected %f; got %f", k, unit.TotalCost(), a.TotalCost())
 		}
 	})
 }

pkg/kubecost/asset.go

@@ -220,7 +220,6 @@ func AssetToExternalAllocation(asset Asset, aggregateBy []string) (*Allocation,
 		Start:        asset.Start(),
 		End:          asset.End(),
 		ExternalCost: asset.TotalCost(),
-		TotalCost:    asset.TotalCost(),
 	}, nil
 }
 

pkg/kubecost/asset_test.go

@@ -1094,8 +1094,8 @@ func TestAssetToExternalAllocation(t *testing.T) {
 	if alloc.ExternalCost != 10.00 {
 		t.Fatalf("expected external allocation with ExternalCost %f; got %f", 10.00, alloc.ExternalCost)
 	}
-	if alloc.TotalCost != 10.00 {
-		t.Fatalf("expected external allocation with TotalCost %f; got %f", 10.00, alloc.TotalCost)
+	if alloc.TotalCost() != 10.00 {
+		t.Fatalf("expected external allocation with TotalCost %f; got %f", 10.00, alloc.TotalCost())
 	}
 
 	// 2) multi-prop full match
@@ -1115,8 +1115,8 @@ func TestAssetToExternalAllocation(t *testing.T) {
 	if alloc.ExternalCost != 10.00 {
 		t.Fatalf("expected external allocation with ExternalCost %f; got %f", 10.00, alloc.ExternalCost)
 	}
-	if alloc.TotalCost != 10.00 {
-		t.Fatalf("expected external allocation with TotalCost %f; got %f", 10.00, alloc.TotalCost)
+	if alloc.TotalCost() != 10.00 {
+		t.Fatalf("expected external allocation with TotalCost %f; got %f", 10.00, alloc.TotalCost())
 	}
 
 	// 3) multi-prop partial match
@@ -1133,8 +1133,8 @@ func TestAssetToExternalAllocation(t *testing.T) {
 	if alloc.ExternalCost != 10.00 {
 		t.Fatalf("expected external allocation with ExternalCost %f; got %f", 10.00, alloc.ExternalCost)
 	}
-	if alloc.TotalCost != 10.00 {
-		t.Fatalf("expected external allocation with TotalCost %f; got %f", 10.00, alloc.TotalCost)
+	if alloc.TotalCost() != 10.00 {
+		t.Fatalf("expected external allocation with TotalCost %f; got %f", 10.00, alloc.TotalCost())
 	}
 
 	// 3) no match

pkg/kubecost/bingen.go

@@ -21,4 +21,4 @@ package kubecost
 // @bingen:generate:AllocationSet
 // @bingen:generate:AllocationSetRange
 
-//go:generate bingen -package=kubecost -version=7 -buffer=github.com/kubecost/cost-model/pkg/util
+//go:generate bingen -package=kubecost -version=8 -buffer=github.com/kubecost/cost-model/pkg/util

pkg/kubecost/kubecost_codecs.go

@@ -14,10 +14,11 @@ package kubecost
 import (
 	"encoding"
 	"fmt"
-	util "github.com/kubecost/cost-model/pkg/util"
 	"reflect"
 	"strings"
 	"time"
+
+	util "github.com/kubecost/cost-model/pkg/util"
 )
 
 const (
@@ -25,7 +26,7 @@ const (
 	GeneratorPackageName string = "kubecost"
 
 	// CodecVersion is the version passed into the generator
-	CodecVersion uint8 = 7
+	CodecVersion uint8 = 8
 )
 
 //--------------------------------------------------------------------------
@@ -167,7 +168,6 @@ func (target *Allocation) MarshalBinary() (data []byte, err error) {
 	buff.WriteFloat64(target.RAMCost)                // write float64
 	buff.WriteFloat64(target.SharedCost)             // write float64
 	buff.WriteFloat64(target.ExternalCost)           // write float64
-	buff.WriteFloat64(target.TotalCost)              // write float64
 	return buff.Bytes(), nil
 }
 
@@ -287,9 +287,6 @@ func (target *Allocation) UnmarshalBinary(data []byte) (err error) {
 	ff := buff.ReadFloat64() // read float64
 	target.ExternalCost = ff
 
-	gg := buff.ReadFloat64() // read float64
-	target.TotalCost = gg
-
 	return nil
 }
 

pkg/kubecost/properties.go

@@ -57,7 +57,7 @@ type PropertyValue struct {
 }
 
 // Properties describes a set of Kubernetes objects.
-// TODO make this a struct smdh [TODO:CLEANUP]
+// TODO:CLEANUP make this a struct smdh
 type Properties map[Property]interface{}
 
 // TODO niko/etl make sure Services deep copy works correctly

pkg/kubecost/window.go

@@ -430,10 +430,11 @@ func (w Window) IsOpen() bool {
 	return w.start == nil || w.end == nil
 }
 
+// TODO:CLEANUP make this unmarshalable (make Start and End public)
 func (w Window) MarshalJSON() ([]byte, error) {
 	buffer := bytes.NewBufferString("{")
-	buffer.WriteString(fmt.Sprintf("\"start\":\"%s\",", w.start.Format("2006-01-02T15:04:05-0700")))
-	buffer.WriteString(fmt.Sprintf("\"end\":\"%s\"", w.end.Format("2006-01-02T15:04:05-0700")))
+	buffer.WriteString(fmt.Sprintf("\"start\":\"%s\",", w.start.Format(time.RFC3339)))
+	buffer.WriteString(fmt.Sprintf("\"end\":\"%s\"", w.end.Format(time.RFC3339)))
 	buffer.WriteString("}")
 	return buffer.Bytes(), nil
 }
@@ -449,7 +450,7 @@ func (w Window) Minutes() float64 {
 // Overlaps returns true iff the two given Windows share and amount of temporal
 // coverage.
 // TODO complete (with unit tests!) and then implement in AllocationSet.accumulate
-// [TODO:CLEANUP]
+// TODO:CLEANUP
 func (w Window) Overlaps(x Window) bool {
 	if (w.start == nil && w.end == nil) || (x.start == nil && x.end == nil) {
 		// one window is completely open, so overlap is guaranteed