azurerm_kubernetes_cluster - support for auto scaling #3361

jlpedrosa · 2019-05-02T15:57:08Z

Hi

This PR adds auto scale and availability zones capabilities to AKS resource and data providers.

It bumps the API version for AKS, there are other PR pending regarding that matter so I expect a conflict depends on what gets merged first.
https://github.com/terraform-providers/terraform-provider-azurerm/pull/3262

This depends on the following features:
az feature register --namespace Microsoft.ContainerService -n AvailabilityZonePreview
az feature register --namespace Microsoft.ContainerService --name VMSSPreview
az provider register -n Microsoft.ContainerService

katbyte · 2019-05-07T00:36:43Z

Duplicate of #3174

katbyte

Thanks for the PR @jlpedrosa,

As the other AKS PR was closed i am going to review and proceed with this one. OVerall it looks good, and thanks for the good property combination validation! Whats left is:

merge/rebase off master
test that updates the properties
updating the docs

azurerm/resource_arm_kubernetes_cluster.go

azurerm/resource_arm_kubernetes_cluster_test.go

jlpedrosa · 2019-05-22T11:04:06Z

Hi @katbyte

Your guess was correct regarding the limits, in the docs the thershold is 100 [https://docs.microsoft.com/en-us/azure/aks/quotas-skus-regions](aks limits)

I've rebased the PR (there were some conflicts).
Added the validations
Added the documentation
terraform fmt the test code (taking it to a file outside .go code and fmt then copying it back).

regarding: "test that updates the properties", as I mentioned in #317 the moment we change the type of agent pool to VMScaleSets (which is required for autoscaling, but it can be enabled withoutautoscaling). Then any put request to the api will fail (which means no updates will work with that type of agent pool).

katbyte · 2019-05-22T14:39:58Z

FYI, for future reference i have a tool that will run terraform fmt on blocks of tf code in the tests files: https://github.com/katbyte/terrafmt

katbyte

Thank you for the updates @jlpedrosa, i've left some comments inline

azurerm/data_source_kubernetes_cluster.go

azurerm/resource_arm_kubernetes_cluster.go

azurerm/resource_arm_kubernetes_cluster_test.go

website/docs/d/kubernetes_cluster.html.markdown

website/docs/r/kubernetes_cluster.html.markdown

funnybirdman

minor typo

website/docs/d/kubernetes_cluster.html.markdown

jlpedrosa · 2019-06-13T07:49:36Z

@tombuildsstuff @katbyte

I ran the tests against my subscription, everything seemed fine. Do you know when it could me merged?

tombuildsstuff

hey @jlpedrosa

Thanks for pushing those changes - apologies for the delayed re-review here!

Taking a look through this is looking good, if we can fix up the remaining minor comments this otherwise LGTM 👍

Thanks!

azurerm/data_source_kubernetes_cluster.go

website/docs/r/kubernetes_cluster.html.markdown

website/docs/d/kubernetes_cluster.html.markdown

tombuildsstuff · 2019-06-19T04:47:54Z

azurerm/resource_arm_kubernetes_cluster.go

+
+		if profile.AvailabilityZones != nil && len(*profile.AvailabilityZones) > 0 {
+			agentPoolProfile["availability_zones"] = utils.FlattenStringSlice(profile.AvailabilityZones)
+		}


can we remove this if statement around this flatten? the utils.FlattenStringSlice should handle this being nil (and we should always set this complex type if there's no elements, since that'll then show a diff)

@tombuildsstuff:
In this case, I am protecting us against some behaviours of the API where an empty array is not the same as not sending an array.

But here we are setting the terraform side. Any protection on the API side should be where we set that property on the SDK (only set it if tf data is !nil and len > 0)

I removed it here and just left it on the expand. Is that OK?

azurerm/data_source_kubernetes_cluster.go

azurerm/resource_arm_kubernetes_cluster.go

README.md

jlpedrosa · 2019-06-27T09:56:07Z

Hi @tombuildsstuff

I've rebased it, but your comments were addressed few days ago, I don't know if you could have a look.

katbyte

Hi @jlpedrosa,

Thank you for the updates, i've left some comments inline and replied to yours so that once addressed this should be good to merge

website/docs/r/kubernetes_cluster.html.markdown

katbyte · 2019-06-27T17:09:42Z

website/docs/r/kubernetes_cluster.html.markdown

+
+* `enable_auto_scaling` - (Optional) Whether to enable auto-scaler [Autoscaling details](https://docs.microsoft.com/en-us/azure/aks/cluster-autoscaler) 
+
+~>  **Note:** Support for the `type` of `VirtualMachineScaleSets` is currently in Public Preview on an opt-in basis. You can enable this feature using the Azure CLI by running `az feature register --name VMSSPreview --namespace Microsoft.ContainerService`


This line is spurious as its duplicated below?

Hi
if you mean

~> **Note:** Support for the `type` of `VirtualMachineScaleSets` is currently in Public Preview on an opt-in basis. You can enable this feature using the Azure CLI by running `az feature register --name VMSSPreview --namespace Microsoft.ContainerService`

No, it's not duplicated, it is a requirement for autoscale sets, it only works with VirtualMachineScaleSets. I've added more comments in the doc and remove the line. Let me know if now is clear or how would you prefer it.

katbyte · 2019-06-27T17:11:51Z

azurerm/resource_arm_kubernetes_cluster.go

+
+		if profile.AvailabilityZones != nil && len(*profile.AvailabilityZones) > 0 {
+			agentPoolProfile["availability_zones"] = utils.FlattenStringSlice(profile.AvailabilityZones)
+		}


But here we are setting the terraform side. Any protection on the API side should be where we set that property on the SDK (only set it if tf data is !nil and len > 0)

azurerm/data_source_kubernetes_cluster.go

azurerm/resource_arm_kubernetes_cluster.go

website/docs/d/kubernetes_cluster.html.markdown

website/docs/r/kubernetes_cluster.html.markdown

jlpedrosa · 2019-07-01T11:18:33Z

Hi @katbyte

I've fixed most of the comments, and final questions to few. Please your feedback, I've rebased and squashed it as it was giving merge errors. Sorry for the delay.

katbyte

Thanks for the updates @jlpedrosa, aside from one minor comment this LGTM now 👍

Hope you don't mind but i am going to push a fix for that last comment to get this merged 🙂

katbyte · 2019-07-03T21:54:13Z

azurerm/resource_arm_kubernetes_cluster_test.go

@@ -1481,6 +1546,8 @@ resource "azurerm_kubernetes_cluster" "test" {
    name    = "default"
    count   = "1"
    vm_size = "Standard_DS2_v2"
+    name    = "default"


This property has been duplicated

Suggested change

name = "default"

TVH7 · 2019-07-18T21:25:49Z

When will the new release be pushed? Waiting for this..

TVH7 · 2019-07-19T07:07:36Z

@katbyte I've seen that the milestone deadline was yesterday however no new release has been made. Any updates?

PatricioDanos · 2019-07-23T11:58:31Z

I'm also waiting for this release. Is there any news when it's going to be GA?

TVH7 · 2019-07-24T02:10:55Z

I'm also waiting for this release. Is there any news when it's going to be GA?

So according to my latest info they have a milestone open for the next release. This is already a week overdue (although it says 2 days overdue, but that is because they keep moving the due date.)

However, we are now at 97% complete so I'm expecting @katbyte to release today or tomorrow (24th/25th of July)

A confirmation by @katbyte would be appreciated though.

Link to the milestone: https://github.com/terraform-providers/terraform-provider-azurerm/milestone/48

PatricioDanos · 2019-07-24T11:53:11Z

I'm also waiting for this release. Is there any news when it's going to be GA?

So according to my latest info they have a milestone open for the next release. This is already a week overdue (although it says 2 days overdue, but that is because they keep moving the due date.)

However, we are now at 97% complete so I'm expecting @katbyte to release today or tomorrow (24th/25th of July)

A confirmation by @katbyte would be appreciated though.

Link to the milestone: https://github.com/terraform-providers/terraform-provider-azurerm/milestone/48

We're now at 100% so it should be released asap.

ghost · 2019-07-30T03:48:55Z

This has been released in version 1.32.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.32.0"
}
# ... other configuration ...

ghost · 2019-08-03T14:50:47Z

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 👉 [email protected]. Thanks!

ghost added size/XXL dependencies labels May 2, 2019

katbyte marked this as a duplicate of #3174 May 7, 2019

katbyte force-pushed the master branch from 77d8dd0 to fa53685 Compare May 12, 2019 19:43

katbyte added service/kubernetes-cluster enhancement labels May 15, 2019

katbyte requested changes May 22, 2019

View reviewed changes

katbyte added the waiting-response label May 22, 2019

ghost added size/XL documentation and removed size/XXL labels May 22, 2019

ghost removed the waiting-response label May 22, 2019

ghost added size/L and removed size/XL labels May 24, 2019

katbyte requested changes May 24, 2019

View reviewed changes

ghost added size/XL and removed size/L labels May 24, 2019

jamesbibby mentioned this pull request Jun 1, 2019

[azurerm_kubernetes_cluster] working with autoscaler #2502

Closed

funnybirdman reviewed Jun 7, 2019

View reviewed changes

website/docs/d/kubernetes_cluster.html.markdown Outdated Show resolved Hide resolved

tombuildsstuff reviewed Jun 19, 2019

View reviewed changes

tombuildsstuff added this to the v1.31.0 milestone Jun 19, 2019

tombuildsstuff added the waiting-response label Jun 19, 2019

katbyte requested changes Jun 27, 2019

View reviewed changes

katbyte modified the milestones: v1.31.0, v1.32.0 Jun 28, 2019

Added extra options for autoscaling

d7af2dc

ghost removed the waiting-response label Jul 1, 2019

Avoid sending count of nodes for autoscaling clusters on updates

3ee4c99

katbyte approved these changes Jul 3, 2019

View reviewed changes

fix testAccAzureRMKubernetesCluster_apiServerAuthorizedIPRanges test

a879fc9

katbyte merged commit 6f8adc9 into hashicorp:master Jul 3, 2019

katbyte changed the title ~~Aks autoscale~~ azurerm_kubernetes_cluster - support for auto scaling Jul 3, 2019

katbyte added a commit that referenced this pull request Jul 3, 2019

Update CHANGELOG.md to include #3361

d49fe40

eashweragero101 mentioned this pull request Jul 25, 2019

The link dosent open up https://github.com/terraform-providers/terraform-provider-azurerm/pull/url #3928

Closed

ghost locked and limited conversation to collaborators Aug 3, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

azurerm_kubernetes_cluster - support for auto scaling #3361

azurerm_kubernetes_cluster - support for auto scaling #3361

jlpedrosa commented May 2, 2019 •

edited

Loading

katbyte commented May 7, 2019

katbyte left a comment

jlpedrosa commented May 22, 2019

katbyte commented May 22, 2019 •

edited

Loading

katbyte left a comment

funnybirdman left a comment

jlpedrosa commented Jun 13, 2019 •

edited

Loading

tombuildsstuff left a comment

tombuildsstuff Jun 19, 2019

jlpedrosa Jun 19, 2019

katbyte Jun 27, 2019

jlpedrosa Jul 1, 2019

jlpedrosa commented Jun 27, 2019

katbyte left a comment

katbyte Jun 27, 2019

jlpedrosa Jul 1, 2019 •

edited

Loading

katbyte Jun 27, 2019

jlpedrosa commented Jul 1, 2019

katbyte left a comment

katbyte Jul 3, 2019

TVH7 commented Jul 18, 2019

TVH7 commented Jul 19, 2019

PatricioDanos commented Jul 23, 2019

TVH7 commented Jul 24, 2019 •

edited

Loading

PatricioDanos commented Jul 24, 2019

ghost commented Jul 30, 2019

ghost commented Aug 3, 2019


		* `enable_auto_scaling` - (Optional) Whether to enable auto-scaler [Autoscaling details](https://docs.microsoft.com/en-us/azure/aks/cluster-autoscaler)

		~> Note: Support for the `type` of `VirtualMachineScaleSets` is currently in Public Preview on an opt-in basis. You can enable this feature using the Azure CLI by running `az feature register --name VMSSPreview --namespace Microsoft.ContainerService`

azurerm_kubernetes_cluster - support for auto scaling #3361

azurerm_kubernetes_cluster - support for auto scaling #3361

Conversation

jlpedrosa commented May 2, 2019 • edited Loading

katbyte commented May 7, 2019

katbyte left a comment

Choose a reason for hiding this comment

jlpedrosa commented May 22, 2019

katbyte commented May 22, 2019 • edited Loading

katbyte left a comment

Choose a reason for hiding this comment

funnybirdman left a comment

Choose a reason for hiding this comment

jlpedrosa commented Jun 13, 2019 • edited Loading

tombuildsstuff left a comment

Choose a reason for hiding this comment

tombuildsstuff Jun 19, 2019

Choose a reason for hiding this comment

jlpedrosa Jun 19, 2019

Choose a reason for hiding this comment

katbyte Jun 27, 2019

Choose a reason for hiding this comment

jlpedrosa Jul 1, 2019

Choose a reason for hiding this comment

jlpedrosa commented Jun 27, 2019

katbyte left a comment

Choose a reason for hiding this comment

katbyte Jun 27, 2019

Choose a reason for hiding this comment

jlpedrosa Jul 1, 2019 • edited Loading

Choose a reason for hiding this comment

katbyte Jun 27, 2019

Choose a reason for hiding this comment

jlpedrosa commented Jul 1, 2019

katbyte left a comment

Choose a reason for hiding this comment

katbyte Jul 3, 2019

Choose a reason for hiding this comment

TVH7 commented Jul 18, 2019

TVH7 commented Jul 19, 2019

PatricioDanos commented Jul 23, 2019

TVH7 commented Jul 24, 2019 • edited Loading

PatricioDanos commented Jul 24, 2019

ghost commented Jul 30, 2019

ghost commented Aug 3, 2019

jlpedrosa commented May 2, 2019 •

edited

Loading

katbyte commented May 22, 2019 •

edited

Loading

jlpedrosa commented Jun 13, 2019 •

edited

Loading

jlpedrosa Jul 1, 2019 •

edited

Loading

TVH7 commented Jul 24, 2019 •

edited

Loading