Fix VA Deletion Check in E2E Testing #75
Conversation
Check with Consistently instead of Eventually and use offset
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: razo7 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 |
razo7
changed the title
|
/test 4.13-openshift-e2e |
|
@razo7: The following test failed, say
Full PR test history. Your PR dashboard. 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. |
| @@ -405,7 +405,7 @@ func wasNodeRebooted(nodeName string, nodeBootTimeBefore time.Time) { | |||
|
|
|||
| // checkVaDeleted verifies if the va has already been deleted due to resource deletion | |||
| func checkVaDeleted(va *storagev1.VolumeAttachment) { | |||
| EventuallyWithOffset(1, func() bool { | |||
| ConsistentlyWithOffset(1, func() bool { | |||
There was a problem hiding this comment.
Why this change? For waiting until something happens, Eventually is the right tool 🤔
| By("checking if old pod has been deleted") | ||
| checkPodDeleted(pod) | ||
|
|
||
| By("checking if old volume attachment has been deleted") |
There was a problem hiding this comment.
Why the change in order? AFAIK in a real world the pod won't be deleted before its related VA is deleted (that's why we delete the VA actually). So first checking the VA makes more sense to me.
|
Closing the PR as it is not needed anymore:
|
Check with Consistently instead of Eventually and use offset