azure_kubernetes_service: Add support for BYO cni #15663

[ekarlso]

No description provided.

[ekarlso]

@katbyte can i have a rerun?

[ekarlso]

Does this look ok to merge or ?

[ekarlso]

I tested this locally and it seems that the "none" option is not passed to Azure as my cluster gets kubenet for networking instead of being CNI-less then.

[katbyte]

got a test failure here @ekarlso

------- Stdout: -------
=== RUN   TestAccDataSourceKubernetesCluster_advancedNetworkingNoneByOCNI
=== PAUSE TestAccDataSourceKubernetesCluster_advancedNetworkingNoneByOCNI
=== CONT  TestAccDataSourceKubernetesCluster_advancedNetworkingNoneByOCNI
    testcase.go:110: Step 1/1 error: Check failed: Check 2/6 error: data.azurerm_kubernetes_cluster.test: Attribute 'network_profile.0.network_plugin' expected "none", got "azure"
--- FAIL: TestAccDataSourceKubernetesCluster_advancedNetworkingNoneByOCNI (537.86s)
FAIL

[ekarlso]

@katbyte tried to fix this but still getting this error that I dont know why, I even tried setting it to null but it still reports replacement. - ip_versions = [ - "IPv4", ] -> null # forces replacement

[ekarlso]

If anyone could help take a stab at this I'd be grateful!

[ekarlso]

All tests are passing @katbyte , could you take a look ?

[stephybun]

@ekarlso, thanks for your effort here so far. We generally don't expose the value none explicitly within the provider. When a property is omitted it should imply none or null, this behaviour is widespread throughout the provider and something that we are continually moving towards. However in this particular case we have some concerns about the UX as explained by this comment. We have an upcoming meeting with the AKS team this week where we will discuss the possibility of renaming the enum to a value more representative of this functionality. Until then i'm going to mark this issue as Blocked.

Thanks for your patience.

[ekarlso]

@stephybun Any news on this after the meeting with the AKS team? We're in dire need of this one due to Cilium.

[mr-twendt]

I used this PR to deploy several clusters with BYO CNI and cilium and it works just fine.

[stephybun]

Thanks for your patience @ekarlso.

After discussions with the AKS team and internally we've decided to make an exception in this case and to allow the value none. I hope you don't mind I've pushed a commit to fix up the test since it was failing and updated the docs so that this is ready to go into the next release.

LGTM 👍

[github-actions]

This functionality has been released in v3.6.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

[github-actions]

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.