chore: log average and assessed thresholds #1657
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
Hi @ricardomaraschini. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
| @@ -197,7 +188,7 @@ func (h *HighNodeUtilization) Balance(ctx context.Context, nodes []*v1.Node) *fr | |||
|
|
|||
| lowNodes, schedulableNodes := nodeInfos[0], nodeInfos[1] | |||
|
|
|||
| klog.V(1).InfoS("Criteria for a node below target utilization", h.criteria...) | |||
| klog.V(1).InfoS("Criteria for a node below target utilization", thresholdsToKeysAndValues(h.args.Thresholds)...) | |||
There was a problem hiding this comment.
Originally, the output for InfoS was precomputed to avoid extra computation each time the plugin is run. Let's keep criteria field around and just criteria=thresholdsToKeysAndValues(h.args.Thresholds).
There was a problem hiding this comment.
Do we call Balance() multiple times for the same plugin during the same deschedule run ?
There was a problem hiding this comment.
My point here is that between executions we need to read/parse/reload the descheduler policy file and when we do that we may potentially have different thresholds. This performance gain (populate criteria only when the plugin is instantiated) can be accomplished only if the whole pod goes down when the policy config change and while the latter does not change we don't create a new plugin instance. Please let me know where my assumptions are wrong.
|
|
||
| // decrease the provided threshold from the average to get the low | ||
| // span. also make sure the resulting values are between 0 and 100. | ||
| lowSpan = normalizer.Clamp( |
There was a problem hiding this comment.
Based on https://dave.cheney.net/2017/04/30/if-a-map-isnt-a-reference-variable-what-is-it a map is not a reference. So this should not change the actual value of lowSpan passed by invoker.
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
when calculating the average an applying the deviations it would be nice to also see the assessed values. this commit makes the descheduler logs these values when using level 3.
|
/retest |
|
/approve |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ingvagabund The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
|
/test pull-descheduler-test-e2e-k8s-master-1-32 |
when calculating the average an applying the deviations it would be nice to also see the assessed values.
this commit makes the descheduler logs these values when using level 3.