test: more resilient “wait for successful pod readiness checks” #2015
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jackfrancis 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 |
| PrintPodsLogs(podPrefix, namespace) | ||
| return false, errors.Errorf("Pods from deployment (%s) in namespace (%s) have been checked out as all Ready %d times, but NotReady %d times. This behavior may mean it is in a crashloop", podPrefix, namespace, successCount, failureCount) | ||
| mostRecentWaitOnSuccessesErr = result.err | ||
| if mostRecentWaitOnSuccessesErr == nil { |
There was a problem hiding this comment.
Total nit, but might be good to invert the if statement to reduce nesting. For example:
if mostRecentWaitOnSuccessesErr != nil {
continue
}
// do the rest of the things without the if nestingThere was a problem hiding this comment.
This is interesting. I propose we bikeshed this sort of abstractly (at later time), and then apply the team-approved pattern generally. Because this pattern is the one we currently use to handle channel responses in these goroutine validations.
Codecov Report
@@ Coverage Diff @@
## master #2015 +/- ##
=======================================
Coverage 76.73% 76.73%
=======================================
Files 135 135
Lines 20574 20574
=======================================
Hits 15787 15787
Misses 3871 3871
Partials 916 916 |
Reason for Change:
This PR tweaks the way we validate that a pod or common set of pods is in a persistent, reliable "Ready" state. Instead of tracking and incrementing "successful" checks, and comparing to "unsuccessful" checks, we instead try to detect "flaps", which are in fact normal Kubernetes pod behaviors, and ensure that the transitions between Ready and non-Ready (i.e., a flap) are within a tolerable spectrum.
Specifically:
Once we get a positive Ready signal, we continue attempting to accumulate the desired success count until we detect a non-Ready signal. A ready --> non-ready transition counts as a single "flap". We then wait for the re-occurrence of a successive Ready signal, which then counts towards the targeted number of successes. Rinse, repeat.
We will return successful if the number of desired Ready counts is reached, and is at least 2 more than the number of flaps. In practice this means that a simple check of "just give me signal that one Ready state has occured" works fine (the "flap" detection will never be engaged in this scenario); and that 4 is probably the minimum number of successes to target for scenarios where you want more than 1 Ready count. 2 is super aggressive as basically any one flap will fail the test, and 3 is arguably not tolerant enough for "normal" reconciliation conditions.
As such all of the relevant "wait for more than 1 Ready signal" usages are set to 4.
Also renamed the func to
WaitOnSucessesto more easily disambiguate from the single-validation-callWaitOnReady.Issue Fixed:
Requirements:
Notes: