Merge pull request #684 from kubecost/niko/adjustedidle

Include adjustment in idle computation

pkg/kubecost/allocation.go

@@ -1108,17 +1108,15 @@ func (as *AllocationSet) Clone() *AllocationSet {
 // ComputeIdleAllocations computes the idle allocations for the AllocationSet,
 // given a set of Assets. Ideally, assetSet should contain only Nodes, but if
 // it contains other Assets, they will be ignored; only CPU, GPU and RAM are
-// considered for idle allocation. One idle allocation per-cluster will be
-// computed and returned, keyed by cluster_id.
+// considered for idle allocation. If the Nodes have adjustments, then apply
+// the adjustments proportionally to each of the resources so that total
+// allocation with idle reflects the adjusted node costs. One idle allocation
+// per-cluster will be computed and returned, keyed by cluster_id.
 func (as *AllocationSet) ComputeIdleAllocations(assetSet *AssetSet) (map[string]*Allocation, error) {
 	if as == nil {
 		return nil, fmt.Errorf("cannot compute idle allocation for nil AllocationSet")
 	}
 
-	// TODO: external allocation: remove after testing and benchmarking
-	profStart := time.Now()
-	defer log.Profile(profStart, fmt.Sprintf("ComputeIdleAllocations: %s", as.Window))
-
 	if assetSet == nil {
 		return nil, fmt.Errorf("cannot compute idle allocation with nil AssetSet")
 	}
@@ -1137,9 +1135,33 @@ func (as *AllocationSet) ComputeIdleAllocations(assetSet *AssetSet) (map[string]
 			if _, ok := assetClusterResourceCosts[node.Properties().Cluster]; !ok {
 				assetClusterResourceCosts[node.Properties().Cluster] = map[string]float64{}
 			}
-			assetClusterResourceCosts[node.Properties().Cluster]["cpu"] += node.CPUCost * (1.0 - node.Discount)
-			assetClusterResourceCosts[node.Properties().Cluster]["gpu"] += node.GPUCost * (1.0 - node.Discount)
-			assetClusterResourceCosts[node.Properties().Cluster]["ram"] += node.RAMCost * (1.0 - node.Discount)
+
+			// adjustmentRate is used to scale resource costs proportionally
+			// by the adjustment. This is necessary because we only get one
+			// adjustment per Node, not one per-resource-per-Node.
+			//
+			// e.g. total cost = $90, adjustment = -$10 => 0.9
+			// e.g. total cost = $150, adjustment = -$300 => 0.3333
+			// e.g. total cost = $150, adjustment = $50 => 1.5
+			adjustmentRate := 1.0
+			if node.TotalCost()-node.Adjustment() == 0 {
+				// If (totalCost - adjustment) is 0.0 then adjustment cancels
+				// the entire node cost and we should make everything 0
+				// without dividing by 0.
+				adjustmentRate = 0.0
+			} else if node.Adjustment() != 0.0 {
+				// adjustmentRate is the ratio of cost-with-adjustment (i.e. TotalCost)
+				// to cost-without-adjustment (i.e. TotalCost - Adjustment).
+				adjustmentRate = node.TotalCost() / (node.TotalCost() - node.Adjustment())
+			}
+
+			cpuCost := node.CPUCost * (1.0 - node.Discount) * adjustmentRate
+			gpuCost := node.GPUCost * (1.0 - node.Discount) * adjustmentRate
+			ramCost := node.RAMCost * (1.0 - node.Discount) * adjustmentRate
+
+			assetClusterResourceCosts[node.Properties().Cluster]["cpu"] += cpuCost
+			assetClusterResourceCosts[node.Properties().Cluster]["gpu"] += gpuCost
+			assetClusterResourceCosts[node.Properties().Cluster]["ram"] += ramCost
 		}
 	})
 

pkg/kubecost/allocation_test.go

@@ -901,34 +901,35 @@ func TestAllocationSet_ComputeIdleAllocations(t *testing.T) {
 	// NOTE: we're re-using generateAllocationSet so this has to line up with
 	// the allocated node costs from that function. See table above.
 
-	// | Hierarchy                               | Cost |  CPU |  RAM |  GPU |
-	// +-----------------------------------------+------+------+------+------+
+	// | Hierarchy                               | Cost |  CPU |  RAM |  GPU | Adjustment |
+	// +-----------------------------------------+------+------+------+------+------------+
 	//   cluster1:
-	//     nodes                                  100.00  50.00  40.00  10.00
-	// +-----------------------------------------+------+------+------+------+
-	//   cluster1 subtotal                        100.00  50.00  40.00  10.00
-	// +-----------------------------------------+------+------+------+------+
-	//   cluster1 allocated                        48.00   6.00  16.00   6.00
-	// +-----------------------------------------+------+------+------+------+
-	//   cluster1 idle                             72.00  44.00  24.00   4.00
-	// +-----------------------------------------+------+------+------+------+
+	//     nodes                                  100.00  55.00  44.00  11.00      -10.00
+	// +-----------------------------------------+------+------+------+------+------------+
+	//   cluster1 subtotal (adjusted)             100.00  50.00  40.00  10.00        0.00
+	// +-----------------------------------------+------+------+------+------+------------+
+	//   cluster1 allocated                        48.00   6.00  16.00   6.00        0.00
+	// +-----------------------------------------+------+------+------+------+------------+
+	//   cluster1 idle                             72.00  44.00  24.00   4.00        0.00
+	// +-----------------------------------------+------+------+------+------+------------+
 	//   cluster2:
-	//     node1                                   35.00  20.00  15.00   0.00
-	//     node2                                   35.00  20.00  15.00   0.00
-	//     node3                                   30.00  10.00  10.00  10.00
+	//     node1                                   35.00  20.00  15.00   0.00        0.00
+	//     node2                                   35.00  20.00  15.00   0.00        0.00
+	//     node3                                   30.00  10.00  10.00  10.00        0.00
 	//     (disks should not matter for idle)
-	// +-----------------------------------------+------+------+------+------+
-	//   cluster2 subtotal                        100.00  50.00  40.00  10.00
-	// +-----------------------------------------+------+------+------+------+
-	//   cluster2 allocated                        28.00   6.00   6.00   6.00
-	// +-----------------------------------------+------+------+------+------+
-	//   cluster2 idle                             82.00  44.00  34.00   4.00
-	// +-----------------------------------------+------+------+------+------+
+	// +-----------------------------------------+------+------+------+------+------------+
+	//   cluster2 subtotal                        100.00  50.00  40.00  10.00        0.00
+	// +-----------------------------------------+------+------+------+------+------------+
+	//   cluster2 allocated                        28.00   6.00   6.00   6.00        0.00
+	// +-----------------------------------------+------+------+------+------+------------+
+	//   cluster2 idle                             82.00  44.00  34.00   4.00        0.00
+	// +-----------------------------------------+------+------+------+------+------------+
 
 	cluster1Nodes := NewNode("", "cluster1", "", start, end, NewWindow(&start, &end))
-	cluster1Nodes.CPUCost = 50.0
-	cluster1Nodes.RAMCost = 40.0
-	cluster1Nodes.GPUCost = 10.0
+	cluster1Nodes.CPUCost = 55.0
+	cluster1Nodes.RAMCost = 44.0
+	cluster1Nodes.GPUCost = 11.0
+	cluster1Nodes.adjustment = -10.00
 
 	cluster2Node1 := NewNode("node1", "cluster2", "node1", start, end, NewWindow(&start, &end))
 	cluster2Node1.CPUCost = 20.0
@@ -966,6 +967,102 @@ func TestAllocationSet_ComputeIdleAllocations(t *testing.T) {
 			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) {
+		t.Fatalf("expected idle CPU cost for %s to be %.2f; got %.2f", "cluster1", 44.0, idles["cluster1"].CPUCost)
+	}
+	if !util.IsApproximately(idles["cluster1"].RAMCost, 24.0) {
+		t.Fatalf("expected idle RAM cost for %s to be %.2f; got %.2f", "cluster1", 24.0, idles["cluster1"].RAMCost)
+	}
+	if !util.IsApproximately(idles["cluster1"].GPUCost, 4.0) {
+		t.Fatalf("expected idle GPU cost for %s to be %.2f; got %.2f", "cluster1", 4.0, idles["cluster1"].GPUCost)
+	}
+
+	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)
+		}
+	}
+
+	// NOTE: we're re-using generateAllocationSet so this has to line up with
+	// the allocated node costs from that function. See table above.
+
+	// | Hierarchy                               | Cost |  CPU |  RAM |  GPU | Adjustment |
+	// +-----------------------------------------+------+------+------+------+------------+
+	//   cluster1:
+	//     nodes                                  100.00   5.00   4.00   1.00       90.00
+	// +-----------------------------------------+------+------+------+------+------------+
+	//   cluster1 subtotal (adjusted)             100.00  50.00  40.00  10.00        0.00
+	// +-----------------------------------------+------+------+------+------+------------+
+	//   cluster1 allocated                        48.00   6.00  16.00   6.00        0.00
+	// +-----------------------------------------+------+------+------+------+------------+
+	//   cluster1 idle                             72.00  44.00  24.00   4.00        0.00
+	// +-----------------------------------------+------+------+------+------+------------+
+	//   cluster2:
+	//     node1                                   35.00  20.00  15.00   0.00        0.00
+	//     node2                                   35.00  20.00  15.00   0.00        0.00
+	//     node3                                   30.00  10.00  10.00  10.00        0.00
+	//     (disks should not matter for idle)
+	// +-----------------------------------------+------+------+------+------+------------+
+	//   cluster2 subtotal                        100.00  50.00  40.00  10.00        0.00
+	// +-----------------------------------------+------+------+------+------+------------+
+	//   cluster2 allocated                        28.00   6.00   6.00   6.00        0.00
+	// +-----------------------------------------+------+------+------+------+------------+
+	//   cluster2 idle                             82.00  44.00  34.00   4.00        0.00
+	// +-----------------------------------------+------+------+------+------+------------+
+
+	cluster1Nodes = NewNode("", "cluster1", "", start, end, NewWindow(&start, &end))
+	cluster1Nodes.CPUCost = 5.0
+	cluster1Nodes.RAMCost = 4.0
+	cluster1Nodes.GPUCost = 1.0
+	cluster1Nodes.adjustment = 90.00
+
+	cluster2Node1 = NewNode("node1", "cluster2", "node1", start, end, NewWindow(&start, &end))
+	cluster2Node1.CPUCost = 20.0
+	cluster2Node1.RAMCost = 15.0
+	cluster2Node1.GPUCost = 0.0
+
+	cluster2Node2 = NewNode("node2", "cluster2", "node2", start, end, NewWindow(&start, &end))
+	cluster2Node2.CPUCost = 20.0
+	cluster2Node2.RAMCost = 15.0
+	cluster2Node2.GPUCost = 0.0
+
+	cluster2Node3 = NewNode("node3", "cluster2", "node3", start, end, NewWindow(&start, &end))
+	cluster2Node3.CPUCost = 10.0
+	cluster2Node3.RAMCost = 10.0
+	cluster2Node3.GPUCost = 10.0
+
+	cluster2Disk1 = NewDisk("disk1", "cluster2", "disk1", start, end, NewWindow(&start, &end))
+	cluster2Disk1.Cost = 5.0
+
+	assetSet = NewAssetSet(start, end, cluster1Nodes, cluster2Node1, cluster2Node2, cluster2Node3, cluster2Disk1)
+
+	idles, err = as.ComputeIdleAllocations(assetSet)
+	if err != nil {
+		t.Fatalf("unexpected error: %s", err)
+	}
+
+	if len(idles) != 2 {
+		t.Fatalf("idles: expected length %d; got length %d", 2, len(idles))
+	}
+
+	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(idles["cluster1"].CPUCost, 44.0) {
+		t.Fatalf("expected idle CPU cost for %s to be %.2f; got %.2f", "cluster1", 44.0, idles["cluster1"].CPUCost)
+	}
+	if !util.IsApproximately(idles["cluster1"].RAMCost, 24.0) {
+		t.Fatalf("expected idle RAM cost for %s to be %.2f; got %.2f", "cluster1", 24.0, idles["cluster1"].RAMCost)
+	}
+	if !util.IsApproximately(idles["cluster1"].GPUCost, 4.0) {
+		t.Fatalf("expected idle GPU cost for %s to be %.2f; got %.2f", "cluster1", 4.0, idles["cluster1"].GPUCost)
+	}
 
 	if idle, ok := idles["cluster2"]; !ok {
 		t.Fatalf("expected idle cost for %s", "cluster2")

pkg/kubecost/asset_test.go

@@ -837,7 +837,6 @@ func TestAssetSet_AggregateBy(t *testing.T) {
 	if err != nil {
 		t.Fatalf("AssetSet.AggregateBy: unexpected error: %s", err)
 	}
-	fmt.Println(as.assets)
 	assertAssetSet(t, as, "1e", window, map[string]float64{
 		"__undefined__": 53.00,
 		"test=test":     7.00,