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/rabbitmq] Fix incorrect configuration of TCP listen options by memoryHighWatermark settings #31336

Merged
merged 4 commits into from
Jan 15, 2025

Conversation

Kulivox
Copy link
Contributor

@Kulivox Kulivox commented Jan 13, 2025

Description of the change

This PR fixes a small bug reported in #31221. It fixes a bug, which made configuring tcpClientOptions impossible, if memoryHighWatermark was disabled.

Benefits

Makes tcpClientOptions configurable independently from memoryHighWatermark

Possible drawbacks

Applicable issues

Additional information

As of now, the memoryHighWatermark is disabled by default, which the default bitnami tcpClientOptions are disabled as well. This PR makes them configurable using tcpClientOptions.enabled field, and makes the field true by default.

Checklist

  • Chart version bumped in Chart.yaml according to semver. This is not necessary when the changes only affect README.md files.
  • Variables are documented in the values.yaml and added to the README.md using readme-generator-for-helm
  • 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)

Signed-off-by: Bitnami Containers <[email protected]>
@carrodher carrodher added verify Execute verification workflow for these changes in-progress labels Jan 13, 2025
@github-actions github-actions bot removed the triage Triage is needed label Jan 13, 2025
@github-actions github-actions bot removed the request for review from carrodher January 13, 2025 13:12
@github-actions github-actions bot requested a review from juan131 January 13, 2025 13:12
Copy link
Contributor

@juan131 juan131 left a comment

Choose a reason for hiding this comment

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

Thanks so much for this fix! Please check my comment, it's a very tiny thing

## TCP Listen Options
##
{{- if .Values.tcpListenOptions.enabled }}
Copy link
Contributor

Choose a reason for hiding this comment

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

This "if" should be included before the comment in line 518

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

Kulivox and others added 2 commits January 15, 2025 10:04
Signed-off-by: Bitnami Containers <[email protected]>
Copy link
Contributor

@juan131 juan131 left a comment

Choose a reason for hiding this comment

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

LGTM

@juan131 juan131 enabled auto-merge (squash) January 15, 2025 09:29
@juan131 juan131 merged commit 7432a31 into bitnami:main Jan 15, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rabbitmq solved verify Execute verification workflow for these changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tcpListenOptions configuration is incorrectly controlled by memoryHighWatermark.enabled setting
4 participants