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

azurem_kubernetes_cluster/azurerm_kubernetes_cluster_node_pool: support for node_public_ip_prefix_id #11635

Merged

Conversation

aristosvo
Copy link
Collaborator

@aristosvo aristosvo commented May 8, 2021

Fixes #11424

Tasks done:

  • Add Node Public IP Prefix for kubernetes_cluster resource
    • Acctest added (TestAccKubernetesCluster_nodePublicIPPrefix)
    • Acctest fixed, enable_node_public_ip cannot be changed (TestAccKubernetesCluster_enableNodePublicIP)
    • Docs
  • Add Node Public IP Prefix for kubernetes_cluster data source
    • Acctest adjusted (TestAccDataSourceKubernetesCluster_enableNodePublicIP -> TestAccDataSourceKubernetesCluster_nodePublicIP)
    • Docs
    • Reordered some code
  • Add Node Public IP Prefix for kubernetes_cluster_node_pool resource
    • Acctest adjusted for kubernetes_cluster_node_pool resource (TestAccKubernetesClusterNodePool_enableNodePublicIP -> TestAccKubernetesClusterNodePool_nodePublicIP)
    • Docs
  • Add Node Public IP Prefix for kubernetes_cluster_node_pool data source
    • Docs

Acceptance tests:

❯ make acctests SERVICE='containers' TESTARGS='-run=nodePublicIP\|NodePublicIP'
==> Checking that code complies with gofmt requirements...
==> Checking that Custom Timeouts are used...
==> Checking that acceptance test packages are used...
TF_ACC=1 go test -v ./azurerm/internal/services/containers -run=nodePublicIP\|NodePublicIP -timeout 180m -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.ProviderVersion=acc"
2021/05/08 18:18:13 [DEBUG] not using binary driver name, it's no longer needed
2021/05/08 18:18:14 [DEBUG] not using binary driver name, it's no longer needed
=== RUN   TestAccDataSourceKubernetesCluster_nodePublicIP
=== PAUSE TestAccDataSourceKubernetesCluster_nodePublicIP
=== RUN   TestAccKubernetesCluster_enableNodePublicIP
=== PAUSE TestAccKubernetesCluster_enableNodePublicIP
=== RUN   TestAccKubernetesCluster_nodePublicIPPrefix
=== PAUSE TestAccKubernetesCluster_nodePublicIPPrefix
=== RUN   TestAccKubernetesClusterNodePool_nodePublicIP
=== PAUSE TestAccKubernetesClusterNodePool_nodePublicIP
=== CONT  TestAccDataSourceKubernetesCluster_nodePublicIP
=== CONT  TestAccKubernetesClusterNodePool_nodePublicIP
=== CONT  TestAccKubernetesCluster_nodePublicIPPrefix
=== CONT  TestAccKubernetesCluster_enableNodePublicIP
--- PASS: TestAccKubernetesCluster_nodePublicIPPrefix (626.92s)
--- PASS: TestAccKubernetesCluster_enableNodePublicIP (671.72s)
--- PASS: TestAccDataSourceKubernetesCluster_nodePublicIP (675.04s)
--- PASS: TestAccKubernetesClusterNodePool_nodePublicIP (976.65s)
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/containers     980.290s

@aristosvo aristosvo force-pushed the feature/aks-node-public-ip-prefix branch from 85e85f7 to a3ad4db Compare May 21, 2021 07:54
@nickvth
Copy link

nickvth commented Jun 1, 2021

@aristosvo great job, only the conflicts need to be fixed.
We hope this MR will be merged soon, because we need this 👍

@aristosvo
Copy link
Collaborator Author

@nickvth Thanks! I reordered some code without separated commits, so I basically asked for it :)

@aristosvo aristosvo force-pushed the feature/aks-node-public-ip-prefix branch from a3ad4db to 57b4042 Compare June 1, 2021 17:14
@aristosvo
Copy link
Collaborator Author

❯ make acctests SERVICE='containers' TESTARGS='-run=nodePublicIP\|NodePublicIP'
==> Checking that code complies with gofmt requirements...
==> Checking that Custom Timeouts are used...
==> Checking that acceptance test packages are used...
TF_ACC=1 go test -v ./azurerm/internal/services/containers -run=nodePublicIP\|NodePublicIP -timeout 180m -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.ProviderVersion=acc"
2021/06/01 20:12:53 [DEBUG] not using binary driver name, it's no longer needed
2021/06/01 20:12:54 [DEBUG] not using binary driver name, it's no longer needed
=== RUN   TestAccDataSourceKubernetesCluster_nodePublicIP
=== PAUSE TestAccDataSourceKubernetesCluster_nodePublicIP
=== RUN   TestAccKubernetesCluster_enableNodePublicIP
=== PAUSE TestAccKubernetesCluster_enableNodePublicIP
=== RUN   TestAccKubernetesCluster_nodePublicIPPrefix
=== PAUSE TestAccKubernetesCluster_nodePublicIPPrefix
=== RUN   TestAccKubernetesClusterNodePool_nodePublicIP
=== PAUSE TestAccKubernetesClusterNodePool_nodePublicIP
=== CONT  TestAccDataSourceKubernetesCluster_nodePublicIP
=== CONT  TestAccKubernetesClusterNodePool_nodePublicIP
=== CONT  TestAccKubernetesCluster_nodePublicIPPrefix
=== CONT  TestAccKubernetesCluster_enableNodePublicIP
--- PASS: TestAccKubernetesCluster_enableNodePublicIP (682.92s)
--- PASS: TestAccDataSourceKubernetesCluster_nodePublicIP (691.89s)
--- PASS: TestAccKubernetesCluster_nodePublicIPPrefix (700.33s)
--- PASS: TestAccKubernetesClusterNodePool_nodePublicIP (979.57s)
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/containers  983.162s

