integration: revisit TestIssue9103#9337
Closed
samuelkarp wants to merge 1 commit intocontainerd:mainfrom
Closed
Conversation
Container execution and task context timeout are both set to 30 seconds, which introduces the opportunity for a race condition. Allow the container to run indefinitely (sleep inf) so that an exit does not race with the task timeout. Signed-off-by: Samuel Karp <[email protected]>
kzys
approved these changes
Nov 3, 2023
henry118
reviewed
Nov 4, 2023
| cntrOpts: []NewContainerOpts{ | ||
| WithNewSpec(oci.WithImageConfig(image), | ||
| withProcessArgs("sleep", "30"), | ||
| withProcessArgs("sleep", "inf"), |
Member
There was a problem hiding this comment.
IIUC the container task was never started in this test case, wonder why would this make a difference?
Member
There was a problem hiding this comment.
Yes. The test case just calls NewTask so that the runc-init is still waiting for signal from task.Start.
When the handleProcessExit doesn't have chance to update the status, the task.Status will return Created status. So I think we can use task.Wait to ensure that we can get stopped status when the expectedStatus is stopped.
Member
Author
There was a problem hiding this comment.
Thanks, I missed this.
Member
|
Can you elaborate on the race condition here? How would the 30s-sleep race with the task context timeout? |
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
5 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.
Container execution and task context timeout are both set to 30 seconds, which introduces the opportunity for a race condition. Allow the container to run indefinitely (sleep inf) so that an exit does not race with the task timeout.
I think this helps with #9334. This same commit against
release/1.6seems to be more reliable (I reran CI on #9335 several times to check). Opening this againstmainand then will cherry-pick back.