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

Diagnostic mode handle SIGTERM #80

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

Conversation

a-thomas-22
Copy link
Collaborator

No description provided.

@a-thomas-22 a-thomas-22 requested a review from a team as a code owner September 16, 2024 18:55
@a-thomas-22 a-thomas-22 enabled auto-merge (squash) September 16, 2024 18:56
Copy link
Contributor

@lambchr lambchr left a comment

Choose a reason for hiding this comment

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

nice, looks good in general, I noticed a couple of issues when trying this out on a test pod. Let me know if you think this is due to an error with the test pod I used:

apiVersion: v1
kind: Pod
metadata:
  name: clamb-test
spec:
  containers:
  - name: clamb
    image: offchainlabs/nitro-node:v3.0.3-3ecd01e
    command: ["/bin/sh"]
    args:
      - -c
      - |
        #!/usr/bin/env bash
        set -e pipefail
        trap 'exit 0' TERM
        sleep infinity &
        wait $!
  # I used this to check that the signal handling was working
  terminationGracePeriodSeconds: 10

- -c
- |
#!/usr/bin/env bash
set -eo pipefail
Copy link
Contributor

Choose a reason for hiding this comment

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

I got the following error when running this on a test pod /bin/sh: 2: set: Illegal option -o pipefail

Suggested change
set -eo pipefail
set -e pipefail

#!/usr/bin/env bash
set -eo pipefail

trap 'exit 0' SIGTERM
Copy link
Contributor

Choose a reason for hiding this comment

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

I got the following error when running this on a test pod trap: SIGTERM: bad trap seems like some shells expect the signal without the SIG part

Suggested change
trap 'exit 0' SIGTERM
trap 'exit 0' TERM

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.

2 participants