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

feat!: add gpu node autoscaling support (#807) #944

Merged

Conversation

JamesTimms
Copy link
Contributor

@JamesTimms JamesTimms commented Jun 30, 2021

  • add gpu node autoscaling support

I haven't added this for any other modules yet as I only need it for beta-private-cluster. If you're happy with the change I'll make it for other modules where it's needed.

edit: I've now read the CONTRIBUTING.md guide and have updated my PR 🔨

  cluster_autoscaling = {
    enabled             = true
    autoscaling_profile = "BALANCED"
    min_cpu_cores       = 1
    max_cpu_cores       = 100
    min_memory_gb       = 1
    max_memory_gb       = 1000
    gpu_resources = [{
      resource_type = "nvidia-tesla-t4"
      minimum       = 1
      maximum       = 3
    }]
  }

* add gpu node autoscaling support for top level module

* add gpu node autoscaling support for beta-private-cluster module
@comment-bot-dev
Copy link

comment-bot-dev commented Jun 30, 2021

Thanks for the PR! 🚀
✅ Lint checks have passed.

James Timms added 3 commits June 30, 2021 16:47
Copy link
Member

@bharathkkb bharathkkb left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @JamesTimms

Comment on lines 253 to 254
max_memory_gb = number
gpu_resources = list(object({
Copy link
Member

Choose a reason for hiding this comment

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

the tf fmt lint error is possibly due to this

Suggested change
max_memory_gb = number
gpu_resources = list(object({
max_memory_gb = number
gpu_resources = list(object({

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried that originally but no luck. I've now moved the object all on one line like other object definitions.

examples/node_pool/variables.tf Show resolved Hide resolved
@bharathkkb bharathkkb changed the title feature: add gpu node autoscaling support (#807) feat!: add gpu node autoscaling support (#807) Jul 1, 2021
@bharathkkb
Copy link
Member

We will also need a small upgrade guide like this showing gpu_resources set to [] to retain previous functionality if they have set cluster_autoscaling

James Timms and others added 3 commits July 4, 2021 11:05
…google-modules#807)

* Format gpu_resource to meet linter requirements
* Update examples/node_pool/
…google-modules#807)

* Add v16.0 upgrade guide
* Update node_pool test to specify `gpu_resources`
docs/upgrading_to_v16.0.md Outdated Show resolved Hide resolved
docs/upgrading_to_v16.0.md Outdated Show resolved Hide resolved
@bharathkkb bharathkkb merged commit e53a949 into terraform-google-modules:master Jul 9, 2021
@release-please release-please bot mentioned this pull request Jul 9, 2021
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.

None yet

3 participants