Merge pull request #2200 from opencost/v1.106-pick-bd142

Fix sharing coefficients when unmounted PVs are folded into namespaces

pkg/kubecost/allocation.go

@@ -94,7 +94,10 @@ type Allocation struct {
 	ProportionalAssetResourceCosts ProportionalAssetResourceCosts `json:"proportionalAssetResourceCosts"` //@bingen:field[ignore]
 	SharedCostBreakdown            SharedCostBreakdowns           `json:"sharedCostBreakdown"`            //@bingen:field[ignore]
 	LoadBalancers                  LbAllocations                  `json:"LoadBalancers"`                  // @bingen:field[version=18]
-
+	// UnmountedPVCost is used to track how much of the cost in PVs is for an
+	// unmounted PV. It is not additive of PVCost() and need not be sent in API
+	// responses.
+	UnmountedPVCost float64 `json:"-"`
 }
 
 type LbAllocations map[string]*LbAllocation
@@ -688,6 +691,7 @@ func (a *Allocation) Clone() *Allocation {
 		ProportionalAssetResourceCosts: a.ProportionalAssetResourceCosts.Clone(),
 		SharedCostBreakdown:            a.SharedCostBreakdown.Clone(),
 		LoadBalancers:                  a.LoadBalancers.Clone(),
+		UnmountedPVCost:                a.UnmountedPVCost,
 	}
 }
 
@@ -787,6 +791,10 @@ func (a *Allocation) Equal(that *Allocation) bool {
 		return false
 	}
 
+	if !util.IsApproximately(a.UnmountedPVCost, that.UnmountedPVCost) {
+		return false
+	}
+
 	return true
 }
 
@@ -1082,21 +1090,12 @@ func (a *Allocation) IsUnallocated() bool {
 }
 
 // IsUnmounted is true if the given Allocation represents unmounted volume costs.
-// Note: Due to change in https://github.com/opencost/opencost/pull/1477 made to include Unmounted
-// PVC cost inside namespace we need to check unmounted suffix across all the three major properties
-// to actually classify it as unmounted.
 func (a *Allocation) IsUnmounted() bool {
 	if a == nil {
 		return false
 	}
 
-	props := a.Properties
-	if props != nil {
-		if props.Container == UnmountedSuffix && props.Namespace == UnmountedSuffix && props.Pod == UnmountedSuffix {
-			return true
-		}
-	}
-	return false
+	return strings.Contains(a.Name, UnmountedSuffix)
 }
 
 // Minutes returns the number of minutes the Allocation represents, as defined
@@ -1109,6 +1108,21 @@ func (a *Allocation) Minutes() float64 {
 	return a.End.Sub(a.Start).Minutes()
 }
 
+// SetUnmountedPVCost determines if the Allocation is unmounted and, if so, it
+// sets the UnmountedPVCost field appropriately.
+func (a *Allocation) SetUnmountedPVCost() float64 {
+	if a == nil {
+		return 0.0
+	}
+
+	if a.IsUnmounted() {
+		a.UnmountedPVCost = a.PVTotalCost()
+		return a.UnmountedPVCost
+	}
+
+	return 0.0
+}
+
 // Share adds the TotalCost of the given Allocation to the SharedCost of the
 // receiving Allocation. No Start, End, Window, or AllocationProperties are considered.
 // Neither Allocation is mutated; a new Allocation is always returned.
@@ -1259,6 +1273,7 @@ func (a *Allocation) add(that *Allocation) {
 	a.LoadBalancerCost += that.LoadBalancerCost
 	a.SharedCost += that.SharedCost
 	a.ExternalCost += that.ExternalCost
+	a.UnmountedPVCost += that.UnmountedPVCost
 
 	// Sum PVAllocations
 	a.PVs = a.PVs.Add(that.PVs)
@@ -2093,8 +2108,8 @@ func computeShareCoeffs(aggregateBy []string, options *AllocationAggregationOpti
 		} else {
 			// Both are additive for weighted distribution, where each
 			// cumulative coefficient will be divided by the total.
-			coeffs[name] += alloc.TotalCost() - alloc.SharedCost
-			total += alloc.TotalCost() - alloc.SharedCost
+			coeffs[name] += alloc.TotalCost() - alloc.SharedCost - alloc.UnmountedPVCost
+			total += alloc.TotalCost() - alloc.SharedCost - alloc.UnmountedPVCost
 		}
 	}
 
@@ -2886,6 +2901,30 @@ func (as *AllocationSet) Set(alloc *Allocation) error {
 	return nil
 }
 
