Change CPU sample sanitization in HPA. - #68068
Conversation
k8s-ci-robot
added
release-note
k8s-ci-robot
added
needs-ok-to-test
|
/assign mwielgus |
|
/assign DirectXMan12 |
|
/assign @DirectXMan12 |
|
/ok-to-test |
k8s-ci-robot
removed
the
needs-ok-to-test
DirectXMan12
left a comment
DirectXMan12
left a comment
There was a problem hiding this comment.
Couple of quick comments inline. Need to do a full scan later today.
There was a problem hiding this comment.
Should this say CPU in it? Doesn't it apply to other metrics as well?
There was a problem hiding this comment.
+mwielgus
Currently this is only for CPU and I belive it will be only used for CPU.
There was a problem hiding this comment.
it got changed a PR or two ago. It used to be universal across all metrics, so we probably need to decide if we really want to change it.
There was a problem hiding this comment.
We didn't have such flag before and we want to change behavior only for CPU as it is kind of special.
faa800c to
b56352e
Compare
|
/test pull-kubernetes-integration |
DirectXMan12
left a comment
DirectXMan12
left a comment
There was a problem hiding this comment.
minor nit inline, otherwise fine modulo the conversation about where this applies
There was a problem hiding this comment.
this should have a way better name. These tests are already hard to follow (maybe call it offsetTimestampBy or something).
There was a problem hiding this comment.
Fixed.
b56352e to
739cf7c
Compare
k8s-ci-robot
added
size/L
|
Is there anything else that I should fix apart calcTimestamp name? |
|
I think I'm fine with this otherwise. |
|
/cc @wenjiaswe please figure out why it's been incorrectly assigned to api-machinery |
Ignore samples if:
- Pod is beeing initalized - 5 minutes from start defined by flag
- pod is unready
- pod is ready but full window of metric hasn't been colected since
transition
- Pod is initialized - 5 minutes from start defined by flag:
- Pod has never been ready after initial readiness period.
739cf7c to
62f771f
Compare
|
Please give me LGTM so I can collect approvals tomorrow. |
|
/test pull-kubernetes-integration |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/assign wojtek-t |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: krzysztof-jastrzebski, mwielgus, wojtek-t The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
|
/test all [submit-queue is verifying that this PR is safe to merge] |
|
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here: https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md. |
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here: https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md. Change CPU sample sanitization in HPA. **What this PR does / why we need it**: Change CPU sample sanitization in HPA. Ignore samples if: - Pod is beeing initalized - 5 minutes from start defined by flag - pod is unready - pod is ready but full window of metric hasn't been colected since transition - Pod is initialized - 5 minutes from start defined by flag: - Pod has never been ready after initial readiness period. **Release notes:** ```release-note Improve CPU sample sanitization in HPA by taking metric's freshness into account. ```
8 participants
What this PR does / why we need it:
Change CPU sample sanitization in HPA.
Ignore samples if:
- Pod is beeing initalized - 5 minutes from start defined by flag
- pod is unready
- pod is ready but full window of metric hasn't been colected since
transition
- Pod is initialized - 5 minutes from start defined by flag:
- Pod has never been ready after initial readiness period.
Release notes: