-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
buildd: graceful shutdown on termination signals #51
Conversation
util/appcontext/appcontext.go
Outdated
"golang.org/x/sys/unix" | ||
) | ||
|
||
// appcontext is a static context that reacts to termination signals of the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is appcontext
(or appContext
) now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment is supposed to be generally about the package. I'll move it to the Context()
function.
util/appcontext/appcontext.go
Outdated
@@ -29,6 +33,7 @@ func appContext() context.Context { | |||
cancel() | |||
retries++ | |||
if retries >= exitLimit { | |||
logrus.Errorf("forcing shutdown") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about got %d SIGTERM/SIGINTs, forcing shutdown
?
vendor.conf
Outdated
@@ -20,7 +20,7 @@ github.com/opencontainers/runc 429a5387123625040bacfbb60d96b1cbd02293ab | |||
github.com/Microsoft/go-winio v0.4.1 | |||
github.com/containerd/fifo 69b99525e472735860a5269b75af1970142b3062 | |||
github.com/opencontainers/runtime-spec 198f23f827eea397d4331d7eb048d9d4c7ff7bee | |||
github.com/containerd/go-runc 5d38580d03f4fbf2f09b2b0065deeeaf8d21aac2 | |||
github.com/containerd/go-runc df573a0e124617148f49720e1447061131675694 git://github.com/tonistiigi/go-runc.git |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: https://
seems preferred over git://
on github: https://stackoverflow.com/questions/19317264/does-github-support-git-protocol-for-pull
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
1b093b1
to
dd4c575
Compare
@AkihiroSuda Thanks for the review. Updated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (if green)
Currently shutting down buildd would potentially leak mounts and breaks the output of running clients. This update gracefully shuts down all current handlers before shutting down the process.
based on #49 (only last commit is new)includes fix for
go-runc
containerd/go-runc#19@AkihiroSuda