Skip to content

Update filestore timeout config based on high capacity tier#3900

Merged
mohitchaurasia91 merged 5 commits into
GoogleCloudPlatform:developfrom
mohitchaurasia91:fix-filestore-tier-timeout-config
Apr 9, 2025
Merged

Update filestore timeout config based on high capacity tier#3900
mohitchaurasia91 merged 5 commits into
GoogleCloudPlatform:developfrom
mohitchaurasia91:fix-filestore-tier-timeout-config

Conversation

@mohitchaurasia91

@mohitchaurasia91 mohitchaurasia91 commented Apr 8, 2025

Copy link
Copy Markdown
Contributor

Timeout config for filestore provisioning, require an update based on newly supported tiers definition.

Update README with supported filestore resource tiers.

Submission Checklist

NOTE: Community submissions can take up to 2 weeks to be reviewed.

Please take the following actions before submitting this pull request.

  • Fork your PR branch from the Toolkit "develop" branch (not main)
  • Test all changes with pre-commit in a local branch #
  • Confirm that "make tests" passes all tests
  • Add or modify unit tests to cover code changes
  • Ensure that unit test coverage remains above 80%
  • Update all applicable documentation
  • Follow Cluster Toolkit Contribution guidelines #

@mohitchaurasia91 mohitchaurasia91 added the release-module-improvements Added to release notes under the "Module Improvements" heading. label Apr 8, 2025
@mohitchaurasia91 mohitchaurasia91 self-assigned this Apr 8, 2025
@mohitchaurasia91 mohitchaurasia91 requested review from a team and samskillman as code owners April 8, 2025 12:07
@mohitchaurasia91

Copy link
Copy Markdown
Contributor Author

/gcbrun

@tpdownes tpdownes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please do not add legacy tiers to the error message. We should be actively steering customers away from HIGH_SCALE_SSD. The gap is intentional. It allows older customers to continue using HIGH_SCALE_SSD without modification but new blueprints to be guided to current recommended values.

https://cloud.google.com/filestore/docs/service-tiers#legacy-tiers

Comment thread modules/file-system/filestore/main.tf Outdated

@annuay-google annuay-google left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Mostly LGTM, please look at the comment

@tpdownes

tpdownes commented Apr 8, 2025

Copy link
Copy Markdown
Contributor

Please do not add legacy tiers to the error message. We should be actively steering customers away from HIGH_SCALE_SSD. The gap is intentional. It allows older customers to continue using HIGH_SCALE_SSD without modification but new blueprints to be guided to current recommended values.

https://cloud.google.com/filestore/docs/service-tiers#legacy-tiers

It would probably help to add a comment that the error message intentionally does not list legacy tiers even though they pass the validation logic. I wrote this code and it probably would have helped you. :)

tpdownes
tpdownes previously approved these changes Apr 8, 2025

@tpdownes tpdownes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approving on assumption that you'll accept my minimal change to the README.

I think we may eventually want to expose these settings as var.timeouts but, for now, this is an improvement that helps.

Comment thread modules/file-system/filestore/README.md Outdated

@annuay-google annuay-google left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@mohitchaurasia91 mohitchaurasia91 merged commit fa3347c into GoogleCloudPlatform:develop Apr 9, 2025
@mohitchaurasia91 mohitchaurasia91 deleted the fix-filestore-tier-timeout-config branch April 9, 2025 04:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-module-improvements Added to release notes under the "Module Improvements" heading.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants