Skip to content

Commit

Permalink
azurerm_private_endpoint Try to add retry on creation (#16315)
Browse files Browse the repository at this point in the history 
Co-authored-by: zjhe <[email protected]>
  • Loading branch information
@lonegunmanb
lonegunmanb and lonegunmanb authored Aug 18, 2022
1 parent 7f5b160 commit 2089f76
Show file tree
Hide file tree
Showing 2 changed files with 181 additions and 25 deletions.
126 changes: 109 additions & 17 deletions internal/services/network/private_endpoint_resource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/hashicorp/go-azure-sdk/resource-manager/postgresql/2017-12-01/servers"
"github.com/hashicorp/go-azure-sdk/resource-manager/privatedns/2018-09-01/privatezones"
"github.com/hashicorp/go-azure-sdk/resource-manager/signalr/2022-02-01/signalr"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-provider-azurerm/helpers/azure"
"github.com/hashicorp/terraform-provider-azurerm/helpers/tf"
"github.com/hashicorp/terraform-provider-azurerm/internal/clients"
Expand Down Expand Up @@ -282,24 +283,61 @@ func resourcePrivateEndpointCreate(d *pluginsdk.ResourceData, meta interface{})
Tags: tags.Expand(d.Get("tags").(map[string]interface{})),
}

err = validatePrivateLinkServiceId(*parameters.PrivateEndpointProperties.PrivateLinkServiceConnections)
if err != nil {
return err
}
err = validatePrivateLinkServiceId(*parameters.PrivateEndpointProperties.ManualPrivateLinkServiceConnections)
if err != nil {
return err
}

cosmosDbResIds := getCosmosDbResIdInPrivateServiceConnections(parameters.PrivateEndpointProperties)
for _, cosmosDbResId := range cosmosDbResIds {
log.Printf("[DEBUG] Add Lock For Private Endpoint %q, lock name: %q", id.Name, cosmosDbResId)
locks.ByName(cosmosDbResId, "azurerm_private_endpoint")
//goland:noinspection GoDeferInLoop
defer locks.UnlockByName(cosmosDbResId, "azurerm_private_endpoint")
}
locks.ByName(subnetId, "azurerm_private_endpoint")
defer locks.UnlockByName(subnetId, "azurerm_private_endpoint")

