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

Unnecessary vpc/subnet recreation planned when modifying ipv6 settings #13588

Closed
philipl opened this issue Apr 12, 2017 · 6 comments · Fixed by #13909
Closed

Unnecessary vpc/subnet recreation planned when modifying ipv6 settings #13588

philipl opened this issue Apr 12, 2017 · 6 comments · Fixed by #13909

Comments

@philipl
Copy link

philipl commented Apr 12, 2017

Using the AWS API or Console, it is possible to add and remove ipv6 cidr block associations from vpcs and subnets without recreating them. However, terraform insists that recreation is required, which is obviously highly disruptive.

Additionally, if you experiment in the console with toggling IPv6 (so you turn it on and then turn it off), terraform will think that it is still on and say it must do a re-creation to turn it off (even though it is off). This effectively leads to a situation where you cannot actually turn IPv6 off on a vpc or subnet because terraform will demand to recreate.

Additionally, to turn IPv6 on without re-creation, you must turn it on using the API or console and then terraform will see that the state is consistent and not force re-creation.

For example:

-/+ module.networking.aws_subnet.private_subnet.1
    assign_ipv6_address_on_creation: "false" => "false"
    availability_zone:               "us-west-2b" => "us-west-2b"
    cidr_block:                      "10.6.128.0/18" => "10.6.128.0/18"
    ipv6_cidr_block:                 "2600:1f14:917:700::/64" => "" (forces new resource)
    ipv6_cidr_block_association_id:  "subnet-cidr-assoc-722fcb1b" => "<computed>"
    map_public_ip_on_launch:         "false" => "false"
    tags.%:                          "1" => "1"
    tags.Name:                       "thunderhead-dev.internal.us-west-2b" => "thunderhead-dev.internal.us-west-2b"
    vpc_id:                          "vpc-3e61cd5a" => "vpc-3e61cd5a"

Terraform Version

0.9.2

Affected Resource(s)

  • aws_vpc
  • aws_subnet

Terraform Configuration Files

resource "aws_vpc" "vpc" {
    cidr_block = "${var.vpc_cidr}"
    instance_tenancy = "default"

    enable_dns_hostnames = true
    enable_dns_support = true

    assign_generated_ipv6_cidr_block = "${var.use_ipv6}"
    lifecycle {
        prevent_destroy = true
    }

    tags {
        Name = "${var.vpc_name}"
    }
}

# Public Subnets
resource "aws_subnet" "public_subnet" {
    count = "${var.public_subnets_count}"

    vpc_id = "${aws_vpc.vpc.id}"

    cidr_block = "${element(split(",", var.public_subnets), count.index)}"
    ipv6_cidr_block = "${var.use_ipv6 == 0 ? "" :
                         cidrsubnet(aws_vpc.vpc.ipv6_cidr_block, 8, count.index)}"

    availability_zone = "${element(split(",", var.availability_zones), count.index)}"

    map_public_ip_on_launch = "${var.public_subnet_map_public_ip_on_launch}"
    assign_ipv6_address_on_creation = "${var.use_ipv6}"                                                                              

    tags {
        Name = "${format("%s.external.%s", var.vpc_name, element(split(",", var.availability_zones), count.index) )}"
    }
}

Expected Behavior

Terraform should be able to add/remove ipv6 cidr blocks from VPCs and Subnets without recreating them.

Actual Behavior

Terraform says it must recreate the vpc or subnet to add/remove ipv6 cidr blocks

Steps to Reproduce

  1. terraform plan

Important Factoids

@stack72
Copy link
Contributor

stack72 commented Apr 21, 2017

Hi @philipl

Terraform can now, as of 0.9.3, enable / disable IPv6 from aws_vpc. The only actions available on a subnet to modify are map_public_ip_on_launch and assign_ipv6_address_on_creation so any changes to IPv6 CIDR block will definitely need a subnet recreation

Please let me know if this isn't the case for you

Thanks

Paul

@stack72 stack72 closed this as completed Apr 21, 2017
@philipl
Copy link
Author

philipl commented Apr 21, 2017

I'm glad this works for VPCs now, but you can use the '(dis)associate-subnet-cidr-block' to add/remove an ipv6 block from an existing subnet. So it is possible.

@stack72
Copy link
Contributor

stack72 commented Apr 21, 2017

You are 100% correct - reopening and will work on this today :)

@stack72 stack72 reopened this Apr 21, 2017
stack72 added a commit that referenced this issue Apr 24, 2017
…eNew

Fixes: #13588

It was pointed out in #13588 that we don't need to ForceNew on a change
of IPv6 CIDR block. The logic I decided to implement here was to
disassociate then associate. We should only be able to be associated to
1 IPv6 CIDR block at once. This feels like a risky move. We can
disassociate and then error on the associate. This would leave us in a
situation where we have no IPv6 CIDR block associated

The alternative here would be that the failure of association, triggers
a reassociation with the old IPv6 CIDR block
stack72 added a commit that referenced this issue Apr 24, 2017
…eNew

Fixes: #13588

It was pointed out in #13588 that we don't need to ForceNew on a change
of IPv6 CIDR block. The logic I decided to implement here was to
disassociate then associate. We should only be able to be associated to
1 IPv6 CIDR block at once. This feels like a risky move. We can
disassociate and then error on the associate. This would leave us in a
situation where we have no IPv6 CIDR block associated

The alternative here would be that the failure of association, triggers
a reassociation with the old IPv6 CIDR block

I added a test to make sure that the subnet Ids don't change as the ipv6
block changes. Before removing the ForceNew from the ipv6_cidr_block,
the test results in the following:

```
=== RUN   TestAccAWSSubnet_ipv6
--- FAIL: TestAccAWSSubnet_ipv6 (92.09s)
	resource_aws_subnet_test.go:105: Expected SubnetIDs not to change, but both got before: subnet-0d2b6a6a and after: subnet-742c6d13
```

After the removal of ForceNew, the test result looks as follows:

```
=== RUN   TestAccAWSSubnet_ipv6
--- PASS: TestAccAWSSubnet_ipv6 (188.34s)
```

```
% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSSubnet_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/04/24 21:26:36 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSSubnet_ -timeout 120m
=== RUN   TestAccAWSSubnet_importBasic
--- PASS: TestAccAWSSubnet_importBasic (85.63s)
=== RUN   TestAccAWSSubnet_basic
--- PASS: TestAccAWSSubnet_basic (80.28s)
=== RUN   TestAccAWSSubnet_ipv6
--- PASS: TestAccAWSSubnet_ipv6 (188.34s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/aws	354.283s
```
@stack72
Copy link
Contributor

stack72 commented Apr 24, 2017

Hi @philipl

Just sent a PR that fixes this :)

Paul

@philipl
Copy link
Author

philipl commented Apr 24, 2017

Cheers!

stack72 added a commit that referenced this issue Apr 24, 2017
…eNew (#13909)

Fixes: #13588

It was pointed out in #13588 that we don't need to ForceNew on a change
of IPv6 CIDR block. The logic I decided to implement here was to
disassociate then associate. We should only be able to be associated to
1 IPv6 CIDR block at once. This feels like a risky move. We can
disassociate and then error on the associate. This would leave us in a
situation where we have no IPv6 CIDR block associated

The alternative here would be that the failure of association, triggers
a reassociation with the old IPv6 CIDR block

I added a test to make sure that the subnet Ids don't change as the ipv6
block changes. Before removing the ForceNew from the ipv6_cidr_block,
the test results in the following:

```
=== RUN   TestAccAWSSubnet_ipv6
--- FAIL: TestAccAWSSubnet_ipv6 (92.09s)
	resource_aws_subnet_test.go:105: Expected SubnetIDs not to change, but both got before: subnet-0d2b6a6a and after: subnet-742c6d13
```

After the removal of ForceNew, the test result looks as follows:

```
=== RUN   TestAccAWSSubnet_ipv6
--- PASS: TestAccAWSSubnet_ipv6 (188.34s)
```

```
% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSSubnet_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/04/24 21:26:36 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSSubnet_ -timeout 120m
=== RUN   TestAccAWSSubnet_importBasic
--- PASS: TestAccAWSSubnet_importBasic (85.63s)
=== RUN   TestAccAWSSubnet_basic
--- PASS: TestAccAWSSubnet_basic (80.28s)
=== RUN   TestAccAWSSubnet_ipv6
--- PASS: TestAccAWSSubnet_ipv6 (188.34s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/aws	354.283s
```
@ghost
Copy link

ghost commented Apr 13, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants