-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add isGarbageCollected to perfscale-load-generator ObjListWatcher #8027
Add isGarbageCollected to perfscale-load-generator ObjListWatcher #8027
Conversation
Hi @alaypatel07. Thanks for your PR. I'm waiting for a kubevirt 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. |
8796942
to
705c834
Compare
e9d39e5
to
1b4d9f5
Compare
/ok-to-test |
This helps in asserting the Delete semantics of the api during performance testing. It is required that when Delete request is processed against kubevirt api objects (VMI, VM and VirtualMachineReplicaSet), kubevirt system is able to observe the complete garbage collection (from delete request all the way to finalizer being removed and object being removed from the kube api) In order to avoid stomping apiserver with get requests from the e2e test suite a further optimisation of introducing a cache is required Signed-off-by: Alay Patel <alayp@nvidia.com>
Signed-off-by: Alay Patel <alayp@nvidia.com>
Signed-off-by: Alay Patel <alayp@nvidia.com>
396f771
to
0e8194d
Compare
/test pull-kubevirt-build
Docker did not come up for some reason. trying to see if this is a flake.. |
0e8194d
to
b149486
Compare
https://asciinema.org/a/MNURPsH2alH3EzQ6JIJv6eJBt I was able to make a local run wait for all the deletions with this. |
/test periodic-kubevirt-performance-cluster-100-density-test trying to see if I can trigger the performance test on this PR |
@alaypatel07: The specified target(s) for
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/test-infra repository. |
/test pull-kubevirt-e2e-k8s-1.22-sig-performance |
/test pull-kubevirt-e2e-k8s-1.22-sig-performance |
/test pull-kubevirt-fossa |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One nit
if vmi.Status.Phase == kvv1.Succeeded { | ||
w.deleteObj(string(vmi.GetUID())) | ||
if vmi.Status.Phase == kvv1.Succeeded || vmi.Status.Phase == kvv1.Failed { | ||
log.Log.V(4).Errorf("VirtualMachineInstance obj: %v Succeeded, waiting for garbage collection", fmt.Sprintf("%s/%s", vmi.Namespace, vmi.Name)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically we could be in either Succeeded or failed. I think the right way to say this is, " VMI obj: %s is in a final state,.."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, thanks for the catch @rthallisey
Signed-off-by: Alay Patel <alayp@nvidia.com>
b149486
to
8e37903
Compare
/lgtm |
/retest-required |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rthallisey 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 |
@alaypatel07: The following tests failed, say
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. |
seems like a flak in getting releases https://prow.ci.kubevirt.io/view/gs/kubevirt-prow/pr-logs/pull/kubevirt_kubevirt/8027/pull-kubevirt-e2e-k8s-1.23-operator/1552675673892458496 /test pull-kubevirt-e2e-k8s-1.23-operator |
reported the failure for this test here #7802 (comment), retrying... /test pull-kubevirt-e2e-kind-1.23-vgpu-nonroot |
reported here the failure here: #7168 (comment) /test pull-kubevirt-e2e-k8s-1.23-sig-compute-migrations-nonroot |
/override pull-kubevirt-fossa this failure is well-known and being discussed in the mailing list |
@alaypatel07: alaypatel07 unauthorized: /override is restricted to Repo administrators, approvers in top level OWNERS file. 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. |
What this PR does / why we need it:
This helps in asserting the Delete semantics of the api during
performance testing. It is required that when Delete request is processed
against kubevirt api objects (VMI, VM and VirtualMachineReplicaSet),
kubevirt system is able to observe the complete garbage collection
(from delete request all the way to finalizer being removed and object being
removed from the kube api)
In order to avoid stomping apiserver with get requests from the e2e test suite
a further optimization of introducing a cache is required
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #8025
Special notes for your reviewer:
Release note: