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

tfexec: add InterruptChannel option to allow a user triggered interrupt #334

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

twmb
Copy link

@twmb twmb commented Aug 31, 2022

This is an alternative to #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.

Refs

@hashicorp-cla
Copy link

hashicorp-cla commented Aug 31, 2022

CLA assistant check
All committers have signed the CLA.

@twmb twmb force-pushed the twmb/interrupt-channel branch 2 times, most recently from 8004d24 to fe44afb Compare August 31, 2022 20:20
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 twmb force-pushed the twmb/interrupt-channel branch from fe44afb to 06beb79 Compare September 1, 2022 01:14
go func() {
select {
case <-interruptCh.(<-chan struct{}):
cmd.Process.Signal(os.Interrupt)

Choose a reason for hiding this comment

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

I think SIGINT is not available in Windows and that's what's making the test to fail 🤔

Copy link

@simon0191 simon0191 Sep 1, 2022

Choose a reason for hiding this comment

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

I'm wondering why cmd_linux can't be used for macos as well?

Choose a reason for hiding this comment

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

I've made cmd_linux -> cmd_unix as the implementation should work in all unix systems
44715fe

@simon0191 simon0191 force-pushed the twmb/interrupt-channel branch 2 times, most recently from 32b2eb7 to 6594828 Compare September 1, 2022 09:28
simon0191 and others added 2 commits September 1, 2022 11:35
All unix platforms should support SIGKILL and SIGINT.
This change uses the implementation of cmd that sends SIGKILL and SIGINT
in all unix platforms.
The list of platform tags is extracted from go/build/syslist.go and
emulates what's done by the unix build tag introduced in go1.19.
@bilalcaliskan
Copy link

is there any update about this feature request and this PR? cheers

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.

4 participants