This repository was archived by the owner on Apr 20, 2026. It is now read-only.
fix never hitting the "TooLate (MissedStart)" condition#16
Merged
yangkev merged 1 commit intorelease-1.14.10-lyftfrom Apr 1, 2020
Merged
fix never hitting the "TooLate (MissedStart)" condition#16yangkev merged 1 commit intorelease-1.14.10-lyftfrom
yangkev merged 1 commit intorelease-1.14.10-lyftfrom
Conversation
Author
|
For review, choose the "split" option, and "hide whitespace changes". |
vllry
reviewed
Mar 30, 2020
vllry
reviewed
Mar 30, 2020
There was a problem hiding this comment.
This "count the events" part of the test is starting to get a little goofy, but I guess it's not worth ripping the bandage off now.
|
/lgtm |
yangkev
force-pushed
the
yangkev-test-toolate-patch
branch
from
fbf9850 to
542f1dc
Compare
yangkev
changed the base branch from
release-1.14.10-lyft
to
yangkev-make-tests-table-driven
yangkev
force-pushed
the
yangkev-test-toolate-patch
branch
from
542f1dc to
81ea72b
Compare
yangkev
force-pushed
the
yangkev-test-toolate-patch
branch
from
81ea72b to
ad3088c
Compare
vllry
reviewed
Apr 1, 2020
|
Still looks good. |
yangkev
changed the base branch from
yangkev-make-tests-table-driven
to
release-1.14.10-lyft
This issue was first raised in kubernetes#73169. In the past, we've seen that when the cronjob controller's syncAll loop time exceeds startingDeadlineSeconds, deadlines get missed. However, we currently do not get any notifications or telemetry on when deadlines are missed. Allowing the `MissedStart` event emission code path is the first step to getting telemetry on missed deadlines. There have been attempts to fix this by removing the "TooLate (MissedStart)" check wholesale: 1. kubernetes#81557 (open) 2. kubernetes#74058 (closed) I don't believe this check should be removed since the user should be notified when their deadline is missed. Changes: - test when startingDeadline elapses, should count as an unmet start time - make tests expect MissedStart event when startingDeadline is missed - fix TooLate (MissedStart) condition never being hit
yangkev
force-pushed
the
yangkev-test-toolate-patch
branch
from
ad3088c to
4928a20
Compare
theatrus
pushed a commit
that referenced
this pull request
Apr 7, 2020
This issue was first raised in kubernetes#73169. In the past, we've seen that when the cronjob controller's syncAll loop time exceeds startingDeadlineSeconds, deadlines get missed. However, we currently do not get any notifications or telemetry on when deadlines are missed. Allowing the `MissedStart` event emission code path is the first step to getting telemetry on missed deadlines. There have been attempts to fix this by removing the "TooLate (MissedStart)" check wholesale: 1. kubernetes#81557 (open) 2. kubernetes#74058 (closed) I don't believe this check should be removed since the user should be notified when their deadline is missed. Changes: - test when startingDeadline elapses, should count as an unmet start time - make tests expect MissedStart event when startingDeadline is missed - fix TooLate (MissedStart) condition never being hit
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This issue was first raised in kubernetes#73169.
In the past, we've seen that when the cronjob controller's syncAll loop
time exceeds startingDeadlineSeconds, deadlines get missed.
However, we currently do not get any notifications or telemetry on when
deadlines are missed.
Allowing the
MissedStartevent emission code path is the first stepto getting telemetry on missed deadlines.
There have been attempts to fix kubernetes#73169 by removing the "TooLate
(MissedStart)" check wholesale:
I don't believe this check should be removed since the user should be
notified when their deadline is missed.
Changes:
test when startingDeadline elapses, should count as an unmet start time
wrap table driven tests in t.Run to allow running individual tests
make tests expect MissedStart event when startingDeadline is missed
fix TooLate (MissedStart) condition never being hit