Repository navigation
NetApp Volumes: mount large-capacity volumes via Cloud DNS - #6186
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces support for mounting large-capacity NetApp volumes and FlexCache caches via Cloud DNS. By configuring a private DNS zone, clients can now mount storage using an FQDN that resolves to multiple NFS endpoint IPs in a round-robin fashion. This change addresses the limitation where clients previously only utilized a single IP address, significantly improving load distribution and performance for compute-heavy workloads like Slurm. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces private Cloud DNS integration for Google Cloud NetApp Volumes and FlexCache, enabling clients to mount large-capacity volumes and caches using a single FQDN that resolves to multiple NFS endpoint IPs via DNS round-robin. It updates the EDA blueprints, adds a new Slurm example blueprint (netapp-volumes-slurm.yaml), and updates the relevant modules and documentation. Feedback on the changes suggests removing the redundant google_project_service resource for dns.googleapis.com inside the netapp-volume module to prevent potential permission conflicts and avoid requiring elevated Service Usage permissions during deployment.
69f3ea8 to
91cd3f7
Compare
|
Please NOTE : We recently upgraded our repository's Go version to 1.26 (#6173). We strongly recommend rebasing your open branches onto the latest develop branch to avoid or resolve any PR test failures. Thank you! |
91cd3f7 to
ec40b80
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for mounting large-capacity Google Cloud NetApp Volumes and FlexCache caches using private Cloud DNS round-robin FQDNs, which distributes Slurm clients across multiple NFS endpoints. It updates the netapp-volume and dns-managed-zone modules, adds a new netapp-volumes-slurm example blueprint, and updates the EDA blueprints and documentation. Feedback suggests stripping the trailing dot from the generated FQDN in dns.tf using trimsuffix to prevent potential NFS client mount failures.
ec40b80 to
9002dc5
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for mounting large-capacity Google Cloud NetApp Volumes and FlexCache caches using private Cloud DNS round-robin FQDNs, which distributes client mounts across multiple NFS endpoints in Slurm clusters. It updates the netapp-volume and dns-managed-zone modules, adds a new netapp-volumes-slurm.yaml example, and updates the EDA blueprints and documentation. Feedback on the changes highlights a potential race condition in dns.tf where downstream modules might attempt to mount the volume before the DNS record is fully active; establishing an implicit dependency on the DNS record set resource is recommended to ensure proper provisioning order.
9002dc5 to
e05a0de
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for mounting large-capacity Google Cloud NetApp Volumes and FlexCache caches using private Cloud DNS round-robin A records, allowing Slurm clients to distribute mounts across all available NFS endpoint IPs via FQDNs. The changes update the netapp-volume and dns-managed-zone modules, add a new netapp-volumes-slurm example blueprint, and update existing EDA blueprints and documentation. Feedback on the changes suggests restricting the unix_permissions for the /home filesystem in the new blueprint from 0777 to 0755 to prevent security risks associated with world-writable home directories.
|
@agrawalkhushi18 This is ready to review. |
e05a0de to
539107b
Compare
22c03c6 to
539107b
Compare
|
I accidentally pushed other code on this branch. I reverted it back to good commit. My apologies. |
|
this PR has been inactive for 6 days and has no unresolved comments. @vikramvs-gg, please review. |
|
/gcbrun |
|
/gcbrun |
|
this PR has been inactive for 2 days and has no unresolved comments. @vikramvs-gg, please review. |
|
/gcbrun |
|
@vikramvs-gg @agrawalkhushi18 Thank you for your approval. Very appreciated. Can you tell me why the pipeline it not moving forward? We have two approvals. |
|
/gcbrun |
Large-capacity volumes and FlexCache expose multiple NFS endpoint IPs. Add volume dns_config and private-zone support so clients, including Slurm, mount by FQDN and use every endpoint. Update EDA and Slurm examples plus docs.
Head branch was pushed to by a user without write access
fdb8a23
539107b to
fdb8a23
Compare
|
Thank you for triggering the run. Turns our region and zone where missing from the new examples/netapp-volumes-slurm.yaml blueprint. I added them. Should work now. |
Thanks @okrause . But why are we hardcoding the region and zone values in the sample blueprint? |
@agrawalkhushi18 One of the PR workflows (IIRC it is PR-validation) validates all the example blueprints. There seem to be two types of tests. The smaller ones are ran in The other category of tests are bigger test cases. examples/netapp-volumes.yaml uses that approach. They have a dedicated test setup in Does that make sense? |
|
/gcbrun |
|
Got it, thanks for the explaination @okrause . |
9df9fe6
into
GoogleCloudPlatform:develop
|
Thank you @vikramvs-gg and @agrawalkhushi18 for your support. Very appreciated! |
Large-capacity volumes and FlexCache expose multiple NFS endpoint IPs. Add volume dns_config and private-zone support so clients, including Slurm, mount by FQDN and use every endpoint. Update EDA and Slurm examples plus docs.
Goal is to have a larger populations of clients spread their clients evenly (statistically) over all endpoint IPs to even out load on storage backend.