future, err := client.CreateOrUpdate(ctx, id.ResourceGroup, id.Name, parameters)
if err != nil {
if strings.EqualFold(err.Error(), "is missing required parameter 'group Id'") {
return fmt.Errorf("creating Private Endpoint %q (Resource Group %q) due to missing 'group Id', ensure that the 'subresource_names' type is populated: %+v", id.Name, id.ResourceGroup, err)
} else {
return fmt.Errorf("creating Private Endpoint %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err)
err = pluginsdk.Retry(d.Timeout(pluginsdk.TimeoutCreate), func() *resource.RetryError {
future, err := client.CreateOrUpdate(ctx, id.ResourceGroup, id.Name, parameters)
if err != nil {
switch {
case strings.EqualFold(err.Error(), "is missing required parameter 'group Id'"):
{
return &resource.RetryError{
Err: fmt.Errorf("creating Private Endpoint %q (Resource Group %q) due to missing 'group Id', ensure that the 'subresource_names' type is populated: %+v", id.Name, id.ResourceGroup, err),
Retryable: false,
}
}
case strings.Contains(err.Error(), "PrivateLinkServiceId Invalid private link service id"):
{
return &resource.RetryError{
Err: fmt.Errorf("creating Private Endpoint %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err),
Retryable: true,
}
}
default:
return &resource.RetryError{
Err: fmt.Errorf("creating Private Endpoint %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err),
Retryable: false,
}
}
}
}
if err = future.WaitForCompletionRef(ctx, client.Client); err != nil {
return fmt.Errorf("waiting for creation of Private Endpoint %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err)

if err = future.WaitForCompletionRef(ctx, client.Client); err != nil {
return &resource.RetryError{
Err: fmt.Errorf("waiting for creation of Private Endpoint %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err),
Retryable: false,
}
}
return nil
})
if err != nil {
return err
}

d.SetId(id.ID())
Expand All @@ -317,6 +355,20 @@ func resourcePrivateEndpointCreate(d *pluginsdk.ResourceData, meta interface{})
return resourcePrivateEndpointRead(d, meta)
}

func validatePrivateLinkServiceId(endpoints []network.PrivateLinkServiceConnection) error {
for _, connection := range endpoints {
_, errors := azure.ValidateResourceID(*connection.PrivateLinkServiceID, "PrivateLinkServiceID")
if len(errors) == 0 {
continue
}
_, errors = validate.PrivateConnectionResourceAlias(*connection.PrivateLinkServiceID, "PrivateLinkServiceID")
if len(errors) != 0 {
return fmt.Errorf("PrivateLinkServiceId Invalid: %q", *connection.PrivateLinkServiceID)
}
}
return nil
}

func getCosmosDbResIdInPrivateServiceConnections(p *network.PrivateEndpointProperties) []string {
var ids []string
exists := make(map[string]struct{})
Expand Down Expand Up @@ -379,16 +431,53 @@ func resourcePrivateEndpointUpdate(d *pluginsdk.ResourceData, meta interface{})
Tags: tags.Expand(d.Get("tags").(map[string]interface{})),
}

future, err := client.CreateOrUpdate(ctx, id.ResourceGroup, id.Name, parameters)
err = validatePrivateLinkServiceId(*parameters.PrivateEndpointProperties.PrivateLinkServiceConnections)
if err != nil {
if strings.EqualFold(err.Error(), "is missing required parameter 'group Id'") {
return fmt.Errorf("updating Private Endpoint %q (Resource Group %q) due to missing 'group Id', ensure that the 'subresource_names' type is populated: %+v", id.Name, id.ResourceGroup, err)
} else {
return fmt.Errorf("updating Private Endpoint %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err)
}
return err
}
if err = future.WaitForCompletionRef(ctx, client.Client); err != nil {
return fmt.Errorf("waiting for update of Private Endpoint %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err)
err = validatePrivateLinkServiceId(*parameters.PrivateEndpointProperties.ManualPrivateLinkServiceConnections)
if err != nil {
return err
}

locks.ByName(subnetId, "azurerm_private_endpoint")
defer locks.UnlockByName(subnetId, "azurerm_private_endpoint")

err = pluginsdk.Retry(d.Timeout(pluginsdk.TimeoutCreate), func() *resource.RetryError {
future, err := client.CreateOrUpdate(ctx, id.ResourceGroup, id.Name, parameters)
if err != nil {
switch {
case strings.EqualFold(err.Error(), "is missing required parameter 'group Id'"):
{
return &resource.RetryError{
Err: fmt.Errorf("updating Private Endpoint %q (Resource Group %q) due to missing 'group Id', ensure that the 'subresource_names' type is populated: %+v", id.Name, id.ResourceGroup, err),
Retryable: false,
}
}
case strings.Contains(err.Error(), "PrivateLinkServiceId Invalid private link service id"):
{
return &resource.RetryError{
Err: fmt.Errorf("creating Private Endpoint %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err),
Retryable: true,
}
}
default:
return &resource.RetryError{
Err: fmt.Errorf("updating Private Endpoint %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err),
}
}
}

if err = future.WaitForCompletionRef(ctx, client.Client); err != nil {
return &resource.RetryError{
Err: fmt.Errorf("waiting for update of Private Endpoint %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err),
Retryable: false,
}
}
return nil
})
if err != nil {
return err
}

// 1 Private Endpoint can have 1 Private DNS Zone Group - so to update we need to Delete & Recreate
Expand Down Expand Up @@ -547,6 +636,7 @@ func resourcePrivateEndpointDelete(d *pluginsdk.ResourceData, meta interface{})
}
log.Printf("[DEBUG] Deleted the Private DNS Zone Group associated with Private Endpoint %q / Resource Group %q.", id.Name, id.ResourceGroup)

subnetId := d.Get("subnet_id").(string)
privateServiceConnections := d.Get("private_service_connection").([]interface{})
parameters := network.PrivateEndpoint{
PrivateEndpointProperties: &network.PrivateEndpointProperties{
Expand All @@ -560,6 +650,8 @@ func resourcePrivateEndpointDelete(d *pluginsdk.ResourceData, meta interface{})
//goland:noinspection GoDeferInLoop
defer locks.UnlockByName(cosmosDbResId, "azurerm_private_endpoint")
}
locks.ByName(subnetId, "azurerm_private_endpoint")
defer locks.UnlockByName(subnetId, "azurerm_private_endpoint")

log.Printf("[DEBUG] Deleting the Private Endpoint %q / Resource Group %q..", id.Name, id.ResourceGroup)
future, err := client.Delete(ctx, id.ResourceGroup, id.Name)
Expand Down
80 changes: 72 additions & 8 deletions internal/services/network/private_endpoint_resource_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ func TestAccPrivateEndpoint_privateConnectionAlias(t *testing.T) {

data.ResourceTest(t, r, []acceptance.TestStep{
{
Config: r.privateConnectionAlias(data),
Config: r.privateConnectionAlias(data, false),
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
check.That(data.ResourceName).Key("subnet_id").Exists(),
Expand All @@ -228,6 +228,22 @@ func TestAccPrivateEndpoint_privateConnectionAlias(t *testing.T) {
})
}

func TestAccPrivateEndpoint_updateToPrivateConnectionAlias(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_private_endpoint", "test")
r := PrivateEndpointResource{}

data.ResourceTest(t, r, []acceptance.TestStep{
{
Config: r.privateConnectionAlias(data, false),
},
data.ImportStep(),
{
Config: r.privateConnectionAlias(data, true),
},
data.ImportStep(),
})
}

func (t PrivateEndpointResource) Exists(ctx context.Context, clients *clients.Client, state *pluginsdk.InstanceState) (*bool, error) {
id, err := parse.PrivateEndpointID(state.ID)
if err != nil {
Expand All @@ -242,6 +258,25 @@ func (t PrivateEndpointResource) Exists(ctx context.Context, clients *clients.Cl
return utils.Bool(resp.ID != nil), nil
}

func TestAccPrivateEndpoint_multipleInstances(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_private_endpoint", "test")
r := PrivateEndpointResource{}

instanceCount := 5
var checks []pluginsdk.TestCheckFunc
for i := 0; i < instanceCount; i++ {
checks = append(checks, check.That(fmt.Sprintf("%s.%d", data.ResourceName, i)).ExistsInAzure(r))
}

config := r.multipleInstances(data, instanceCount)
data.ResourceTest(t, r, []acceptance.TestStep{
{
Config: config,
Check: acceptance.ComposeTestCheckFunc(checks...),
},
})
}

func (PrivateEndpointResource) template(data acceptance.TestData, seviceCfg string) string {
return fmt.Sprintf(`
provider "azurerm" {
Expand All @@ -251,7 +286,7 @@ provider "azurerm" {
data "azurerm_subscription" "current" {}
resource "azurerm_resource_group" "test" {
name = "acctestRG-privatelink-%d"
name = "zjhe-acctestRG-privatelink-%d"
location = "%s"
}
Expand Down Expand Up @@ -416,7 +451,7 @@ provider "azurerm" {
}
resource "azurerm_resource_group" "test" {
name = "acctestRG-privatelink-%d"
name = "zjhe-acctestRG-privatelink-%d"
location = "%s"
}
Expand Down Expand Up @@ -496,7 +531,7 @@ provider "azurerm" {
}
resource "azurerm_resource_group" "test" {
name = "acctestRG-privatelink-%d"
name = "zjhe-acctestRG-privatelink-%d"
location = "%s"
}
Expand Down Expand Up @@ -571,7 +606,7 @@ provider "azurerm" {
}
resource "azurerm_resource_group" "test" {
name = "acctestRG-privatelink-%d"
name = "zjhe-acctestRG-privatelink-%d"
location = "%s"
}
Expand Down Expand Up @@ -656,7 +691,7 @@ provider "azurerm" {
}
resource "azurerm_resource_group" "test" {
name = "acctestRG-privatelink-%d"
name = "zjhe-acctestRG-privatelink-%d"
location = "%s"
}
Expand Down Expand Up @@ -729,7 +764,15 @@ resource "azurerm_private_endpoint" "test" {
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger, data.RandomInteger, data.RandomInteger, data.RandomInteger, data.RandomInteger, data.RandomInteger)
}

func (r PrivateEndpointResource) privateConnectionAlias(data acceptance.TestData) string {
func (r PrivateEndpointResource) privateConnectionAlias(data acceptance.TestData, withTags bool) string {
tags := `
tags = {
env = "TEST"
}
`
if !withTags {
tags = ""
}
return fmt.Sprintf(`
%s
Expand All @@ -745,6 +788,27 @@ resource "azurerm_private_endpoint" "test" {
private_connection_resource_alias = azurerm_private_link_service.test.alias
request_message = "test"
}
%s
}
`, r.template(data, r.serviceAutoApprove(data)), data.RandomInteger)
`, r.template(data, r.serviceAutoApprove(data)), data.RandomInteger, tags)
}

func (r PrivateEndpointResource) multipleInstances(data acceptance.TestData, count int) string {
return fmt.Sprintf(`
%s
resource "azurerm_private_endpoint" "test" {
count = %d
name = "acctest-privatelink-%d-${count.index}"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
subnet_id = azurerm_subnet.endpoint.id
private_service_connection {
name = azurerm_private_link_service.test.name
is_manual_connection = false
private_connection_resource_id = azurerm_private_link_service.test.id
}
}
`, r.template(data, r.serviceAutoApprove(data)), count, data.RandomInteger)
}

0 comments on commit 2089f76

Please sign in to comment.