🐛 Add CleanupAndWait call for flaky ClusterClass test #7589
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/S
internal/test/envtest/environment.go
Outdated
| @@ -422,6 +422,28 @@ func (e *Environment) CreateAndWait(ctx context.Context, obj client.Object, opts | |||
| return nil | |||
| } | |||
|
|
|||
| // DeleteAndWait deletes the given object and waits for it to be not available from the API Server. | |||
| func (e *Environment) DeleteAndWait(ctx context.Context, obj client.Object) error { | |||
There was a problem hiding this comment.
This is very close to CleanupAndWait (takes one object instead of several) but I think it's worth adding just for the semantics of being closer to the normal client calls.
There was a problem hiding this comment.
If the only difference is the vararg + the corresponding loop, let's please rename the existing func instead of introducing a new one
There was a problem hiding this comment.
(is it the only difference?)
There was a problem hiding this comment.
We shouldn't really rename it though, right - it would have to be deprecated. I'm fine with just using the existing function - I just always end up typing this in and wondering why it doesn't exist 😅
There was a problem hiding this comment.
+1 to rename current func to DeleteAndWait + adding
// Deprecated: use DeleteAndWait instead
func (e *Environment) CleanupAndWait(ctx context.Context, objs ...client.Object) error {
return DeleteAndWait(ctx, objs...)
}
So we give proper notice to consumers
There was a problem hiding this comment.
In my opinion we don't have to deprecate as it's all just internal: internal/test/envtest/environment.go :)
There was a problem hiding this comment.
I hadn't realized this was in internal - I'll rename the func and update this PR.
There was a problem hiding this comment.
I assumed that you didn't noticed it :)
There was a problem hiding this comment.
Can't actually create a 'Delete' function here as it interferes with the Client interface (answering probably how it ended up with this name in the first place), and I don't like having Cleanup paired with DeleteAndWait I think the easiest thing to do is just to continue to use cleanup as this is really not important 😄
killianmuldoon
force-pushed
the
fix-flaky-clusterclass-delete
branch
from
62ed2dc to
1ebd903
Compare
Signed-off-by: killianmuldoon <[email protected]>
killianmuldoon
force-pushed
the
fix-flaky-clusterclass-delete
branch
from
1ebd903 to
1bc99f6
Compare
k8s-ci-robot
added
size/XS
sbueringer
changed the title
|
Thx for the fix! /lgtm |
k8s-ci-robot
added
the
lgtm
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fabriziopandini 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
|
/cherry-pick release-1.3 |
|
@sbueringer: new pull request created: #7600 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. |
|
/cherry-pick release-1.2 |
|
@sbueringer: new pull request created: #7609 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. |
Add a
DeleteAndWaitfunction to our test package which allows waiting until an object is deleting before moving on.Fixes flake in https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_cluster-api/7587/pull-cluster-api-test-main/1595021734782701568