provider/aws: Opsworks instance #4276

mrjefftang · 2015-12-11T23:39:32Z

This is a work in progress to support issue #3459.

There's still a lot of work to be done.

TODO:

CRUD functionality
Code cleanup
Documentation

mrjefftang · 2015-12-11T23:59:38Z

P.S. Any comments / suggestions would be greatly appreciated.

jmcclell · 2015-12-15T20:40:17Z

@mrjefftang I actually have this implemented already. I haven't created a PR yet because I'm still testing and also trying to add opsworks app, the ability to register existing ec2 instances via an opsworks_instance_registration resource, and trying to figure out the best way to do instance scheduling for time based instances. You can see the current stuff I have pushed at https://github.com/jmcclell/terraform. Currently it's in the mater branch.

I'm very new to Go so if you have any feedback I'd appreciate it.

mrjefftang · 2015-12-15T20:43:28Z

@jmcclell Looks like we've came to mostly the same code with the exception that you've added some additional validation and I've handled root block device definitions.

jmcclell · 2015-12-15T21:21:16Z

Root block devices aren't part of the opsworks instance level, as far as I can tell?

mrjefftang · 2015-12-15T21:24:47Z

You can actually define the root block device via the API. I've tested it with the code on this pull request.

I've also incorporated your validation checks into my branch via the ValidateFunc specification on the schema.

jmcclell · 2015-12-15T21:29:59Z

Aye. It looks like you've done some other neat stuff, as well. Elastic IP/Vol options, for example. Some of my validation checks are actually probably not necessary since the AWS struct itself will yell - I didn't realize this at first.

Curious - what is the advantage of pulling back items such as CreateTime, etc? I wasn't sure if it made sense to do so or not since it isn't part of managing the resource. Since I'm very new to all of this perhaps I am missing something?

Also - is your goal to have it actually start the underlying instance on create? I was thinking of adding a property to determine the desired state of the underlying instance or something. Thoughts?

mrjefftang · 2015-12-15T22:05:10Z

I figure validation should be front loaded as possible.

I pulled back all of the extra attributes just out of completeness, I was just going down the list from the structure definition in the AWS SDK.

I haven't thought about the instance state yet, not sure what the different use cases will be. It makes sense to have a "desired state" and react to that.

mrjefftang · 2015-12-16T19:29:36Z

This PR is completed.

jmcclell · 2015-12-16T21:46:41Z

builtin/providers/aws/resource_aws_opsworks_instance.go

+	return
+}
+
+func resourceAwsOpsworksInstanceValidate(d *schema.ResourceData) error {


Are you certain this is the case? I don't see this in the documentation, but haven't had a chance to test myself. Seems odd that this would be restricted since you can add block devices to any other ec2 instance regardless of AMI.

It errors out when attempting to add a block device mapping when using a
custom AMI.

I believe the only way to do it is by specifying the block device mapping
when creating your custom AMI.
On Wed, Dec 16, 2015 at 5:07 PM Jason McClellan (dsc) <
notifications@github.com> wrote:

In builtin/providers/aws/resource_aws_opsworks_instance.go
#4276 (comment):

errors = append(errors, fmt.Errorf(

"%q must be one of \"running\" or \"stopped\"", k))

}

return
+}

+func validateVirtualizationType(v interface{}, k string) (ws []string, errors []error) {

value := v.(string)

if value != "paravirtual" && value != "hvm" {

errors = append(errors, fmt.Errorf(

"%q must be one of \"paravirtual\" or \"hvm\"", k))

}

return
+}

