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

New Resource: azurerm_kubernetes_cluster_node_pool #4899

Merged
merged 1 commit into from
Nov 21, 2019

Conversation

tombuildsstuff
Copy link
Member

This PR re-opens @titilambert's original work from #4046 which I've rebased on top of #4898 - unfortunately pushing a rebase caused Github closed the PR and gives no way of reopening it, hence re-opening this PR

This introduces a new resource azurerm_kubernetes_cluster_node_pool which allows for managing Virtual Machine Scale Set Node Pools independently of the AKS Cluster.

Fixes #4001

Dependent on #4898 - once that's merged this'll want to be rebased on top of master which should make the diff more relevant

@tombuildsstuff tombuildsstuff added this to the v1.37.0 milestone Nov 17, 2019
@tombuildsstuff tombuildsstuff requested a review from a team November 17, 2019 10:55
@tombuildsstuff tombuildsstuff force-pushed the f/aks-pool-rebased branch 3 times, most recently from 957029b to 3e9082c Compare November 17, 2019 12:59
@tombuildsstuff tombuildsstuff self-assigned this Nov 17, 2019
@tombuildsstuff tombuildsstuff changed the title New Resource: azurerm_kubernetes_cluster_node_pool [WIP] New Resource: azurerm_kubernetes_cluster_node_pool Nov 17, 2019
@tombuildsstuff tombuildsstuff force-pushed the f/aks-pool-rebased branch 4 times, most recently from 4207452 to aa60de6 Compare November 18, 2019 11:37
@tombuildsstuff
Copy link
Member Author

Tests pass:

Screenshot 2019-11-19 at 08 12 32

@tombuildsstuff tombuildsstuff force-pushed the f/aks-pool-rebased branch 2 times, most recently from 67babcc to 5cdb3b7 Compare November 20, 2019 11:18
@tombuildsstuff tombuildsstuff changed the title [WIP] New Resource: azurerm_kubernetes_cluster_node_pool New Resource: azurerm_kubernetes_cluster_node_pool Nov 20, 2019
Co-authored-by: titilambert <titilambert@ttb.lt>
Co-authored-by: djsly <sylvain.boily@gmail.com>

This commit rebases @titilambert's original commits on top of #4898
In addition it also makes a couple of changes:

- The resource has been renamed `azurerm_kubernetes_cluster_node_pool` to match
  the latest terminology used in the Portal
- The tests have been updated to use a Default Node Pool block in the AKS resource
- During creation we now check for the presence of the parent Cluster and then
  confirm that the Default Node Pool is a VirtualMachineScaleSet type
- Removes support for `orchestrator_version` temporarily - since this wants to be
  added to both the Default Node Pool and this resource at the same time/in the same PR
  since this wants some more specific tests (which'd be easier to review in a separate PR)
- Matches the name/schema for all fields with the `default_node_pool` block in the AKS resource

I've ended up `git reset HEAD~N` here to work around the merge conflict due to the large changes
within the AKS resource in #4898, but this seemed like the most pragmatic way to ship these changes.
Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@tombuildsstuff tombuildsstuff merged commit 60218f2 into master Nov 21, 2019
@tombuildsstuff tombuildsstuff deleted the f/aks-pool-rebased branch November 21, 2019 14:03
tombuildsstuff added a commit that referenced this pull request Nov 21, 2019
@ghost
Copy link

ghost commented Nov 26, 2019

This has been released in version 1.37.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 1.37.0"
}
# ... other configuration ...

@rem-aj
Copy link

rem-aj commented Dec 4, 2019

Can someone please provide a code snippet of using both azurerm_kubernetes_cluster and azurerm_kubernetes_cluster_node_pool with a default node pool and an additional node pool.

I'm trying to make the switch now but unfortunately I'm not clear on the example provided in the terraform documentation.

https://www.terraform.io/docs/providers/azurerm/r/kubernetes_cluster_node_pool.html
The example here references the default node pool inside azurerm_kubernetes_cluster

However no reference to the additional node pool. Is it safe to assume that the parameter passed to azurerm_kubernetes_cluster_node_pool represent the non default option node pool option?

Thanks!

@tombuildsstuff
Copy link
Member Author

@rem-aj

Is it safe to assume that the parameter passed to azurerm_kubernetes_cluster_node_pool represent the non default option node pool option?

Yes - only the default node pool is defined within the azurerm_kubernetes_cluster resource - additional pools can be defined using the azurerm_kubernetes_cluster_node_pool resource as many times as needed (up to the service limits, anyway 😄)

@ghost
Copy link

ghost commented Dec 21, 2019

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 Dec 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New resource - azurerm_kubernetes_cluster_agentpool
3 participants