From 2e3124633b7dea98b03827155f7296446930dc7b Mon Sep 17 00:00:00 2001 From: henglu Date: Mon, 14 Jun 2021 20:47:04 +0800 Subject: [PATCH 1/7] fix zone behavior change for loadbalancer --- .../loadbalancer/loadbalancer_resource.go | 66 +++++++++++++++++-- website/docs/r/lb.html.markdown | 7 +- 2 files changed, 64 insertions(+), 9 deletions(-) diff --git a/azurerm/internal/services/loadbalancer/loadbalancer_resource.go b/azurerm/internal/services/loadbalancer/loadbalancer_resource.go index c14d37c64435..ed23d56d334e 100644 --- a/azurerm/internal/services/loadbalancer/loadbalancer_resource.go +++ b/azurerm/internal/services/loadbalancer/loadbalancer_resource.go @@ -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, @@ -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, @@ -358,7 +385,25 @@ func expandAzureRmLoadBalancerFrontendIpConfigurations(d *pluginsdk.ResourceData } name := data["name"].(string) - zones := azure.ExpandZones(data["zones"].([]interface{})) + zones := &[]string{"1", "2"} + // 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"} + case "No-Zone": + zones = &[]string{} + } + } frontEndConfig := network.FrontendIPConfiguration{ Name: &name, FrontendIPConfigurationPropertiesFormat: &properties, @@ -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) diff --git a/website/docs/r/lb.html.markdown b/website/docs/r/lb.html.markdown index 96bef47ecebd..3ebe13cbf780 100644 --- a/website/docs/r/lb.html.markdown +++ b/website/docs/r/lb.html.markdown @@ -52,15 +52,16 @@ 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`. + +-> **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. + * `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 From 22233464d74285c10516e7d543df076041e0da74 Mon Sep 17 00:00:00 2001 From: henglu Date: Wed, 16 Jun 2021 10:45:33 +0800 Subject: [PATCH 2/7] add test for availability_zone --- .../loadbalancer_resource_test.go | 73 +++++++++++++++++++ website/docs/r/lb.html.markdown | 3 +- 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/azurerm/internal/services/loadbalancer/loadbalancer_resource_test.go b/azurerm/internal/services/loadbalancer/loadbalancer_resource_test.go index 2564109ad1db..802141b551d3 100644 --- a/azurerm/internal/services/loadbalancer/loadbalancer_resource_test.go +++ b/azurerm/internal/services/loadbalancer/loadbalancer_resource_test.go @@ -165,6 +165,36 @@ func TestAccAzureRMLoadBalancer_privateIP(t *testing.T) { }) } +func TestAccAzureRMLoadBalancer_ZoneRedundant(t *testing.T) { + 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"] @@ -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) +} diff --git a/website/docs/r/lb.html.markdown b/website/docs/r/lb.html.markdown index 3ebe13cbf780..f6462315df1d 100644 --- a/website/docs/r/lb.html.markdown +++ b/website/docs/r/lb.html.markdown @@ -53,7 +53,8 @@ The following arguments are supported: * `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. + -> **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. * `subnet_id` - The ID of the Subnet which should be associated with the IP Configuration. From 64102da97138d940770583eff34fe4e5e44a4e46 Mon Sep 17 00:00:00 2001 From: henglu Date: Wed, 16 Jun 2021 11:51:45 +0800 Subject: [PATCH 3/7] add test for single zone --- .../loadbalancer/loadbalancer_resource_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/azurerm/internal/services/loadbalancer/loadbalancer_resource_test.go b/azurerm/internal/services/loadbalancer/loadbalancer_resource_test.go index 802141b551d3..20742792980f 100644 --- a/azurerm/internal/services/loadbalancer/loadbalancer_resource_test.go +++ b/azurerm/internal/services/loadbalancer/loadbalancer_resource_test.go @@ -195,6 +195,21 @@ func TestAccAzureRMLoadBalancer_NoZone(t *testing.T) { }) } +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"] From 8488584ef9c20280a7357e2e7cc2889782dc6318 Mon Sep 17 00:00:00 2001 From: ms-henglu <79895375+ms-henglu@users.noreply.github.com> Date: Wed, 16 Jun 2021 12:34:45 +0800 Subject: [PATCH 4/7] Apply document suggestions from code review Co-authored-by: WS <20408400+WodansSon@users.noreply.github.com> --- website/docs/r/lb.html.markdown | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/website/docs/r/lb.html.markdown b/website/docs/r/lb.html.markdown index f6462315df1d..19dcd905fb7d 100644 --- a/website/docs/r/lb.html.markdown +++ b/website/docs/r/lb.html.markdown @@ -53,9 +53,13 @@ The following arguments are supported: * `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. + `No-Zones` - A `non-zonal` resource will be created and the resource will not be replicated or distributed to any Availability Zones. --> **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. + `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. From d3b95992bca260adb8670d511ae08575536080eb Mon Sep 17 00:00:00 2001 From: henglu Date: Thu, 17 Jun 2021 00:12:07 +0800 Subject: [PATCH 5/7] fix acctest test --- .../loadbalancer/loadbalancer_resource.go | 44 +++++++++++++++---- 1 file changed, 35 insertions(+), 9 deletions(-) diff --git a/azurerm/internal/services/loadbalancer/loadbalancer_resource.go b/azurerm/internal/services/loadbalancer/loadbalancer_resource.go index ed23d56d334e..7fcd6bd56d24 100644 --- a/azurerm/internal/services/loadbalancer/loadbalancer_resource.go +++ b/azurerm/internal/services/loadbalancer/loadbalancer_resource.go @@ -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" @@ -108,7 +109,9 @@ func resourceArmLoadBalancer() *pluginsdk.Resource { "private_ip_address_version": { Type: pluginsdk.TypeString, Optional: true, - Default: string(network.IPVersionIPv4), + // this property should only be applied for frontendIpConfigurations which reference a subnet + //Default: string(network.IPVersionIPv4), + Computed: true, ValidateFunc: validation.StringInSlice([]string{ string(network.IPVersionIPv4), string(network.IPVersionIPv6), @@ -243,7 +246,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{ @@ -348,11 +355,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) @@ -364,8 +372,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, @@ -379,6 +386,11 @@ 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, } @@ -386,15 +398,18 @@ func expandAzureRmLoadBalancerFrontendIpConfigurations(d *pluginsdk.ResourceData name := data["name"].(string) zones := &[]string{"1", "2"} + zonesSet := false // TODO - Remove in 3.0 - if deprecatedZonesRaw, ok := data["zones"]; ok { + 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 := data["availability_zone"]; ok { + 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)} @@ -404,6 +419,17 @@ func expandAzureRmLoadBalancerFrontendIpConfigurations(d *pluginsdk.ResourceData 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{ Name: &name, FrontendIPConfigurationPropertiesFormat: &properties, @@ -413,7 +439,7 @@ func expandAzureRmLoadBalancerFrontendIpConfigurations(d *pluginsdk.ResourceData frontEndConfigs = append(frontEndConfigs, frontEndConfig) } - return &frontEndConfigs + return &frontEndConfigs, nil } func flattenLoadBalancerFrontendIpConfiguration(ipConfigs *[]network.FrontendIPConfiguration) []interface{} { From 7f9c289835f6b53c237a284232a0abdf8c29c6a0 Mon Sep 17 00:00:00 2001 From: ms-henglu Date: Thu, 17 Jun 2021 10:24:32 +0800 Subject: [PATCH 6/7] add todo for replacing hardcode with getting zone list by resource api --- azurerm/internal/services/loadbalancer/loadbalancer_resource.go | 1 + 1 file changed, 1 insertion(+) diff --git a/azurerm/internal/services/loadbalancer/loadbalancer_resource.go b/azurerm/internal/services/loadbalancer/loadbalancer_resource.go index 7fcd6bd56d24..28e1129f6c51 100644 --- a/azurerm/internal/services/loadbalancer/loadbalancer_resource.go +++ b/azurerm/internal/services/loadbalancer/loadbalancer_resource.go @@ -397,6 +397,7 @@ func expandAzureRmLoadBalancerFrontendIpConfigurations(d *pluginsdk.ResourceData } name := data["name"].(string) + // TODO - get zone list for each location by Resource API, instead of hardcode zones := &[]string{"1", "2"} zonesSet := false // TODO - Remove in 3.0 From 071468df358ad077d3c2dd5048b1f30d4954be2a Mon Sep 17 00:00:00 2001 From: ms-henglu Date: Thu, 17 Jun 2021 13:38:19 +0800 Subject: [PATCH 7/7] remove comments --- azurerm/internal/services/loadbalancer/loadbalancer_resource.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/azurerm/internal/services/loadbalancer/loadbalancer_resource.go b/azurerm/internal/services/loadbalancer/loadbalancer_resource.go index 28e1129f6c51..31cf30cbd219 100644 --- a/azurerm/internal/services/loadbalancer/loadbalancer_resource.go +++ b/azurerm/internal/services/loadbalancer/loadbalancer_resource.go @@ -109,8 +109,6 @@ func resourceArmLoadBalancer() *pluginsdk.Resource { "private_ip_address_version": { Type: pluginsdk.TypeString, Optional: true, - // this property should only be applied for frontendIpConfigurations which reference a subnet - //Default: string(network.IPVersionIPv4), Computed: true, ValidateFunc: validation.StringInSlice([]string{ string(network.IPVersionIPv4),