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

Manage terraform AWS IP address and ranges #313

Merged
merged 3 commits into from
Feb 7, 2025

Conversation

mpagot
Copy link
Collaborator

@mpagot mpagot commented Feb 3, 2025

Move all the locals about ip address and ranges to a dedicated file. Change indentation to split long lines.
Create intermediate new locals to make algorithm hopefully more readable. Start considering 4 initial IP address that AWS reserve. Calculate each range and IP dynamically using the previous one as starting point.

Ticket: TEAM-9980

Verification

No peering

sle-15-SP5-HanaSr-Aws-Byos-x86_64-Build15-SP5_2025-02-05T03:03:21Z-hanasr_aws_test_fencing_native ec2_r4.8xlarge

sle-15-SP3-HanaSr-Aws-Byos-x86_64-Build15-SP3_2025-02-05T03:03:21Z-hanasr_aws_test_fencing_native ec2_r4.8xlarge

sle-15-SP6-HanaSr-Aws-Payg-x86_64-Build15-SP6_2025-02-05T03:03:21Z-hanasr_aws_test_fencing_native_stop_kill ec2_r4.8xlarge

Peering

sle-15-SP5-Qesap-Aws-Payg-x86_64-BuildLATEST_AWS_SLE15_5_PAYG-qesap_aws_saptune_test

Move all the locals about ip address and ranges to a dedicated file.
Change indentation to split long lines.
Create intermediate new locals to make algorithm hopefully more readable.
Start considering 4 initial IP address that AWS reserve.
Calculate each range and IP dinamically using the previous one as
starting point.
Copy link
Collaborator

@lpalovsky lpalovsky left a comment

Choose a reason for hiding this comment

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

LGTM aside the naming comment I left.

@@ -49,6 +28,9 @@ locals {
netweaver_os_image = var.netweaver_os_image != "" ? var.netweaver_os_image : var.os_image
netweaver_os_owner = var.netweaver_os_owner != "" ? var.netweaver_os_owner : var.os_owner

netweaver_xscs_server_count = var.netweaver_enabled ? (var.netweaver_ha_enabled ? 2 : 1) : 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

isn't it supposed to be ascs instead of xscs ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

same acronym is used everywhere down to the module

Copy link
Collaborator

@alvarocarvajald alvarocarvajald left a comment

Choose a reason for hiding this comment

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

LGTM

@mpagot mpagot merged commit 6d72a6f into SUSE:main Feb 7, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants