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

azurerm_kubernetes_cluster_node_pool needs validation for creating node pools with the same name #8202

Closed
amasover opened this issue Aug 21, 2020 · 2 comments
Labels
upstream/terraform This issue is blocked on an upstream issue within Terraform (Terraform Core/CLI, The Plugin SDK etc)

Comments

@amasover
Copy link
Contributor

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Terraform (and AzureRM Provider) Version

Terraform v0.12.29
Azurerm v2.23.0

Affected Resource(s)

  • azurerm_kubernetes_cluster_node_pool

Terraform Configuration Files

# Copy-paste your Terraform configurations here - for large Terraform configs,
# please use a service like Dropbox and share a link to the ZIP file. For
# security, you can also encrypt the files using our GPG public key: https://keybase.io/hashicorp
resource "azurerm_kubernetes_cluster_node_pool" "load" {
  name = "load"
  kubernetes_cluster_id = azurerm_kubernetes_cluster.newskies_k8s.id
  vm_size = "Standard_F4s"
  node_count = "1"
  node_taints = ["purpose=load:NoSchedule"]
}

resource "azurerm_kubernetes_cluster_node_pool" "small" {
  name = "load"
  kubernetes_cluster_id = azurerm_kubernetes_cluster.newskies_k8s.id
  vm_size = "Standard_F2s"
  node_count = "1"
  node_taints = ["purpose=small:NoSchedule"]
}

resource "azurerm_kubernetes_cluster_node_pool" "large" {
  name = "load"
  kubernetes_cluster_id = azurerm_kubernetes_cluster.newskies_k8s.id
  vm_size = "Standard_F8s"
  node_count = "1"
  node_taints = ["purpose=large:NoSchedule"]
}

Debug Output

Panic Output

Expected Behavior

There should be a validation error because you can't create multiple node pools with the same name.

Actual Behavior

A single node pool called "load" was created, though it was based on the "large" node pool that I defined in Terraform.

Interesting to node that the Azure API doesn't seem to have validation for this either.

Steps to Reproduce

  1. Create a kubernetes cluster, and then create multiple node pools with the same name that use that cluster id
@tombuildsstuff tombuildsstuff added upstream/terraform This issue is blocked on an upstream issue within Terraform (Terraform Core/CLI, The Plugin SDK etc) and removed service/kubernetes-cluster labels Aug 24, 2020
@tombuildsstuff
Copy link
Contributor

hey @amasover

Thanks for opening this issue.

Most (but not all) of the Azure API's are Upserts - meaning that when sending a "CreateOrUpdate" request (as we do via the Azure SDK) that if a resource doesn't exist it'll be created, but otherwise it'll be updated. Unfortunately the Azure API (and thus the Azure SDK) doesn't expose more granular Create/Update methods to differentiate these in most cases - so we end up having to use the CreateOrUpdate methods.

From Terraform's side, in version 2.0 of the Azure Provider (#2807) we introduced a concept to help avoid this, "requiring existing resources to be imported" - since some Azure API's behave differently when creating/updating an object and thus the field behaviours differ. Whilst this fixes this when trying to create two duplicate resources sequentially - unfortunately it won't prevent someone doing this in parallel (without us introducing a lock into each resource, which'd slow provisioning down considerably).

Whilst that's clearly not ideal (as you've raised this issue) - it's something we're looking to improve in the future, however to solve this without introducing locks into every resource requires support for defining what the unique identifiers are for each resource within Terraform's Plugin SDK - which is being tracked in hashicorp/terraform-plugin-sdk#224. As such whilst I'd like to thank you for opening this issue, since resolving is dependent on support in Terraform's Plugin SDK - I'm going to close this issue in favour of the upstream issue for the moment, however we'll re-open this once that's fixed - would you mind subscribing to that issue for updates?

Thanks!

@ghost
Copy link

ghost commented Sep 23, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

@ghost ghost locked and limited conversation to collaborators Sep 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
upstream/terraform This issue is blocked on an upstream issue within Terraform (Terraform Core/CLI, The Plugin SDK etc)
Projects
None yet
Development

No branches or pull requests

3 participants