chore: add stress test that validates retries #587
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/L
| @@ -114,7 +114,7 @@ func thousandDeploymentsTest(ctx context.Context, c client.Client, invConfig inv | |||
| Expect(e.ErrorEvent.Err).To(BeNil()) | |||
| } | |||
| for _, e := range destroyerEvents { | |||
| Expect(e.PruneEvent.Error).To(BeNil(), "PruneEvent: %v", e.PruneEvent) | |||
| Expect(e.DeleteEvent.Error).To(BeNil(), "DeleteEvent: %v", e.DeleteEvent) | |||
There was a problem hiding this comment.
Why change PruneEvent to DeleteEvent?
Is there a reason that we run the same for loop 3 times instead of combining them?
There was a problem hiding this comment.
Why change PruneEvent to DeleteEvent?
For the Destroyer there are no PruneEvents. It replaces them with DeleteEvents. The PruneEvents are only used by the Applier. This change fixes the test to catch the right event errors.
Personally, I think they should all be DeleteEvents, since there's no real difference between them, but that's tech debt from years ago that we haven't gotten around to cleaning up.
Is there a reason that we run the same for loop 3 times instead of combining them?
No particular reason. That was just the existing behavior. I could clean it up like I did in the other test. That would make it easier to debug by printing the whole event list.
There was a problem hiding this comment.
added an function to clean it up
There was a problem hiding this comment.
Yeah, distinguishing between PruneEvent and DeleteEvent seemed like a good idea at the time. But agree with @karlkfi that they should all just be DeleteEvents.
karlkfi
force-pushed
the
karl-stress-retry
branch
4 times, most recently
from
a3b8e93 to
42805ed
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: karlkfi, mortent 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
- Duplicate the 1,000 Deployment test, but use a 1m reconcile timeout in a retry loop. - This verifies that the applier and destroyer are re-entrant at scale.
karlkfi
force-pushed
the
karl-stress-retry
branch
from
42805ed to
9dcbe66
Compare
|
/lgtm |
k8s-ci-robot
added
the
lgtm
a retry loop.