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

Bug: azurerm_kubernetes_cluster errors out with InvalidLoadbalancerProfile on changes #6534

Merged

Conversation

aristosvo
Copy link
Collaborator

Fixes #6525

@ghost ghost added the size/XS label Apr 19, 2020
@aristosvo aristosvo force-pushed the fix/kubernetes_cluster-loadbalancer branch from edd985c to aa37bd2 Compare April 19, 2020 14:18
@aristosvo
Copy link
Collaborator Author

aristosvo commented Apr 19, 2020

Before:

==> Checking that code complies with gofmt requirements...
==> Checking that Custom Timeouts are used...
TF_ACC=1 go test -v ./azurerm/internal/services/containers/tests/ -run=TestAccAzureRMKubernetesCluster_addAgent -timeout 60m -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccAzureRMKubernetesCluster_addAgent
=== PAUSE TestAccAzureRMKubernetesCluster_addAgent
=== CONT  TestAccAzureRMKubernetesCluster_addAgent
--- PASS: TestAccAzureRMKubernetesCluster_addAgent (1276.60s)
PASS
ok  	github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/containers/tests	1276.669s

After test enhancement:

==> Checking that code complies with gofmt requirements...
==> Checking that Custom Timeouts are used...
TF_ACC=1 go test -v ./azurerm/internal/services/containers/tests/ -run=TestAccAzureRMKubernetesCluster_addAgent -timeout 60m -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccAzureRMKubernetesCluster_addAgent
=== PAUSE TestAccAzureRMKubernetesCluster_addAgent
=== CONT  TestAccAzureRMKubernetesCluster_addAgent
--- FAIL: TestAccAzureRMKubernetesCluster_addAgent (1190.94s)
    testing.go:640: Step 2 error: errors during apply:
        
        Error: updating Managed Kubernetes Cluster "acctestaks200419171809471201" (Resource Group "acctestRG-200419171809471201"): containerservice.ManagedClustersClient#CreateOrUpdate: Failure sending request: StatusCode=400 -- Original Error: Code="InvalidLoadBalancerProfile" Message="Load balancer profile must specify one of ManagedOutboundIPs, OutboundIPPrefixes and OutboundIPs." Target="networkProfile.loadBalancerProfile"
        
          on /tmp/tf-test124628542/main.tf line 21:
          (source code not available)
        
        
FAIL
FAIL	github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/containers/tests	1190.991s
FAIL
GNUmakefile:100: recipe for target 'acctests' failed

After code change:

==> Checking that code complies with gofmt requirements...
==> Checking that Custom Timeouts are used...
TF_ACC=1 go test -v ./azurerm/internal/services/containers/tests/ -run=TestAccAzureRMKubernetesCluster_addAgent -timeout 60m -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccAzureRMKubernetesCluster_addAgent
=== PAUSE TestAccAzureRMKubernetesCluster_addAgent
=== CONT  TestAccAzureRMKubernetesCluster_addAgent
--- PASS: TestAccAzureRMKubernetesCluster_addAgent (1507.55s)
PASS
ok  	github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/containers/tests	1507.635s

@sbkg0002
Copy link

This would really help us! Is there anything we can do to help the merge?

@AntonChernysh
Copy link

AntonChernysh commented Apr 24, 2020

Another one is looking forward for this PR to be merged.
...
kubelet_identity is not exposed in 2.5.0 and we really need 2.6.0 or above but can't do it due to this bug.

@leiter-jakab
Copy link

@AntonChernysh

I used this to expose the kubelet identity before 2.6:

data "azuread_service_principal" "node_identity" {
  display_name = "${azurerm_kubernetes_cluster.my_cluster.name}-agentpool"
}

output "node_service_principal_id" {
  value = data.azuread_service_principal.node_identity.id
}

@angelbarrera92
Copy link

We are facing this issue. Is this PR planned to be merged any time soon?

@ekarlso
Copy link
Contributor

ekarlso commented May 4, 2020

@tombuildsstuff Any idea on getting this in for a quick 2.8.0 or 2.7.1 ? It's a really sad issue.

@twendt
Copy link

twendt commented May 6, 2020

I have just successfully tested this PR.

@sbkg0002
Copy link

sbkg0002 commented May 8, 2020

It's not part of the 2.9 release, is it?

@aristosvo
Copy link
Collaborator Author

Nope. I'll rebase it to resolve the conflicts

@gitflo1
Copy link

gitflo1 commented May 11, 2020

Will this PR be part of the 2.10 release? This is really blocking us from updating our clusters...

@aristosvo
Copy link
Collaborator Author

I discussed it with the maintainers, if it doesn’t make it into 2.10.0 (May 14th) expect it for 2.11.0.

@atrauzzi
Copy link

Can we please have this in a patch release 2.9.1, ASAP? This bug is far too disruptive to sit on a fix for the regular release cycle.

@aristosvo aristosvo changed the title Fix for azurerm_kubernetes_cluster InvalidLoadbalancerProfile Bug: azurerm_kubernetes_cluster errors out with InvalidLoadbalancerProfile on changes May 12, 2020
@djsly
Copy link
Contributor

djsly commented May 12, 2020

this needs to be picked up ASAP :) we will be reverting back to a custom local provider code, but we hate doing this... @aristosvo any feedback on your PR from @tombuildsstuff / @katbyte yet ? I quickly reviewed , and it seems good.

@aristosvo
Copy link
Collaborator Author

@djsly this:

if it doesn’t make it into 2.10.0 expect it for 2.11.0.

@katbyte has taken a look at the PR and told me she will discuss it internally with someone more familiar with AKS, probably @tombuildsstuff. One of the main reasons they are hesitant to merge AKS PRs at the moment is due to a lot of breaking API changes for AKS lately..

@djsly
Copy link
Contributor

djsly commented May 12, 2020

@aristosvo I understand, so many changes. but this not being merged is breaking the usage of terraform + aks

@katbyte katbyte added this to the v2.10.0 milestone May 12, 2020
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.

Thanks for this fix @aristosvo, LGTM 👍

@katbyte katbyte merged commit c8c4e0e into hashicorp:master May 12, 2020
katbyte added a commit that referenced this pull request May 12, 2020
@ghost
Copy link

ghost commented May 15, 2020

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

@ghost
Copy link

ghost commented Jun 12, 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 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Jun 12, 2020
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.

InvalidLoadBalancerProfile in AKS