Graceful job termination #26311

jupe · 2020-07-03T09:50:46Z

jupe
Jul 3, 2020

When cancelling job there is chance that running software handle some resources that needs to release when job is cancelled. To make this robust way GitHub Actions should send signals for running process so that application can tear-down tasks properly. Gentle terminator would send first SIGINT, wait some seconds and if app still doesn’t die it would send another signal SIGTERM and finally SIGKILL to force terminate it eventually.

Probably GH already manage this someway, but at least I didn’t find any documentation about subject. would be good to document it properly how it behaviours at the moment so it’s easier to propose changes if needed.

Here is nice document for Jenkins about same issue: https://gist.github.com/datagrok/dfe9604cb907523f4a2f

Answered by BrightRan

Jul 20, 2020

@jupe,

According the introduction from the engineering team, after the user click “Cancel workflow”:

The server will re-evaluate job-if condition on all running jobs.
If the job condition is always(), it will not get canceled.
For the rest of the jobs that need cancellation, the server will send a cancellation message to all the runners.
Each runner has 5 minutes to finish the cancellation process before the server force terminate the job.
The runner will re-evaluate if condition on the current running step.
If the step condition is always(), it will not get canceled.
Otherwise, the runner will send Ctrl-C to the action entry process (node for javascript action, docker for c…

View full answer

BrightRan · 2020-07-06T02:21:19Z

BrightRan
Jul 6, 2020

@jupe,

Thanks for your feedback.
I have created an internal ticket to help you report this question to the appropriate engineering team for further discussion and evaluation. If they have any update, I will notify you in time, and sometimes the appropriate engineers may directly reply you here.

1 reply

lundstrj Oct 11, 2023

Hello @BrightRan any movement since 2020?

BrightRan · 2020-07-20T01:34:38Z

BrightRan
Jul 20, 2020

@jupe,

According the introduction from the engineering team, after the user click “Cancel workflow”:

The server will re-evaluate job-if condition on all running jobs.
If the job condition is always(), it will not get canceled.
For the rest of the jobs that need cancellation, the server will send a cancellation message to all the runners.
Each runner has 5 minutes to finish the cancellation process before the server force terminate the job.
The runner will re-evaluate if condition on the current running step.
If the step condition is always(), it will not get canceled.
Otherwise, the runner will send Ctrl-C to the action entry process (node for javascript action, docker for container action, and bash/cmd/pwd for run action), if the process doesn’t exit within 7500ms, the runner will send Ctrl-Break to the process, then wait for 2500ms for the process to exit. the runner will terminate the process tree if the process is still running.
The runner will try all the following steps that have condition sets to always() as many as it can within the 5 minutes cancellation timeout.

Hope this can help you understand better.

3 replies

briceburg Jun 20, 2023

this does not seem correct. the cancelled job is not sent SIGINT/Ctrl-C , it exits immediately without sending any termination messages. https://github.com/orgs/community/discussions/26311#discussioncomment-6231898

also, is the 7500ms configurable? if each runner has 5minutes to finish cancellation process, 7.5s for process termination seems way to low.

justinmchase Dec 4, 2023

We are using shell: bash and run: terraform ... and I am seeing the same thing as @briceburg, the process is not gracefully terminated it just abruptly exits and leaves our terraform state file in a locked state.

Do we need to use traps or what is the proper way to terminate a process in a bash shell action?

justinmchase Mar 21, 2024

I ended up using a node20 action and making a simple js file which just uses nodes child_process module to invoke terraform and process.on('SIGINT', ...) to propagate cancellation signals. For some reason I just could not make it work with the bash action.

acvejic · 2021-07-01T09:02:32Z

acvejic
Jul 1, 2021

Hi @brightran

I have a terraform/terragrunt action that when it is canceled it leaves state file locked in AWS dynamoDB. Then I need to force unlock it and most of the resources are not in state file so my state is corrupted.

jobs:
    terragrunt:  
      runs-on: ubuntu-18.04
      if: "!(contains(github.event.head_commit.message, 'skip all') || \
            startsWith(github.event.head_commit.message, 'Merge branch ''develop'' into') \
          )"
...
          - name: terragrunt apply
            if: "!contains(github.event.head_commit.message, 'skip tf') && env.DIR_EXISTS=='true' && env.DESTROY_QA_ENV!='true'"
            working-directory: .terraform/environments/${{ steps.parse_branch.outputs.qa_name }}
            run: |
                 terragrunt apply -auto-approve

And when I cancel the job all I see in action console is:

module.sisCodeDeploy.null_resource.wait_for_admin[0]: Still creating... [2m30s elapsed]
Error: The operation was canceled.

So it does not look like job allowed terraform to do all the necessary steps, to save state, release lock etc.
Any suggestions what to do?

0 replies

james-portman-az · 2022-02-10T11:14:35Z

james-portman-az
Feb 10, 2022

Hi,

This is happening for us too, terraform does not have chance to release the state locks.
It does not appear to be working as documented, it just immediately terminates.

If it is working as documented then it would be good to be able to extend the 7500ms shutdown time

0 replies

ruohola · 2022-03-27T15:14:15Z

ruohola
Mar 27, 2022

I can also confirm that using cancel-in-progress: true with a GitHub Actions CI pipeline that runs terraform plan does not work well. If the plan step is canceled it will just immediately terminate and not gracefully release the lock.

I’ve tested locally sending SIGINT and SIGTERM signals to a running terraform plan and it does exit gracefully and releases the lock. GitHub Actions seems to be sending some other, more aggressive signal, when killing the workflow.

0 replies

jsoref · 2022-03-28T15:05:31Z

jsoref
Mar 28, 2022

If you’re using terraform, I’d suggest using atlantis:

  <a href="https://www.runatlantis.io/" target="_blank" rel="noopener">runatlantis.io</a>

Terraform Pull Request Automation | Atlantis

Atlantis: Terraform Pull Request Automation

Yes, it means you’re running a small VM somewhere, but, it also means you don’t have to worry about it being killed.

0 replies

pauldraper · 2022-06-07T20:39:57Z

pauldraper
Jun 7, 2022

I confirmed; Github runner is using SIGKILL for cancel-in-progress.

1 reply

briceburg Jun 20, 2023

Ideally the runner would attempt a SIGINT, to allow the executing process time to cleanup, before SIGKILL / halting...

@BrightRan suggests that a SIGINT is sent, but clearly this is not happening per the job logs (terraform will log that it is trying to gracefully exit)... again, this is requesting we try to gracefully terminate processes in jobs where concurrency has cancel-in-progress: true

CharlesLiu-TOPNetwork · 2022-10-25T08:43:41Z

CharlesLiu-TOPNetwork
Oct 25, 2022

Can we make job termination operate more gentle?
Uncompleted compilation file won't be clean after cancel-in-progress KILL make process.

0 replies

cornerman · 2023-02-24T21:50:04Z

cornerman
Feb 24, 2023

This seems like a common issue for everybody running tasks like terraform - that need to gracefully exit. You can and should disable fail-fast in the github action, so jobs don't get cancelled if a new one arrives. But having the cancel button in the UI which could be accidentally clicked by some user is really dangerous. In case of terraform that means potentially leaking resources - that need to be cleaned up manually afterwards.

Is there any solution for this?

0 replies

ringerc · 2023-06-21T04:07:00Z

ringerc
Jun 21, 2023

I wrote a demo to illustrate and confirm the behaviour described above, which really should be in the docs.

At least on a Linux runner, "CTRL-C" means SIGINT and "CTRL-Break" (also CTRL-) means SIGTERM.

.github/workflows/cancel-test.yaml:

name: Cancel test
on:
  - workflow_dispatch
jobs:
  cancel-test:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v3
      - run: |
          exec python3 signaller.py
      - if: always()
        run: |
          cat signaller.log

signaller.py:

#!/usr/bin/env python3

import time
import signal
import sys

signaller_log = open("signaller.log", "w")

def handler(signum, frame):
  signame = signal.Signals(signum).name
  print(f'Signal handler called with signal {signame} ({signum}) at {time.time()}', file=sys.stderr)
  print(f'Signal handler called with signal {signame} ({signum}) at {time.time()}', file=signaller_log)
  sys.stderr.flush()
  signaller_log.flush()

for sig in (signal.SIGINT, signal.SIGTERM, signal.SIGHUP, signal.SIGUSR1, signal.SIGUSR2, signal.SIGABRT, signal.SIGQUIT, signal.SIGPIPE):
  signal.signal(sig, handler)

while True:
  print(f"tick {time.time()}", file=sys.stderr)
  print(f"tick {time.time()}", file=signaller_log)
  sys.stderr.flush()
  signaller_log.flush()
  time.sleep(1);

in this simple case the result observed was consistent with above docs - SIGINT, about 7.5s, SIGTERM, about 2.5s, and presumably SIGKILL + cleanup.

But ... note that I've exec'd the Python interpreter running the script so it's the leader process.

If I don't exec the interpreter, the Python script dies without warning or any chance to clean anything up!

It looks like the github actions runner probably waits for the session leader process to exit, then hard-kills anything under it when it exits.

It doesn't appear to deliver signals to the process tree by signalling the process group; AFAICS it only signals the leader process. So the leader must install a signal handler that explicitly propagates signals to child processes then waits for them to exit.

0 replies

ringerc · 2023-06-22T00:55:44Z

ringerc
Jun 22, 2023

I wrote this up better in a demo at https://github.com/ringerc/github-actions-signal-handling-demo since I wasn't satisfied with the answers from @BrightRan above, nor my earlier quick tests.

It's a right mess. It looks like you really need to rely on if: always() blocks, and assume the whole process tree of a step can be nuked without warning. It's possible to implement signal-propagation to child procs, but it's ugly, and without any way to change the timeouts it's still going to be fragile.

2 replies

briceburg Jun 29, 2023

✅ this is super helpful @ringerc . I've opened a discussion to track a configurable grace period here: https://github.com/orgs/community/discussions/59652

ringerc Jul 24, 2023

Thanks @briceburg . It's especially difficult to correctly handle failures because a given step's processes could be SIGKILL'd without a chance to clean up, but per the test case I supplied it's also possible for processes from a step to keep running after the step is cancelled, so they're still running when an if: always() cleanup or recovery step runs. And it's easy to leak such processes into the background if, say, you're using shell job control to run two coupled commands in a step.

You can imagine that this would be bad if, say, your if: always() or if: failed() step does a terraform force-unlock to make sure that a terraform lock is released when a job gets cancelled, but the terraform process run by the cancelled step got detached from the step's process group so it kept running in the background when the step got killed...

AFAICS the only correct way to do it is to:

have your critical commands write pid-files when they start, recording the pid and process start time
have them delete the pid file on termination
in an if: always() or if: failure() step, test if the pid file exists, and if it does:
- test the pid it points to is still running, and the pid it points to has the same process start time as in the pid file (to detect pid reuse). If still running, kill -KILL the process tree of the target process (and hope nothing detached/daemonized off it).
- now that you know for sure the critical process is terminated, perform your cleanup action, which must not take longer than 7.5 seconds allowing for possible worker load, scheduler delays, network I/O etc, so it'd better be simple and minimal

Even then, this is subject to a race between the critical process succeeding and when it then deletes its pid file on completion or otherwise locally records success. So your cleanup step must be idempotent and not do anything wrong if the critical step actually succeeded, then crashed or timed out in the moment before it could delete its pid file.

kedare · 2023-07-28T10:00:37Z

kedare
Jul 28, 2023

Has something changed in the way Github processing of the job termination ?
I am not receiving any signal that can be catched anymore, everything just gets killed directly.

1 reply

ringerc Aug 2, 2023

@kedare you won't get a signal unless your process is the session leader. If you run it via a shell, it won't get a signal. See the detailed demos I wrote in the repo linked above.

restfulhead · 2023-09-13T14:40:16Z

restfulhead
Sep 13, 2023

Why is this marked as answered when it isn't really answered? How to avoid e.g. terraform apply to be killed without cleaning up?

3 replies

ringerc Sep 17, 2023

Because GH don't really expect Actions to work reliably I guess. It's not even documented. Maybe they figure if they make it bad enough people will be driven to try Azure Devops.

jsoref Sep 18, 2023

Here's a way to avoid the problem: #26311 (comment)

gerbyzation Oct 25, 2023

Yes this is quite annoying. For terraform we are using terraform cloud, so that the actual apply is triggered but not executed on github actions runners. If a job is terminated this won't brick the terraform operation halfway through.

We have a similar problem with helm releases, which can get stuck in an upgrade-pending state when the job is killed while updating a release. Our current workaround is to have another step that will run if a job is cancelled and rollback if the release is in a bad state.

      - name: Rollback if cancelled
        if: always() && (steps.helm-update.outcome == 'cancelled' || steps.helm-update.outcome == 'failure')
        run: |
          helm status ${{ inputs.chart_values_file_name }} | \
            grep -qi 'pending-upgrade' && \
            helm rollback ${{ inputs.chart_values_file_name }}

breathe · 2023-11-15T00:31:47Z

breathe
Nov 15, 2023

Could we maybe adjust the wrapper so that it invokes terraform via tini -- krallin/tini#8 -- or something like it which 'does a good job' (I'm guessing?) of forwarding signals to child processes ...

0 replies

breathe · 2023-11-15T02:02:45Z

breathe
Nov 15, 2023

This is the workaround I'm using now ...

No terraform wrapper because the node process which implements the wrapper doesn't (and can't?) forward signals

    - uses: hashicorp/setup-terraform@v2
      with:
        terraform_wrapper: false

dumb bash script wrapper: forward as much as possible to terraform and only respond to the first signal to maximize the time terraform will spend trying to cleanup. Save this to a file in the repo (${{ github.workspace }}/terraform/terraform-ci.bash in example code below)

#!/bin/bash 

COUNTER=0
_term() { 
  echo "Caught SIGTERM signal!" 

  if [[ $COUNTER -lt 1 ]] ; then
    echo "Passing signal to terraform"
    kill -TERM "$child" 2>/dev/null
  else
    echo "Already passed signal to terraform"  
  fi

  let COUNTER++
}

_int() { 
  echo "Caught SIGINT signal!"

  if [[ $COUNTER -lt 1 ]] ; then
    echo "Passing signal to terraform"
    kill -INT "$child" 2>/dev/null
  else
    echo "Already passed signal to terraform"  
  fi

  let COUNTER++
}

_other() { 
  echo "Caught OTHER signal!" 

  if [[ $COUNTER -lt 1 ]] ; then
    echo "Passing signal to terraform"
    kill -INT "$child" 2>/dev/null
  else
    echo "Already passed signal to terraform"  
  fi

  let COUNTER++
}

trap _term SIGTERM
trap _int SIGINT

trap _other SIGHUP
trap _other SIGUSR1
trap _other SIGUSR2
trap _other SIGABRT
trap _other SIGQUIT
trap _other SIGPIPE

