`kubernetes_cluster` -`pod_cidrs`/`service_cidrs` #16657

qiqingzhang · 2022-05-05T08:49:08Z

public pr for `kubernetes_cluster` supports pod_cidrs/service_cidrs

test result after update：

TestAccKubernetesCluster_serviceCidrs：

TestAccKubernetesCluster_podCidrs：

…ervice_cidrs

…ter(d

stephybun

Thanks for this PR @heiazuo.

I can't seem to find any docs on Namespace Resources. The test fails because the preview feature isn't enabled on the subscription. Registering it doesn't seem to work - usually when a preview feature is being registered I see the state as Registering but in this case it's stuck on Pending

$ az feature list -o table --query "[?contains(name, 'Microsoft.ContainerService/EnableNamespaceResourcesPreview')].{Name:name,State:properties.state}"
Name                                                        State
----------------------------------------------------------  -------
Microsoft.ContainerService/EnableNamespaceResourcesPreview  Pending

stephybun · 2022-05-30T09:47:14Z

internal/services/containers/kubernetes_cluster_other_resource_test.go

@@ -624,6 +624,62 @@ func TestAccKubernetesCluster_oidcIssuer(t *testing.T) {
 	})
 }

+func TestAccKubernetesCluster_namespace(t *testing.T) {


Can we rename this to

Suggested change

func TestAccKubernetesCluster_namespace(t *testing.T) {

func TestAccKubernetesCluster_namespaceToggle(t *testing.T) {

stephybun · 2022-05-30T09:47:24Z

internal/services/containers/kubernetes_cluster_other_resource_test.go

+	})
+}
+
+func (KubernetesClusterResource) namespaceEnabled(data acceptance.TestData, enabled bool) string {


And this to

Suggested change

func (KubernetesClusterResource) namespaceEnabled(data acceptance.TestData, enabled bool) string {

func (KubernetesClusterResource) namespace(data acceptance.TestData, enabled bool) string {

stephybun · 2022-05-30T10:00:52Z

website/docs/r/kubernetes_cluster.html.markdown

@@ -134,6 +134,8 @@ In addition, one of either `identity` or `service_principal` blocks must be spec

 -> **Note:** This requires that the Preview Feature `Microsoft.ContainerService/AKS-AzureDefender` is enabled, see [the documentation](https://docs.microsoft.com/azure/defender-for-cloud/defender-for-containers-enable?tabs=aks-deploy-portal%2Ck8s-deploy-asc%2Ck8s-verify-asc%2Ck8s-remove-arc%2Caks-removeprofile-api&pivots=defender-for-container-aks) for more information.

+* `namespace_resources_enabled` - (Optional) It can be enabled/disabled on creation and updation of the managed cluster. The default value is false.


Suggested change

* `namespace_resources_enabled` - (Optional) It can be enabled/disabled on creation and updation of the managed cluster. The default value is false.

* `namespace_resources_enabled` - (Optional) Specifies whether Namespace Resources should be enabled. Defaults to `false`.

stephybun · 2022-05-30T10:05:18Z

website/docs/r/kubernetes_cluster.html.markdown

@@ -515,8 +517,12 @@ A `network_profile` block supports the following:

 * `pod_cidr` - (Optional) The CIDR to use for pod IP addresses. This field can only be set when `network_plugin` is set to `kubenet`. Changing this forces a new resource to be created.

+* `pod_cidrs` - (Optional) One IPv4 CIDR is expected for single-stack networking. Two CIDRs, one for each IP family (IPv4/IPv6), is expected for dual-stack networking. Changing this forces a new resource to be created.


Suggested change

* `pod_cidrs` - (Optional) One IPv4 CIDR is expected for single-stack networking. Two CIDRs, one for each IP family (IPv4/IPv6), is expected for dual-stack networking. Changing this forces a new resource to be created.

* `pod_cidrs` - (Optional) A list of CIDRs to use for pod IP addresses. For single-stack networking a single IPv4 CIDR is expected. For dual-stack networking an IPv4 and IPv6 CIDR are expected. Changing this forces a new resource to be created.

stephybun · 2022-05-30T10:05:34Z

website/docs/r/kubernetes_cluster.html.markdown

 * `service_cidr` - (Optional) The Network Range used by the Kubernetes service. Changing this forces a new resource to be created.

+* `service_cidrs` - (Optional) One IPv4 CIDR is expected for single-stack networking. Two CIDRs, one for each IP family (IPv4/IPv6), is expected for dual-stack networking. Changing this forces a new resource to be created.


Suggested change

* `service_cidrs` - (Optional) One IPv4 CIDR is expected for single-stack networking. Two CIDRs, one for each IP family (IPv4/IPv6), is expected for dual-stack networking. Changing this forces a new resource to be created.

* `service_cidrs` - (Optional) A list of CIDRs to use for Kubernetes services. For single-stack networking a single IPv4 CIDR is expected. For dual-stack networking an IPv4 and IPv6 CIDR are expected. Changing this forces a new resource to be created.

…etesCluster_namespaceToggle`

ms-henglu · 2022-06-01T04:57:56Z

Hi @stephybun - It seems Microsoft.ContainerService/EnableNamespaceResourcesPreview needs to be manually approved, I'll contact service team see if this is a private preview feature.

ms-henglu · 2022-06-01T05:22:39Z

Hi @stephybun - Just confirmed with service team, this feature is not ready to be exposed.

@heiazuo , would you please help remove this property?

stephybun

One more question, what happens if both service_cidr and service_cidrs or pod_cidr and pod_cidrs are supplied, does the API accept that?

qiqingzhang · 2022-06-02T09:33:24Z

One more question, what happens if both service_cidr and service_cidrs or pod_cidr and pod_cidrs are supplied, does the API accept that?

Hi @stephybun, user can not set service_cidr and service_cidrs at the same time, we have validation function to check:
validateKubernetesCluster

stephybun

Thanks for your patience @qiqingzhang, left a few more suggestions in-line, would you mind taking a look at them? Thanks!

stephybun · 2022-08-25T16:33:31Z

internal/services/containers/kubernetes_cluster_validate.go


 				// Azure network plugin is not compatible with pod_cidr
 				if podCidr != "" && networkPlugin == "azure" {
 					return fmt.Errorf("`pod_cidr` and `azure` cannot be set together")
 				}

 				// if not All empty values or All set values.
-				if !(dockerBridgeCidr == "" && dnsServiceIP == "" && serviceCidr == "") && !(dockerBridgeCidr != "" && dnsServiceIP != "" && serviceCidr != "") {
+				if !(dockerBridgeCidr == "" && dnsServiceIP == "" && !isCidrset) && !(dockerBridgeCidr != "" && dnsServiceIP != "" && isCidrset) {


This validation makes sure that docker_bridge_cidr, dns_service_ip and service_cidr are either all set or empty rather than preventing the user from setting both service_cidr/service_cidrs and pod_cidr/pod_cidrs. In any case after trying this out it is indeed possible to set both _cidr and _cidrs.

However it seems this validation may no longer be relevant or needed since AKS no longer supports docker for the container runtime (see #18119). Could you please update the validation here while you're at it?

About depreciating docker_bridge_cidr, I think it's better to make another PR for it.

stephybun · 2022-08-25T16:34:10Z

internal/services/containers/kubernetes_cluster_network_resource_test.go

@@ -1800,6 +1831,70 @@ resource "azurerm_kubernetes_cluster" "test" {
 `, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger, enablePrivateCluster, data.RandomInteger)
 }

+func (KubernetesClusterResource) podCidrs(data acceptance.TestData) string {


Could we also add a test case where we're providing both an IPv4 and IPv6 address?

ms-henglu · 2022-08-29T08:43:40Z

Hi @stephybun ,

I've added a commit to update this PR, and it allows user to supply both service_cidr and service_cidrs or pod_cidr and pod_cidrs.

stephybun · 2022-08-29T09:38:19Z

internal/services/containers/kubernetes_cluster_network_resource_test.go

+  network_profile {
+    network_plugin = "kubenet"
+    pod_cidrs      = ["10.1.1.0/24", "2002::1234:abcd:ffff:c0a8:101/120"]
+    ip_versions    = ["IPv4", "IPv6"]


I'm assuming this needs to be set in order to be able to provide both an IPv4 and IPv6 address? In which case we should probably add some validation in the create method to inform users as well as a note in the docs?

Thanks! I've added that info in the doc. But I think maybe leave the complicated validation to the service side?

The service side error message isn't particularly descriptive. AKS is complex enough as it is, so it would be beneficial to the users if we returned a more informative message such as
fmt.Errorf("dual-stack networking must be enabled and ip_versions must be set to [IPv4, IPv6] in order to specify multiple %s", field")
Can you please add that in?

Sorry I misunderstood, it's not necessary to set ip_versions = ["IPv4", "IPv6"] in order.

I'm not sure what you mean by order but the test TestAccKubernetesCluster_serviceCidrsDualStack will fail if ip_versions isn't set to ["IPv4", "IPv6"], this is what we should add validation for.

Got it, I'll test it and add this validation.

Hi @stephybun , I just tried, it still works.

resource "azurerm_kubernetes_cluster" "test" { name = "henglu96" location = azurerm_resource_group.test.location resource_group_name = azurerm_resource_group.test.name dns_prefix = "henglu96" default_node_pool { name = "default" node_count = 1 vm_size = "Standard_DS2_v2" } network_profile { network_plugin = "kubenet" dns_service_ip = "10.1.1.10" docker_bridge_cidr = "172.18.0.1/16" service_cidrs = ["10.1.1.0/24", "2002::1234:abcd:ffff:c0a8:101/120"] ip_versions = ["IPv6", "IPv4"] // Please notice here } identity { type = "SystemAssigned" } }

@ms-henglu, the order of the arguments IPv4/IPv6 is irrelevant here. If a user supplies the following configuration

network_profile { network_plugin = "kubenet" dns_service_ip = "10.1.1.10" docker_bridge_cidr = "172.18.0.1/16" service_cidrs = ["10.1.1.0/24", "2002::1234:abcd:ffff:c0a8:101/120"] ip_versions = ["IPv4"] // <---- Only one ip family }

It won't be accepted by the API

{ "code": "InvalidNetworkingConfig", "message": "Service CIDRs do not match IP families.", "subcode": "", "target": "networkProfile.serviceCIDRs" }

Got it, I'll add validation for this case.

@stephybun , I've added this check, thanks!

stephybun

Thanks @ms-henglu and @qiqingzhang, LGTM 🌈

github-actions · 2022-10-11T19:04:34Z

This functionality has been released in v3.26.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 · 2022-11-11T02:32:43Z

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.

qiqingzhang added 12 commits April 28, 2022 16:01

kubernetes_cluster supports namespace_resources_enabled/pod_cidrs/s…

399ab1d

…ervice_cidrs

del run command

79cd0a0

modif CDIRS prop

1a63bf5

remove useless line

f3fda51

clean up code

5367e77

order by character

7a96793

namespace_resources_enabled add test false/true/false

796a3cf

remove "namespace_resources_enabled" default false

6aa5e1b

add force new in doc and add test for pod_cidrs and service_cidrs

a0e0fbf

move cidrs testcase to network test and modify validateKubernetesClus…

15cf1ad

…ter(d

modify test case and validateKubernetesCluster(d

bb7268c

modify validateKubernetesCluster

a3877fa

github-actions bot added documentation size/L labels May 5, 2022

qiqingzhang changed the title ~~kubernetes_cluster supports namespace_resources_enabled/pod_cidrs/service_cidrs~~ kubernetes_cluster -namespace_resources_enabled/pod_cidrs/service_cidrs May 11, 2022

stephybun added service/kubernetes-cluster enhancement labels May 13, 2022

stephybun requested changes May 31, 2022

View reviewed changes

stephybun added the waiting-response label May 31, 2022

fix(kubernetes_cluster):modify docs and rename test to `TestAccKubern…

4c81277

…etesCluster_namespaceToggle`

github-actions bot removed the waiting-response label Jun 1, 2022

[fix] remove namespace_resources_enabled

0d12d6c

github-actions bot added size/M and removed size/L labels Jun 1, 2022

qiqingzhang changed the title ~~kubernetes_cluster -namespace_resources_enabled/pod_cidrs/service_cidrs~~ kubernetes_cluster -pod_cidrs/service_cidrs Jun 1, 2022

stephybun reviewed Jun 2, 2022

View reviewed changes

stephybun requested changes Aug 25, 2022

View reviewed changes

github-actions bot added the waiting-response label Aug 25, 2022

update

c6e5698

github-actions bot added waiting-response and removed waiting-response labels Aug 29, 2022

stephybun reviewed Aug 29, 2022

View reviewed changes

add note

a042d9a

github-actions bot added waiting-response and removed waiting-response labels Aug 31, 2022

add validation

feb0c28

github-actions bot added waiting-response and removed waiting-response labels Sep 6, 2022

update docs and formatting on error message

47e8bb4

github-actions bot removed the waiting-response label Oct 6, 2022

stephybun approved these changes Oct 6, 2022

View reviewed changes

stephybun merged commit 1307680 into hashicorp:main Oct 6, 2022

github-actions bot added this to the v3.26.0 milestone Oct 6, 2022

stephybun added a commit that referenced this pull request Oct 6, 2022

Update for #16657

de38bbc

github-actions bot locked as resolved and limited conversation to collaborators Nov 11, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`kubernetes_cluster` -`pod_cidrs`/`service_cidrs` #16657

`kubernetes_cluster` -`pod_cidrs`/`service_cidrs` #16657

qiqingzhang commented May 5, 2022 •

edited

Loading

stephybun left a comment

stephybun May 30, 2022

stephybun May 30, 2022

stephybun May 30, 2022

stephybun May 30, 2022

stephybun May 30, 2022

ms-henglu commented Jun 1, 2022

ms-henglu commented Jun 1, 2022

stephybun left a comment

qiqingzhang commented Jun 2, 2022

stephybun left a comment

stephybun Aug 25, 2022

ms-henglu Aug 29, 2022

stephybun Aug 25, 2022

ms-henglu Aug 29, 2022

ms-henglu commented Aug 29, 2022

stephybun Aug 29, 2022

ms-henglu Aug 31, 2022

stephybun Sep 5, 2022

ms-henglu Sep 6, 2022

stephybun Sep 6, 2022 •

edited

Loading

ms-henglu Sep 6, 2022

ms-henglu Sep 6, 2022

stephybun Sep 6, 2022

ms-henglu Sep 6, 2022

ms-henglu Sep 6, 2022

stephybun left a comment

github-actions bot commented Oct 11, 2022

github-actions bot commented Nov 11, 2022

	func TestAccKubernetesCluster_namespace(t *testing.T) {
	func TestAccKubernetesCluster_namespaceToggle(t *testing.T) {

	func (KubernetesClusterResource) namespaceEnabled(data acceptance.TestData, enabled bool) string {
	func (KubernetesClusterResource) namespace(data acceptance.TestData, enabled bool) string {

		@@ -134,6 +134,8 @@ In addition, one of either `identity` or `service_principal` blocks must be spec

		-> Note: This requires that the Preview Feature `Microsoft.ContainerService/AKS-AzureDefender` is enabled, see [the documentation](https://docs.microsoft.com/azure/defender-for-cloud/defender-for-containers-enable?tabs=aks-deploy-portal%2Ck8s-deploy-asc%2Ck8s-verify-asc%2Ck8s-remove-arc%2Caks-removeprofile-api&pivots=defender-for-container-aks) for more information.

		* `namespace_resources_enabled` - (Optional) It can be enabled/disabled on creation and updation of the managed cluster. The default value is false.

		@@ -515,8 +517,12 @@ A `network_profile` block supports the following:

		* `pod_cidr` - (Optional) The CIDR to use for pod IP addresses. This field can only be set when `network_plugin` is set to `kubenet`. Changing this forces a new resource to be created.

		* `pod_cidrs` - (Optional) One IPv4 CIDR is expected for single-stack networking. Two CIDRs, one for each IP family (IPv4/IPv6), is expected for dual-stack networking. Changing this forces a new resource to be created.

	* `pod_cidrs` - (Optional) One IPv4 CIDR is expected for single-stack networking. Two CIDRs, one for each IP family (IPv4/IPv6), is expected for dual-stack networking. Changing this forces a new resource to be created.
	* `pod_cidrs` - (Optional) A list of CIDRs to use for pod IP addresses. For single-stack networking a single IPv4 CIDR is expected. For dual-stack networking an IPv4 and IPv6 CIDR are expected. Changing this forces a new resource to be created.

		* `service_cidr` - (Optional) The Network Range used by the Kubernetes service. Changing this forces a new resource to be created.

		* `service_cidrs` - (Optional) One IPv4 CIDR is expected for single-stack networking. Two CIDRs, one for each IP family (IPv4/IPv6), is expected for dual-stack networking. Changing this forces a new resource to be created.

	* `service_cidrs` - (Optional) One IPv4 CIDR is expected for single-stack networking. Two CIDRs, one for each IP family (IPv4/IPv6), is expected for dual-stack networking. Changing this forces a new resource to be created.
	* `service_cidrs` - (Optional) A list of CIDRs to use for Kubernetes services. For single-stack networking a single IPv4 CIDR is expected. For dual-stack networking an IPv4 and IPv6 CIDR are expected. Changing this forces a new resource to be created.

kubernetes_cluster -pod_cidrs/service_cidrs #16657

kubernetes_cluster -pod_cidrs/service_cidrs #16657

Conversation

qiqingzhang commented May 5, 2022 • edited Loading

public pr for kubernetes_cluster supports pod_cidrs/service_cidrs

stephybun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ms-henglu commented Jun 1, 2022

ms-henglu commented Jun 1, 2022

stephybun left a comment

Choose a reason for hiding this comment

qiqingzhang commented Jun 2, 2022

stephybun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ms-henglu commented Aug 29, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stephybun Sep 6, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stephybun left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 11, 2022

github-actions bot commented Nov 11, 2022

`kubernetes_cluster` -`pod_cidrs`/`service_cidrs` #16657

`kubernetes_cluster` -`pod_cidrs`/`service_cidrs` #16657

qiqingzhang commented May 5, 2022 •

edited

Loading

public pr for `kubernetes_cluster` supports pod_cidrs/service_cidrs

stephybun Sep 6, 2022 •

edited

Loading