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

provider/aws: Allow aws_alb to have the name auto-generated #8673

Merged
merged 1 commit into from Sep 12, 2016

Conversation

stack72
Copy link
Contributor

@stack72 stack72 commented Sep 6, 2016

In order to satisify scenarios where a lifecycle is block is used, we
would need the AWS ALB name field to be autogenerated. We follow the
same work as AWS ELB, we allow either a name-prefix or we automatically assign a prefix of tf-lb

% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSALB_'                                                                                                                            2 ↵ ✹
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/09/08 12:43:40 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 (79.81s)
=== RUN   TestAccAWSALB_generatedName
--- PASS: TestAccAWSALB_generatedName (93.81s)
=== RUN   TestAccAWSALB_namePrefix
--- PASS: TestAccAWSALB_namePrefix (73.48s)
=== RUN   TestAccAWSALB_tags
--- PASS: TestAccAWSALB_tags (181.32s)
=== RUN   TestAccAWSALB_noSecurityGroup
--- PASS: TestAccAWSALB_noSecurityGroup (66.03s)
=== RUN   TestAccAWSALB_accesslogs
--- PASS: TestAccAWSALB_accesslogs (130.82s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    625.285s

@catsby
Copy link
Member

catsby commented Sep 6, 2016

LGTM – should probably add name_prefix in there while we're at it

@stack72 stack72 changed the title provider/aws: Allow aws_alb to have the name auto-generated [WIP] provider/aws: Allow aws_alb to have the name auto-generated Sep 7, 2016
@stack72 stack72 force-pushed the f-aws-alb-generated-name branch 3 times, most recently from 38bf96d to 8bc428c Compare September 8, 2016 11:59
@stack72 stack72 changed the title [WIP] provider/aws: Allow aws_alb to have the name auto-generated provider/aws: Allow aws_alb to have the name auto-generated Sep 8, 2016
@stack72
Copy link
Contributor Author

stack72 commented Sep 8, 2016

@catsby added :)

testAccCheckAWSALBExists("aws_alb.alb_test", &conf),
resource.TestCheckResourceAttrSet("aws_alb.alb_test", "name"),
resource.TestMatchResourceAttr("aws_alb.alb_test", "name",
regexp.MustCompile("^tf-lb+")),
Copy link
Member

Choose a reason for hiding this comment

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

It is basically working, but the + quantifier at the end doesn't make much sense as it belongs to b.
I assume "^tf-lb.+" is what you intended to type here? Perhaps "^tf-lb" would be sufficient and less greedy.

In order to satisify scenarios where a lifecycle is block is used, we
would need the AWS ALB name field to be autogenerated. WE follow the
same work as AWS ELB, we prefix it with `tl-lb-`

```
% 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/09/08 12:43:40 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 (79.81s)
=== RUN   TestAccAWSALB_generatedName
--- PASS: TestAccAWSALB_generatedName (93.81s)
=== RUN   TestAccAWSALB_namePrefix
--- PASS: TestAccAWSALB_namePrefix (73.48s)
=== RUN   TestAccAWSALB_tags
--- PASS: TestAccAWSALB_tags (181.32s)
=== RUN   TestAccAWSALB_noSecurityGroup
--- PASS: TestAccAWSALB_noSecurityGroup (66.03s)
=== RUN   TestAccAWSALB_accesslogs
--- PASS: TestAccAWSALB_accesslogs (130.82s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    625.285s
```
@stack72
Copy link
Contributor Author

stack72 commented Sep 12, 2016

Hi @radeksimko

I have made the change as requested - the new 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/09/12 10:32:12 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 (65.21s)
=== RUN   TestAccAWSALB_generatedName
--- PASS: TestAccAWSALB_generatedName (64.01s)
=== RUN   TestAccAWSALB_namePrefix
--- PASS: TestAccAWSALB_namePrefix (66.22s)
=== RUN   TestAccAWSALB_tags
--- PASS: TestAccAWSALB_tags (98.58s)
=== RUN   TestAccAWSALB_noSecurityGroup
--- PASS: TestAccAWSALB_noSecurityGroup (59.35s)
=== RUN   TestAccAWSALB_accesslogs
--- PASS: TestAccAWSALB_accesslogs (118.57s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    471.966s

@stack72 stack72 merged commit 44bc709 into master Sep 12, 2016
@stack72 stack72 deleted the f-aws-alb-generated-name branch September 12, 2016 10:04
@levinse
Copy link

levinse commented Sep 20, 2016

@stack72 thanks for making the change to allow name to be optional so create_before_destroy can be used.

Unfortunately this is still not usable. Using name_prefix and create_before_destroy makes the auto-generated name (including the prefix) over 32 chars, which results in an error from Terraform.

Can you guys cut the length of the auto-generated portion in half so name_prefix can be used? Or better yet, adjust the length of the auto-generated name based on the length of name_prefix so the final name is under 32 chars?

@stack72
Copy link
Contributor Author

stack72 commented Sep 22, 2016

Hi @levinse

Can you post what you autogenerated name is? When i don't specify a name i definitely have less than 32 characters

P.

@levinse
Copy link

levinse commented Sep 22, 2016

@stack72 I don't have access to the test environment now but if you use name_prefix of e.g. 10 chars the full string with the prefix was over 40 chars. I'll try to post the specifics later but it should be easy to repro.

@levinse
Copy link

levinse commented Oct 8, 2016

Hi @stack72 any updates on this? Thanks

@ghost
Copy link

ghost commented Apr 21, 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.

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

Successfully merging this pull request may close these issues.

None yet

4 participants