+// GetUnmountedPVCost returns the sum of all UnmountedPVCost fields across all
+// allocations in the set.
+func (as *AllocationSet) GetUnmountedPVCost() float64 {
+	upvc := 0.0
+
+	for _, a := range as.Allocations {
+		upvc += a.UnmountedPVCost
+	}
+
+	return upvc
+}
+
+// SetUnmountedPVCost sets the UnmountedPVCost field for all allocations in the
+// set.
+func (as *AllocationSet) SetUnmountedPVCost() float64 {
+	upvc := 0.0
+
+	for _, a := range as.Allocations {
+		upvc += a.SetUnmountedPVCost()
+	}
+
+	return upvc
+}
+
 // Start returns the Start time of the AllocationSet window
 func (as *AllocationSet) Start() time.Time {
 	if as == nil {

pkg/kubecost/summaryallocation.go

@@ -40,6 +40,7 @@ type SummaryAllocation struct {
 	SharedCost             float64               `json:"sharedCost"`
 	ExternalCost           float64               `json:"externalCost"`
 	Share                  bool                  `json:"-"`
+	UnmountedPVCost        float64               `json:"-"`
 }
 
 // NewSummaryAllocation converts an Allocation to a SummaryAllocation by
@@ -68,6 +69,7 @@ func NewSummaryAllocation(alloc *Allocation, reconcile, reconcileNetwork bool) *
 		RAMCost:                alloc.RAMCost + alloc.RAMCostAdjustment,
 		SharedCost:             alloc.SharedCost,
 		ExternalCost:           alloc.ExternalCost,
+		UnmountedPVCost:        alloc.UnmountedPVCost,
 	}
 
 	// Revert adjustments if reconciliation is off. If only network
@@ -83,6 +85,11 @@ func NewSummaryAllocation(alloc *Allocation, reconcile, reconcileNetwork bool) *
 		sa.NetworkCost -= alloc.NetworkCostAdjustment
 	}
 
+	// If the allocation is unmounted, set UnmountedPVCost to the full PVCost.
+	if sa.IsUnmounted() {
+		sa.UnmountedPVCost = sa.PVCost
+	}
+
 	return sa
 }
 
