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

Retry for deleting default DHCP options, plus pagination plus a test sweeper #8907

Merged
merged 2 commits into from
Jun 13, 2019
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
20 changes: 16 additions & 4 deletions aws/resource_aws_default_vpc_dhcp_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,16 +65,28 @@ func resourceAwsDefaultVpcDhcpOptionsCreate(d *schema.ResourceData, meta interfa
},
}

resp, err := conn.DescribeDhcpOptions(req)
var dhcpOptions []*ec2.DhcpOptions
err := conn.DescribeDhcpOptionsPages(req, func(page *ec2.DescribeDhcpOptionsOutput, lastPage bool) bool {
dhcpOptions = append(dhcpOptions, page.DhcpOptions...)
return !lastPage
})

if err != nil {
return err
return fmt.Errorf("Error describing DHCP options: %s", err)
}

if len(resp.DhcpOptions) != 1 || resp.DhcpOptions[0] == nil {
if len(dhcpOptions) == 0 {
return fmt.Errorf("Default DHCP Options Set not found")
}

d.SetId(aws.StringValue(resp.DhcpOptions[0].DhcpOptionsId))
if len(dhcpOptions) > 1 {
return fmt.Errorf("Multiple default DHCP Options Sets found")
}

if dhcpOptions[0] == nil {
return fmt.Errorf("Default DHCP Options Set is empty")
}
d.SetId(aws.StringValue(dhcpOptions[0].DhcpOptionsId))

return resourceAwsVpcDhcpOptionsUpdate(d, meta)
}
Expand Down
4 changes: 0 additions & 4 deletions aws/resource_aws_default_vpc_dhcp_options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,6 @@ func testAccCheckAWSDefaultVpcDhcpOptionsDestroy(s *terraform.State) error {
}

const testAccAWSDefaultVpcDhcpOptionsConfigBasic = `
provider "aws" {
region = "us-west-2"
}

resource "aws_default_vpc_dhcp_options" "foo" {
tags = {
Name = "Default DHCP Option Set"
Expand Down
9 changes: 8 additions & 1 deletion aws/resource_aws_vpc_dhcp_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ func resourceAwsVpcDhcpOptionsUpdate(d *schema.ResourceData, meta interface{}) e
func resourceAwsVpcDhcpOptionsDelete(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).ec2conn

return resource.Retry(3*time.Minute, func() *resource.RetryError {
err := resource.Retry(3*time.Minute, func() *resource.RetryError {
log.Printf("[INFO] Deleting DHCP Options ID %s...", d.Id())
_, err := conn.DeleteDhcpOptions(&ec2.DeleteDhcpOptionsInput{
DhcpOptionsId: aws.String(d.Id()),
Expand Down Expand Up @@ -239,6 +239,13 @@ func resourceAwsVpcDhcpOptionsDelete(d *schema.ResourceData, meta interface{}) e
return resource.NonRetryableError(err)
}
})

if isResourceTimeoutError(err) {
_, err = conn.DeleteDhcpOptions(&ec2.DeleteDhcpOptionsInput{
DhcpOptionsId: aws.String(d.Id()),
})
}
return err
}

func findVPCsByDHCPOptionsID(conn *ec2.EC2, id string) ([]*ec2.Vpc, error) {
Expand Down
76 changes: 76 additions & 0 deletions aws/resource_aws_vpc_dhcp_options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package aws

import (
"fmt"
"log"
"testing"

"github.com/aws/aws-sdk-go/aws"
Expand All @@ -11,6 +12,81 @@ import (
"github.com/hashicorp/terraform/terraform"
)

func init() {
resource.AddTestSweepers("aws_vpc_dhcp_options", &resource.Sweeper{
Name: "aws_vpc_dhcp_options",
F: testSweepVpcDhcpOptions,
})
}

func testSweepVpcDhcpOptions(region string) error {
client, err := sharedClientForRegion(region)
if err != nil {
return fmt.Errorf("error getting client: %s", err)
}
conn := client.(*AWSClient).ec2conn

input := &ec2.DescribeDhcpOptionsInput{}

err = conn.DescribeDhcpOptionsPages(input, func(page *ec2.DescribeDhcpOptionsOutput, lastPage bool) bool {
for _, dhcpOption := range page.DhcpOptions {
var defaultDomainNameFound, defaultDomainNameServersFound bool

domainName := region + ".compute.internal"
if region == "us-east-1" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit lets start with domainName = region + ".compute.internal" then use the if to change the value is the condition region == "us-east-1" is true.

## remove 'var domainName string' on line #33 
domainName := region + ".compute.internal"
if region == "us-east-1" {
  domainName = "ec2.internal"
}

domainName = "ec2.internal"
}

// This skips the default dhcp configurations so they don't get deleted
for _, dhcpConfiguration := range dhcpOption.DhcpConfigurations {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be helpful to add a comment that here indicating that we are skipping the default DHCP options here.

if aws.StringValue(dhcpConfiguration.Key) == "domain-name" {
if len(dhcpConfiguration.Values) != 1 || dhcpConfiguration.Values[0] == nil {
continue
}

if aws.StringValue(dhcpConfiguration.Values[0].Value) == domainName {
defaultDomainNameFound = true
}
} else if aws.StringValue(dhcpConfiguration.Key) == "domain-name-servers" {
if len(dhcpConfiguration.Values) != 1 || dhcpConfiguration.Values[0] == nil {
continue
}

if aws.StringValue(dhcpConfiguration.Values[0].Value) == "AmazonProvidedDNS" {
defaultDomainNameServersFound = true
}
}
}

if defaultDomainNameFound && defaultDomainNameServersFound {
continue
}

input := &ec2.DeleteDhcpOptionsInput{
DhcpOptionsId: dhcpOption.DhcpOptionsId,
}

_, err := conn.DeleteDhcpOptions(input)

if err != nil {
log.Printf("[ERROR] Error deleting EC2 DHCP Option (%s): %s", aws.StringValue(dhcpOption.DhcpOptionsId), err)
}
}
return !lastPage
})

if testSweepSkipSweepError(err) {
log.Printf("[WARN] Skipping EC2 DHCP Option sweep for %s: %s", region, err)
return nil
}

if err != nil {
return fmt.Errorf("error describing DHCP Options: %s", err)
}

return nil
}

func TestAccAWSDHCPOptions_importBasic(t *testing.T) {
resourceName := "aws_vpc_dhcp_options.foo"

Expand Down