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

[bitnami/redis] Handle SIGTERM in kubectl-shared container #32085

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

Conversation

kppullin
Copy link

@kppullin kppullin commented Feb 20, 2025

Description of the change

When Redis Sentinel has the master service option enabled, the kubectl-shared container runs a script with an infinite while+sleep loop.

This script currently does not handle SIGTERM and causes the pod to hang upon termination until the terminationGracePeriodSeconds timeout elapses.

This change introduces a preStop hack to create a terminate file, which is used as a signal to the script that it should exit the infinite loop.

Possible drawbacks

No known drawbacks

Applicable issues

N/A

Checklist

  • Chart version bumped in Chart.yaml according to semver. This is not necessary when the changes only affect README.md files.
  • Title of the pull request follows this pattern [bitnami/<name_of_the_chart>] Descriptive title
  • All commits signed off and in agreement of Developer Certificate of Origin (DCO)

@github-actions github-actions bot added the triage Triage is needed label Feb 20, 2025
@github-actions github-actions bot requested a review from carrodher February 20, 2025 19:08
@carrodher
Copy link
Member

Thanks for your contribution! Please submit changes to multiple charts in separate PRs to facilitate automated testing and merge of pull requests.

@kppullin
Copy link
Author

Thanks @carrodher . I've removed the valkey changes.

@kppullin kppullin force-pushed the redis-sentinel-kubectl-exit-on-term branch from d3dd475 to a327f3c Compare February 20, 2025 21:00
@carrodher
Copy link
Member

Unfortunately, there is still one more thing... Your fork doesn't allow @bitnami-bot to access it as a maintainer, which is required for the GitHub Action in charge of automatically updating the README, CHANGELOG, etc to succeed.

remote: Permission to iherbllc/bitnami-charts.git denied to bitnami-bot.
fatal: unable to access 'https://github.com/iherbllc/bitnami-charts/': The requested URL returned error: 403
Error: Process completed with exit code 128.

Could you please check this?

@kppullin
Copy link
Author

@carrodher I've granted bitnami-bot access to the repo, however the invitation is pending and the pipeline still fails. Not sure what else I can do from my end.

@carrodher carrodher added verify Execute verification workflow for these changes in-progress redis and removed in-progress labels Feb 26, 2025
@github-actions github-actions bot removed the triage Triage is needed label Feb 26, 2025
@github-actions github-actions bot removed the request for review from carrodher February 26, 2025 11:18
@github-actions github-actions bot requested a review from migruiz4 February 26, 2025 11:18
@kppullin kppullin force-pushed the redis-sentinel-kubectl-exit-on-term branch 3 times, most recently from 55d2c6f to 461e8ab Compare February 28, 2025 22:34
…tl-shared`

container runs a script with an infinite `while+sleep` loop.

This script does not handle SIGTERM and causes the pod to hang upon termination
until the termination grace period elapses.

This change introduces a `preStop` hack to create a `terminate` file, which is used
as a singal to the script that it should exit the infinite loop.

Signed-off-by: Kevin Pullin <[email protected]>
@kppullin kppullin force-pushed the redis-sentinel-kubectl-exit-on-term branch from 90aacb8 to 28eee5e Compare March 4, 2025 19:59
Signed-off-by: Bitnami Containers <[email protected]>
@kppullin kppullin changed the title [bitnami/redis] - handle SIGTERM in kubectl-shared container [bitnami/redis] Handle SIGTERM in kubectl-shared container Mar 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in-progress redis verify Execute verification workflow for these changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants