Separate rate limiters for Pod evictions for different zones in NodeController #28843
Conversation
k8s-github-robot
added
size/XL
k8s-github-robot
added
size/L
| @@ -239,8 +238,7 @@ func StartControllers(s *options.CMServer, kubeClient *client.Client, kubeconfig | |||
| glog.Warningf("Unsuccessful parsing of service CIDR %v: %v", s.ServiceCIDR, err) | |||
| } | |||
| nodeController := nodecontroller.NewNodeController(cloud, clientset.NewForConfigOrDie(restclient.AddUserAgent(kubeconfig, "node-controller")), | |||
| s.PodEvictionTimeout.Duration, flowcontrol.NewTokenBucketRateLimiter(s.DeletingPodsQps, int(s.DeletingPodsBurst)), | |||
| flowcontrol.NewTokenBucketRateLimiter(s.DeletingPodsQps, int(s.DeletingPodsBurst)), | |||
| s.PodEvictionTimeout.Duration, s.DeletingPodsQps, int(s.DeletingPodsBurst), s.DeletingPodsQps, int(s.DeletingPodsBurst), | |||
There was a problem hiding this comment.
Do you have any usecase on your mind of having different deletionEviction & terminationEvictions rate-limitters? I mean with different parameters.
Currently, there is no way to have different values for them, so maybe they should have always the same values, and then we can pass those values once instead of twice.
|
@gmarek - I think this PR requires release note. |
gmarek
added
release-note
gmarek
changed the title
gmarek
changed the title
|
Changed the name of the PR so it's more release-note'y. @wojtek-t PTAL |
| @@ -1175,7 +1243,8 @@ func TestCheckPod(t *testing.T) { | |||
| }, | |||
| } | |||
|
|
|||
| nc := NewNodeController(nil, nil, 0, nil, nil, 0, 0, 0, nil, nil, 0, false) | |||
| nc := NewNodeController(nil, nil, 0, 0, 0, | |||
| 0, 0, 0, nil, nil, 0, false) | |||
There was a problem hiding this comment.
nit: merge those two lines into 1 line
|
Very minor nits - lgtm after that. |
|
Fixed the nits. Applying lgtm. |
gmarek
added
the
lgtm
|
GCE e2e build/test passed for commit 5677a98. |
|
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
|
GCE e2e build/test passed for commit 5677a98. |
|
Automatic merge from submit-queue |
Ref. #28832
NodeController needs to be able to separately adjust rate-limits for eviction for different zones. This PR splits rate limiters.
cc @davidopp