Skip to content

Commit

Permalink
Fix bug where Summary Allocation would not account for disabled recon…
Browse files Browse the repository at this point in the history
…ciliation in sharing coefficients
  • Loading branch information
nikovacevic committed Mar 10, 2022
1 parent bfb98b7 commit bb847ad
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 5 deletions.
2 changes: 2 additions & 0 deletions pkg/kubecost/allocation.go
Original file line number Diff line number Diff line change
Expand Up @@ -834,6 +834,8 @@ type AllocationAggregationOptions struct {
IdleByNode bool
LabelConfig *LabelConfig
MergeUnallocated bool
Reconcile bool
ReconcileNetwork bool
ShareFuncs []AllocationMatchFunc
ShareIdle string
ShareSplit string
Expand Down
24 changes: 19 additions & 5 deletions pkg/kubecost/summaryallocation.go
Original file line number Diff line number Diff line change
Expand Up @@ -326,14 +326,14 @@ func NewSummaryAllocationSet(as *AllocationSet, ffs, kfs []AllocationMatchFunc,
for _, alloc := range as.allocations {
// First, detect if the allocation should be kept. If so, mark it as
// such, insert it, and continue.
sholdKeep := false
shouldKeep := false
for _, kf := range kfs {
if kf(alloc) {
sholdKeep = true
shouldKeep = true
break
}
}
if sholdKeep {
if shouldKeep {
sa := NewSummaryAllocation(alloc, reconcile, reconcileNetwork)
sa.Share = true
sas.Insert(sa)
Expand Down Expand Up @@ -454,8 +454,8 @@ func (sas *SummaryAllocationSet) AggregateBy(aggregateBy []string, options *Allo
// an empty slice implies that we should aggregate everything. (See
// generateKey for why that makes sense.)
shouldAggregate := aggregateBy != nil
sholdKeep := len(options.SharedHourlyCosts) > 0 || len(options.ShareFuncs) > 0
if !shouldAggregate && !sholdKeep {
shouldKeep := len(options.SharedHourlyCosts) > 0 || len(options.ShareFuncs) > 0
if !shouldAggregate && !shouldKeep {
return nil
}

Expand Down Expand Up @@ -627,6 +627,20 @@ func (sas *SummaryAllocationSet) AggregateBy(aggregateBy []string, options *Allo
}
}

// If reconciliation has been fully or partially disabled, clear the
// relevant adjustments from the alloc totals
if allocTotals != nil && (!options.Reconcile || !options.ReconcileNetwork) {
if !options.Reconcile {
for _, tot := range allocTotals {
tot.ClearAdjustments()
}
} else if !options.ReconcileNetwork {
for _, tot := range allocTotals {
tot.NetworkCostAdjustment = 0.0
}
}
}

// If filters have been applied, then we need to record allocation resource
// totals after filtration (i.e. the allocations that are present) so that
// we can identify the proportion of idle cost to keep. That is, we should
Expand Down

0 comments on commit bb847ad

Please sign in to comment.