+func resourceAwsOpsworksInstanceValidate(d *schema.ResourceData) error {

Are you certain this is the case? I don't see this in the documentation,
but haven't had a chance to test myself. Seems odd that this would be
restricted since you can add block devices to any other ec2 instance
regardless of AMI.

—
Reply to this email directly or view it on GitHub
https://github.com/hashicorp/terraform/pull/4276/files#r47838567.

cmcarthur · 2015-12-16T22:15:11Z

👍

jmcclell · 2015-12-17T02:03:04Z

Cool deal! I'll finish up opsworks application and submit that, as well.

Any ideas on run state?

mrjefftang · 2015-12-17T02:09:49Z

@jmcclell 'state' is provided. If set to "running" it will start the instance, if set to "stopped" it will stop it.

beedub · 2016-03-03T17:31:17Z

builtin/providers/aws/provider.go

@@ -240,6 +240,7 @@ func Provider() terraform.ResourceProvider {
 			"aws_opsworks_mysql_layer":             resourceAwsOpsworksMysqlLayer(),
 			"aws_opsworks_ganglia_layer":           resourceAwsOpsworksGangliaLayer(),
 			"aws_opsworks_custom_layer":            resourceAwsOpsworksCustomLayer(),
+			"aws_opsworks_instance":		resourceAwsOpsworksInstance(),


is this causing the build failure?

mrjefftang · 2016-03-03T17:57:18Z

Thanks @beedub .

It looks like there were a few changes in Terraform after this PR was submitted.

I had been using this code successfully until now, I'll give it a quick look and try to get it working again.

I need to figure out what happened to makwAwsStringList

beedub · 2016-03-03T18:10:22Z

np, sorry if these comments came off as pushy. i'm personally very interested in getting these changes merged, so just trying to help 😃 thx for making the changes!

mrjefftang · 2016-03-03T18:12:26Z

No worries. I didn't see them as pushy. I completely understand wanting to be able to use the functionality.

I lost interest in getting these merged a few months ago since there was no response from the maintainers. I was using a local build of terraform with my changes so I could use them internally.

mrjefftang · 2016-03-04T19:14:12Z

Added acceptance testing which is the final requirement

New resource checklist - [x] Acceptance testing - [x] Documentation - [x] Well-formed code

mrjefftang · 2016-03-14T19:21:16Z

@apparentlymart @radeksimko Any updates on getting this merged? I just rebased it again to fix a merge conflict as this PR.

jonathanq · 2016-03-17T06:17:44Z

I am going to start looking at terraform and I use OpsWorks heavily, so being able to define the instances would be nice. Hope this gets merged soon!

mrjefftang · 2016-03-21T14:13:42Z

@phinze @jen20 What else is needed to get this merged?

phinze · 2016-03-23T21:03:11Z

Hi @mrjefftang - sorry about the delay here - great work on this so far! I have a few minor comments that I'll make in line, but this is really close to being ready to ship!

phinze · 2016-03-23T21:06:10Z

builtin/providers/aws/resource_aws_opsworks_instance.go

+	}
+
+	instance := resp.Instances[0]
+	instanceId := *instance.InstanceId


We've learned in our time with the AWS API that all sorts of different shapes of responses are possible! 😀

So we tend to be extra defensive in handling types out of the SDK - I'd double check both the length of resp.Instances and that the InstanceId pointer is non-null. (For the rest of the d.Set calls below the null check is handled in the implementation so we're good there.)

phinze · 2016-03-23T21:07:17Z

website/source/docs/providers/aws/r/opsworks_instance.html.markdown

+  ]
+  instance_type = "t2.micro"
+  os = "Amazon Linux 2015.09"
+  state = "stopped"


Style nit - can you format this like hclfmt would? (Basically align equals for adjacent attributes.)

u2mejc · 2016-03-30T23:40:56Z

@mrjefftang Awesome work, I'm looking forward to having this merged as well.

stack72 · 2016-04-05T03:13:27Z

Hi @mrjefftang

thanks for fixing up the comments that @phinze made, I tried to have another look at this (by running the tests) and get the following:

make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSOpsworksInstance'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSOpsworksInstance -timeout 120m
=== RUN   TestAccAWSOpsworksInstance
--- FAIL: TestAccAWSOpsworksInstance (13.67s)
    testing.go:154: Step 0 error: Error applying: 1 error(s) occurred:

        * aws_opsworks_stack.tf-acc: ValidationException: Vpc Id: Failed to check with EC2 - please validate IAM role permissions
            status code: 400, request id: 1965ec6d-fadb-11e5-8764-23fe7fda5cec
    testing.go:175: Error destroying resource! WARNING: Dangling resources
        may exist. The full state and error is shown below.

        Error: Check failed: Fall through error on OpsWorks instance test

        State: <no state>
FAIL
exit status 1
FAIL    github.com/hashicorp/terraform/builtin/providers/aws    13.689s
make: *** [testacc] Error 1

I know that at the top of the file you have the following:

// These tests assume the existence of predefined Opsworks IAM roles named `aws-opsworks-ec2-role`
// and `aws-opsworks-service-role`.

but we cannot guarantee that these roles exist. What is needed here to include these roles as part of the test?

Thanks

Paul

