-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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
provider/aws: Allows aws_alb security_groups to be updated #9804
Conversation
Fixes #9658 Fixes #8728 Originally, this would ForceNew as follows: ``` -/+ aws_alb.alb_test arn: "arn:aws:elasticloadbalancing:us-west-2:187416307283:loadbalancer/app/test-alb-9658/3459cd2446b76901" => "<computed>" arn_suffix: "app/test-alb-9658/3459cd2446b76901" => "<computed>" dns_name: "test-alb-9658-1463108301.us-west-2.elb.amazonaws.com" => "<computed>" enable_deletion_protection: "false" => "false" idle_timeout: "30" => "30" internal: "false" => "false" name: "test-alb-9658" => "test-alb-9658" security_groups.#: "2" => "1" (forces new resource) security_groups.1631253634: "sg-3256274b" => "" (forces new resource) security_groups.3505955000: "sg-1e572667" => "sg-1e572667" (forces new resource) subnets.#: "2" => "2" subnets.2407170741: "subnet-ee536498" => "subnet-ee536498" subnets.2414619308: "subnet-f1a7b595" => "subnet-f1a7b595" tags.%: "1" => "1" tags.TestName: "TestAccAWSALB_basic" => "TestAccAWSALB_basic" vpc_id: "vpc-dd0ff9ba" => "<computed>" zone_id: "Z1H1FL5HABSF5" => "<computed>" Plan: 1 to add, 0 to change, 1 to destroy. ``` When the ALB was ForceNew, the ARN changed. The test has been updated to include a check to make sure that the ARNs are the same after the update After this change, it looks as follows: ``` ~ aws_alb.alb_test security_groups.#: "1" => "2" security_groups.1631253634: "" => "sg-3256274b" security_groups.3505955000: "sg-1e572667" => "sg-1e572667" Plan: 0 to add, 1 to change, 0 to destroy. ``` Test Results: ``` % make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSALB_' ✹ ==> Checking that code complies with gofmt requirements... go generate $(go list ./... | grep -v /terraform/vendor/) 2016/11/02 12:20:58 Generated command/internal_plugin_list.go TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSALB_ -timeout 120m === RUN TestAccAWSALB_basic --- PASS: TestAccAWSALB_basic (64.25s) === RUN TestAccAWSALB_generatedName --- PASS: TestAccAWSALB_generatedName (65.04s) === RUN TestAccAWSALB_namePrefix --- PASS: TestAccAWSALB_namePrefix (67.02s) === RUN TestAccAWSALB_tags --- PASS: TestAccAWSALB_tags (96.06s) === RUN TestAccAWSALB_updatedSecurityGroups --- PASS: TestAccAWSALB_updatedSecurityGroups (101.61s) === RUN TestAccAWSALB_noSecurityGroup --- PASS: TestAccAWSALB_noSecurityGroup (59.83s) === RUN TestAccAWSALB_accesslogs --- PASS: TestAccAWSALB_accesslogs (162.65s) PASS ok github.com/hashicorp/terraform/builtin/providers/aws 616.489s ```
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, LGTM.
I just tried this out; I cannot remove all the security groups from an ALB now. Removing |
…#9804) Fixes hashicorp#9658 Fixes hashicorp#8728 Originally, this would ForceNew as follows: ``` -/+ aws_alb.alb_test arn: "arn:aws:elasticloadbalancing:us-west-2:187416307283:loadbalancer/app/test-alb-9658/3459cd2446b76901" => "<computed>" arn_suffix: "app/test-alb-9658/3459cd2446b76901" => "<computed>" dns_name: "test-alb-9658-1463108301.us-west-2.elb.amazonaws.com" => "<computed>" enable_deletion_protection: "false" => "false" idle_timeout: "30" => "30" internal: "false" => "false" name: "test-alb-9658" => "test-alb-9658" security_groups.#: "2" => "1" (forces new resource) security_groups.1631253634: "sg-3256274b" => "" (forces new resource) security_groups.3505955000: "sg-1e572667" => "sg-1e572667" (forces new resource) subnets.#: "2" => "2" subnets.2407170741: "subnet-ee536498" => "subnet-ee536498" subnets.2414619308: "subnet-f1a7b595" => "subnet-f1a7b595" tags.%: "1" => "1" tags.TestName: "TestAccAWSALB_basic" => "TestAccAWSALB_basic" vpc_id: "vpc-dd0ff9ba" => "<computed>" zone_id: "Z1H1FL5HABSF5" => "<computed>" Plan: 1 to add, 0 to change, 1 to destroy. ``` When the ALB was ForceNew, the ARN changed. The test has been updated to include a check to make sure that the ARNs are the same after the update After this change, it looks as follows: ``` ~ aws_alb.alb_test security_groups.#: "1" => "2" security_groups.1631253634: "" => "sg-3256274b" security_groups.3505955000: "sg-1e572667" => "sg-1e572667" Plan: 0 to add, 1 to change, 0 to destroy. ``` Test Results: ``` % make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSALB_' ✹ ==> Checking that code complies with gofmt requirements... go generate $(go list ./... | grep -v /terraform/vendor/) 2016/11/02 12:20:58 Generated command/internal_plugin_list.go TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSALB_ -timeout 120m === RUN TestAccAWSALB_basic --- PASS: TestAccAWSALB_basic (64.25s) === RUN TestAccAWSALB_generatedName --- PASS: TestAccAWSALB_generatedName (65.04s) === RUN TestAccAWSALB_namePrefix --- PASS: TestAccAWSALB_namePrefix (67.02s) === RUN TestAccAWSALB_tags --- PASS: TestAccAWSALB_tags (96.06s) === RUN TestAccAWSALB_updatedSecurityGroups --- PASS: TestAccAWSALB_updatedSecurityGroups (101.61s) === RUN TestAccAWSALB_noSecurityGroup --- PASS: TestAccAWSALB_noSecurityGroup (59.83s) === RUN TestAccAWSALB_accesslogs --- PASS: TestAccAWSALB_accesslogs (162.65s) PASS ok github.com/hashicorp/terraform/builtin/providers/aws 616.489s ```
I'm having similar problems with this now, terraform 0.8.4. In my case, I've modified a sg so that it has to be recreated. It times out trying to delete the sg - it doesn't properly detach it from the ALB first. |
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. |
Fixes #9658
Fixes #8728
Originally, this would ForceNew as follows:
When the ALB was ForceNew, the ARN changed. The test has been updated to include a check to make sure that the ARNs are the same after the update
After this change, it looks as follows:
Test Results: