Skip to content

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