@@ -300,20 +307,12 @@ func (sa *SummaryAllocation) IsUnallocated() bool {
 
 // IsUnmounted is true if the given SummaryAllocation represents unmounted
 // volume costs.
-// Note: Due to change in https://github.com/opencost/opencost/pull/1477 made to include Unmounted
-// PVC cost inside namespace we need to check unmounted suffix across all the three major properties
-// to actually classify it as unmounted.
 func (sa *SummaryAllocation) IsUnmounted() bool {
 	if sa == nil {
 		return false
 	}
-	props := sa.Properties
-	if props != nil {
-		if props.Container == UnmountedSuffix && props.Namespace == UnmountedSuffix && props.Pod == UnmountedSuffix {
-			return true
-		}
-	}
-	return false
+
+	return strings.Contains(sa.Name, UnmountedSuffix)
 }
 
 // Minutes returns the number of minutes the SummaryAllocation represents, as
@@ -581,6 +580,16 @@ func (sas *SummaryAllocationSet) Add(that *SummaryAllocationSet) (*SummaryAlloca
 	return acc, nil
 }
 
+func (sas *SummaryAllocationSet) GetUnmountedPVCost() float64 {
+	upvc := 0.0
+
+	for _, sa := range sas.SummaryAllocations {
+		upvc += sa.UnmountedPVCost
+	}
+
+	return upvc
+}
+
 // AggregateBy aggregates the Allocations in the given AllocationSet by the given
 // AllocationProperty. This will only be legal if the AllocationSet is divisible by the
 // given AllocationProperty; e.g. Containers can be divided by Namespace, but not vice-a-versa.
@@ -724,6 +733,7 @@ func (sas *SummaryAllocationSet) AggregateBy(aggregateBy []string, options *Allo
 			sharedResourceTotals[key].NetworkCost += sa.NetworkCost
 			sharedResourceTotals[key].PersistentVolumeCost += sa.PVCost
 			sharedResourceTotals[key].RAMCost += sa.RAMCost
+			sharedResourceTotals[key].UnmountedPVCost += sa.UnmountedPVCost
 
 			shareSet.Insert(sa)
 			delete(sas.SummaryAllocations, sa.Name)
@@ -883,8 +893,9 @@ func (sas *SummaryAllocationSet) AggregateBy(aggregateBy []string, options *Allo
 		// 0.0 and 1.0.
 		// NOTE: SummaryAllocation does not support ShareEven, so only record
 		// by cost for cost-weighted distribution.
-		if sharingCoeffs != nil {
-			sharingCoeffs[key] += sa.TotalCost() - sa.SharedCost
+		// if sharingCoeffs != nil {
+		if sharingCoeffs != nil && !sa.IsUnmounted() {
+			sharingCoeffs[key] += sa.TotalCost() - sa.SharedCost - sa.UnmountedPVCost
 		}
 
 		// 6. Distribute idle allocations according to the idle coefficients.
@@ -1043,21 +1054,24 @@ func (sas *SummaryAllocationSet) AggregateBy(aggregateBy []string, options *Allo
 	// NOTE: ShareEven is not supported
 	if len(shareSet.SummaryAllocations) > 0 {
 
+		shareCoeffSum := 0.0
+
 		sharingCoeffDenominator := 0.0
 		for _, rt := range allocTotals {
-			sharingCoeffDenominator += rt.TotalCost()
+			// Here, the allocation totals
+			sharingCoeffDenominator += rt.TotalCost() // does NOT include unmounted PVs at all
 		}
 
 		// Do not include the shared costs, themselves, when determining
 		// sharing coefficients.
 		for _, rt := range sharedResourceTotals {
-			sharingCoeffDenominator -= rt.TotalCost()
+			// Due to the fact that sharingCoeffDenominator already has no
+			// unmounted PV costs, we need to be careful not to additionally
+			// subtract the unmounted PV cost when we remove shared costs
+			// from the denominator.
+			sharingCoeffDenominator -= (rt.TotalCost() - rt.UnmountedPVCost)
 		}
 
-		// Do not include the unmounted costs when determining sharing
-		// coefficients because they do not receive shared costs.
-		sharingCoeffDenominator -= totalUnmountedCost
-
 		if sharingCoeffDenominator <= 0.0 {
 			log.Warnf("SummaryAllocation: sharing coefficient denominator is %f", sharingCoeffDenominator)
 		} else {
@@ -1071,6 +1085,7 @@ func (sas *SummaryAllocationSet) AggregateBy(aggregateBy []string, options *Allo
 				}
 				if sharingCoeffs[key] > 0.0 {
 					sharingCoeffs[key] /= sharingCoeffDenominator
+					shareCoeffSum += sharingCoeffs[key]
 				} else {
 					log.Warnf("SummaryAllocation: detected illegal sharing coefficient for %s: %v (setting to zero)", key, sharingCoeffs[key])
 					sharingCoeffs[key] = 0.0

pkg/kubecost/summaryallocation_json.go

@@ -24,6 +24,8 @@ type SummaryAllocationResponse struct {
 	RAMCost                *float64  `json:"ramCost"`
 	SharedCost             *float64  `json:"sharedCost"`
 	ExternalCost           *float64  `json:"externalCost"`
+	TotalEfficiency        *float64  `json:"totalEfficiency"`
+	TotalCost              *float64  `json:"totalCost"`
 }
 
 // ToResponse converts a SummaryAllocation to a SummaryAllocationResponse,
@@ -49,6 +51,8 @@ func (sa *SummaryAllocation) ToResponse() *SummaryAllocationResponse {
 		RAMCost:                formatutil.Float64ToResponse(sa.RAMCost),
 		SharedCost:             formatutil.Float64ToResponse(sa.SharedCost),
 		ExternalCost:           formatutil.Float64ToResponse(sa.ExternalCost),
+		TotalEfficiency:        formatutil.Float64ToResponse(sa.TotalEfficiency()),
+		TotalCost:              formatutil.Float64ToResponse(sa.TotalCost()),
 	}
 }
 

pkg/kubecost/totals.go

@@ -10,6 +10,16 @@ import (
 	"github.com/patrickmn/go-cache"
 )
 
+type AllocationTotalsResult struct {
+	Cluster map[string]*AllocationTotals `json:"cluster"`
+	Node    map[string]*AllocationTotals `json:"node"`
+}
+
+type AssetTotalsResult struct {
+	Cluster map[string]*AssetTotals `json:"cluster"`
+	Node    map[string]*AssetTotals `json:"node"`
+}
+
 // AllocationTotals represents aggregate costs of all Allocations for
 // a given cluster or tuple of (cluster, node) between a given start and end
 // time, where the costs are aggregated per-resource. AllocationTotals
@@ -35,6 +45,10 @@ type AllocationTotals struct {
 	PersistentVolumeCostAdjustment float64   `json:"persistentVolumeCostAdjustment"`
 	RAMCost                        float64   `json:"ramCost"`
 	RAMCostAdjustment              float64   `json:"ramCostAdjustment"`
+	// UnmountedPVCost is used to track how much of the cost in
+	// PersistentVolumeCost is for an unmounted PV. It is not additive of that
+	// field, and need not be sent in API responses.
+	UnmountedPVCost float64 `json:"-"`
 }
 
 // ClearAdjustments sets all adjustment fields to 0.0

pkg/kubecost/totals_json.go

@@ -0,0 +1,77 @@
+package kubecost
+
+import (
+	"time"
+
+	"github.com/opencost/opencost/pkg/util/formatutil"
+)
+
+type AllocationTotalsResponse struct {
+	Start                          time.Time `json:"start"`
+	End                            time.Time `json:"end"`
+	Cluster                        string    `json:"cluster"`
+	Node                           string    `json:"node"`
+	Count                          int       `json:"count"`
+	CPUCost                        *float64  `json:"cpuCost"`
+	CPUCostAdjustment              *float64  `json:"cpuCostAdjustment"`
+	GPUCost                        *float64  `json:"gpuCost"`
+	GPUCostAdjustment              *float64  `json:"gpuCostAdjustment"`
+	LoadBalancerCost               *float64  `json:"loadBalancerCost"`
+	LoadBalancerCostAdjustment     *float64  `json:"loadBalancerCostAdjustment"`
+	NetworkCost                    *float64  `json:"networkCost"`
+	NetworkCostAdjustment          *float64  `json:"networkCostAdjustment"`
+	PersistentVolumeCost           *float64  `json:"persistentVolumeCost"`
+	PersistentVolumeCostAdjustment *float64  `json:"persistentVolumeCostAdjustment"`
+	RAMCost                        *float64  `json:"ramCost"`
+	RAMCostAdjustment              *float64  `json:"ramCostAdjustment"`
+	TotalCost                      *float64  `json:"totalCost"`
+}
+
+func (arts *AllocationTotals) ToResponse() *AllocationTotalsResponse {
+	if arts == nil {
+		return nil
+	}
+
+	return &AllocationTotalsResponse{
+		Start:                          arts.Start,
+		End:                            arts.End,
+		Cluster:                        arts.Cluster,
+		Node:                           arts.Node,
+		Count:                          arts.Count,
+		CPUCost:                        formatutil.Float64ToResponse(arts.CPUCost),
+		CPUCostAdjustment:              formatutil.Float64ToResponse(arts.CPUCostAdjustment),
+		GPUCost:                        formatutil.Float64ToResponse(arts.GPUCost),
+		GPUCostAdjustment:              formatutil.Float64ToResponse(arts.GPUCostAdjustment),
+		LoadBalancerCost:               formatutil.Float64ToResponse(arts.LoadBalancerCost),
+		LoadBalancerCostAdjustment:     formatutil.Float64ToResponse(arts.LoadBalancerCostAdjustment),
+		NetworkCost:                    formatutil.Float64ToResponse(arts.NetworkCost),
+		NetworkCostAdjustment:          formatutil.Float64ToResponse(arts.NetworkCostAdjustment),
+		PersistentVolumeCost:           formatutil.Float64ToResponse(arts.PersistentVolumeCost),
+		PersistentVolumeCostAdjustment: formatutil.Float64ToResponse(arts.PersistentVolumeCostAdjustment),
+		RAMCost:                        formatutil.Float64ToResponse(arts.RAMCost),
+		RAMCostAdjustment:              formatutil.Float64ToResponse(arts.RAMCostAdjustment),
+		TotalCost:                      formatutil.Float64ToResponse(arts.TotalCost()),
+	}
+}
+
+type AllocationTotalsResultResponse struct {
+	Cluster map[string]*AllocationTotalsResponse `json:"cluster"`
+	Node    map[string]*AllocationTotalsResponse `json:"node"`
+}
+
+func (atr *AllocationTotalsResult) ToResponse() *AllocationTotalsResultResponse {
+	response := &AllocationTotalsResultResponse{
+		Cluster: map[string]*AllocationTotalsResponse{},
+		Node:    map[string]*AllocationTotalsResponse{},
+	}
+
+	for k, v := range atr.Cluster {
+		response.Cluster[k] = v.ToResponse()
+	}
+
+	for k, v := range atr.Node {
+		response.Node[k] = v.ToResponse()
+	}
+
+	return response
+}