From 9aa9d3d4b7c637a0796e5a25c312a999ac628bd9 Mon Sep 17 00:00:00 2001 From: Mike Spreitzer Date: Wed, 12 Jun 2024 03:38:15 -0400 Subject: [PATCH] Add unit test of borrowing by exempt, fix bug Signed-off-by: Mike Spreitzer --- .../pkg/util/flowcontrol/apf_controller.go | 10 +- .../util/flowcontrol/exempt_borrowing_test.go | 164 ++++++++++++++++++ 2 files changed, 169 insertions(+), 5 deletions(-) create mode 100644 staging/src/k8s.io/apiserver/pkg/util/flowcontrol/exempt_borrowing_test.go diff --git a/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/apf_controller.go b/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/apf_controller.go index 88f7ec7a13e..c707884e133 100644 --- a/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/apf_controller.go +++ b/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/apf_controller.go @@ -399,9 +399,9 @@ func (cfgCtlr *configController) updateBorrowing() { func (cfgCtlr *configController) updateBorrowingLocked(setCompleters bool, plStates map[string]*priorityLevelState) { items := make([]allocProblemItem, 0, len(plStates)) nonExemptPLNames := make([]string, 0, len(plStates)) - idxOfNonExempt := map[string]int{} - cclOfExempt := map[string]int{} - var minCLSum, minCurrentCLSum int + idxOfNonExempt := map[string]int{} // items index of non-exempt classes + cclOfExempt := map[string]int{} // minCurrentCL of exempt classes + var minCLSum, minCurrentCLSum int // sums over non-exempt classes remainingServerCL := cfgCtlr.nominalCLSum for plName, plState := range plStates { obs := plState.seatDemandIntegrator.Reset() @@ -425,9 +425,9 @@ func (cfgCtlr *configController) updateBorrowingLocked(setCompleters bool, plSta upperBound: float64(plState.maxCL), target: math.Max(float64(minCurrentCL), plState.seatDemandStats.smoothed), }) + minCLSum += plState.minCL + minCurrentCLSum += minCurrentCL } - minCLSum += plState.minCL - minCurrentCLSum += minCurrentCL } if len(items) == 0 && cfgCtlr.nominalCLSum > 0 { klog.ErrorS(nil, "Impossible: no priority levels", "plStates", cfgCtlr.priorityLevelStates) diff --git a/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/exempt_borrowing_test.go b/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/exempt_borrowing_test.go new file mode 100644 index 00000000000..308c972f7cb --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/exempt_borrowing_test.go @@ -0,0 +1,164 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package flowcontrol + +import ( + "testing" + "time" + + fcboot "k8s.io/apiserver/pkg/apis/flowcontrol/bootstrap" + fqs "k8s.io/apiserver/pkg/util/flowcontrol/fairqueuing/queueset" + testeventclock "k8s.io/apiserver/pkg/util/flowcontrol/fairqueuing/testing/eventclock" + "k8s.io/apiserver/pkg/util/flowcontrol/metrics" + "k8s.io/client-go/informers" + clientsetfake "k8s.io/client-go/kubernetes/fake" + + flowcontrol "k8s.io/api/flowcontrol/v1" +) + +func TestUpdateBorrowing(t *testing.T) { + startTime := time.Now() + clk, _ := testeventclock.NewFake(startTime, 0, nil) + plcExempt := fcboot.MandatoryPriorityLevelConfigurationExempt + plcHigh := fcboot.SuggestedPriorityLevelConfigurationWorkloadHigh + plcMid := fcboot.SuggestedPriorityLevelConfigurationWorkloadLow + plcLow := fcboot.MandatoryPriorityLevelConfigurationCatchAll + plcs := []*flowcontrol.PriorityLevelConfiguration{plcHigh, plcExempt, plcMid, plcLow} + fses := []*flowcontrol.FlowSchema{} + k8sClient := clientsetfake.NewSimpleClientset(plcLow, plcExempt, plcHigh, plcMid) + informerFactory := informers.NewSharedInformerFactory(k8sClient, 0) + flowcontrolClient := k8sClient.FlowcontrolV1() + serverCL := int(*plcHigh.Spec.Limited.NominalConcurrencyShares+ + *plcMid.Spec.Limited.NominalConcurrencyShares+ + *plcLow.Spec.Limited.NominalConcurrencyShares) * 6 + config := TestableConfig{ + Name: "test", + Clock: clk, + AsFieldManager: "testfm", + FoundToDangling: func(found bool) bool { return !found }, + InformerFactory: informerFactory, + FlowcontrolClient: flowcontrolClient, + ServerConcurrencyLimit: serverCL, + ReqsGaugeVec: metrics.PriorityLevelConcurrencyGaugeVec, + ExecSeatsGaugeVec: metrics.PriorityLevelExecutionSeatsGaugeVec, + QueueSetFactory: fqs.NewQueueSetFactory(clk), + } + ctlr := newTestableController(config) + _ = ctlr.lockAndDigestConfigObjects(plcs, fses) + if ctlr.nominalCLSum != serverCL { + t.Fatalf("Unexpected rounding: nominalCLSum=%d", ctlr.nominalCLSum) + } + stateExempt := ctlr.priorityLevelStates[plcExempt.Name] + stateHigh := ctlr.priorityLevelStates[plcHigh.Name] + stateMid := ctlr.priorityLevelStates[plcMid.Name] + stateLow := ctlr.priorityLevelStates[plcLow.Name] + + // Scenario 1: everybody wants more than ServerConcurrencyLimit. + // Test the case of exempt borrowing so much that less than minCL + // is available to each non-exempt. + stateExempt.seatDemandIntegrator.Set(float64(serverCL + 100)) + stateHigh.seatDemandIntegrator.Set(float64(serverCL + 100)) + stateMid.seatDemandIntegrator.Set(float64(serverCL + 100)) + stateLow.seatDemandIntegrator.Set(float64(serverCL + 100)) + clk.SetTime(startTime.Add(borrowingAdjustmentPeriod)) + ctlr.updateBorrowing() + if expected, actual := serverCL+100, stateExempt.currentCL; expected != actual { + t.Errorf("Scenario 1: expected %d, got %d for exempt", expected, actual) + } else { + t.Logf("Scenario 1: expected and got %d for exempt", expected) + } + if expected, actual := stateHigh.minCL, stateHigh.currentCL; expected != actual { + t.Errorf("Scenario 1: expected %d, got %d for hi", expected, actual) + } else { + t.Logf("Scenario 1: expected and got %d for hi", expected) + } + if expected, actual := stateMid.minCL, stateMid.currentCL; expected != actual { + t.Errorf("Scenario 1: expected %d, got %d for mid", expected, actual) + } else { + t.Logf("Scenario 1: expected and got %d for mid", expected) + } + if expected, actual := stateLow.minCL, stateLow.currentCL; expected != actual { + t.Errorf("Scenario 1: expected %d, got %d for lo", expected, actual) + } else { + t.Logf("Scenario 1: expected and got %d for lo", expected) + } + + // Scenario 2: non-exempt want more than serverCL but get halfway between minCL and minCurrentCL. + expectedHigh := (stateHigh.nominalCL + stateHigh.minCL) / 2 + expectedMid := (stateMid.nominalCL + stateMid.minCL) / 2 + expectedLow := (stateLow.nominalCL + stateLow.minCL) / 2 + expectedExempt := serverCL - (expectedHigh + expectedMid + expectedLow) + stateExempt.seatDemandIntegrator.Set(float64(expectedExempt)) + clk.SetTime(startTime.Add(2 * borrowingAdjustmentPeriod)) + ctlr.updateBorrowing() + clk.SetTime(startTime.Add(3 * borrowingAdjustmentPeriod)) + ctlr.updateBorrowing() + if expected, actual := expectedExempt, stateExempt.currentCL; expected != actual { + t.Errorf("Scenario 2: expected %d, got %d for exempt", expected, actual) + } else { + t.Logf("Scenario 2: expected and got %d for exempt", expected) + } + if expected, actual := expectedHigh, stateHigh.currentCL; expected != actual { + t.Errorf("Scenario 2: expected %d, got %d for hi", expected, actual) + } else { + t.Logf("Scenario 2: expected and got %d for hi", expected) + } + if expected, actual := expectedMid, stateMid.currentCL; expected != actual { + t.Errorf("Scenario 2: expected %d, got %d for mid", expected, actual) + } else { + t.Logf("Scenario 2: expected and got %d for mid", expected) + } + if expected, actual := expectedLow, stateLow.currentCL; expected != actual { + t.Errorf("Scenario 2: expected %d, got %d for lo", expected, actual) + } else { + t.Logf("Scenario 2: expected and got %d for lo", expected) + } + + // Scenario 3: only mid is willing to lend, and exempt borrows all of that. + // Test the case of regular borrowing. + expectedHigh = stateHigh.nominalCL + expectedMid = stateMid.minCL + expectedLow = stateLow.nominalCL + expectedExempt = serverCL - (expectedHigh + expectedMid + expectedLow) + stateExempt.seatDemandIntegrator.Set(float64(expectedExempt)) + stateMid.seatDemandIntegrator.Set(float64(1)) + clk.SetTime(startTime.Add(4 * borrowingAdjustmentPeriod)) + ctlr.updateBorrowing() + clk.SetTime(startTime.Add(5 * borrowingAdjustmentPeriod)) + ctlr.updateBorrowing() + if expected, actual := expectedExempt, stateExempt.currentCL; expected != actual { + t.Errorf("Scenario 3: expected %d, got %d for exempt", expected, actual) + } else { + t.Logf("Scenario 3: expected and got %d for exempt", expected) + } + if expected, actual := expectedHigh, stateHigh.currentCL; expected != actual { + t.Errorf("Scenario 3: expected %d, got %d for hi", expected, actual) + } else { + t.Logf("Scenario 3: expected and got %d for hi", expected) + } + if expected, actual := expectedMid, stateMid.currentCL; expected != actual { + t.Errorf("Scenario 3: expected %d, got %d for mid", expected, actual) + } else { + t.Logf("Scenario 3: expected and got %d for mid", expected) + } + if expected, actual := expectedLow, stateLow.currentCL; expected != actual { + t.Errorf("Scenario 3: expected %d, got %d for lo", expected, actual) + } else { + t.Logf("Scenario 3: expected and got %d for lo", expected) + } + +}