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

azurerm_lb - fix zone behaviour bug introduced in recent API upgrade #12208

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
66 changes: 60 additions & 6 deletions azurerm/internal/services/loadbalancer/loadbalancer_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,21 @@ func resourceArmLoadBalancer() *pluginsdk.Resource {
ValidateFunc: validation.StringIsNotEmpty,
},

"availability_zone": {
Type: pluginsdk.TypeString,
Optional: true,
//Default: "Zone-Redundant",
Computed: true,
ForceNew: true,
ValidateFunc: validation.StringInSlice([]string{
"No-Zone",
"1",
"2",
"3",
"Zone-Redundant",
}, false),
},

"subnet_id": {
Type: pluginsdk.TypeString,
Optional: true,
Expand Down Expand Up @@ -156,7 +171,19 @@ func resourceArmLoadBalancer() *pluginsdk.Resource {
Set: pluginsdk.HashString,
},

"zones": azure.SchemaSingleZone(),
// TODO - 3.0 make Computed only
"zones": {
Type: pluginsdk.TypeList,
Optional: true,
Computed: true,
ForceNew: true,
Deprecated: "This property has been deprecated in favour of `availability_zone` due to a breaking behavioural change in Azure: https://azure.microsoft.com/en-us/updates/zone-behavior-change/",
MaxItems: 1,
Elem: &pluginsdk.Schema{
Type: pluginsdk.TypeString,
ValidateFunc: validation.StringIsNotEmpty,
},
},

"id": {
Type: pluginsdk.TypeString,
Expand Down Expand Up @@ -358,7 +385,25 @@ func expandAzureRmLoadBalancerFrontendIpConfigurations(d *pluginsdk.ResourceData
}

name := data["name"].(string)
zones := azure.ExpandZones(data["zones"].([]interface{}))
zones := &[]string{"1", "2"}
ms-henglu marked this conversation as resolved.
Show resolved Hide resolved
// TODO - Remove in 3.0
if deprecatedZonesRaw, ok := data["zones"]; ok {
deprecatedZones := azure.ExpandZones(deprecatedZonesRaw.([]interface{}))
if deprecatedZones != nil {
zones = deprecatedZones
}
}

if availabilityZones, ok := data["availability_zone"]; ok {
switch availabilityZones.(string) {
case "1", "2", "3":
zones = &[]string{availabilityZones.(string)}
case "Zone-Redundant":
zones = &[]string{"1", "2"}
ms-henglu marked this conversation as resolved.
Show resolved Hide resolved
case "No-Zone":
zones = &[]string{}
}
}
frontEndConfig := network.FrontendIPConfiguration{
ms-henglu marked this conversation as resolved.
Show resolved Hide resolved
Name: &name,
FrontendIPConfigurationPropertiesFormat: &properties,
Expand Down Expand Up @@ -388,11 +433,20 @@ func flattenLoadBalancerFrontendIpConfiguration(ipConfigs *[]network.FrontendIPC
ipConfig["id"] = *config.ID
}

zones := make([]string, 0)
if zs := config.Zones; zs != nil {
zones = *zs
availabilityZones := "No-Zone"
zonesDeprecated := make([]string, 0)
if config.Zones != nil {
if len(*config.Zones) > 1 {
availabilityZones = "Zone-Redundant"
}
if len(*config.Zones) == 1 {
zones := *config.Zones
availabilityZones = zones[0]
zonesDeprecated = zones
}
}
ipConfig["zones"] = zones
ipConfig["availability_zone"] = availabilityZones
ipConfig["zones"] = zonesDeprecated

if props := config.FrontendIPConfigurationPropertiesFormat; props != nil {
ipConfig["private_ip_address_allocation"] = string(props.PrivateIPAllocationMethod)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,36 @@ func TestAccAzureRMLoadBalancer_privateIP(t *testing.T) {
})
}

func TestAccAzureRMLoadBalancer_ZoneRedundant(t *testing.T) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we add one more test that takes a "1", "2" or "3"?

data := acceptance.BuildTestData(t, "azurerm_lb", "test")
r := LoadBalancer{}

data.ResourceTest(t, r, []acceptance.TestStep{
{
Config: r.availability_zone(data, "Zone-Redundant"),
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
),
},
data.ImportStep(),
})
}

func TestAccAzureRMLoadBalancer_NoZone(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_lb", "test")
r := LoadBalancer{}

data.ResourceTest(t, r, []acceptance.TestStep{
{
Config: r.availability_zone(data, "No-Zone"),
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
),
},
data.ImportStep(),
})
}

