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.

Sign up for GitHub

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

Jump to bottom

kubernetes_cluster -pod_cidrs/service_cidrs #16657

Merged
merged 18 commits into from
Oct 6, 2022
Merged

kubernetes_cluster -pod_cidrs/service_cidrs #16657

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
399ab1d
`kubernetes_cluster` supports namespace_resources_enabled/pod_cidrs/s…
qiqingzhang Apr 28, 2022
79cd0a0
del run command
qiqingzhang Apr 28, 2022
1a63bf5
modif CDIRS prop
qiqingzhang Apr 28, 2022
f3fda51
remove useless line
qiqingzhang Apr 28, 2022
5367e77
clean up code
qiqingzhang Apr 28, 2022
7a96793
order by character
qiqingzhang Apr 28, 2022
796a3cf
namespace_resources_enabled add test false/true/false
qiqingzhang Apr 28, 2022
6aa5e1b
remove "namespace_resources_enabled" default false
qiqingzhang Apr 28, 2022
a0e0fbf
add force new in doc and add test for pod_cidrs and service_cidrs
qiqingzhang Apr 29, 2022
15cf1ad
move cidrs testcase to network test and modify validateKubernetesClus…
qiqingzhang Apr 29, 2022
bb7268c
modify test case and validateKubernetesCluster(d
qiqingzhang Apr 29, 2022
a3877fa
modify validateKubernetesCluster
qiqingzhang Apr 29, 2022
4c81277
fix(kubernetes_cluster):modify docs and rename test to `TestAccKubern…
qiqingzhang Jun 1, 2022
0d12d6c
[fix] remove `namespace_resources_enabled`
qiqingzhang Jun 1, 2022
c6e5698
update
ms-henglu Aug 29, 2022
a042d9a
add note
ms-henglu Aug 31, 2022
feb0c28
add validation
ms-henglu Sep 6, 2022
47e8bb4
update docs and formatting on error message
stephybun Oct 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
191 changes: 191 additions & 0 deletions internal/services/containers/kubernetes_cluster_network_resource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ func TestAccKubernetesCluster_advancedNetworkingKubenet(t *testing.T) {
data.ImportStep(),
})
}

func TestAccKubernetesCluster_advancedNetworkingIPVersionsIPv4(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_kubernetes_cluster", "test")
r := KubernetesClusterResource{}
Expand Down Expand Up @@ -411,6 +412,66 @@ func TestAccKubernetesCluster_privateClusterOff(t *testing.T) {
})
}

func TestAccKubernetesCluster_podCidrs(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_kubernetes_cluster", "test")
r := KubernetesClusterResource{}

data.ResourceTest(t, r, []acceptance.TestStep{
{
Config: r.podCidrs(data),
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
),
},
data.ImportStep(),
})
}

func TestAccKubernetesCluster_podCidrsDualStack(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_kubernetes_cluster", "test")
r := KubernetesClusterResource{}

data.ResourceTest(t, r, []acceptance.TestStep{
{
Config: r.podCidrsDualStack(data),
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
),
},
data.ImportStep(),
})
}

func TestAccKubernetesCluster_serviceCidrs(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_kubernetes_cluster", "test")
r := KubernetesClusterResource{}

data.ResourceTest(t, r, []acceptance.TestStep{
{
Config: r.serviceCidrs(data),
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
),
},
data.ImportStep(),
})
}

func TestAccKubernetesCluster_serviceCidrsDualStack(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_kubernetes_cluster", "test")
r := KubernetesClusterResource{}

data.ResourceTest(t, r, []acceptance.TestStep{
{
Config: r.serviceCidrsDualStack(data),
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
),
},
data.ImportStep(),
})
}

func TestAccKubernetesCluster_standardLoadBalancer(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_kubernetes_cluster", "test")
r := KubernetesClusterResource{}
Expand Down Expand Up @@ -1800,6 +1861,136 @@ resource "azurerm_kubernetes_cluster" "test" {
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger, enablePrivateCluster, data.RandomInteger)
}

func (KubernetesClusterResource) podCidrs(data acceptance.TestData) string {
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure

return fmt.Sprintf(`
resource "azurerm_resource_group" "test" {
name = "acctestRG-aks-%d"
location = "%s"
}

resource "azurerm_kubernetes_cluster" "test" {
name = "acctestaks%d"
location = azurerm_resource_group.test.location
resource_group_name = azurerm_resource_group.test.name
dns_prefix = "acctestaks%d"

default_node_pool {
name = "default"
node_count = 1
vm_size = "Standard_DS2_v2"
}

network_profile {
network_plugin = "kubenet"
pod_cidrs = ["10.1.1.0/24"]
}

identity {
type = "SystemAssigned"
}
}
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger)
}

func (KubernetesClusterResource) podCidrsDualStack(data acceptance.TestData) string {
return fmt.Sprintf(`
resource "azurerm_resource_group" "test" {
name = "acctestRG-aks-%d"
location = "%s"
}

resource "azurerm_kubernetes_cluster" "test" {
name = "acctestaks%d"
location = azurerm_resource_group.test.location
resource_group_name = azurerm_resource_group.test.name
dns_prefix = "acctestaks%d"

default_node_pool {
name = "default"
node_count = 1
vm_size = "Standard_DS2_v2"
}

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

Choose a reason for hiding this comment

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

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member

@stephybun stephybun Sep 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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"
  }
}

Copy link
Member

Choose a reason for hiding this comment

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

@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"
}

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

}

identity {
type = "SystemAssigned"
}
}
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger)
}

func (KubernetesClusterResource) serviceCidrs(data acceptance.TestData) string {
return fmt.Sprintf(`
resource "azurerm_resource_group" "test" {
name = "acctestRG-aks-%d"
location = "%s"
}

resource "azurerm_kubernetes_cluster" "test" {
name = "acctestaks%d"
location = azurerm_resource_group.test.location
resource_group_name = azurerm_resource_group.test.name
dns_prefix = "acctestaks%d"

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"]
}

identity {
type = "SystemAssigned"
}
}
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger)
}

func (KubernetesClusterResource) serviceCidrsDualStack(data acceptance.TestData) string {
return fmt.Sprintf(`
resource "azurerm_resource_group" "test" {
name = "acctestRG-aks-%d"
location = "%s"
}

resource "azurerm_kubernetes_cluster" "test" {
name = "acctestaks%d"
location = azurerm_resource_group.test.location
resource_group_name = azurerm_resource_group.test.name
dns_prefix = "acctestaks%d"

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 = ["IPv4", "IPv6"]
}

identity {
type = "SystemAssigned"
}
}
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger)
}

func (KubernetesClusterResource) standardLoadBalancerConfig(data acceptance.TestData) string {
return fmt.Sprintf(`
provider "azurerm" {
Expand Down
33 changes: 33 additions & 0 deletions internal/services/containers/kubernetes_cluster_resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,17 @@ func resourceKubernetesCluster() *pluginsdk.Resource {
ValidateFunc: validate.CIDR,
},

"pod_cidrs": {
Type: pluginsdk.TypeList,
Optional: true,
Computed: true,
ForceNew: true,
Elem: &pluginsdk.Schema{
Type: pluginsdk.TypeString,
ValidateFunc: validation.StringIsNotEmpty,
},
},

"service_cidr": {
Type: pluginsdk.TypeString,
Optional: true,
Expand All @@ -478,6 +489,17 @@ func resourceKubernetesCluster() *pluginsdk.Resource {
ValidateFunc: validate.CIDR,
},

"service_cidrs": {
Type: pluginsdk.TypeList,
Optional: true,
Computed: true,
ForceNew: true,
Elem: &pluginsdk.Schema{
Type: pluginsdk.TypeString,
ValidateFunc: validation.StringIsNotEmpty,
},
},

"load_balancer_sku": {
Type: pluginsdk.TypeString,
Optional: true,
Expand Down Expand Up @@ -595,6 +617,7 @@ func resourceKubernetesCluster() *pluginsdk.Resource {
},
},
},

"ip_versions": {
Type: pluginsdk.TypeList,
Optional: true,
Expand Down Expand Up @@ -2134,6 +2157,10 @@ func expandKubernetesClusterNetworkProfile(input []interface{}) (*containerservi
networkProfile.PodCidr = utils.String(podCidr)
}

if v, ok := config["pod_cidrs"]; ok {
networkProfile.PodCidrs = utils.ExpandStringSlice(v.([]interface{}))
}

if v, ok := config["docker_bridge_cidr"]; ok && v.(string) != "" {
dockerBridgeCidr := v.(string)
networkProfile.DockerBridgeCidr = utils.String(dockerBridgeCidr)
Expand All @@ -2144,6 +2171,10 @@ func expandKubernetesClusterNetworkProfile(input []interface{}) (*containerservi
networkProfile.ServiceCidr = utils.String(serviceCidr)
}

if v, ok := config["service_cidrs"]; ok {
networkProfile.ServiceCidrs = utils.ExpandStringSlice(v.([]interface{}))
}

return &networkProfile, nil
}

Expand Down Expand Up @@ -2364,7 +2395,9 @@ func flattenKubernetesClusterNetworkProfile(profile *containerservice.NetworkPro
"network_mode": string(profile.NetworkMode),
"network_policy": string(profile.NetworkPolicy),
"pod_cidr": podCidr,
"pod_cidrs": utils.FlattenStringSlice(profile.PodCidrs),
"service_cidr": serviceCidr,
"service_cidrs": utils.FlattenStringSlice(profile.ServiceCidrs),
"outbound_type": string(profile.OutboundType),
},
}
Expand Down
13 changes: 12 additions & 1 deletion internal/services/containers/kubernetes_cluster_validate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,27 @@ func validateKubernetesCluster(d *pluginsdk.ResourceData, cluster *containerserv
dnsServiceIP := profile["dns_service_ip"].(string)
serviceCidr := profile["service_cidr"].(string)
podCidr := profile["pod_cidr"].(string)
podCidrs := profile["pod_cidrs"].([]interface{})
serviceCidrs := profile["service_cidrs"].([]interface{})
isServiceCidrSet := serviceCidr != "" || len(serviceCidrs) != 0

// 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 == "" && !isServiceCidrSet) && !(dockerBridgeCidr != "" && dnsServiceIP != "" && isServiceCidrSet) {
return fmt.Errorf("`docker_bridge_cidr`, `dns_service_ip` and `service_cidr` should all be empty or all should be set")
}

ipVersions := profile["ip_versions"].([]interface{})
if len(serviceCidrs) == 2 && len(ipVersions) != 2 {
return fmt.Errorf("dual-stack networking must be enabled and `ip_versions` must be set to [\"IPv4\", \"IPv6\"] in order to specify multiple values in `service_cidrs`")
}
if len(podCidrs) == 2 && len(ipVersions) != 2 {
return fmt.Errorf("dual-stack networking must be enabled and `ip_versions` must be set to [\"IPv4\", \"IPv6\"] in order to specify multiple values in `pod_cidrs`")
}
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions website/docs/r/kubernetes_cluster.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -515,8 +515,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) 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) 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.

~> **Note:** This range should not be used by any network element on or connected to this VNet. Service address CIDR must be smaller than /12. `docker_bridge_cidr`, `dns_service_ip` and `service_cidr` should all be empty or all should be set.

Examples of how to use [AKS with Advanced Networking](https://docs.microsoft.com/en-us/azure/aks/networking-overview#advanced-networking) can be [found in the `./examples/kubernetes/` directory in the Github repository](https://github.com/hashicorp/terraform-provider-azurerm/tree/main/examples/kubernetes).
Expand Down