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

fix(cluster): use external_address in cql_ip_address #6382

Merged
merged 1 commit into from
Jul 18, 2023

Conversation

enaydanov
Copy link
Contributor

Fixing the changes introduced in #6062 for the case if INTRA_NODE_COMM_PUBLIC is False.

Note, that it's not required to replace second occurrence of ip_address with external_address.

PR pre-checks (self review)

  • I followed KISS principle and best practices
  • I didn't leave commented-out/debugging code
  • I added the relevant backport labels
  • New configuration option are added and documented (in sdcm/sct_config.py)
  • I have added tests to cover my changes (Infrastructure only - under unit-test/ folder)
  • All new and existing unit tests passed (CI)
  • I have updated the Readme/doc folder accordingly (if needed)

Replace usage of ip_address with external_address as it more
suitable for purpose to be a public address.
@enaydanov enaydanov added Ready for review backport/5.2 Need backport to 5.2 backport/2023.1 Need to backport to 2023.1 labels Jul 18, 2023
@enaydanov enaydanov requested review from fruch and soyacz July 18, 2023 05:16
Copy link
Contributor

@fgelcer fgelcer left a comment

Choose a reason for hiding this comment

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

LGTM

@fruch
Copy link
Contributor

fruch commented Jul 18, 2023

I'll need more information here, where did we see an issue with this code, with siren ? I thought we were using VPC peering and not using public addresses ?

Anyhow please run azure artifact test to validate this doesn't break it.

@enaydanov
Copy link
Contributor Author

Yes, siren-tests is the case when we use False for INTRA_NODE_COMM_PUBLIC and True for public for IP_SSH_CONNECTIONS

@enaydanov
Copy link
Contributor Author

enaydanov commented Jul 18, 2023

Copy link
Contributor

@soyacz soyacz left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@fruch fruch left a comment

Choose a reason for hiding this comment

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

LGTM

@fruch fruch merged commit 1e35aed into scylladb:master Jul 18, 2023
@fruch fruch added the backport/2023.1-done Commit backported to 2023.1 label Jul 18, 2023
@enaydanov enaydanov deleted the fix-cql-ip-address branch July 18, 2023 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/5.2 Need backport to 5.2 backport/2023.1-done Commit backported to 2023.1 backport/2023.1 Need to backport to 2023.1 Ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants