Skip to content

Commit b4d4f98

Browse files
author
jo.perez
committed
fix(ndb): correct disruptionsAllowed calculation in NodeDisruptionBudget
Stop subtracting `currentDisruptions` twice when computing `NodeDisruptionBudget.status.disruptionsAllowed`. This was producing negative values at capacity (e.g. `max=1, current=1 -> -1`). Add unit tests for boundary and limiting-factor cases to prevent regressions.
1 parent cb252e7 commit b4d4f98

2 files changed

Lines changed: 73 additions & 3 deletions

File tree

internal/controller/nodedisruptionbudget_controller.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,12 @@ type NodeDisruptionBudgetResolver struct {
176176
Resolver resolver.Resolver
177177
}
178178

179+
func computeNodeDisruptionBudgetDisruptionsAllowed(maxDisruptedNodes, minUndisruptedNodes, watchedNodes, currentDisruptions int) int {
180+
disruptionsForMax := maxDisruptedNodes - currentDisruptions
181+
disruptionsForMin := (watchedNodes - currentDisruptions) - minUndisruptedNodes
182+
return int(math.Min(float64(disruptionsForMax), float64(disruptionsForMin)))
183+
}
184+
179185
// Sync ensure the budget's status is up to date
180186
func (r *NodeDisruptionBudgetResolver) Sync(ctx context.Context) error {
181187
nodeNames, err := r.GetSelectedNodes(ctx)
@@ -192,9 +198,12 @@ func (r *NodeDisruptionBudgetResolver) Sync(ctx context.Context) error {
192198

193199
r.NodeDisruptionBudget.Status.WatchedNodes = nodes
194200
r.NodeDisruptionBudget.Status.CurrentDisruptions = disruptionCount
195-
disruptionsForMax := r.NodeDisruptionBudget.Spec.MaxDisruptedNodes - disruptionCount
196-
disruptionsForMin := (len(nodes) - disruptionCount) - r.NodeDisruptionBudget.Spec.MinUndisruptedNodes
197-
r.NodeDisruptionBudget.Status.DisruptionsAllowed = int(math.Min(float64(disruptionsForMax), float64(disruptionsForMin))) - disruptionCount
201+
r.NodeDisruptionBudget.Status.DisruptionsAllowed = computeNodeDisruptionBudgetDisruptionsAllowed(
202+
r.NodeDisruptionBudget.Spec.MaxDisruptedNodes,
203+
r.NodeDisruptionBudget.Spec.MinUndisruptedNodes,
204+
len(nodes),
205+
disruptionCount,
206+
)
198207
r.NodeDisruptionBudget.Status.Disruptions = disruptions
199208
return nil
200209
}
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
package controller
2+
3+
import "testing"
4+
5+
func TestComputeNodeDisruptionBudgetDisruptionsAllowed(t *testing.T) {
6+
tests := []struct {
7+
name string
8+
maxDisruptedNodes int
9+
minUndisruptedNodes int
10+
watchedNodes int
11+
currentDisruptions int
12+
expected int
13+
}{
14+
{
15+
name: "at max disruption returns zero",
16+
maxDisruptedNodes: 1,
17+
minUndisruptedNodes: 0,
18+
watchedNodes: 34,
19+
currentDisruptions: 1,
20+
expected: 0,
21+
},
22+
{
23+
name: "max side is limiting factor",
24+
maxDisruptedNodes: 3,
25+
minUndisruptedNodes: 0,
26+
watchedNodes: 10,
27+
currentDisruptions: 2,
28+
expected: 1,
29+
},
30+
{
31+
name: "min undisrupted side is limiting factor",
32+
maxDisruptedNodes: 10,
33+
minUndisruptedNodes: 10,
34+
watchedNodes: 12,
35+
currentDisruptions: 1,
36+
expected: 1,
37+
},
38+
{
39+
name: "returns negative when min undisrupted cannot be respected",
40+
maxDisruptedNodes: 10,
41+
minUndisruptedNodes: 10,
42+
watchedNodes: 5,
43+
currentDisruptions: 1,
44+
expected: -6,
45+
},
46+
}
47+
48+
for _, tt := range tests {
49+
t.Run(tt.name, func(t *testing.T) {
50+
got := computeNodeDisruptionBudgetDisruptionsAllowed(
51+
tt.maxDisruptedNodes,
52+
tt.minUndisruptedNodes,
53+
tt.watchedNodes,
54+
tt.currentDisruptions,
55+
)
56+
if got != tt.expected {
57+
t.Fatalf("unexpected disruptions allowed: got=%d expected=%d", got, tt.expected)
58+
}
59+
})
60+
}
61+
}

0 commit comments

Comments
 (0)