Skip to content
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 and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Graceful shutdown #690

Merged
merged 3 commits into from
Mar 6, 2020
Merged

Conversation

andreygolev
Copy link
Contributor

When I started to test graceful shutdown of worker type agents, it appeared that they don't wait for their running jobs before shutdown.
If I recall correctly, plugin.CleanupClients() kill plugins immediately, causing childs (in case of shell executor) to die.

This PR does the following:

  • SIGTERM received
  • Agent leaves cluster to not accept new jobs
  • Agent waits for active jobs to finish during 3h
  • Exits when running jobs finished or gracefulTimeout reached

Note: Running jobs successfully can send ExecutionDone to leader even after agent left cluster.

log.Info("agent: Gracefully shutting down agent...")
go func() {
plugin.CleanupClients()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When is this going to happen?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested it only with containers. Container exits, and the everything that was running inside dies also. So it doesn't matter in my case :)
But you're right, not everyone is using containers.
I'll add plugin.CleanupClients() somewhere below

cmd/agent.go Outdated
time.Sleep(1 * time.Second)
}

plugin.CleanupClients()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this gofmt'ed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, edited it with vi first :)

Copy link
Member

@vcastellm vcastellm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@vcastellm vcastellm merged commit 43c994f into distribworks:master Mar 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants