orchestrator: respect task stop grace period in stop-first updates (#3274) - #3298
Open
adilalperenciftci wants to merge 1 commit into
Open
adilalperenciftci wants to merge 1 commit into
adilalperenciftci wants to merge 1 commit into
Conversation
…oby#3274) when doing a stop-first service update, DelayStart was waiting on r.TaskTimeout (defaults to 1m) to give up on the old task. if the container had a StopGracePeriod longer then 1m, this timer expired while the old container was still shutting down and relased the new task too early, so both containers ended up running at the same time. this was a regression from 47ddece where service wasn't passed to DelayStart anymore. but since StopGracePeriod is already in oldTask.Spec, we can just read it directly from the container spec without needing the service object, and use max(TaskTimeout, grace + 5s buffer). fixes moby#3274 Signed-off-by: ADİL ALPEREN ÇİFTCİ <134228585+adilalperenciftci@users.noreply.github.com>
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
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.
when updating a service with stop-first order, DelayStart was waiting for
r.TaskTimeout (default 1m) to give up on the old task. if the old task
had a StopGracePeriod longer then 1 minute, the timer expired while the old
container was still shutting down and relased the replacement task, causing
both containers to run at the same time and violating stop-first.
this was a regression from 47ddece where service paramater was removed from
DelayStart. since StopGracePeriod is inside oldTask.Spec.GetContainer(), we can
read it directly from oldTask and use max(TaskTimeout, grace + 5s buffer)
so we wait for the container to actually stop before starting the new one.
fixes #3274