[FG:InPlacePodVerticalScaling] fix InPlacePodVerticalScaling e2e tests #128598
Conversation
k8s-ci-robot
added
release-note-none
|
This issue is currently awaiting triage. If a SIG or subproject determines this is a relevant issue, they will accept it by applying the The 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. |
k8s-ci-robot
added
needs-triage
|
/test pull-kubernetes-e2e-inplace-pod-resize-containerd-main-v2 |
k8s-ci-robot
added
area/test
sig/node
|
@pacoxu would you be able to review this? |
|
/cc @tallclair @esotsal |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: e017dd4439ffa1f932e67008cc343b2026309a87
|
|
/test |
|
@aojea: The
The following commands are available to trigger optional jobs:
Use
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-sigs/prow repository. |
|
/test pull-kubernetes-node-e2e-alpha-ec2 /approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: AnishShah, aojea, esotsal 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-alpha-features |
| @@ -110,9 +110,9 @@ func doPodResizeAdmissionPluginsTests() { | |||
| } | |||
|
|
|||
| for _, tc := range testcases { | |||
| ginkgo.It(tc.name, func(ctx context.Context) { | |||
| f := framework.NewDefaultFramework(tc.name) | |||
There was a problem hiding this comment.
This is not a mistake that can be found by the existing linters for E2E suites. The test has to run, then Ginkgo detects the mistake.
This begs the question: how was this test developed? Was it executed at all before merging #128266?
There was a problem hiding this comment.
It was executed and failed but for some reason did not block the PR from merging - https://prow.k8s.io/pr-history/?org=kubernetes&repo=kubernetes&pr=128266
There was a problem hiding this comment.
we don't block on flake tests or optional jobs, alpha features are very flaky, blocking presubmits jobs need to be very reliable and fast, is on reviewers authors to ensure their code is fine to merge. It is a trade off we have to have and stable, reliable and performance CI system
There was a problem hiding this comment.
I agree that they are non-blocking and optional jobs. But it should at least inform failures on GitHub. In my case, the job was triggered but did not appear under GitHub checks.
There was a problem hiding this comment.
the job config has a boolean to indicate to report status,. I didn;t check but maybe that was happened, we should switch that bool to on so people can look into the failures
Thanks for doing the retrospective with us
|
Failure seems unrelated. /retest |
@AnishShah a good practice is to leave the test that failed and the link to the jobs with the reason you believe are unrelated, that simplifies a lot other reviewers life |
|
The job failures in the kind-alpha-* jobs sounds related to #128600 |
|
/test pull-kubernetes-e2e-inplace-pod-resize-containerd-main-v2 ^ This triggers a presubmit job. But it is only visible from Prow dashboard and not here on GitHub |
Please check #125205 (comment), i think failures might have to do with this commit. |
|
@AnishShah: The following tests 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-sigs/prow repository. I understand the commands that are listed here. |
What type of PR is this?
/kind failing-test
What this PR does / why we need it:
I broke InPlacePodVerticalScaling e2e tests in #128266. This change fixes it.
Failure message:
Which issue(s) this PR fixes:
Fixes #128607
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: