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: Elastic Beanstalk Application Version #3871

Conversation

dharrisio
Copy link
Contributor

This is still a work in progress, but I've added some basic support for using beanstalk application versions. This PR also includes a merge from the master branch, since I wasn't able to build without those updates.

Still need to add:

  • Tests
  • Documentation

Basic document to use the new resource:

provider "aws" {
  region = "us-east-1"
}

resource "aws_s3_bucket" "default" {
  bucket = "tftest.applicationversion.bucket"
}

resource "aws_s3_bucket_object" "default" {
  bucket = "${aws_s3_bucket.default.id}"
  key = "beanstalk/go-v1.zip"
  source = "go-v1.zip"
}

resource "aws_elastic_beanstalk_application" "default" {
  name = "tf-test-name"
  description = "tf-test-desc"
}

resource "aws_elastic_beanstalk_environment" "default" {
  name = "tf-test-name"
  application = "${aws_elastic_beanstalk_application.default.name}"
  version_label = "${aws_elastic_beanstalk_application_version.default.name}"
  solution_stack_name = "64bit Amazon Linux 2015.09 v2.0.4 running Go 1.4"
}

resource "aws_elastic_beanstalk_application_version" "default" {
  application = "tf-test-name"
  name = "tf-test-version-label"
  bucket = "${aws_s3_bucket.default.id}"
  key = "${aws_s3_bucket_object.default.id}"
}

