Fix removing tasks when a jobs service is removed by dperny · Pull Request #3112 · moby/swarmkit (original) (raw)
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation4 Commits1 Checks0 Files changed
Conversation
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
[ Show hidden characters]({{ revealButtonHref }})
Previously, the jobs orchestrators were missing the appropriate code to handle the deletion of a service. As a result, when a service was deleted, the tasks for that service would hang around indefinitely. It's likely that on a leadership change or restart, the tasks would have been deleted, but that's generally not an acceptable process as leadership changes or restarts aren't guaranteed.
Fixes this issue by adding event handling for services, similar to global or replicated services, that moves all tasks for a deleted service into Remove state, flagging them for deletion.
Previously, the jobs orchestrators were missing the appropriate code to handle the deletion of a service. As a result, when a service was deleted, the tasks for that service would hang around indefinitely. It's likely that on a leadership change or restart, the tasks would have been deleted, but that's generally not an acceptable process as leadership changes or restarts aren't guaranteed.
Fixes this issue by adding event handling for services, similar to global or replicated services, that moves all tasks for a deleted service into Remove state, flagging them for deletion.
Signed-off-by: Drew Erny derny@mirantis.com
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SGTM, but perhaps someone else wants to give it some eyes as well
@@ -201,6 +201,11 @@ func (o *Orchestrator) handleEvent(ctx context.Context, event events.Event) { |
---|
service = ev.Service |
case api.EventUpdateService: |
service = ev.Service |
case api.EventDeleteService: |
if orchestrator.IsReplicatedJob(ev.Service) | |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a blocker, but wondering if we should have IsJob()
and/or IsService()
utility for these
Not sure what's wrong with CircleCI, but as we're in the process of removing it altogether (things are running in GitHub actions now), I'll probably should just ignore it;
Using SSH Config Dir '/home/circleci/.ssh'
git version 2.38.0
Cloning git repository
Cloning into '.'...
remote: Enumerating objects: 45657, done.
remote: Counting objects: 100% (8854/8854), done.
remote: Compressing objects: 100% (4595/4595), done.
remote: Total 45657 (delta 3960), reused 8218 (delta 3699), pack-reused 36803
Receiving objects: 100% (45657/45657), 39.04 MiB | 36.74 MiB/s, done.
Resolving deltas: 100% (28065/28065), done.
Checking out branch
fatal: reference is not a tree: 75e563b4b3c63a740ac0aa413ad33c039d891f63
exit status 128
CircleCI received exit code 128