Copy link
Member

@mbfrahry mbfrahry left a comment

Choose a reason for hiding this comment

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

Hey @aristosvo! About 30 of the cluster tests are now failing with these changes.

TestAccKubernetesClusterNodePool_autoScaleUpdate fails with

Code="LinkedInvalidPropertyId" Message="Property id '' at path 'properties.nodePublicIPPrefixID'

and TestAccKubernetesCluster_enableNodePublicIP is failing as well

testing.go:620: Step 1/2 error: Error running pre-apply refresh: exit status 1
Error: "default_node_pool.0.enable_node_public_ip": all of `default_node_pool.0.enable_node_public_ip,default_node_pool.0.node_public_ip_prefix_id` must be specified
with azurerm_kubernetes_cluster.test,
on terraform_plugin_test.tf line 11, in resource "azurerm_kubernetes_cluster" "test":
11: resource "azurerm_kubernetes_cluster" "test" {
--- FAIL: TestAccKubernetesCluster_enableNodePublicIP (5.99s)

I think we should be nearly there if we get those issues fixed up

@aristosvo
Copy link
Collaborator Author

aristosvo commented Jun 1, 2021

@mbfrahry Thanks for running all the AccTests!

I believe I fixed the last one already with commit 21d0359, RequiredWith the other way around:

❯ make acctests SERVICE='containers' TESTARGS='-run=enableNodePublicIP'
==> Checking that code complies with gofmt requirements...
==> Checking that Custom Timeouts are used...
==> Checking that acceptance test packages are used...
TF_ACC=1 go test -v ./azurerm/internal/services/containers -run=enableNodePublicIP -timeout 180m -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.ProviderVersion=acc"
2021/06/01 22:29:57 [DEBUG] not using binary driver name, it's no longer needed
2021/06/01 22:29:59 [DEBUG] not using binary driver name, it's no longer needed
=== RUN   TestAccKubernetesCluster_enableNodePublicIP
=== PAUSE TestAccKubernetesCluster_enableNodePublicIP
=== CONT  TestAccKubernetesCluster_enableNodePublicIP
--- PASS: TestAccKubernetesCluster_enableNodePublicIP (678.25s)
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/containers    682.116s

I've tried to fix the first one as well, checks are running atm! Are these failing tests only ClusterNodePool related tests or should I dig deeper here?

❯ make acctests SERVICE='containers' TESTARGS='-run=TestAccKubernetesClusterNodePool_autoScaleUpdate'
==> Checking that code complies with gofmt requirements...
==> Checking that Custom Timeouts are used...
==> Checking that acceptance test packages are used...
TF_ACC=1 go test -v ./azurerm/internal/services/containers -run=TestAccKubernetesClusterNodePool_autoScaleUpdate -timeout 180m -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.ProviderVersion=acc"
2021/06/01 22:44:23 [DEBUG] not using binary driver name, it's no longer needed
2021/06/01 22:44:24 [DEBUG] not using binary driver name, it's no longer needed
=== RUN   TestAccKubernetesClusterNodePool_autoScaleUpdate
=== PAUSE TestAccKubernetesClusterNodePool_autoScaleUpdate
=== CONT  TestAccKubernetesClusterNodePool_autoScaleUpdate
--- PASS: TestAccKubernetesClusterNodePool_autoScaleUpdate (1233.28s)
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/containers  1236.973s

@mbfrahry
Copy link
Member

mbfrahry commented Jun 1, 2021

Ack! I'm so sorry! I was looking at the wrong commit. I'm running through the tests again against the latest changes here. I'll report back as soon as they are done

@aristosvo
Copy link
Collaborator Author

No problem, I hadn't noticed the failing ClusterNodePool tests yet.

Thanks a lot!

@mbfrahry mbfrahry changed the title kubernetes_cluster/kubernetes_cluster_node_pool: Node Public IP Prefix added for resources and data sources azurem_kubernetes_cluster/azurerm_kubernetes_cluster_node_pool: support for node_public_ip_prefix_id Jun 2, 2021
Copy link
Member

@mbfrahry mbfrahry left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @aristosvo!

@mbfrahry mbfrahry added this to the v2.62.0 milestone Jun 2, 2021
@mbfrahry mbfrahry merged commit a30ef14 into hashicorp:master Jun 2, 2021
mbfrahry added a commit that referenced this pull request Jun 2, 2021
@ghost
Copy link

ghost commented Jun 4, 2021

This has been released in version 2.62.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 = "~> 2.62.0"
}
# ... other configuration ...

@github-actions
Copy link

github-actions bot commented Jul 5, 2021

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 5, 2021
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.

Support for Public IP Prefix in Azure Kubernetes Cluster
3 participants