Merge pull request #810 from kubecost/sean/idle-by-node-bugfix

Allow unmounted resources past idle filters

pkg/kubecost/allocation.go

@@ -928,10 +928,9 @@ func (as *AllocationSet) AggregateBy(aggregateBy []string, options *AllocationAg
 
 	// (3-5) Filter, distribute idle cost, and aggregate (in that order)
 	for _, alloc := range as.allocations {
-		idleKey, err := alloc.getIdleKey(options)
+		idleId, err := alloc.getIdleId(options)
 		if err != nil {
-			log.DedupedInfof(5,"AllocationSet.AggregateBy: missing idleKey for allocation: %s", alloc.Name)
-			continue
+			log.DedupedWarningf(3,"AllocationSet.AggregateBy: missing idleId for allocation: %s", alloc.Name)
 		}
 
 		skip := false
@@ -949,7 +948,7 @@ func (as *AllocationSet) AggregateBy(aggregateBy []string, options *AllocationAg
 			// entry will result in that proportional amount being removed
 			// from the idle allocation at the end of the process.)
 			if idleFiltrationCoefficients != nil {
-				if ifcc, ok := idleFiltrationCoefficients[idleKey]; ok {
+				if ifcc, ok := idleFiltrationCoefficients[idleId]; ok {
 					delete(ifcc, alloc.Name)
 				}
 			}
@@ -962,35 +961,37 @@ func (as *AllocationSet) AggregateBy(aggregateBy []string, options *AllocationAg
 		// all idle allocations will be in the aggSet at this point, so idleSet
 		// will be empty and we won't enter this block.
 		if idleSet.Length() > 0 {
-			// Distribute idle allocations by coefficient per-idleKey, per-allocation
+			// Distribute idle allocations by coefficient per-idleId, per-allocation
 			for _, idleAlloc := range idleSet.allocations {
-				// Only share idle if the idleKey matches; i.e. the allocation
-				// is from the same idleKey as the idle costs
-				iaIdleKey, err := idleAlloc.getIdleKey(options)
+				// Only share idle if the idleId matches; i.e. the allocation
+				// is from the same idleId as the idle costs
+				iaidleId, err := idleAlloc.getIdleId(options)
 				if err != nil {
+					log.Errorf("AllocationSet.AggregateBy: Idle allocation is missing idleId %s", idleAlloc.Name)
 					return err
 				}
-				if iaIdleKey != idleKey {
+
+				if iaidleId != idleId {
 					continue
 				}
 
 				// Make sure idle coefficients exist
-				if _, ok := idleCoefficients[idleKey]; !ok {
-					log.Warningf("AllocationSet.AggregateBy: error getting idle coefficient: no idleKey '%s' for '%s'", idleKey, alloc.Name)
+				if _, ok := idleCoefficients[idleId]; !ok {
+					log.Warningf("AllocationSet.AggregateBy: error getting idle coefficient: no idleId '%s' for '%s'", idleId, alloc.Name)
 					continue
 				}
-				if _, ok := idleCoefficients[idleKey][alloc.Name]; !ok {
+				if _, ok := idleCoefficients[idleId][alloc.Name]; !ok {
 					log.Warningf("AllocationSet.AggregateBy: error getting idle coefficient for '%s'", alloc.Name)
 					continue
 				}
 
-				alloc.CPUCoreHours += idleAlloc.CPUCoreHours * idleCoefficients[idleKey][alloc.Name]["cpu"]
-				alloc.GPUHours += idleAlloc.GPUHours * idleCoefficients[idleKey][alloc.Name]["gpu"]
-				alloc.RAMByteHours += idleAlloc.RAMByteHours * idleCoefficients[idleKey][alloc.Name]["ram"]
+				alloc.CPUCoreHours += idleAlloc.CPUCoreHours * idleCoefficients[idleId][alloc.Name]["cpu"]
+				alloc.GPUHours += idleAlloc.GPUHours * idleCoefficients[idleId][alloc.Name]["gpu"]
+				alloc.RAMByteHours += idleAlloc.RAMByteHours * idleCoefficients[idleId][alloc.Name]["ram"]
 
-				idleCPUCost := idleAlloc.CPUCost * idleCoefficients[idleKey][alloc.Name]["cpu"]
-				idleGPUCost := idleAlloc.GPUCost * idleCoefficients[idleKey][alloc.Name]["gpu"]
-				idleRAMCost := idleAlloc.RAMCost * idleCoefficients[idleKey][alloc.Name]["ram"]
+				idleCPUCost := idleAlloc.CPUCost * idleCoefficients[idleId][alloc.Name]["cpu"]
+				idleGPUCost := idleAlloc.GPUCost * idleCoefficients[idleId][alloc.Name]["gpu"]
+				idleRAMCost := idleAlloc.RAMCost * idleCoefficients[idleId][alloc.Name]["ram"]
 				alloc.CPUCost += idleCPUCost
 				alloc.GPUCost += idleGPUCost
 				alloc.RAMCost += idleRAMCost
@@ -1016,41 +1017,37 @@ func (as *AllocationSet) AggregateBy(aggregateBy []string, options *AllocationAg
 	// before sharing with the aggregated allocations.
 	if idleSet.Length() > 0 && shareSet.Length() > 0 {
 		for _, alloc := range shareSet.allocations {
-			idleKey, err := alloc.getIdleKey(options)
+			idleId, err := alloc.getIdleId(options)
 			if err != nil {
-				log.DedupedWarningf(5, "AllocationSet.AggregateBy: missing idleKey for allocation: %s", alloc.Name)
-				continue
+				log.DedupedWarningf(3, "AllocationSet.AggregateBy: missing idleId for allocation: %s", alloc.Name)
 			}
-
-			// Distribute idle allocations by coefficient per-idleKey, per-allocation
+			// Distribute idle allocations by coefficient per-idleId, per-allocation
 			for _, idleAlloc := range idleSet.allocations {
-				// Only share idle if the idleKey matches; i.e. the allocation
-				// is from the same idleKey as the idle costs
-				iaIdleKey, err := idleAlloc.getIdleKey(options)
-				if err != nil {
-					return nil
-				}
-				if iaIdleKey != idleKey {
+				// Only share idle if the idleId matches; i.e. the allocation
+				// is from the same idleId as the idle costs
+				iaidleId, _ := idleAlloc.getIdleId(options)
+
+				if iaidleId != idleId {
 					continue
 				}
 
 				// Make sure idle coefficients exist
-				if _, ok := idleCoefficients[idleKey]; !ok {
-					log.Warningf("AllocationSet.AggregateBy: error getting idle coefficient: no idleKey '%s' for '%s'", idleKey, alloc.Name)
+				if _, ok := idleCoefficients[idleId]; !ok {
+					log.Warningf("AllocationSet.AggregateBy: error getting idle coefficient: no idleId '%s' for '%s'", idleId, alloc.Name)
 					continue
 				}
-				if _, ok := idleCoefficients[idleKey][alloc.Name]; !ok {
+				if _, ok := idleCoefficients[idleId][alloc.Name]; !ok {
 					log.Warningf("AllocationSet.AggregateBy: error getting idle coefficient for '%s'", alloc.Name)
 					continue
 				}
 
-				alloc.CPUCoreHours += idleAlloc.CPUCoreHours * idleCoefficients[idleKey][alloc.Name]["cpu"]
-				alloc.GPUHours += idleAlloc.GPUHours * idleCoefficients[idleKey][alloc.Name]["gpu"]
-				alloc.RAMByteHours += idleAlloc.RAMByteHours * idleCoefficients[idleKey][alloc.Name]["ram"]
+				alloc.CPUCoreHours += idleAlloc.CPUCoreHours * idleCoefficients[idleId][alloc.Name]["cpu"]
+				alloc.GPUHours += idleAlloc.GPUHours * idleCoefficients[idleId][alloc.Name]["gpu"]
+				alloc.RAMByteHours += idleAlloc.RAMByteHours * idleCoefficients[idleId][alloc.Name]["ram"]
 
-				idleCPUCost := idleAlloc.CPUCost * idleCoefficients[idleKey][alloc.Name]["cpu"]
-				idleGPUCost := idleAlloc.GPUCost * idleCoefficients[idleKey][alloc.Name]["gpu"]
-				idleRAMCost := idleAlloc.RAMCost * idleCoefficients[idleKey][alloc.Name]["ram"]
+				idleCPUCost := idleAlloc.CPUCost * idleCoefficients[idleId][alloc.Name]["cpu"]
+				idleGPUCost := idleAlloc.GPUCost * idleCoefficients[idleId][alloc.Name]["gpu"]
+				idleRAMCost := idleAlloc.RAMCost * idleCoefficients[idleId][alloc.Name]["ram"]
 				alloc.CPUCost += idleCPUCost
 				alloc.GPUCost += idleGPUCost
 				alloc.RAMCost += idleRAMCost
@@ -1067,9 +1064,9 @@ func (as *AllocationSet) AggregateBy(aggregateBy []string, options *AllocationAg
 	if idleFiltrationCoefficients != nil {
 		groupingIdleFiltrationCoeffs = map[string]map[string]float64{}
 
-		for idleKey, m := range idleFiltrationCoefficients {
-			if _, ok := groupingIdleFiltrationCoeffs[idleKey]; !ok {
-				groupingIdleFiltrationCoeffs[idleKey] = map[string]float64{
+		for idleId, m := range idleFiltrationCoefficients {
+			if _, ok := groupingIdleFiltrationCoeffs[idleId]; !ok {
+				groupingIdleFiltrationCoeffs[idleId] = map[string]float64{
 					"cpu": 0.0,
 					"gpu": 0.0,
 					"ram": 0.0,
@@ -1078,7 +1075,7 @@ func (as *AllocationSet) AggregateBy(aggregateBy []string, options *AllocationAg
 
 			for _, n := range m {
 				for resource, val := range n {
-					groupingIdleFiltrationCoeffs[idleKey][resource] += val
+					groupingIdleFiltrationCoeffs[idleId][resource] += val
 				}
 			}
 		}
@@ -1089,14 +1086,13 @@ func (as *AllocationSet) AggregateBy(aggregateBy []string, options *AllocationAg
 	if len(aggSet.idleKeys) > 0 && groupingIdleFiltrationCoeffs != nil {
 		for idleKey := range aggSet.idleKeys {
 			idleAlloc := aggSet.Get(idleKey)
-
-			iaIdleKey, err := idleAlloc.getIdleKey(options)
+			iaidleId, err := idleAlloc.getIdleId(options)
 			if err != nil {
-				log.Warningf("AllocationSet.AggregateBy: idle allocation without IdleKey: %s", idleAlloc)
-				continue
+				log.Errorf("AllocationSet.AggregateBy: Idle allocation is missing idleId %s", idleAlloc.Name)
+				return err
 			}
 
-			if resourceCoeffs, ok := groupingIdleFiltrationCoeffs[iaIdleKey]; ok {
+			if resourceCoeffs, ok := groupingIdleFiltrationCoeffs[iaidleId]; ok {
 				idleAlloc.CPUCost *= resourceCoeffs["cpu"]
 				idleAlloc.CPUCoreHours *= resourceCoeffs["cpu"]
 				idleAlloc.RAMCost *= resourceCoeffs["ram"]
@@ -1238,43 +1234,42 @@ func computeIdleCoeffs(options *AllocationAggregationOptions, as *AllocationSet,
 			continue
 		}
 
-		idleKey, err := alloc.getIdleKey(options)
+		idleId, err := alloc.getIdleId(options)
 		if err != nil {
-			// skip allocations that are missing idleKey
-			continue
+			log.DedupedWarningf(3, "Missing Idle Key for %s", alloc.Name)
 		}
 
 		// get the name key for the allocation
 		name := alloc.Name
 
 		// Create key based tables if they don't exist
-		if _, ok := coeffs[idleKey]; !ok {
-			coeffs[idleKey] = map[string]map[string]float64{}
+		if _, ok := coeffs[idleId]; !ok {
+			coeffs[idleId] = map[string]map[string]float64{}
 		}
-		if _, ok := totals[idleKey]; !ok {
-			totals[idleKey] = map[string]float64{}
+		if _, ok := totals[idleId]; !ok {
+			totals[idleId] = map[string]float64{}
 		}
 
-		if _, ok := coeffs[idleKey][name]; !ok {
-			coeffs[idleKey][name] = map[string]float64{}
+		if _, ok := coeffs[idleId][name]; !ok {
+			coeffs[idleId][name] = map[string]float64{}
 		}
 
 		if shareType == ShareEven {
 			for _, r := range types {
 				// Not additive - hard set to 1.0
-				coeffs[idleKey][name][r] = 1.0
+				coeffs[idleId][name][r] = 1.0
 
 				// totals are additive
-				totals[idleKey][r] += 1.0
+				totals[idleId][r] += 1.0
 			}
 		} else {
-			coeffs[idleKey][name]["cpu"] += alloc.CPUTotalCost()
-			coeffs[idleKey][name]["gpu"] += alloc.GPUTotalCost()
-			coeffs[idleKey][name]["ram"] += alloc.RAMTotalCost()
+			coeffs[idleId][name]["cpu"] += alloc.CPUTotalCost()
+			coeffs[idleId][name]["gpu"] += alloc.GPUTotalCost()
+			coeffs[idleId][name]["ram"] += alloc.RAMTotalCost()
 
-			totals[idleKey]["cpu"] += alloc.CPUTotalCost()
-			totals[idleKey]["gpu"] += alloc.GPUTotalCost()
-			totals[idleKey]["ram"] += alloc.RAMTotalCost()
+			totals[idleId]["cpu"] += alloc.CPUTotalCost()
+			totals[idleId]["gpu"] += alloc.GPUTotalCost()
+			totals[idleId]["ram"] += alloc.RAMTotalCost()
 		}
 	}
 
@@ -1285,43 +1280,43 @@ func computeIdleCoeffs(options *AllocationAggregationOptions, as *AllocationSet,
 			continue
 		}
 
-		// idleKey will be providerId or cluster
-		idleKey, err := alloc.getIdleKey(options)
+		// idleId will be providerId or cluster
+		idleId, err := alloc.getIdleId(options)
 		if err != nil {
-			return nil, err
+			log.DedupedWarningf(3, "Missing Idle Key in share set for %s", alloc.Name)
 		}
 
 		// get the name key for the allocation
 		name := alloc.Name
 
-		// Create idleKey based tables if they don't exist
-		if _, ok := coeffs[idleKey]; !ok {
-			coeffs[idleKey] = map[string]map[string]float64{}
+		// Create idleId based tables if they don't exist
+		if _, ok := coeffs[idleId]; !ok {
+			coeffs[idleId] = map[string]map[string]float64{}
 		}
-		if _, ok := totals[idleKey]; !ok {
-			totals[idleKey] = map[string]float64{}
+		if _, ok := totals[idleId]; !ok {
+			totals[idleId] = map[string]float64{}
 		}
 
-		if _, ok := coeffs[idleKey][name]; !ok {
-			coeffs[idleKey][name] = map[string]float64{}
+		if _, ok := coeffs[idleId][name]; !ok {
+			coeffs[idleId][name] = map[string]float64{}
 		}
 
 		if shareType == ShareEven {
 			for _, r := range types {
 				// Not additive - hard set to 1.0
-				coeffs[idleKey][name][r] = 1.0
+				coeffs[idleId][name][r] = 1.0
 
 				// totals are additive
-				totals[idleKey][r] += 1.0
+				totals[idleId][r] += 1.0
 			}
 		} else {
-			coeffs[idleKey][name]["cpu"] += alloc.CPUTotalCost()
-			coeffs[idleKey][name]["gpu"] += alloc.GPUTotalCost()
-			coeffs[idleKey][name]["ram"] += alloc.RAMTotalCost()
+			coeffs[idleId][name]["cpu"] += alloc.CPUTotalCost()
+			coeffs[idleId][name]["gpu"] += alloc.GPUTotalCost()
+			coeffs[idleId][name]["ram"] += alloc.RAMTotalCost()
 
-			totals[idleKey]["cpu"] += alloc.CPUTotalCost()
-			totals[idleKey]["gpu"] += alloc.GPUTotalCost()
-			totals[idleKey]["ram"] += alloc.RAMTotalCost()
+			totals[idleId]["cpu"] += alloc.CPUTotalCost()
+			totals[idleId]["gpu"] += alloc.GPUTotalCost()
+			totals[idleId]["ram"] += alloc.RAMTotalCost()
 		}
 	}
 
@@ -1339,24 +1334,24 @@ func computeIdleCoeffs(options *AllocationAggregationOptions, as *AllocationSet,
 	return coeffs, nil
 }
 
-// getIdleKey returns the providerId or cluster of an Allocation depending on the IdleByNode
+// getIdleId returns the providerId or cluster of an Allocation depending on the IdleByNode
 // option in the AllocationAggregationOptions and an error if the respective field is missing
-func (a *Allocation) getIdleKey(options *AllocationAggregationOptions) (string, error) {
-	var idleKey string
+func (a *Allocation) getIdleId(options *AllocationAggregationOptions) (string, error) {
+	var idleId string
 	if options.IdleByNode {
 		// Key allocations to ProviderId to match against node
-		idleKey = a.Properties.ProviderID
-		if idleKey == "" {
-			return idleKey, fmt.Errorf("ProviderId is not set")
+		idleId = a.Properties.ProviderID
+		if idleId == "" {
+			return idleId, fmt.Errorf("ProviderId is not set")
 		}
 	} else {
 		// key the allocations by cluster id
-		idleKey = a.Properties.Cluster
-		if idleKey == "" {
-			return idleKey, fmt.Errorf("ClusterProp is not set")
+		idleId = a.Properties.Cluster
+		if idleId == "" {
+			return idleId, fmt.Errorf("ClusterProp is not set")
 		}
 	}
-	return idleKey, nil
+	return idleId, nil
 }
 
 func (a *Allocation) generateKey(aggregateBy []string) string {
@@ -1683,6 +1678,7 @@ func (as *AllocationSet) ComputeIdleAllocationsByNode(assetSet *AssetSet) (map[s
 	assetSet.Each(func(key string, a Asset) {
 		if node, ok := a.(*Node); ok {
 			if _, ok := assetNodeResourceCosts[node.Properties().ProviderID]; ok || node.Properties().ProviderID == "" {
+				log.DedupedWarningf(5, "Compute Idle Allocations By Node: Node missing providerId: %s", node.properties.Name)
 				return
 			}
 
@@ -1729,6 +1725,7 @@ func (as *AllocationSet) ComputeIdleAllocationsByNode(assetSet *AssetSet) (map[s
 		providerId := a.Properties.ProviderID
 		if providerId == "" {
 			// Failed to find allocation's node
+			log.DedupedWarningf(5, "Compute Idle Allocations By Node: Allocation missing providerId: %s", a.Name)
 			return
 		}