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-cluster] Allow setting IP_VERSION for DNS lookup #73489

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mtenrero
Copy link

Description of the change

Allows to set IP Lookup mode

Benefits

It's required for DualStack environments in the case it's some stack preference. Specially on OpenShift

Possible drawbacks

Applicable issues

DualStack networks
Helm Chart

Signed-off-by: Marcos Tenrero <mtenrero@allot.com>
@github-actions github-actions bot added redis-cluster triage Triage is needed labels Oct 22, 2024
@carrodher carrodher added verify Execute verification workflow for these changes in-progress labels Oct 22, 2024
@github-actions github-actions bot removed the triage Triage is needed label Oct 22, 2024
@github-actions github-actions bot removed the request for review from carrodher October 22, 2024 17:05
@jotamartos
Copy link
Contributor

Hi,

Thank you for taking the time to contribute. The main problem I see with this change is that there will not be any documentation about that env var and nobody will know that it's possible to use it. Let us review the changes and come up with a better alternative to fix this problem.

Copy link
Contributor

@jotamartos jotamartos left a comment

Choose a reason for hiding this comment

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

Hi again,

As I mentioned above, using the IP_VERSION env var will difficult its usage as it won't be documented anywhere. However, I'd make the following changes

  • add the ip_version parameter as argument in each function. I'd include as first/second parameter depending on if host/hostname is also an argument of the function.
  • The default value if the argument is not provided will be empty
  • Add a new env var in the redis-cluster-env.sh script that allows you set the IP version (REDIS_DNS_LOOKUP_IP_VERSION or similar)
  • Update the librediscluster.sh script to use that env var as argument of the different methods.

Let me know if you have any questions.

Thanks

Copy link

This Pull Request has been automatically marked as "stale" because it has not had recent activity (for 15 days). It will be closed if no further activity occurs. Thank you for your contribution.

@github-actions github-actions bot added the stale 15 days without activity label Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in-progress redis-cluster stale 15 days without activity verify Execute verification workflow for these changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants