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

Reserve IP addresses for Slurm instances. #149

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

wiktorn
Copy link
Contributor

@wiktorn wiktorn commented Jun 1, 2024

Reserve IP addresses using google_compute_address for all Slurm instances so during the reconfiguration they maintain the same IP address, regardless if static IP addresses where provided or not.

@wiktorn wiktorn force-pushed the reserve_ip_addresses branch from c40beda to 6134ae8 Compare June 10, 2024 07:07
subnetwork = each.value.subnetwork
address_type = "INTERNAL"
region = var.region
address = each.value.network_ip
Copy link
Collaborator

@mr0re1 mr0re1 Aug 2, 2024

Choose a reason for hiding this comment

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

Isn't project should be set to handle SharedVPC?
I guess it should be a project from subnetwork.
Caveat: we never provide var.subnetwork_project, instead subnetwork is always a selfLink, that is sufficient for the providers.
In your case you would need to:

  • Rename var.subnetwork -> var.subnetwork_self_link
  • And either parse selfLink or create data source to extract project_id.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The project doesn't have to (or even can't?) be in the SharedVPC. For sure what worked for me, creating them in service project. And I think it is better from management perspective (you see all the reserved IP addresses in service project).

But I'll do the renames and remove subnetwork_project and see how it works.

Copy link
Contributor Author

@wiktorn wiktorn Aug 6, 2024

Choose a reason for hiding this comment

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

Rename in _slurm_instance will require aligning the call sites of the module in cluster-toolkit.

Preliminary version of changes is available here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Per discussion, I rolled back renaming subnetwork to subnetwork_self_link, but I dropped the subnetwork_project.

Should I mark it as DEPRECATED instead of silently dropping, as it is done in Cluster Toolkit?

wiktorn added 2 commits August 6, 2024 13:25
Reserve IP addresses using google_compute_address for all Slurm
instances so during the reconfiguration they maintain the same IP
address, regardless if static IP addresses where provided or not.
@wiktorn wiktorn force-pushed the reserve_ip_addresses branch from 6134ae8 to 449951d Compare August 6, 2024 13:25
@mr0re1
Copy link
Collaborator

mr0re1 commented Dec 12, 2024

@wiktorn , [slrum]_instance_template terraform modules have been moved to the toolkit repo.

@mr0re1 mr0re1 self-assigned this Dec 12, 2024
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.

2 participants