terraform "$@" &
child=$! 
wait "$child"

use exec when invoking terraform to avoid an extra shell process in the signal propogation chain

    - name: Terraform Apply
      run: exec ${{ github.workspace }}/terraform/terraform-ci.bash apply -input=false ...

2 replies

justinmchase Dec 4, 2023

Very nice, this is the right answer.

What signals are you seeing actually trapped in _other?

breathe Dec 5, 2023

Not seeing any output through the _other handlers so far. Put that there initially to try and get visibility into what github actions does -- and then left it there for a little extra visibility in case something changes

npwolf · 2024-03-20T23:03:28Z

npwolf
Mar 20, 2024

Here is my workaround. Like @breathe , I don't use the terraform wrapper. Aside from that, I instead use tini to make sure all signals get propagated:

      - name: Setup terraform
        uses: hashicorp/setup-terraform@v3
        with:
          terraform_wrapper: false

      - name: Initialize terraform environment
        run: |
          terraform init
          terraform validate
          sudo apt install -y tini  

      - name: terraform apply
        # Need tini so terraform gets SIGTERM appropriately 
        run: |
          exec tini -s -g -- terraform apply -auto-approve

Which results in something like the following when the job gets cancelled:

Interrupt received.
Please wait for Terraform to exit or data loss may occur.
Gracefully shutting down...

Stopping operation...
╷
│ Error: execution halted
│ 
│ 
╵
╷
│ Error: execution halted
│ 
│ 
╵
╷
│ Error: waiting for ECS Service (arn:aws:ecs:....) create: context canceled
│ 
│   <error about resource>
│ 
╵
Error: The operation was canceled.

1 reply

r34son Mar 21, 2024

Great! Thanks a lot

dimikot · 2024-04-11T20:24:35Z

dimikot
Apr 11, 2024

I built a simple tool which speeds up the signals propagation based on the above material:
dimikot/signal-fanout

It can be just put to shell: key of the step specification. And then, all of the processes launched from that step (including nested and called without exec) will receive SIGTERM immediately on the workflow cancellation.

...
jobs:
  my-job:
    runs-on: ubuntu-latest
    steps:
      - name: Long-running step
        shell: signal-fanout {0}
        run: |
          for i in $(seq 1 30); do echo "$(date): $i"; sleep 1; done

0 replies

Graceful job termination #26311

Replies: 17 comments · 14 replies

Replies: 17 comments 14 replies