Skip to content

Commit

Permalink
fix: node pie graphs to use consistent powers in divisor (#4726)
Browse files Browse the repository at this point in the history
1. The code determining whether or not the units of requests, limits and
capacity  required normalization was leaving out capacity in its
determination. As such, if the capacity was of different units than
requests and limits, they'd be left  inconsistent and the percentage
would be off by at least a factor of 1000 or 1024.

2. The code normalizing units was erroneously reversing a value in one
conditional, and thus erroneously triggering a second conditional which
was reversing the work of the first one (multiplying by the same factor
by which it had just divided). Rather than fix the bug, it's easier and
better to just eliminate this code. The conditionals aren't even necessary,
because negative exponentiation "just works".
  • Loading branch information
laurelnaiad authored and k8s-ci-robot committed Jan 7, 2020
1 parent 8d94344 commit acb9f1f
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 17 deletions.
19 changes: 4 additions & 15 deletions src/app/frontend/common/components/graph/helper.ts
Expand Up @@ -57,21 +57,10 @@ export class FormattedValue {
if (expectedPower < 0) {
throw new Error(`Suffix '${suffix}' not recognized.`);
}

let powerDiff = expectedPower - currentPower;

if (powerDiff < 0) {
powerDiff = -powerDiff;
this.value_ = this.value_ * Math.pow(this.base_, powerDiff);
this.suffix_ = suffix;
}

if (powerDiff > 0) {
this.value_ = this.value_ / Math.pow(this.base_, powerDiff);
this.suffix_ = suffix;
}

this.value_ = Number(this.value_.toPrecision(3));
const powerDiff = expectedPower - currentPower;
const value = this.value_ / Math.pow(this.base_, powerDiff);
this.value_ = Number(value.toPrecision(3));
this.suffix_ = suffix;
}

static NewFormattedCoreValue(value: number): FormattedValue {
Expand Down
10 changes: 8 additions & 2 deletions src/app/frontend/resource/cluster/node/detail/component.ts
Expand Up @@ -98,7 +98,10 @@ export class NodeDetailComponent implements OnInit, OnDestroy {
this.node.allocatedResources.memoryCapacity,
);

if (cpuLimitsValue.suffixPower !== cpuRequestsValue.suffixPower) {
if (
cpuLimitsValue.suffixPower !== cpuRequestsValue.suffixPower ||
cpuLimitsValue.suffixPower !== cpuCapacityValue.suffixPower
) {
const suffix =
cpuLimitsValue.suffixPower < cpuRequestsValue.suffixPower
? cpuLimitsValue.suffix
Expand All @@ -109,7 +112,10 @@ export class NodeDetailComponent implements OnInit, OnDestroy {
cpuCapacityValue.normalize(suffix);
}

if (memoryLimitsValue.suffixPower !== memoryRequestsValue.suffixPower) {
if (
memoryLimitsValue.suffixPower !== memoryRequestsValue.suffixPower ||
memoryLimitsValue.suffixPower !== memoryCapacityValue.suffixPower
) {
const suffix =
memoryLimitsValue.suffixPower < memoryRequestsValue.suffixPower
? memoryLimitsValue.suffix
Expand Down

0 comments on commit acb9f1f

Please sign in to comment.