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] allow to configure trafficDistribution #32443

Merged
merged 3 commits into from
Mar 19, 2025

Conversation

rossifumax
Copy link
Contributor

Hello !

Description of the change

Add trafficDistribution feature on svc-headless of RabbitMQ.
This feature is available since k8s v1.31.

Benefits

Take advantage of the new feature trafficDistribution.
cf. https://kubernetes.io/docs/concepts/services-networking/service/#traffic-distribution

Possible drawbacks

Normally, no drawbacks.

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)

Sorry, something went wrong.

@github-actions github-actions bot added rabbitmq triage Triage is needed labels Mar 13, 2025
@github-actions github-actions bot requested a review from carrodher March 13, 2025 16:26
@carrodher carrodher added verify Execute verification workflow for these changes in-progress labels Mar 13, 2025
@github-actions github-actions bot removed the triage Triage is needed label Mar 13, 2025
@github-actions github-actions bot removed the request for review from carrodher March 13, 2025 20:15
@github-actions github-actions bot requested a review from alvneiayu March 13, 2025 20:15
rossifumax and others added 2 commits March 17, 2025 11:29

Unverified

The email in this signature doesn’t match the committer email.
Signed-off-by: Rémi Beaufils <remi.beaufils@claranet.com>

Unverified

The email in this signature doesn’t match the committer email.
Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>
@rossifumax rossifumax force-pushed the feat/rabbitmq-traffic-distribution branch from 159512d to 74466ad Compare March 17, 2025 10:29
Signed-off-by: Bitnami Bot <bitnami.bot@broadcom.com>
## @param service.trafficDistribution Traffic Distribution provides another
## way to influence traffic routing within a Kubernetes Service.
##
trafficDistribution: "PreferClose"
Copy link
Contributor

Choose a reason for hiding this comment

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

hi @rossifumax

first of all, thanks a lot for your PR.

I have a question about your changes. Why by default "PreferClose"? Maybe it is a good idea to set up the value to empty one and just include the trafficDistribution in case that the users want to set up it to trafficDistribution. What do you think?

Thanks a lot again

Álvaro

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello @alvneiayu,

In my opinion, the default value should be set to PreferClose for all services for various reasons (performance, cost, reliability).
However, depending on the use cases, it might be necessary to change this setting when pods on the same node or zone become overloaded.

What do you think ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello @alvneiayu

Btw, i already had a conversation about this feature and the defaults to apply on the Opensearch helm chart if you want to take a look : #32442 (comment)

Regards

Copy link
Contributor

Choose a reason for hiding this comment

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

awesome @rossifumax , thanks a lot for your answer. Everything clear then on my side. Again, thanks a lot for your contribution

Copy link
Contributor

@alvneiayu alvneiayu left a comment

Choose a reason for hiding this comment

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

LGTM

@alvneiayu alvneiayu merged commit 49e8182 into bitnami:main Mar 19, 2025
20 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.

None yet

4 participants