-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
kubernetes_cluster
-pod_cidrs
/service_cidrs
#16657
Changes from 12 commits
399ab1d
79cd0a0
1a63bf5
f3fda51
5367e77
7a96793
796a3cf
6aa5e1b
a0e0fbf
15cf1ad
bb7268c
a3877fa
4c81277
0d12d6c
c6e5698
a042d9a
feb0c28
47e8bb4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -624,6 +624,62 @@ func TestAccKubernetesCluster_oidcIssuer(t *testing.T) { | |||||
}) | ||||||
} | ||||||
|
||||||
func TestAccKubernetesCluster_namespace(t *testing.T) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we rename this to
Suggested change
|
||||||
data := acceptance.BuildTestData(t, "azurerm_kubernetes_cluster", "test") | ||||||
r := KubernetesClusterResource{} | ||||||
|
||||||
data.ResourceTest(t, r, []acceptance.TestStep{ | ||||||
{ | ||||||
Config: r.namespaceEnabled(data, false), | ||||||
Check: acceptance.ComposeTestCheckFunc( | ||||||
check.That(data.ResourceName).ExistsInAzure(r), | ||||||
), | ||||||
}, | ||||||
data.ImportStep(), | ||||||
{ | ||||||
Config: r.namespaceEnabled(data, true), | ||||||
Check: acceptance.ComposeTestCheckFunc( | ||||||
check.That(data.ResourceName).ExistsInAzure(r), | ||||||
), | ||||||
}, | ||||||
data.ImportStep(), | ||||||
{ | ||||||
Config: r.namespaceEnabled(data, false), | ||||||
Check: acceptance.ComposeTestCheckFunc( | ||||||
check.That(data.ResourceName).ExistsInAzure(r), | ||||||
), | ||||||
}, | ||||||
data.ImportStep(), | ||||||
}) | ||||||
} | ||||||
|
||||||
func (KubernetesClusterResource) namespaceEnabled(data acceptance.TestData, enabled bool) string { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And this to
Suggested change
|
||||||
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" | ||||||
namespace_resources_enabled = "%t" | ||||||
|
||||||
default_node_pool { | ||||||
name = "default" | ||||||
node_count = 1 | ||||||
vm_size = "Standard_DS2_v2" | ||||||
} | ||||||
|
||||||
identity { | ||||||
type = "SystemAssigned" | ||||||
} | ||||||
} | ||||||
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger, enabled) | ||||||
} | ||||||
|
||||||
func (KubernetesClusterResource) basicAvailabilitySetConfig(data acceptance.TestData) string { | ||||||
return fmt.Sprintf(` | ||||||
provider "azurerm" { | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,14 +26,16 @@ func validateKubernetesCluster(d *pluginsdk.ResourceData, cluster *containerserv | |
dnsServiceIP := profile["dns_service_ip"].(string) | ||
serviceCidr := profile["service_cidr"].(string) | ||
podCidr := profile["pod_cidr"].(string) | ||
serviceCidrs := profile["service_cidrs"].([]interface{}) | ||
isCidrset := 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 == "" && !isCidrset) && !(dockerBridgeCidr != "" && dnsServiceIP != "" && isCidrset) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This validation makes sure that 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. About depreciating |
||
return fmt.Errorf("`docker_bridge_cidr`, `dns_service_ip` and `service_cidr` should all be empty or all should be set") | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
* `network_profile` - (Optional) A `network_profile` block as defined below. | ||||||
|
||||||
-> **Note:** If `network_profile` is not defined, `kubenet` profile will be used by default. | ||||||
|
@@ -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. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
* `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. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
~> **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). | ||||||
|
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure