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

Support graceful cancellation #332

Closed

Conversation

simon0191
Copy link

@simon0191 simon0191 commented Aug 30, 2022

This PR adds support for graceful cancellation and a GracefulShutdownTimeout option for apply and destroy commands.

Currently when the context is cancelled a SIGKILL is sent to Terraform. This is the default behaviour of exec.CommandContext . The SIGKILL forcefully kills the Terraform process and therefore it doesn’t start a graceful shutdown which sometimes ends up with the state lock not being released.

The proposed fix is that on context cancellation, a SIGINT is sent so that Terraform starts a graceful shutdown with the hope of releasing the lock and allowing providers to gracefully terminate, and then, after GracefulShutdownTimeout if the command hasn’t finished, then a SIGKILL is sent to forcefully terminate the process.


When a command is started, the command that is created is run without a context (equivalent to passing context.Background()), and in a concurrent go routine the cancellation of the parent context is monitored.

When the parent context is cancelled, a SIGINT is sent to the command. After GracefulShutdownTimeout passes, if the command hasn't finished, a SIGKILL is sent to the command to forcefully kill it.

The GracefulShutdownTimeout is also useful so that the caller of tf-exec is able to configure for how long it's willing to wait for the cancellation, which depends on how each provider handles cancellations.

Refs

@hashicorp-cla
Copy link

hashicorp-cla commented Aug 30, 2022

CLA assistant check
All committers have signed the CLA.

@simon0191 simon0191 force-pushed the ss/graceful-cancellation branch 5 times, most recently from 6a4226d to 59b00d5 Compare August 31, 2022 13:53
twmb added a commit to redpanda-data/terraform-exec that referenced this pull request Aug 31, 2022
This is an alternative to hashicorp#332, adding an option that enables the user
to implement graceful shutdowns for Apply and Destroy operations.

Rather than adding an option that changes the behavior of the input
context, we instead add an option that specifically sends an interrupt
to the terraform process. The input context behavior remains unchanged.

This requires the caller to do a bit more orchestration work for
timeouts, but keeps context truer to the "abandon work" intent. This
also allows users to force quit _even if_ they are in the middle of a
graceful shutdown, rathern than having one behavior mutually exclusive
with the other.
twmb added a commit to redpanda-data/terraform-exec that referenced this pull request Aug 31, 2022
This is an alternative to hashicorp#332, adding an option that enables the user
to implement graceful shutdowns for Apply and Destroy operations.

Rather than adding an option that changes the behavior of the input
context, we instead add an option that specifically sends an interrupt
to the terraform process. The input context behavior remains unchanged.

This requires the caller to do a bit more orchestration work for
timeouts, but keeps context truer to the "abandon work" intent. This
also allows users to force quit _even if_ they are in the middle of a
graceful shutdown, rathern than having one behavior mutually exclusive
with the other.
twmb added a commit to redpanda-data/terraform-exec that referenced this pull request Aug 31, 2022
This is an alternative to hashicorp#332, adding an option that enables the user
to implement graceful shutdowns for Apply and Destroy operations.

Rather than adding an option that changes the behavior of the input
context, we instead add an option that specifically sends an interrupt
to the terraform process. The input context behavior remains unchanged.

This requires the caller to do a bit more orchestration work for
timeouts, but keeps context truer to the "abandon work" intent. This
also allows users to force quit _even if_ they are in the middle of a
graceful shutdown, rathern than having one behavior mutually exclusive
with the other.
@simon0191
Copy link
Author

Closing in favor of #334 which is a much simpler and nicer implementation

@simon0191 simon0191 closed this Aug 31, 2022
twmb added a commit to redpanda-data/terraform-exec that referenced this pull request Sep 1, 2022
This is an alternative to hashicorp#332, adding an option that enables the user
to implement graceful shutdowns for Apply and Destroy operations.

Rather than adding an option that changes the behavior of the input
context, we instead add an option that specifically sends an interrupt
to the terraform process. The input context behavior remains unchanged.

This requires the caller to do a bit more orchestration work for
timeouts, but keeps context truer to the "abandon work" intent. This
also allows users to force quit _even if_ they are in the middle of a
graceful shutdown, rathern than having one behavior mutually exclusive
with the other.
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.

None yet

2 participants