Promote pod autoscaling #79954
Promote pod autoscaling #79954
Conversation
k8s-ci-robot
added
release-note-none
|
Hi @BobyMCbobs. Thanks for your PR. I'm waiting for a kubernetes 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/test-infra repository. |
k8s-ci-robot
added
the
needs-ok-to-test
|
@BobyMCbobs: Reiterating the mentions to trigger a notification: In response to this:
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/test-infra repository. |
k8s-ci-robot
added
the
sig/testing
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
|
/ok-to-test |
| Release : v1.16 | ||
| Testname: Up-scale pod replicas and resources | ||
| Description: Pod replicas MUST increase from 1 pod to 2 pods. | ||
| Pod CPU request will increase to 50%. |
There was a problem hiding this comment.
It should be: Average Pod CPU utilization will drop to lower than 50%
Utilization is usage/request
The test starts with 1 pod, 150 mCpu usage, 200mcpu request - that gives us 75% utilization, while target is 250.
Autoscaler will add second pod. Then we have 150 mCpu total usage, 400 mCPU total request which is <50% utilization.
k8s-ci-robot
added
size/S
|
Thank you! |
k8s-ci-robot
added
the
lgtm
| Then the HP Autoscaler will create another pod | ||
| And the 150mCPU usage will be spread across both Pods | ||
| */ | ||
| framework.ConformanceIt("Should scale from 1 pod to 2 pods", func() { |
There was a problem hiding this comment.
Please lowercase Should so it is like all the others.
Also the description now is accurate but it's still not in the right format - RFC2119. So I think this would be better:
Given 1 Pod with 150mCPU usage, 200mCPU per Pod request, and targeted CPU utilization of 50%, the HP Autoscaler MUST create another pod, spreading the 150mCPU usage across both Pods.
k8s-ci-robot
added
priority/important-longterm
k8s-ci-robot
removed
the
lgtm
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/assign @smarterclayton @bgrant0607 Clayton, Brian, |
|
This test isn't still flaky, is it? I know in 1.14 it was pretty bad, are we set on overall flakiness? |
|
/approve CPU autoscaling is reasonably well adopted and has minimal dependencies. If a specific vendor has an issue with this in practice (because they feel this is out of scope) I'm willing to entertain removing this from the conformance suite and putting into a profile. So this is provisionally approved for 1.16 conformance but if there is significant pushback I am willing to approve removing it from conformance. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: BobyMCbobs, bskiba, smarterclayton 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-kubernetes-e2e-kind |
|
@BobyMCbobs: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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/test-infra repository. I understand the commands that are listed here. |
|
So this essentially requires that the metrics server addon be deployed right? Seems like as a semi-related follow up we should move that into kubernetes-sigs instead of kubernetes-incubator at least, and some of the Kubernetes setup docs should be updated to include this. |
|
@smarterclayton @bgrant0607 - @BenTheElder caught something after this merged ... https://kubernetes.slack.com/archives/C20HH14P7/p1563324207095300 Should we "promote" it to |
|
HPA demanding status report to a metric system, or in our case a non-essential addons such as the metric server, seems to require further discussion. in my option, for this test, we should be reporting to a mock/test system and not an actual community maintained addon. by adding this test to conformance we just enabled the notion that any conformant cluster should host a metrics system, which is not true in the wild. maybe for some k8s based products yes, but not for all clusters out there. for now, kubeadm will not enable the metric server to be able to run this test. |
|
I am going to revert this given the number of components that don't add the component. The revert is conditional on us discussing how to address this (a number of options have been suggested, from promoting components to required to having the test skip if metrics is not installed but be enforced if it is) and we should try to reach a decision in 3 weeks time (arbitrarily chosen). |
|
thanks @smarterclayton ! |
Yes I think so, although, I think somewhat tangential to this discussion. |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Promotes the following E2E tests to Conformance:
test/e2e/autoscaling/horizontal_pod_autoscaling.go: "Should scale from 1 pod to 2 pods"
Special notes for your reviewer:
Part of Umbrella Issue #78747
Test grid results.
Does this PR introduce a user-facing change?:
/area conformance
/area test
@kubernetes/sig-autoscaling-pr-reviews
@kubernetes/sig-architecture-pr-reviews
@kubernetes/cncf-conformance-wg