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

release lock on SIGTERM too. #61

Closed
mildred opened this issue Dec 1, 2016 · 7 comments
Closed

release lock on SIGTERM too. #61

mildred opened this issue Dec 1, 2016 · 7 comments
Labels
bug Something isn't working

Comments

@mildred
Copy link
Contributor

mildred commented Dec 1, 2016

On SIGINT (Interrupt from keyboard), terragrunt release the lock, but not on SIGTERM (Termination signal). This should probably be fixed. See locks/lock.go.

diff --git a/locks/lock.go b/locks/lock.go
index 58ace31..13af32b 100644
--- a/locks/lock.go
+++ b/locks/lock.go
@@ -5,6 +5,7 @@ import (
        "github.com/gruntwork-io/terragrunt/errors"
        "os"
        "os/signal"
+       "syscall"
 )
 
 // Every type of lock must implement this interface
@@ -48,6 +49,7 @@ func WithLock(lock Lock, action func() error) (finalErr error) {
        // the blocking call to action() to return normally.
        signalChannel := make(chan os.Signal, 1)
        signal.Notify(signalChannel, os.Interrupt)
+       signal.Notify(signalChannel, syscall.SIGTERM)
        go func() { util.Logger.Printf("Caught signal '%s'. Terraform should be shutting down gracefully now.", <- signalChannel) }()
 
        return action()
@mildred
Copy link
Contributor Author

mildred commented Dec 2, 2016

Additionally, terragrunt should forward SIGTERM signals to terraform. There is no guaantee that terraform will have received it. See also hashicorp/terraform#10459

@mildred
Copy link
Contributor Author

mildred commented Dec 2, 2016

In fact, on some cases (default on systemd services) SIGTERM is sent to all processes in the cgroup. In other cases (sysvservice, docker) the SIGTERM signal is only sent to the parent process.

@brikis98 brikis98 added bug Something isn't working help wanted labels Dec 2, 2016
@brikis98
Copy link
Member

brikis98 commented Dec 2, 2016

Good point. Looks like you have some code written for this already. Would you like to submit a PR?

@mildred
Copy link
Contributor Author

mildred commented Dec 5, 2016

Yes, I will submit a PR once I get something with terraform. There are two aspects:

  • handle SIGTERM as well as SIGINT
  • make sure terraform has received SIGTERM. When we type Ctrl-C in a terminal, the VT will make sure that all VT clients are sent a SIGINT signal. With SIGTERM, we are not sure that terraform received SIGTERM and we must probably forward the signal.

Are you ok with forwarding the SIGTERM signal ?

When forwarding SIGTERM we must make sure terraform can handle it. Currently it does not respond to SIGTERM and doesn't cleanly finish like it does with Ctrl-C. I'm currently submitting a PR on terraform for that.

@brikis98
Copy link
Member

brikis98 commented Dec 5, 2016

Probably makes sense to wait for your PR to be merged into Terraform itself.

@mildred
Copy link
Contributor Author

mildred commented Dec 5, 2016

yes

@brikis98
Copy link
Member

brikis98 commented Jan 8, 2017

This should have been fixed by #72.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants