Increase timeout in ExpectMirroredRequest #4206
Conversation
k8s-ci-robot
added
release-note-none
|
/assign @robscott |
conformance/utils/http/mirror.go
Outdated
| } | ||
| return false | ||
| }, 60*time.Second, time.Millisecond*100, `Couldn't find mirrored request in "%s/%s" logs`, mirrorPod.Namespace, mirrorPod.Name) | ||
| }, 2*time.Minute, time.Second*1, `Couldn't find mirrored request in "%s/%s" logs`, mirrorPod.Namespace, mirrorPod.Name) |
There was a problem hiding this comment.
While I don't think the interval needs to be configurable, I think the timeout should be. Would recommend a new entry in https://github.com/kubernetes-sigs/gateway-api/blob/main/conformance/utils/config/timeout.go for this.
There was a problem hiding this comment.
good point, what do you think about reusing RequestTimeout from the config?
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kl52752, robscott 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
| } | ||
| return false | ||
| }, 60*time.Second, time.Millisecond*100, `Couldn't find mirrored request in "%s/%s" logs`, mirrorPod.Namespace, mirrorPod.Name) | ||
| }, timeoutConfig.RequestTimeout, time.Second*1, `Couldn't find mirrored request in "%s/%s" logs`, mirrorPod.Namespace, mirrorPod.Name) |
There was a problem hiding this comment.
very nit: time.Second*1 == time.Second :) you can get rid of the number here.
I will lgtm here, keep it on the queue for some next batch of fixes!
Thanks!
|
/lgtm |
k8s-ci-robot
added
the
lgtm
4 participants
What type of PR is this?
/area conformance-test
What this PR does / why we need it:
Increase pooling time and timeout for checking the condition in HTTPRouteRequestMirror test suite.
Which issue(s) this PR fixes:
Fixes # NA
Does this PR introduce a user-facing change?: