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

Opsworks Application support #4419

Merged
merged 1 commit into from
Apr 16, 2016

Conversation

odise
Copy link
Contributor

@odise odise commented Dec 22, 2015

This PR implements Opsworks App resources along with some tests. I tried to include all app related features however aws-flow-ruby type is incomplete (AwsFlowRubySettings). opsworks.UpdateApp doesn't support application Shortname updates so those will be ignored.

@u2mejc
Copy link
Contributor

u2mejc commented Apr 2, 2016

@odise Would you be willing to squash your commits and rebase this?

Also here's a patch to fix the current compilation issues: (you can apply it with git apply or manually)

From 26146c9c9d50493680da3999c75fd3e9d87fdd4a Mon Sep 17 00:00:00 2001
From: Justin Clark <u2mejc@users.noreply.github.com>
Date: Fri, 1 Apr 2016 17:00:31 -0700
Subject: [PATCH] Refactor OpsWorks Applications PR for Terraform 0.6.x

---
 .../providers/aws/resource_aws_opsworks_application.go | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/builtin/providers/aws/resource_aws_opsworks_application.go b/builtin/providers/aws/resource_aws_opsworks_application.go
index 3b9a2af..cf63c3b 100644
--- a/builtin/providers/aws/resource_aws_opsworks_application.go
+++ b/builtin/providers/aws/resource_aws_opsworks_application.go
@@ -268,7 +268,7 @@ func resourceAwsOpsworksApplicationRead(d *schema.ResourceData, meta interface{}
    d.Set("stack_id", app.StackId)
    d.Set("type", app.Type)
    d.Set("description", app.Description)
-   d.Set("domains", unwrapAwsStringList(app.Domains))
+   d.Set("domains", flattenStringList(app.Domains))
    d.Set("enable_ssl", app.EnableSsl)
    resourceAwsOpsworksSetApplicationSsl(d, app.SslConfiguration)
    resourceAwsOpsworksSetApplicationSource(d, app.AppSource)
@@ -292,7 +292,7 @@ func resourceAwsOpsworksApplicationCreate(d *schema.ResourceData, meta interface
        StackId:          aws.String(d.Get("stack_id").(string)),
        Type:             aws.String(d.Get("type").(string)),
        Description:      aws.String(d.Get("description").(string)),
-       Domains:          makeAwsStringList(d.Get("domains").([]interface{})),
+       Domains:          expandStringList(d.Get("domains").([]interface{})),
        EnableSsl:        aws.Bool(d.Get("enable_ssl").(bool)),
        SslConfiguration: resourceAwsOpsworksApplicationSsl(d),
        AppSource:        resourceAwsOpsworksApplicationSource(d),
@@ -302,7 +302,7 @@ func resourceAwsOpsworksApplicationCreate(d *schema.ResourceData, meta interface
    }

    var resp *opsworks.CreateAppOutput
-   err = resource.Retry(2*time.Minute, func() error {
+   err = resource.Retry(2*time.Minute, func() *resource.RetryError {
        var cerr error
        resp, cerr = client.CreateApp(req)
        if cerr != nil {
@@ -310,9 +310,9 @@ func resourceAwsOpsworksApplicationCreate(d *schema.ResourceData, meta interface
            if opserr, ok := cerr.(awserr.Error); ok {
                // XXX: handle errors
                log.Printf("[ERROR] OpsWorks error: %s message: %s", opserr.Code(), opserr.Message())
-               return cerr
+               return resource.RetryableError(cerr)
            }
-           return resource.RetryError{Err: cerr}
+           return resource.NonRetryableError(cerr)
        }
        return nil
    })
@@ -336,7 +336,7 @@ func resourceAwsOpsworksApplicationUpdate(d *schema.ResourceData, meta interface
        Name:             aws.String(d.Get("name").(string)),
        Type:             aws.String(d.Get("type").(string)),
        Description:      aws.String(d.Get("description").(string)),
-       Domains:          makeAwsStringList(d.Get("domains").([]interface{})),
+       Domains:          expandStringList(d.Get("domains").([]interface{})),
        EnableSsl:        aws.Bool(d.Get("enable_ssl").(bool)),
        SslConfiguration: resourceAwsOpsworksApplicationSsl(d),
        AppSource:        resourceAwsOpsworksApplicationSource(d),
@@ -348,7 +348,7 @@ func resourceAwsOpsworksApplicationUpdate(d *schema.ResourceData, meta interface
    log.Printf("[DEBUG] Updating OpsWorks layer: %s", d.Id())

    var resp *opsworks.UpdateAppOutput
-   err := resource.Retry(2*time.Minute, func() error {
+   err := resource.Retry(2*time.Minute, func() *resource.RetryError {
        var cerr error
        resp, cerr = client.UpdateApp(req)
        if cerr != nil {
@@ -356,9 +356,9 @@ func resourceAwsOpsworksApplicationUpdate(d *schema.ResourceData, meta interface
            if opserr, ok := cerr.(awserr.Error); ok {
                // XXX: handle errors
                log.Printf("[ERROR] OpsWorks error: %s message: %s", opserr.Code(), opserr.Message())
-               return cerr
+               return resource.NonRetryableError(cerr)
            }
-           return resource.RetryError{Err: cerr}
+           return resource.RetryableError(cerr)
        }
        return nil
    })
-- 
2.7.3

@janschumann janschumann force-pushed the opsworks-application branch 2 times, most recently from 5c5fd56 to 1d13224 Compare April 4, 2016 10:51
@janschumann
Copy link
Contributor

@u2mejc This branch is now squashed and rebased. Tests should also pass.

There are some issues. E.g. Attributes are not passed to the api client for some reason, resulting in default values for document_root etc.

Would you now get this PR merged, so we can continue working on issues and refactoring (e.g. renaming application to app etc.) in separate PRs/Issues?

@u2mejc
Copy link
Contributor

u2mejc commented Apr 4, 2016

@janschumann said:

There are some issues. E.g. Attributes are not passed to the api client for some reason, resulting in default values for document_root etc.

ah I just worked quickly to get it to compile and just a quick terraform apply test on friday, sorry I missed that. Do you have time to see if you can find that bug and do an amend?

@janschumann
Copy link
Contributor

@u2mejc I tried to figure out what the problem is.But debugging ist not very easy in terraform. Could you assist me with debugging? I am using IntelliJ IDEA. But the go debugging feature does not work due tu multiple file builds.

@u2mejc
Copy link
Contributor

u2mejc commented Apr 7, 2016

@janschumann I'm not sure if I can help much, I'm not familiar with IntelliJ. I'm a vim user and I've been compiling using make plugin-dev PLUGIN=provider-aws. I didn't have to do much debug for the refactor.

Good news is I'm not able to reproduce the bug you mentioned. IMHO this better then just MVP and ready to ship. I just rebased and dropped the errant new line in the .travis.yml.

@apparentlymart when you have a sec, could you please give this a peak and let me know if you'd think we need anything else before we merge?

@apparentlymart apparentlymart merged commit 6bf9f21 into hashicorp:master Apr 16, 2016
@apparentlymart
Copy link
Contributor

Sorry for the delay on this one @odise, and thanks for the contribution and your patience with getting it merged.

Some drift in the stack acceptance tests was causing the acceptance tests here to fail but I fixed that up in 2d597f0.

@ghost
Copy link

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

6 participants