Additional workspace states (e.g. post_plan_running) the provider does not expect
lucymhdavies opened this issue · 3 comments
Moved out of #68
Found a few more instances of unhandled workspace states, so either a similar fix to #8 is needed, or perhaps something more forwards-compatible.
(i.e. list all the states we do expect, and do nothing for the ones we do not expect)
Debug Output
-----------------------------------------------------: timestamp=2022-10-07T14:01:17.596+0100
2022-10-07T14:01:17.597+0100 [INFO] provider.terraform-provider-multispace_v0.2.0: 2022/10/07 14:01:17 [DEBUG] non-progressive state, exiting "post_plan_running": timestamp=2022-10-07T14:01:17.597+0100
2022-10-07T14:01:17.597+0100 [INFO] provider.terraform-provider-multispace_v0.2.0: 2022/10/07 14:01:17 [INFO] plan complete, confirming apply. "run-sQQsTk8Nt8qQXxXB": timestamp=2022-10-07T14:01:17.597+0100
2022-10-07T14:01:17.597+0100 [INFO] provider.terraform-provider-multispace_v0.2.0: 2022/10/07 14:01:17 [DEBUG] TFE API Request Details:
---[ REQUEST ]---------------------------------------
POST /api/v2/runs/run-sQQsTk8Nt8qQXxXB/actions/apply HTTP/1.1
Host: app.terraform.io
User-Agent: go-tfe
Content-Length: 109
Accept: application/vnd.api+json
Authorization: Bearer S6JcndyjRR8ZJQ.atlasv1.REVOKED_TOKEN
Content-Type: application/vnd.api+json
Accept-Encoding: gzip
{
"data": {
"type": "",
"attributes": {
"comment": "terraform-provider-multispace on Fri Oct 7 14:01:17 BST 2022"
}
}
}
-----------------------------------------------------: timestamp=2022-10-07T14:01:17.597+0100
2022-10-07T14:01:17.720+0100 [TRACE] NodeAbstractResouceInstance.writeResourceInstanceState to workingState for multispace_run.trigger_workspaces["webserver-aws-dev"]
2022-10-07T14:01:17.720+0100 [TRACE] NodeAbstractResouceInstance.writeResourceInstanceState: writing state object for multispace_run.trigger_workspaces["webserver-aws-dev"]
2022-10-07T14:01:17.721+0100 [INFO] provider.terraform-provider-multispace_v0.2.0: 2022/10/07 14:01:17 [DEBUG] TFE API Response Details:
---[ RESPONSE ]--------------------------------------
HTTP/2.0 409 Conflict
Content-Length: 62
Cache-Control: no-cache
Content-Type: application/vnd.api+json; charset=utf-8
Date: Fri, 07 Oct 2022 13:01:17 GMT
Referrer-Policy: strict-origin-when-cross-origin
Strict-Transport-Security: max-age=63072000; includeSubDomains; preload
Tfp-Api-Version: 2.5
Vary: Accept-Encoding
Vary: Accept, Origin
X-Content-Type-Options: nosniff
X-Download-Options: noopen
X-Frame-Options: SAMEORIGIN
X-Permitted-Cross-Domain-Policies: none
X-Ratelimit-Limit: 30
X-Ratelimit-Remaining: 27
X-Ratelimit-Reset: 0.043
X-Request-Id: 8419f58a-d2d5-46c0-2ba0-f9e3cffe4a30
X-Xss-Protection: 1; mode=block
{
"errors": [
{
"status": "409",
"title": "transition not allowed"
}
]
}
The problem (i.e. the reason the provider is not forwards-compatible), is that our waitForRun function takes among its options two parameters:
terminal []tfe.RunStatus- the list of all run statuses we're looking for which would signal that we're done waiting at this stage, and can move on to the next oneprogress []tfe.RunStatus- the list of all run statuses we know about which indicate that we're not yet done waiting
And the problem is that there's a third set of statuses to consider: all run statuses we do not know about (because of forwards compatibility).
As such, one option to potentially resolve this is to no longer require that progress list, and just consider the terminal list. This works on the assumption that we're more likely to see new progress states added to TFC than terminal states, and so enumerating all of them (and keeping that list updated) is not practical.
We should then also double-check all of the stages we're listing as terminal, add new ones as needed from https://github.com/hashicorp/go-tfe/blob/main/run.go#L51-L78, and update existing ones
Though even then I'm not entirely sure that is going to work...
For example, in my PR https://github.com/mitchellh/terraform-provider-multispace/pull/8/files
I added RunCostEstimated to the terminal list because, if you have no policies on a workspace, then you'll never reach a RunPolicyChecked state. But this only works on the assumption that if you do have some policies to check, then the time between RunCostEstimated and RunPolicyChecking is so small that the API never returns the RunCostEstimated. I'm not entirely sure whether that's true or not.
Perhaps, rather than just considering the list of possible states, we also check run.Actions for tfe.IsConfirmable which, as far as I can tell, means "this run is awaiting (human) confirmation"
Yeah, that seems to do the trick. Simple as that.
✔ strawb@HashiBook ~/git_src/github.com/mitchellh/terraform-provider-multispace [wait_for_apply|✚ 1]
$ git diff
diff --git a/internal/provider/wait.go b/internal/provider/wait.go
index 1cefa64..e374b77 100644
--- a/internal/provider/wait.go
+++ b/internal/provider/wait.go
@@ -78,8 +78,12 @@ func waitForRun(
}
}
if !found {
- log.Printf("[DEBUG] non-progressive state, exiting %q", r.Status)
- return r, nil
+ if r.Actions.IsConfirmable {
+ log.Printf("[DEBUG] non-progressive is-confirmable state, exiting %q", r.Status)
+ return r, nil
+ }
+
+ log.Printf("[DEBUG] non-progressive state, waiting %q", r.Status)
}
// Check if 30 seconds have passed since the last update.
An example where we're not handling a particular status, but the run is not yet confirmable...
2022-10-07T18:45:45.487+0100 [INFO] provider.terraform-provider-multispace_v0.2.0: 2022/10/07 18:45:45 [DEBUG] non-progressive state, waiting "post_plan_running": timestamp=2022-10-07T18:45:45.487+0100
I'll add it to #68
It'd probably be wise to bump to the latest go-tfe... but looking at #64 that's a bigger change than simply bumping the numbers.
With functionality from this provider being incorporated into the main TFE provider...
hashicorp/terraform-provider-tfe#742
I'm gonna close this off.