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.

Sign up for GitHub

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

Jump to bottom

provider/aws: Elastic Beanstalk Application Version #7407

Closed
wants to merge 6 commits into from
Closed

provider/aws: Elastic Beanstalk Application Version #7407

wants to merge 6 commits into from

Conversation

lestephane
Copy link

No description provided.

@radeksimko radeksimko changed the title This is a pull-request update of https://github.com/hashicorp/terraform/pull/5770 to resolve conflicts, and hopefully speed the inclusion of this enhancement into master. provider/aws: Elastic Beanstalk Application Version Jun 29, 2016
@lestephane lestephane mentioned this pull request Jun 29, 2016
Merged
matchaxnb pushed a commit to matchaxnb/terraform that referenced this pull request Jun 29, 2016
@phinze
core: Fix destroy when module vars used in provider config
bf0e770 
For `terraform destroy`, we currently build up the same graph we do for
`plan` and `apply` and we do a walk with a special Diff that says
"destroy everything".

We have fought the interpolation subsystem time and again through this
code path. Beginning in hashicorp#2775 we gained a new feature to selectively
prune out problematic graph nodes. The past chain of destroy fixes I
have been involved with (hashicorp#6557, hashicorp#6599, hashicorp#6753) have attempted to massage
the "noop" definitions to properly handle the edge cases reported.

"Variable is depended on by provider config" is another edge case we add
here and try to fix.

This dive only makes me more convinced that the whole `terraform
destroy` code path needs to be reworked.

For now, I went with a "surgical strike" approach to the problem
expressed in hashicorp#7047. I found a couple of issues with the existing
Noop and DestroyEdgeInclude logic, especially with regards to
flattening, but I'm explicitly ignoring these for now so we can get this
particular bug fixed ahead of the 0.7 release. My hope is that we can
circle around with a fully specced initiative to refactor `terraform
destroy`'s graph to be more state-derived than config-derived.

Until then, this fixes hashicorp#7407
@kevtainer kevtainer mentioned this pull request Jul 6, 2016
Closed
@lestephane
Copy link
Author

I have rebased and run the acceptance tests successfully:

    lestephane@le-studio: [  ] :~/GoGitRepos/src/github.com/hashicorp/terraform $ aws-vault exec --debug --assume-role-ttl=1h terraform-acceptance-test@sandbox -- make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSBeanstalk'
    ...
    ==> Checking that code complies with gofmt requirements...
    /home/lestephane/GoGitRepos/bin/stringer
    go generate $(go list ./... | grep -v /vendor/)
    2016/07/08 15:30:58 Generated command/internal_plugin_list.go
    TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSBeanstalk -timeout 120m
    === RUN   TestAccAWSBeanstalkApp_basic
    --- PASS: TestAccAWSBeanstalkApp_basic (24.48s)
    === RUN   TestAccAWSBeanstalkAppVersion_basic
    --- PASS: TestAccAWSBeanstalkAppVersion_basic (58.88s)
    === RUN   TestAccAWSBeanstalkConfigurationTemplate_basic
    --- PASS: TestAccAWSBeanstalkConfigurationTemplate_basic (33.92s)
    === RUN   TestAccAWSBeanstalkConfigurationTemplate_VPC
    --- PASS: TestAccAWSBeanstalkConfigurationTemplate_VPC (56.67s)
    === RUN   TestAccAWSBeanstalkConfigurationTemplate_Setting
    --- PASS: TestAccAWSBeanstalkConfigurationTemplate_Setting (33.86s)
    === RUN   TestAccAWSBeanstalkEnv_basic
    --- PASS: TestAccAWSBeanstalkEnv_basic (419.71s)
    === RUN   TestAccAWSBeanstalkEnv_tier
    --- PASS: TestAccAWSBeanstalkEnv_tier (632.75s)
    === RUN   TestAccAWSBeanstalkEnv_outputs
    --- PASS: TestAccAWSBeanstalkEnv_outputs (402.95s)
    === RUN   TestAccAWSBeanstalkEnv_cname_prefix
    --- PASS: TestAccAWSBeanstalkEnv_cname_prefix (416.76s)
    === RUN   TestAccAWSBeanstalkEnv_config
    --- PASS: TestAccAWSBeanstalkEnv_config (611.49s)
    === RUN   TestAccAWSBeanstalkEnv_resource
    --- PASS: TestAccAWSBeanstalkEnv_resource (403.15s)
    === RUN   TestAccAWSBeanstalkEnv_version_label
    --- PASS: TestAccAWSBeanstalkEnv_version_label (441.97s)
    PASS
    ok      github.com/hashicorp/terraform/builtin/providers/aws    3536.610s

@stack72
Copy link
Contributor

stack72 commented Jul 18, 2016

@dharrisio / @djlestephane

Can you folks please let me know about this one? Do we use this and close off #5770? If so, please can this be rebased

Thanks

Paul

@stack72 stack72 added the waiting-response An issue/pull request is waiting for a response from the community label Jul 18, 2016
@dharrisio
Copy link
Contributor

@stack72 I don't think this is quite ready to be merged. There are still a few pieces outlined in #5770 that make me uncomfortable with the changes. For now, I'm going to update #5770 as a WIP, because I think that is probably more representative of the work that as been done so far on that PR.

I do have some plans for using the application version without a new resource. This should at least help some use cases with Elastic Beanstalk. I'm planning on getting that done soon, as well as working on some of the issues in #5770. I should have more on that this week. Hope this clears up the current status of this for you!

@lestephane
Copy link
Author

@stack72 I've rebased #7407 in the meantime. I'm new at this so if that is
not the right thing to do, you can close my pull request.

On Mon, Jul 18, 2016 at 7:18 PM, David Harris notifications@github.com
wrote:

@stack72 https://github.com/stack72 I don't think this is quite ready
to be merged. There are still a few pieces outlined in #5770
#5770 that make me
uncomfortable with the changes. For now, I'm going to update #5770
#5770 as a WIP, because I
think that is probably more representative of the work that as been done so
far on that PR.

I do have some plans for using the application version without a new
resource. This should at least help some use cases with Elastic Beanstalk.
I'm planning on getting that done soon, as well as working on some of the
issues in #5770 #5770. I
should have more on that this week. Hope this clears up the current status
of this for you!

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#7407 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFNTlF29gw3Hcta6L_hUjVPF6iBzFFC0ks5qW7VLgaJpZM4JA9iJ
.

@marchub
Copy link

marchub commented Aug 12, 2016

+1

@radeksimko radeksimko mentioned this pull request Sep 10, 2016
Open
@dharrisio
Copy link
Contributor

@stack72 This pr can probably be closed now that #5770 has been merged.

@stack72
Copy link
Contributor

stack72 commented Feb 22, 2017

Closed via #5770

@stack72 stack72 closed this Feb 22, 2017
@ghost
Copy link

ghost commented Apr 16, 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 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
enhancement provider/aws waiting-response An issue/pull request is waiting for a response from the community
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@lestephane @stack72 @dharrisio @marchub @radeksimko