mrjefftang · 2016-04-05T13:15:54Z

Hi @stack72 ,

That comment is out of date as it was copied from resource_aws_opsworks_custom_layer_test.go. It no longer relies on the predefined opsworks roles. I'll remove it.

The instance test relies on testAccAwsOpsworksStackConfigVpcCreate in resource_aws_opsworks_custom_layer_test.go which actually defines custom IAM roles as of #4009.

What you have run into is an edge case I've experienced where the IAM role is created but is not available for use. It's a race condition between Terraform attempting to create an Opsworks stack using the newly created IAM role and AWS propagating the existence of the role.

In using my code for some test environments, just rerunning Terraform after a few minutes (once the IAM roles were created in the first run) works just fine.

 09:09:47  jeff@apollo  ...git.luolix.top/hashicorp/terraform  ♦️ system  🐍 system   opsworks_instance ✘ ✹   $  make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSOpsworksInstance'
==> Checking that code complies with gofmt requirements...
/Users/jeff/Projects/go/bin/stringer
go generate $(go list ./... | grep -v /vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSOpsworksInstance -timeout 120m
=== RUN   TestAccAWSOpsworksInstance
--- FAIL: TestAccAWSOpsworksInstance (15.73s)
    testing.go:148: Step 0 error: Error applying: 1 error(s) occurred:

        * aws_opsworks_stack.tf-acc: ValidationException: Vpc Id: Failed to check with EC2 - please validate IAM role permissions
            status code: 400, request id: 13485edb-fb30-11e5-85e6-a119075d1df9
    testing.go:169: Error destroying resource! WARNING: Dangling resources
        may exist. The full state and error is shown below.

        Error: Check failed: Fall through error on OpsWorks instance test

        State: <no state>
FAIL
exit status 1
FAIL    github.com/hashicorp/terraform/builtin/providers/aws    15.760s
make: *** [testacc] Error 1

 09:12:48  ✘ 2  jeff@apollo  ...git.luolix.top/hashicorp/terraform  ♦️ system  🐍 system   opsworks_instance ✘ ✹   $ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSOpsworksInstance'
==> Checking that code complies with gofmt requirements...
/Users/jeff/Projects/go/bin/stringer
go generate $(go list ./... | grep -v /vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSOpsworksInstance -timeout 120m
=== RUN   TestAccAWSOpsworksInstance
--- PASS: TestAccAWSOpsworksInstance (38.20s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    38.224s

mrjefftang · 2016-04-05T13:18:22Z

On subsequent runs with the IAM roles destroyed and recreated it also works fine. Looking through the AWS webconsole, I can find no dangling resources.

u2mejc · 2016-04-06T19:19:37Z

@mrjefftang Would it possible to put a waiter in there for the target resource to become available and avoid the race condition? I think this would be ready to merge then after the tests are stabilized, would you agree @stack72?

mrjefftang · 2016-04-06T19:48:22Z

@u2mejc Looks like it was attempted by @stack72 in #4235 but not accepted.

Also @phinze had an attempt for retryable errors in #4844.

The alternative is to submit a separate PR as a bugfix for Opsworks stack with something similar to https://github.com/hashicorp/terraform/blob/master/builtin/providers/aws/resource_aws_instance.go#L363

The problem is I can't reproduce this issue most of the time. Also this error is slightly different than what I've experienced previously in #2162. Before the actual IAM profile didn't exist. Now it just looks like the permissions for the IAM aren't propagated yet.

u2mejc · 2016-04-12T01:29:27Z

@mrjefftang good call adding to the waiter to prevent errors while it propagates. @stack72 , #6049 should resolve the testing issue. Are there other issues you'd like to see addressed?

apparentlymart · 2016-04-16T17:32:04Z

Thanks for this, @mrjefftang! And again, sorry for the delay in getting these opsworks features merged.

I added a small followup fix da839e7 for a slightly-broken config example in the docs.

ghost · 2020-04-26T02:22:13Z

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.

radeksimko added enhancement provider/aws wip labels Dec 15, 2015

mrjefftang changed the title ~~[WIP] provider/aws: Opsworks instance~~ provider/aws: Opsworks instance Dec 16, 2015

apparentlymart removed the wip label Dec 16, 2015

jmcclell reviewed Dec 16, 2015
View reviewed changes

beedub reviewed Mar 3, 2016
View reviewed changes

Add support for Opsworks Instances

bcd5904

New resource checklist - [x] Acceptance testing - [x] Documentation - [x] Well-formed code

phinze reviewed Mar 23, 2016
View reviewed changes

style updates to documentation and nil checks

88de250

stack72 added the waiting-response An issue/pull request is waiting for a response from the community label Apr 5, 2016

stack72 self-assigned this Apr 5, 2016

add TestCheckExists/TestCheckAttributes for opsworks instance

b857bd1

mrjefftang mentioned this pull request Apr 6, 2016

Handle race condition with IAM role permissions & Opsworks #6049

Merged

apparentlymart removed the waiting-response An issue/pull request is waiting for a response from the community label Apr 16, 2016

apparentlymart merged commit b857bd1 into hashicorp:master Apr 16, 2016

apparentlymart added a commit that referenced this pull request Apr 16, 2016

Merge #4276: aws_opsworks_instance resource

94f338d

apparentlymart mentioned this pull request Apr 16, 2016

provider/aws: Support OpsWorks instances #3459

Closed

mrjefftang deleted the opsworks_instance branch April 19, 2016 14:56

ghost locked and limited conversation to collaborators Apr 26, 2020

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

provider/aws: Opsworks instance #4276

provider/aws: Opsworks instance #4276

mrjefftang commented Dec 11, 2015

mrjefftang commented Dec 11, 2015

jmcclell commented Dec 15, 2015

mrjefftang commented Dec 15, 2015

jmcclell commented Dec 15, 2015

mrjefftang commented Dec 15, 2015

jmcclell commented Dec 15, 2015

mrjefftang commented Dec 15, 2015

mrjefftang commented Dec 16, 2015

jmcclell Dec 16, 2015

mrjefftang Dec 16, 2015

cmcarthur commented Dec 16, 2015

jmcclell commented Dec 17, 2015

mrjefftang commented Dec 17, 2015

beedub Mar 3, 2016

mrjefftang commented Mar 3, 2016

beedub commented Mar 3, 2016

mrjefftang commented Mar 3, 2016

mrjefftang commented Mar 4, 2016

mrjefftang commented Mar 14, 2016

jonathanq commented Mar 17, 2016

mrjefftang commented Mar 21, 2016

phinze commented Mar 23, 2016

phinze Mar 23, 2016

phinze Mar 23, 2016

u2mejc commented Mar 30, 2016

stack72 commented Apr 5, 2016

mrjefftang commented Apr 5, 2016

mrjefftang commented Apr 5, 2016

u2mejc commented Apr 6, 2016

mrjefftang commented Apr 6, 2016

u2mejc commented Apr 12, 2016

apparentlymart commented Apr 16, 2016

ghost commented Apr 26, 2020

provider/aws: Opsworks instance #4276

provider/aws: Opsworks instance #4276

Conversation

mrjefftang commented Dec 11, 2015

mrjefftang commented Dec 11, 2015

jmcclell commented Dec 15, 2015

mrjefftang commented Dec 15, 2015

jmcclell commented Dec 15, 2015

mrjefftang commented Dec 15, 2015

jmcclell commented Dec 15, 2015

mrjefftang commented Dec 15, 2015

mrjefftang commented Dec 16, 2015

jmcclell Dec 16, 2015

Choose a reason for hiding this comment

mrjefftang Dec 16, 2015

Choose a reason for hiding this comment

cmcarthur commented Dec 16, 2015

jmcclell commented Dec 17, 2015

mrjefftang commented Dec 17, 2015

beedub Mar 3, 2016

Choose a reason for hiding this comment

mrjefftang commented Mar 3, 2016

beedub commented Mar 3, 2016

mrjefftang commented Mar 3, 2016

mrjefftang commented Mar 4, 2016

mrjefftang commented Mar 14, 2016

jonathanq commented Mar 17, 2016

mrjefftang commented Mar 21, 2016

phinze commented Mar 23, 2016

phinze Mar 23, 2016

Choose a reason for hiding this comment

phinze Mar 23, 2016

Choose a reason for hiding this comment

u2mejc commented Mar 30, 2016

stack72 commented Apr 5, 2016

mrjefftang commented Apr 5, 2016

mrjefftang commented Apr 5, 2016

u2mejc commented Apr 6, 2016

mrjefftang commented Apr 6, 2016

u2mejc commented Apr 12, 2016

apparentlymart commented Apr 16, 2016

ghost commented Apr 26, 2020