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

Add support for nodepool's gpu_instance #519

Merged
merged 2 commits into from
Mar 5, 2024
Merged

Add support for nodepool's gpu_instance #519

merged 2 commits into from
Mar 5, 2024

Conversation

lonegunmanb
Copy link
Member

Describe your changes

Issue number

#517

Checklist before requesting a review

  • The pr title can be used to describe what this pr did in CHANGELOG.md file
  • I have executed pre-commit on my machine
  • I have passed pr-check on my machine

Thanks for your cooperation!

@lonegunmanb
Copy link
Member Author

Hi @zioproto , would you please give this pr a review? Thanks!

main.tf Outdated
@@ -59,6 +59,7 @@ resource "azurerm_kubernetes_cluster" "main" {
enable_host_encryption = var.enable_host_encryption
enable_node_public_ip = var.enable_node_public_ip
fips_enabled = var.default_node_pool_fips_enabled
gpu_instance = var.default_node_pool_gpu_instance
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why you would need GPUs in the system nodepool ?

variables.tf Outdated
@@ -436,6 +436,12 @@ variable "default_node_pool_fips_enabled" {
description = " (Optional) Should the nodes in this Node Pool have Federal Information Processing Standard enabled? Changing this forces a new resource to be created."
}

variable "default_node_pool_gpu_instance" {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The default nodepool is a system node pool that contains also AKS system workloads. I don't see the value of adding a GPU workload on this nodepool.

I suggest to enable the GPU features only on the extra nodepools.

Why your use case needs a GPU on the system node pool ? Could you please share more about your scenario ? Thanks

@lonegunmanb lonegunmanb merged commit bd52700 into main Mar 5, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

2 participants