@dharrisio dharrisio changed the title F aws elastic beanstalk provider/aws: Elastic Beanstalk Application Version Nov 12, 2015
@@ -178,12 +184,14 @@ func resourceAwsElasticBeanstalkEnvironmentUpdate(d *schema.ResourceData, meta i
func resourceAwsElasticBeanstalkEnvironmentDescriptionUpdate(conn *elasticbeanstalk.ElasticBeanstalk, d *schema.ResourceData) error {
name := d.Get("name").(string)
desc := d.Get("description").(string)
version := d.Get("version_label").(string)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't resourceAwsElasticBeanstalkEnvironmentUpdate check for d.HasChange("version") and trigger an update as well?

Also, (forgive my ignorance of how terraform works here) if a ForceNew is triggered on an application version - for example say the key field changes - would an update propagate to the beanstalk environment telling it to re-deploy the application version? I worry that the application version will be re-created but not actually re-deployed to the environment since the version_label would be the same string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zpatrick you're correct about the version update, 4c3fd07 fixes that.

The answer to your second question is yes, terraform should update the environment with the new version as long as your environment resource references the new version_label. If you do something like the example document above, that will happen by changing the version_label for the aws_elastic_beanstalk_application_version resource. The output of terraform plan would look something like this:

-/+ aws_elastic_beanstalk_application_version.default
    application: "tf-test-name" => "tf-test-name"
    bucket:      "tftest.applicationversion.bucket" => "tftest.applicationversion.bucket"
    key:         "beanstalk/go-v1.zip" => "beanstalk/go-v1.zip"
    name:        "tf-test-version-label" => "tf-test-version-new-label" (forces new resource)

~ aws_elastic_beanstalk_environment.default
    version_label: "tf-test-version-label" => "tf-test-version-new-label"

@@ -172,6 +181,12 @@ func resourceAwsElasticBeanstalkEnvironmentUpdate(d *schema.ResourceData, meta i
}
}

if d.HasChange("version_label") {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you attempt to update a setting option and the version label at the same time AWS returns this error

* aws_elastic_beanstalk_environment.default: InvalidParameterValue: Environment named tf-test-name is in an invalid state for this operation. Must be Ready.

Here is an example document that demonstrates the issue.

provider "aws" {
  region = "us-east-1"
}

resource "aws_s3_bucket" "default" {
  bucket = "tftest.applicationversion.bucket"
}

resource "aws_s3_bucket_object" "default" {
  bucket = "${aws_s3_bucket.default.id}"
  key    = "beanstalk/go-v1.zip"
  source = "go-v1.zip"
}

resource "aws_elastic_beanstalk_application" "default" {
  name        = "tf-test-name"
  description = "tf-test-desc"
}

resource "aws_elastic_beanstalk_environment" "default" {
  name                = "tf-test-name"
  application         = "${aws_elastic_beanstalk_application.default.name}"
  version_label       = "${aws_elastic_beanstalk_application_version.default.name}"
  solution_stack_name = "64bit Amazon Linux 2015.09 v2.0.4 running Go 1.4"

  setting {
    namespace = "aws:autoscaling:asg"
    name      = "MinSize"
    value     = "1"
  }

  setting {
    namespace = "aws:autoscaling:asg"
    name      = "MaxSize"
    value     = "1"
  }
}

resource "aws_elastic_beanstalk_application_version" "default" {
  application = "tf-test-name"
  name        = "tf-test-version-label"
  bucket      = "${aws_s3_bucket.default.id}"
  key         = "${aws_s3_bucket_object.default.id}"
}

Change document to

provider "aws" {
  region = "us-east-1"
}

resource "aws_s3_bucket" "default" {
  bucket = "tftest.applicationversion.bucket"
}

resource "aws_s3_bucket_object" "default" {
  bucket = "${aws_s3_bucket.default.id}"
  key    = "beanstalk/go-v1.zip"
  source = "go-v1.zip"
}

resource "aws_elastic_beanstalk_application" "default" {
  name        = "tf-test-name"
  description = "tf-test-desc"
}

resource "aws_elastic_beanstalk_environment" "default" {
  name                = "tf-test-name"
  application         = "${aws_elastic_beanstalk_application.default.name}"
  version_label       = "${aws_elastic_beanstalk_application_version.default.name}"
  solution_stack_name = "64bit Amazon Linux 2015.09 v2.0.4 running Go 1.4"

  setting {
    namespace = "aws:autoscaling:asg"
    name      = "MinSize"
    value     = "1"
  }

  setting {
    namespace = "aws:autoscaling:asg"
    name      = "MaxSize"
    value     = "2"
  }
}

resource "aws_elastic_beanstalk_application_version" "default" {
  application = "tf-test-name"
  name        = "tf-test-version-label-new"
  bucket      = "${aws_s3_bucket.default.id}"
  key         = "${aws_s3_bucket_object.default.id}"
}

This causes an issue because the environment is still making the option setting changes when it tries to update the environment with the new application version.

I think there are two potential ways to fix this.

  1. Instead of calling update environment for each changed attribute, build one struct that contains all the attribute changes and make one api call to update the environment.
  2. Before calling update environment wait for the state to be ready.

@catsby I'm not sure which of those would be the preferred way, so let me know what you think.

@dharrisio
Copy link
Contributor Author

@catsby Other than the one issue I commented on the diff about, I think this is ready for you to take a look at when you have the time.

@Bowbaq
Copy link
Contributor

Bowbaq commented Jan 13, 2016

I have this code pulled down in my terraform branch. We ended up deciding against using terraform to manage application versions for now (it doesn't fit super well with our CI/CD process).

We created the elastic beanstalk environments without a version_label parameter, which worked fine. We then deployed code to those environments. Now, terraform really wants to override the version from the deployed code with the empty string:

...
version_label:                "commit-59ab401b97-checksum-38d72bd0e0" => ""

I've applied the following patch to my branch to prevent that, but it's not ideal:

diff --git a/builtin/providers/aws/resource_aws_elastic_beanstalk_environment.go b/builtin/providers/aws/resource_aws_elastic_beanstalk_environment.go
index 40af3f4..3ce895e 100644
--- a/builtin/providers/aws/resource_aws_elastic_beanstalk_environment.go
+++ b/builtin/providers/aws/resource_aws_elastic_beanstalk_environment.go
@@ -183,7 +183,7 @@ func resourceAwsElasticBeanstalkEnvironmentUpdate(d *schema.ResourceData, meta i
                }
        }

-       if d.HasChange("version_label") {
+       if d.HasChange("version_label") && d.Get("version_label").(string) != "" {
                if err := resourceAwsElasticBeanstalkEnvironmentApplicationVersionUpdate(conn, d); err != nil {
                        return err
                }

Ideally, if version_label isn't set, it shouldn't even show up in the diff

@dharrisio
Copy link
Contributor Author

@Bowbaq thanks for the feedback. I can't think of a better way to do this then what you have there. The problem sounds similar to this description on the mailing list. https://groups.google.com/forum/#!searchin/terraform-tool/vpc$20nat/terraform-tool/MxEXo9hhqHk/eFXZi-K0AQAJ

We would essentially need a way to ignore a "sub-resource" . @catsby do you have any recommendations on how to handle this?

@dharrisio
Copy link
Contributor Author

@Bowbaq after looking into this some more it looks like there might already be a way to deal with this. Would the lifecylce paramter ignore_changes fix the problem?

For example:

lifecycle {
    ignore_changes = [ "version_label"]
}

Let me know if that fixes the problem for you.

@Bowbaq
Copy link
Contributor

Bowbaq commented Jan 22, 2016

Didn't know about that trick, I'll give it a shot & let you know

@Bowbaq
Copy link
Contributor

Bowbaq commented Jan 26, 2016

Looks like that works fine, thanks

@Bowbaq
Copy link
Contributor

Bowbaq commented Jan 27, 2016

@dharrisio turned out not to work after all. Subsequent changes to environment settings failed to apply with a message about diffs not matching. Running with debug logging on, it seemed to come from version_label being in one diff & not the other

@dharrisio
Copy link
Contributor Author

@Bowbaq Thanks for the info, I'll try and find some time to look into that soon.

@dharrisio
Copy link
Contributor Author

@Bowbaq The version_label issue turned out to not be specific here (#4965). It should be fixed the next time master gets merged into f-aws-elastic-beanstalk branch.

@Bowbaq
Copy link
Contributor

Bowbaq commented Feb 2, 2016

Good to hear, I'll merge that into my custom branch tomorrow & give it a spin

@dharrisio
Copy link
Contributor Author

@catsby I added another test to verify that the correct version_label gets deployed to the Beanstalk Environment, hopefully that makes this easier to merge. When you have a chance can you take a look at this and let me know if it needs anything else before it can be merged?

@pecastro
Copy link

Hi,

I've been testing your branch on 251f151 and I've noticed a couple of things ...
The aws_elastic_beanstalk_application_version resource doesn't seem to be able to handle the 'depends_on' throwing an error like:

Error loading config: Error loading example.tf: Error reading depends_on for aws_elastic_beanstalk_application_version[hello]: At 498:16: unknown slice type: *ast.LiteralType

Which I resorted to after realiing that the deps weren't being properly handled ...

`Error applying plan:

1 error(s) occurred:

  • aws_elastic_beanstalk_application_version.hello: InvalidParameterValue: No Application named 'hello-app' found.`

It seems that in my case aws_elastic_beanstalk_application_version wanted to run first before aws_elastic_beanstalk_application

Can you confirm / make sense of any of this ?

Other than that it all seems to work so far.

@pecastro
Copy link

It also seems to insist on _Updating environment hello-app's configuration settings. on subsequent terraform apply's even though nothing has changed in the .tf file.

@dharrisio
Copy link
Contributor Author

@pecastro thanks for the feedback. I haven't seen any issues like the first one you described. Can you post an example config where the dependencies don't work as expected? Ideally, you shouldn't need to use the depends_on option, but I'll experiment with that and see if I can recreate the issue.

For the second problem, which settings are causing the recurring plans? I think that the root of that problem is in f-aws-elastic-beanstalk. There is probably more work to do with the option_settings and recurring plans, mostly related with how Amazon inconsistently returns values for those settings.

@dharrisio
Copy link
Contributor Author

@pecastro One other thing to note on the Error loading config error. Make sure that depends_on is a list. I am seeing a different error that I am looking into.

This is the document I am using to test this:

provider "aws" {
  region = "us-east-1"
}

resource "aws_s3_bucket" "default" {
  bucket = "tftest.applicationversion.bucket-1234"
}

resource "aws_s3_bucket_object" "default" {
  bucket = "${aws_s3_bucket.default.id}"
  key    = "beanstalk/go-v1.zip"
  source = "test-fixtures/beanstalk-go-v1.zip"
}

resource "aws_elastic_beanstalk_application" "default" {
  name        = "tf-test-name"
  description = "tf-test-desc"
}

resource "aws_elastic_beanstalk_application_version" "default" {
  application = "${aws_elastic_beanstalk_application.default.name}"
  name        = "tf-test-version-label"
  bucket      = "${aws_s3_bucket.default.id}"
  key         = "${aws_s3_bucket_object.default.id}"
  depends_on  = ["${aws_elastic_beanstalk_application.default}"]
}

The new error message is * aws_elastic_beanstalk_application_version.default: missing dependency: ${aws_elastic_beanstalk_application.default}

@dharrisio
Copy link
Contributor Author

@catsby I rebased this against master in #5770, going to close this one.

@dharrisio dharrisio closed this Mar 21, 2016
omeid pushed a commit to omeid/terraform that referenced this pull request Mar 30, 2018
…nal-rate-based-rule

New Resource: aws_wafregional_rate_based_rule
@ghost
Copy link

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

5 participants