func (r LoadBalancer) Exists(ctx context.Context, client *clients.Client, state *pluginsdk.InstanceState) (*bool, error) {
loadBalancerName := state.Attributes["name"]
resourceGroup := state.Attributes["resource_group_name"]
Expand Down Expand Up @@ -497,3 +527,46 @@ resource "azurerm_lb" "test" {
}
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger, data.RandomInteger)
}

func (r LoadBalancer) availability_zone(data acceptance.TestData, zone string) string {
return fmt.Sprintf(`
provider "azurerm" {
features {}
}

resource "azurerm_resource_group" "test" {
name = "acctestRG-lb-%d"
location = "%s"
}

resource "azurerm_virtual_network" "test" {
name = "acctvn-%d"
address_space = ["10.0.0.0/16"]
location = azurerm_resource_group.test.location
resource_group_name = azurerm_resource_group.test.name
}

resource "azurerm_subnet" "test" {
name = "acctsub-%d"
resource_group_name = azurerm_resource_group.test.name
virtual_network_name = azurerm_virtual_network.test.name
address_prefix = "10.0.2.0/24"
}

resource "azurerm_lb" "test" {
name = "acctestlb-%d"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
sku = "Standard"

frontend_ip_configuration {
name = "Internal"
private_ip_address_allocation = "Static"
private_ip_address_version = "IPv4"
private_ip_address = "10.0.2.7"
subnet_id = azurerm_subnet.test.id
availability_zone = "%s"
}
}
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger, data.RandomInteger, zone)
}
8 changes: 5 additions & 3 deletions website/docs/r/lb.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,17 @@ The following arguments are supported:
`frontend_ip_configuration` supports the following:

* `name` - (Required) Specifies the name of the frontend ip configuration.
* `availability_zone` - (Optional) A list of Availability Zones which the Load Balancer's IP Addresses should be created in. Possible values are `Zone-Redundant`, `1`, `2`, `3`, and `No-Zone`. Defaults to `Zone-Redundant`.
`Zone-Redundant` will specify multiple zones in a region with Availability Zones and create a zone-redundant resource.
ms-henglu marked this conversation as resolved.
Show resolved Hide resolved

-> **Please Note**: Availability Zones are only supported with a [Standard SKU](https://docs.microsoft.com/en-us/azure/load-balancer/load-balancer-standard-availability-zones) and [in select regions](https://docs.microsoft.com/en-us/azure/availability-zones/az-overview) at this time. Standard SKU Load Balancer that do not specify a zone are zone redundant by default.
ms-henglu marked this conversation as resolved.
Show resolved Hide resolved

* `subnet_id` - The ID of the Subnet which should be associated with the IP Configuration.
* `private_ip_address` - (Optional) Private IP Address to assign to the Load Balancer. The last one and first four IPs in any range are reserved and cannot be manually assigned.
* `private_ip_address_allocation` - (Optional) The allocation method for the Private IP Address used by this Load Balancer. Possible values as `Dynamic` and `Static`.
* `private_ip_address_version` - The version of IP that the Private IP Address is. Possible values are `IPv4` or `IPv6`.
* `public_ip_address_id` - (Optional) The ID of a Public IP Address which should be associated with the Load Balancer.
* `public_ip_prefix_id` - (Optional) The ID of a Public IP Prefix which should be associated with the Load Balancer. Public IP Prefix can only be used with outbound rules.
* `zones` - (Optional) A list of Availability Zones which the Load Balancer's IP Addresses should be created in.

-> **Please Note**: Availability Zones are only supported with a [Standard SKU](https://docs.microsoft.com/en-us/azure/load-balancer/load-balancer-standard-availability-zones) and [in select regions](https://docs.microsoft.com/en-us/azure/availability-zones/az-overview) at this time. Standard SKU Load Balancer that do not specify a zone are zone redundant by default.

## Attributes Reference

Expand Down