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

Added Step Function resources (State Machine & Activity) #11420

Merged
merged 8 commits into from
Jan 31, 2017

Conversation

Ninir
Copy link
Contributor

@Ninir Ninir commented Jan 25, 2017

Reasoning for this change

This is the second step towards #10652, following #11109.

TODOs

  • Add the SFN State Machine resource
  • Add the SFN State Activity
  • Handle import of state machine resources
  • Handle import of activity resources
  • Add the documentation for SFN State Machine
  • Add the documentation for SFN Activity
  • Add import tests

Acceptance tests

$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSSfnStateMachine_basic'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/01/30 19:27:55 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSSfnStateMachine_basic -timeout 120m
=== RUN   TestAccAWSSfnStateMachine_basic
--- PASS: TestAccAWSSfnStateMachine_basic (41.37s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/aws	41.400s

$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSSfnActivity_importBasic'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/01/30 22:38:42 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSSfnActivity_importBasic -timeout 120m
=== RUN   TestAccAWSSfnActivity_importBasic
--- PASS: TestAccAWSSfnActivity_importBasic (15.64s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/aws	15.667s

$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSSfnActivity_importBasic'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/01/30 18:54:14 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSSfnActivity_importBasic -timeout 120m
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/aws	0.025s

@Ninir Ninir force-pushed the r-step-functions branch 5 times, most recently from e7f4b5d to 0d24ef5 Compare January 25, 2017 22:52
@Ninir Ninir changed the title [WIP] Added Step Function State Machine handling [WIP] Added Step Function resources Jan 29, 2017
@stack72
Copy link
Contributor

stack72 commented Jan 30, 2017

Hi @Ninir

how is this progressing? :)

P.

@Ninir
Copy link
Contributor Author

Ninir commented Jan 30, 2017

Hi @stack72!
Making the last checks (website mainly). Will be good in an hour or so I guess :)

@stack72
Copy link
Contributor

stack72 commented Jan 30, 2017

👍

@Ninir Ninir force-pushed the r-step-functions branch 3 times, most recently from ed01d7c to 7fe0c96 Compare January 30, 2017 21:40
@Ninir
Copy link
Contributor Author

Ninir commented Jan 30, 2017

Hey @stack72
I'm having a weird issue at the moment with the "State machine" import testing, which outputs the following panic:

$ TF_LOG=DEBUG TF_LOG_PATH=terraform.log make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSSfnStateMachine_importBasic'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/01/30 23:03:54 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSSfnStateMachine_importBasic -timeout 120m
=== RUN   TestAccAWSSfnStateMachine_importBasic
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x30 pc=0x111ccb2]

goroutine 1422 [running]:
panic(0x18153e0, 0xc4200100e0)
	/usr/local/opt/go/libexec/src/runtime/panic.go:500 +0x1a1
github.com/hashicorp/terraform/terraform.(*NodeRefreshableDataResourceInstance).EvalTree(0xc42051c358, 0xc42051c358, 0x1165149)
	/Users/ninir/Sites/golang/src/github.com/hashicorp/terraform/terraform/node_data_refresh.go:134 +0x322
github.com/hashicorp/terraform/terraform.(*NodeRefreshableResource).EvalTree(0xc42051c0c0, 0x1a97760, 0xc42051c0c0)
	/Users/ninir/Sites/golang/src/github.com/hashicorp/terraform/terraform/node_resource_refresh.go:31 +0x79
github.com/hashicorp/terraform/terraform.(*Graph).walk.func1(0x1a97760, 0xc42051c0c0, 0x0, 0x0)
	/Users/ninir/Sites/golang/src/github.com/hashicorp/terraform/terraform/graph.go:232 +0xb90
github.com/hashicorp/terraform/dag.(*AcyclicGraph).Walk.func3(0xc420522fd0, 0xc4201d71d0, 0xc420522ff0, 0xc42038ec90, 0xc420523000, 0x1a97760, 0xc42051c0c0, 0xc42046a720, 0xc42046aba0)
	/Users/ninir/Sites/golang/src/github.com/hashicorp/terraform/dag/dag.go:246 +0x251
created by github.com/hashicorp/terraform/dag.(*AcyclicGraph).Walk
	/Users/ninir/Sites/golang/src/github.com/hashicorp/terraform/dag/dag.go:255 +0x6a6
exit status 2
FAIL	github.com/hashicorp/terraform/builtin/providers/aws	27.475s
make: *** [testacc] Error 1

Creating an issue for this, let's see...

@stack72
Copy link
Contributor

stack72 commented Jan 31, 2017

Hi @Ninir

I think we should remove the Import from this to start with and wait for that bug to be fixed - that way we can unblock you and we can follow up with import later

Thoughts?

Paul

@Ninir Ninir changed the title [WIP] Added Step Function resources Added Step Function resources (State Machine & Activity) Jan 31, 2017
@Ninir
Copy link
Contributor Author

Ninir commented Jan 31, 2017

Hey Paul,

Just removed the file. Will investigate the issue.

I think you can run tests and go with it :)

@stack72
Copy link
Contributor

stack72 commented Jan 31, 2017

Hey @Ninir

the code looks great - there is 1 issue on the tests though

% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSSfn'                                                           130 ↵ ✭
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/01/31 14:22:20 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSSfn -timeout 120m
=== RUN   TestAccAWSSfnActivity_importBasic
--- PASS: TestAccAWSSfnActivity_importBasic (20.07s)
=== RUN   TestAccAWSSfnActivity_basic
--- FAIL: TestAccAWSSfnActivity_basic (17.19s)
	testing.go:329: Error destroying resource! WARNING: Dangling resources
		may exist. The full state and error is shown below.

		Error: Check failed: Expected AWS Step Function Activity to be destroyed, but was still found

		State: <no state>
=== RUN   TestAccAWSSfnStateMachine_basic
--- PASS: TestAccAWSSfnStateMachine_basic (38.93s)
FAIL
exit status 1
FAIL	github.com/hashicorp/terraform/builtin/providers/aws	76.218s
make: *** [testacc] Error 1

@Ninir
Copy link
Contributor Author

Ninir commented Jan 31, 2017

Hey @stack72,

I'm a bit of annoyed there. I was able to reproduce the issue, however, here is the REST API:

2017/01/31 17:11:15 [DEBUG] [aws-sdk-go] DEBUG: Request states/DeleteActivity Details:
---[ REQUEST POST-SIGN ]-----------------------------
POST / HTTP/1.1

{"activityArn":"arn:aws:states:eu-west-1:512425551441:activity:bxw3i1oe7c"}
-----------------------------------------------------
2017/01/31 17:11:15 [DEBUG] [aws-sdk-go] DEBUG: Response states/DeleteActivity Details:
---[ RESPONSE ]--------------------------------------
HTTP/1.1 200 OK

{}
-----------------------------------------------------
2017/01/31 17:11:15 [DEBUG] [aws-sdk-go] DEBUG: Request states/DescribeActivity Details:
---[ REQUEST POST-SIGN ]-----------------------------
POST / HTTP/1.1

{"activityArn":"arn:aws:states:eu-west-1:512425551441:activity:bxw3i1oe7c"}
-----------------------------------------------------
2017/01/31 17:11:15 [DEBUG] [aws-sdk-go] DEBUG: Response states/DescribeActivity Details:
---[ RESPONSE ]--------------------------------------
HTTP/1.1 200 OK

{"activityArn":"arn:aws:states:eu-west-1:512425551441:activity:bxw3i1oe7c","creationDate":1.485879064252E9,"name":"bxw3i1oe7c"}
-----------------------------------------------------

Delete is OK, but describe still shows it... not even consistent there :(

@stack72
Copy link
Contributor

stack72 commented Jan 31, 2017

I think we may have to add a retry until it's gone?

@Ninir
Copy link
Contributor Author

Ninir commented Jan 31, 2017

Sure, will add a retry to 2minutes max (we are talking of a few seconds IMO) :)

@stack72
Copy link
Contributor

stack72 commented Jan 31, 2017

Perfect - eventual consistency FTW ;)

@Ninir
Copy link
Contributor Author

Ninir commented Jan 31, 2017

Hey Paul,

Just added the retry. Tests are ok on my side (multiple tries):

$ TF_LOG=DEBUG TF_LOG_PATH=terraform.log make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSSfn'                    
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/01/31 19:25:01 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSSfn -timeout 120m
=== RUN   TestAccAWSSfnActivity_importBasic
--- PASS: TestAccAWSSfnActivity_importBasic (28.04s)
=== RUN   TestAccAWSSfnActivity_basic
--- PASS: TestAccAWSSfnActivity_basic (24.27s)
=== RUN   TestAccAWSSfnStateMachine_basic
--- PASS: TestAccAWSSfnStateMachine_basic (45.12s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/aws	97.464s

And the logs are exposing the eventual consistency, but it is now handled :)

@Ninir Ninir force-pushed the r-step-functions branch 2 times, most recently from 77f435a to 92cede0 Compare January 31, 2017 18:34
@stack72
Copy link
Contributor

stack72 commented Jan 31, 2017

Yay! :) Thanks so much for this

% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSSfn'                                                                    ✭
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/01/31 20:14:41 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSSfn -timeout 120m
=== RUN   TestAccAWSSfnActivity_importBasic
--- PASS: TestAccAWSSfnActivity_importBasic (20.58s)
=== RUN   TestAccAWSSfnActivity_basic
--- PASS: TestAccAWSSfnActivity_basic (21.17s)
=== RUN   TestAccAWSSfnStateMachine_basic
--- PASS: TestAccAWSSfnStateMachine_basic (47.44s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/aws	89.210s

@stack72 stack72 merged commit 4da1451 into hashicorp:master Jan 31, 2017
stack72 pushed a commit that referenced this pull request Jan 31, 2017
* Added Step Function Activity & Step Function State Machine

* Added SFN State Machine documentation

* Added aws_sfn_activity & documentation

* Allowed import of sfn resources

* Added more checks on tests, fixed documentation

* Handled the update case of a SFN function (might be already deleting)

* Removed the State Machine import test file

* Fixed the eventual consistency of the read after delete for SFN functions
@Ninir Ninir deleted the r-step-functions branch February 1, 2017 13:22
arcadiatea pushed a commit to ticketmaster/terraform that referenced this pull request Feb 9, 2017
…1420)

* Added Step Function Activity & Step Function State Machine

* Added SFN State Machine documentation

* Added aws_sfn_activity & documentation

* Allowed import of sfn resources

* Added more checks on tests, fixed documentation

* Handled the update case of a SFN function (might be already deleting)

* Removed the State Machine import test file

* Fixed the eventual consistency of the read after delete for SFN functions
@ghost
Copy link

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

2 participants