test/e2e: add retry with backoff to all updates #3662
Conversation
Closes projectcontour#3660. Signed-off-by: Steve Kriss <krisss@vmware.com>
skriss
requested review from
danehans and
sunjayBhatia
and removed request for
a team
Codecov Report
@@ Coverage Diff @@
## main #3662 +/- ##
==========================================
- Coverage 77.08% 77.03% -0.05%
==========================================
Files 100 100
Lines 7121 7121
==========================================
- Hits 5489 5486 -3
- Misses 1511 1514 +3
Partials 121 121
|
| require.NoError(t, retry.RetryOnConflict(retry.DefaultBackoff, func() error { | ||
| if err := fx.Client.Get(context.TODO(), client.ObjectKeyFromObject(p), p); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| p.Spec.Routes[0].HealthCheckPolicy = &contourv1.HTTPHealthCheckPolicy{ | ||
| Path: "/status/418", | ||
| } | ||
|
|
||
| return fx.Client.Update(context.TODO(), p) | ||
| })) |
There was a problem hiding this comment.
Since this logic is ~repeated (get object, apply changes, update object), we could extract this to a test framework helper method. I was undecided on whether it was worth it or not, since you'd have to use some type-assertions or closure tricks to make it generic for types beyond just HTTPProxy, and it might be unnecessarily complicated.
There was a problem hiding this comment.
The status updating code had a similar problem, and I used a Mutator function there to handle it, I'm not sure how good I feel about it, looking back. Although this is repetitive, this does have the advantage of clarity. I agree we should leave this for now, and see if we should dry it up later.
| require.NoError(t, retry.RetryOnConflict(retry.DefaultBackoff, func() error { | ||
| if err := fx.Client.Get(context.TODO(), client.ObjectKeyFromObject(p), p); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| p.Spec.Routes[0].HealthCheckPolicy = &contourv1.HTTPHealthCheckPolicy{ | ||
| Path: "/status/418", | ||
| } | ||
|
|
||
| return fx.Client.Update(context.TODO(), p) | ||
| })) |
There was a problem hiding this comment.
The status updating code had a similar problem, and I used a Mutator function there to handle it, I'm not sure how good I feel about it, looking back. Although this is repetitive, this does have the advantage of clarity. I agree we should leave this for now, and see if we should dry it up later.
|
cc @danehans we may want to do something similar in the operator e2e's to try to avoid some of the flakes we've been seeing. |
Closes #3660.
Signed-off-by: Steve Kriss krisss@vmware.com