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 all 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
105 changes: 92 additions & 13 deletions azurerm/internal/services/loadbalancer/loadbalancer_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package loadbalancer
import (
"fmt"
"log"
"strings"
"time"

"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-11-01/network"
Expand Down Expand Up @@ -73,6 +74,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 All @@ -93,7 +109,7 @@ func resourceArmLoadBalancer() *pluginsdk.Resource {
"private_ip_address_version": {
Type: pluginsdk.TypeString,
Optional: true,
Default: string(network.IPVersionIPv4),
Computed: true,
ValidateFunc: validation.StringInSlice([]string{
string(network.IPVersionIPv4),
string(network.IPVersionIPv6),
Expand Down Expand Up @@ -156,7 +172,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 @@ -216,7 +244,11 @@ func resourceArmLoadBalancerCreateUpdate(d *pluginsdk.ResourceData, meta interfa
properties := network.LoadBalancerPropertiesFormat{}

if _, ok := d.GetOk("frontend_ip_configuration"); ok {
properties.FrontendIPConfigurations = expandAzureRmLoadBalancerFrontendIpConfigurations(d)
frontendIPConfigurations, err := expandAzureRmLoadBalancerFrontendIpConfigurations(d)
if err != nil {
return err
}
properties.FrontendIPConfigurations = frontendIPConfigurations
}

loadBalancer := network.LoadBalancer{
Expand Down Expand Up @@ -321,11 +353,12 @@ func resourceArmLoadBalancerDelete(d *pluginsdk.ResourceData, meta interface{})
return nil
}

func expandAzureRmLoadBalancerFrontendIpConfigurations(d *pluginsdk.ResourceData) *[]network.FrontendIPConfiguration {
func expandAzureRmLoadBalancerFrontendIpConfigurations(d *pluginsdk.ResourceData) (*[]network.FrontendIPConfiguration, error) {
configs := d.Get("frontend_ip_configuration").([]interface{})
frontEndConfigs := make([]network.FrontendIPConfiguration, 0, len(configs))
sku := d.Get("sku").(string)

for _, configRaw := range configs {
for index, configRaw := range configs {
data := configRaw.(map[string]interface{})

privateIpAllocationMethod := data["private_ip_address_allocation"].(string)
Expand All @@ -337,8 +370,7 @@ func expandAzureRmLoadBalancerFrontendIpConfigurations(d *pluginsdk.ResourceData
properties.PrivateIPAddress = &v
}

properties.PrivateIPAddressVersion = network.IPVersion(data["private_ip_address_version"].(string))

subnetSet := false
if v := data["public_ip_address_id"].(string); v != "" {
properties.PublicIPAddress = &network.PublicIPAddress{
ID: &v,
Expand All @@ -352,13 +384,51 @@ func expandAzureRmLoadBalancerFrontendIpConfigurations(d *pluginsdk.ResourceData
}

if v := data["subnet_id"].(string); v != "" {
subnetSet = true
properties.PrivateIPAddressVersion = network.IPVersionIPv4
if v := data["private_ip_address_version"].(string); v != "" {
properties.PrivateIPAddressVersion = network.IPVersion(v)
}
properties.Subnet = &network.Subnet{
ID: &v,
}
}

name := data["name"].(string)
zones := azure.ExpandZones(data["zones"].([]interface{}))
// TODO - get zone list for each location by Resource API, instead of hardcode
zones := &[]string{"1", "2"}
ms-henglu marked this conversation as resolved.
Show resolved Hide resolved
zonesSet := false
// TODO - Remove in 3.0
if deprecatedZonesRaw, ok := d.GetOk(fmt.Sprintf("frontend_ip_configuration.%d.zones", index)); ok {
zonesSet = true
deprecatedZones := azure.ExpandZones(deprecatedZonesRaw.([]interface{}))
if deprecatedZones != nil {
zones = deprecatedZones
}
}

if availabilityZones, ok := d.GetOk(fmt.Sprintf("frontend_ip_configuration.%d.availability_zone", index)); ok {
zonesSet = true
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{}
}
}
if !strings.EqualFold(sku, string(network.LoadBalancerSkuNameStandard)) {
if zonesSet && len(*zones) > 0 {
return nil, fmt.Errorf("Availability Zones are not available on the `Basic` SKU")
}
zones = &[]string{}
} else if !subnetSet {
if zonesSet && len(*zones) > 0 {
return nil, fmt.Errorf("Networking supports zones only for frontendIpconfigurations which reference a subnet.")
}
zones = &[]string{}
}
frontEndConfig := network.FrontendIPConfiguration{
ms-henglu marked this conversation as resolved.
Show resolved Hide resolved
Name: &name,
FrontendIPConfigurationPropertiesFormat: &properties,
Expand All @@ -368,7 +438,7 @@ func expandAzureRmLoadBalancerFrontendIpConfigurations(d *pluginsdk.ResourceData
frontEndConfigs = append(frontEndConfigs, frontEndConfig)
}

return &frontEndConfigs
return &frontEndConfigs, nil
}

func flattenLoadBalancerFrontendIpConfiguration(ipConfigs *[]network.FrontendIPConfiguration) []interface{} {
Expand All @@ -388,11 +458,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,51 @@ 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 TestAccAzureRMLoadBalancer_SingleZone(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_lb", "test")
r := LoadBalancer{}

data.ResourceTest(t, r, []acceptance.TestStep{
{
Config: r.availability_zone(data, "1"),
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 +542,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)
}
12 changes: 9 additions & 3 deletions website/docs/r/lb.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,21 @@ 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`.
`No-Zones` - A `non-zonal` resource will be created and the resource will not be replicated or distributed to any Availability Zones.

`1`, `2` or `3` (e.g. single Availability Zone) - A `zonal` resource will be created and will be replicate or distribute to a single specific Availability Zone.

`Zone-Redundant` - A `zone-redundant` resource will be created and will replicate or distribute the resource across all three Availability Zones automatically.

-> **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.

* `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