Cloud run tasks (post-plan only) CLI integration #30141

uturunku1 · 2021-12-10T19:48:51Z

Integration with Terraform Cloud Run Tasks (post-plan stage). Run tasks is a beta feature implemented on Terraform Cloud that will display results after a plan like so:

These results are currently not being displayed or consumed by Terraform CLI, so with this PR (as well as some changes made to go-tfe hashicorp/go-tfe#286), is implementing the necessary changes to display the output of Run Tasks' result in the CLI:

IMPLEMENTATION

All changes in this PR are being applied through the op.plan because we want the user to see results of Run Tasks after a plan has found changes to infrastructure and right before the apply operation (so what we call the pre-apply stage).

These changes are also scope to the cloud package. This is to be determined, but we think that the CLI will not offer support for Run Tasks prior to version 1.2

This PR also adds an abstraction when interacting with Cloud Integrations such as Run Tasks, Policy Checks and Cost estimation. The abstraction is not yet applied to Policy Checks and Cost Estimation. But if we were to use it for all three Cloud Integrations, it would simplify and unify output during the pre-apply stage.

TESTING

Ensure your organization has access to the Run Tasks beta. You may have to request access for staging and prod. However, if you are running Atlas on dev mode, you'll have Run Task feature enabled by default.
Follow these steps to add run tasks to the workspace you'll be testing with: https://www.terraform.io/docs/cloud/workspaces/run-tasks.html
We have a go server running on a EC2 instance available for test event hooks. This are the URLs you can add to your run tasks:

http://54.167.177.151/success
http://54.167.177.151/failed
http://54.167.177.151/success?timeout=630 to force a timeout. But you can change the value of timeout to as many seconds you want the server to wait before firing off the webhook back to TFC

Create some config using a cloud config

terraform {
  cloud {
    hostname = "app.staging.terraform.io"
    organization = "test"
    workspaces {
      name = "project"
    }
  }
}

variable "replicas" {
  type = number
  default = 7
}

output "hello_there" {
  value = var.replicas
}

Using a testing workspace, run ./terraform apply to see Run Tasks output right after Cost Estimation output. If you have no tasks added to your workspace in the platform, you should see no output related to Run Tasks.

Screenshots

Task results

No run tasks defined

No infrastructure changes (unreachable tasks)

NOTE: Once this PR has being approved, we will squash the commits.

internal/cloud/backend_plan.go

barrettclark

This all looks good to me. I left 1 nit. The PR also has a merge conflict that will need to be resolved.

barrettclark · 2022-01-04T20:58:24Z

internal/cloud/backend_runTasks.go

+type taskStageReadFunc func(b *Cloud, stopCtx context.Context) (*tfe.TaskStage, error)
+
+func summarizeTaskResults(taskResults []*tfe.TaskResult) *taskResultSummary {
+	var pe, er, erm, pa int


I like to error on the side of more descriptive variable names. pa appears to be the count of tasks that are passed, so perhaps passedCount. This is not a deal-breaker for the PR, though.

Yes, I totally agree with this change. I guess I was trying to stick a bit to the "Go way" of naming variable, which usually is one letter or very few letters (like "p", "ret", "r", more on that topic: https://blog.devgenius.io/write-better-go-code-by-keeping-your-variable-names-short-13f404ac515c), which I actually find a bit annoying because, like you, I prefer descriptive names. Also, I have noticed that this project does not care much for that type of naming conventions, so I think is best for me to stay away from single letters :) @barrettclark

It's wild to me that the Go way would favor terse over descriptive variables. When future me drops into old code and see something like xzz I have to spend more time trying to remember what the heck that means and why. I will try to not swim against the current if our style guide favors brevity, but very much appreciate more verbose and meaningful names in the meantime. Thank you for making this change.

nfagerlund

I did some manual testing, and it looks pretty sweet! One question about the status of the go-tfe work, but otherwise this seems good.

nfagerlund · 2022-01-06T01:39:57Z

go.mod

@@ -41,7 +41,7 @@ require (
 	github.com/hashicorp/go-multierror v1.1.1
 	github.com/hashicorp/go-plugin v1.4.3
 	github.com/hashicorp/go-retryablehttp v0.7.0
-	github.com/hashicorp/go-tfe v0.21.0
+	github.com/hashicorp/go-tfe v0.20.1-0.20211215223223-3057a43c4f5c


Just a gut-check -- are we thinking to wait to merge this PR until the go-tfe changes have merged? Or will we treat this more like the run-up to 1.1.0, when we were building against a WIP branch for a while?

If it's the latter, we should probably rebase the WIP branch and make sure we don't lose any features compared to what were previously at. It looks like with the current status, we might lose run variables?

We need to have a slightly better process I think for pre-release go-tfe. I think we are going to wait on the go-tfe merge and depend directly on a branch until the next release of terraform that includes run tasks, similar to what we did in the run up to 1.1. Reason being, we want to make sure the API doesn't shift before releasing in go-tfe. This shouldn't compile if run variables weren't in the base, so it must be based on a run variables pre-release. I take your point and will make sure to rebase this to v0.22.0.

brandonc · 2022-01-06T16:29:36Z

Updated go-tfe (using branch run-tasks-integration)

internal/cloud/backend_plan.go

chrisarcand · 2022-01-12T18:48:29Z

internal/cloud/backend_plan.go

+		// fetching task_stages.
+		if strings.HasSuffix(err.Error(), "Invalid include parameter") {
+			r, err = b.client.Runs.Read(stopCtx, runID)
+		}


This feels brittle to me. Although it's not ideal, the 'proper' way to do this would be to use the TFP-API-Version header information (exposed in the client here) to conditionally make a single request if the host (TFC or TFE) supports the necessary API version for it; the same as any other addition we've made in the past.

That is my intention! Once this is merged I can supplant the next version with run tasks. Feels bad though because I know run tasks will squat on that version for a long time. https://github.com/hashicorp/atlas/pull/11572

If we hit line 335, and err is nil, and TFE or TFC have Run Tasks defined for that workspace, then the CLI would not display anything about run tasks' results but the platform would still show run tasks' results, which is the discrepancy/problem we were trying to solve to begin with. I wasn't sure if that's the behavior we want, but I want to bring that up just in case. I understand these changes are temporary. @brandonc @chrisarcand

If we hit line 335, run tasks won't be defined for the workspace because we'll be working against a version of TFE that doesn't support them -- that's what "invalid include parameter" would indicate... But it's not a great feature detection strategy and I'd like to rely on the API version instead.

brandonc · 2022-02-02T00:08:43Z

internal/cloud/backend_plan.go

@@ -335,6 +346,22 @@ in order to capture the filesystem context the remote workspace expects:
 	// status of the run will be "errored", but there is still policy
 	// information which should be shown.

+	// Await pre-apply run tasks


Suggested change

// Await pre-apply run tasks

// Await post-plan run tasks

😄

This change will await the completion of pre-apply run tasks if they exist on a run and then report the results. It also adds an abstraction when interacting with cloud integrations such as policy checking and cost estimation that simplify and unify output, although I did not go so far as to refactor those callers to use it yet.

…t previously was incorrectly named TaskStage

go-tfe is pinned to branch run-tasks-integration pending API changes until run tasks support in the CLI is closer to release

Older versions of TFE will not allow "task_stages" as an include parameter. In this case, fall back to reading the Run without additional options.

This commit stems from the change to make post plan the default run task stage, at the time of this commit's writing! Since pre apply is under internal revision, we have removed the block that polls the pre apply stage until the team decides to re-add support for pre apply run tasks.

github-actions · 2022-02-25T20:46:59Z

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

github-actions · 2022-03-28T02:20:31Z

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

chrisarcand added the cloud Related to Terraform Cloud's integration with Terraform label Dec 10, 2021

brandonc force-pushed the preapply-runtasks-clioutput branch from 57bd873 to 5ef0bfb Compare December 15, 2021 17:41

brandonc marked this pull request as ready for review December 15, 2021 17:50

brandonc changed the title ~~Work In Progress: Preapply runtasks clioutput~~ Cloud run tasks (pre-apply only) CLI integration Dec 15, 2021

brandonc reviewed Dec 15, 2021

View reviewed changes

internal/cloud/backend_plan.go Outdated Show resolved Hide resolved

barrettclark approved these changes Jan 4, 2022

View reviewed changes

nfagerlund approved these changes Jan 6, 2022

View reviewed changes

brandonc reviewed Jan 7, 2022

View reviewed changes

internal/cloud/backend_plan.go Outdated Show resolved Hide resolved

chrisarcand reviewed Jan 12, 2022

View reviewed changes

sebasslash changed the title ~~Cloud run tasks (pre-apply only) CLI integration~~ Cloud run tasks (post-plan only) CLI integration Feb 1, 2022

brandonc reviewed Feb 2, 2022

View reviewed changes

sebasslash force-pushed the preapply-runtasks-clioutput branch from 307a904 to 10c5a0d Compare February 7, 2022 17:02

uturunku1 and others added 17 commits February 24, 2022 14:02

[cloud] handle unreachable run tasks

2938ec4

[cloud] run tasks output formatting

6b5da4d

[cloud] report run tasks by name instead of assuming pre_apply

791c36c

[cloud] refactor integration context and add code documentation

0b8bb29

delete unused function

8090b23

pull latest changes from go-tfe branch and use use new field name tha…

77946af

…t previously was incorrectly named TaskStage

run go mod tidy

385589e

rename variables to something more descriptive

a9da859

run go mod tidy

46aaaa0

update go-tfe

391b9c4

go-tfe is pinned to branch run-tasks-integration pending API changes until run tasks support in the CLI is closer to release

Fall back to reading latest run without task_stages

aa0dda8

Older versions of TFE will not allow "task_stages" as an include parameter. In this case, fall back to reading the Run without additional options.

pull latest changes from go-tfe run-tasks-integration branch

e46683d

use new enum string for task stages

383da48

run go mod tidy

542e3f9

Updated for latest go-tfe run task changes

52c5f9f

sebasslash force-pushed the preapply-runtasks-clioutput branch from 10c5a0d to 52c5f9f Compare February 25, 2022 20:33

sebasslash merged commit afb956d into main Feb 25, 2022

github-actions bot locked as resolved and limited conversation to collaborators Mar 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cloud run tasks (post-plan only) CLI integration #30141

Cloud run tasks (post-plan only) CLI integration #30141

uturunku1 commented Dec 10, 2021 •

edited by sebasslash

barrettclark left a comment

barrettclark Jan 4, 2022

uturunku1 Jan 4, 2022

barrettclark Jan 4, 2022

nfagerlund left a comment

nfagerlund Jan 6, 2022

brandonc Jan 6, 2022

brandonc commented Jan 6, 2022

chrisarcand Jan 12, 2022 •

edited

brandonc Jan 12, 2022

uturunku1 Jan 13, 2022 •

edited

brandonc Jan 13, 2022

brandonc Feb 2, 2022

sebasslash Feb 7, 2022

github-actions bot commented Feb 25, 2022

github-actions bot commented Mar 28, 2022

Cloud run tasks (post-plan only) CLI integration #30141

Cloud run tasks (post-plan only) CLI integration #30141

Conversation

uturunku1 commented Dec 10, 2021 • edited by sebasslash

IMPLEMENTATION

TESTING

Screenshots

barrettclark left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nfagerlund left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brandonc commented Jan 6, 2022

chrisarcand Jan 12, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

uturunku1 Jan 13, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Feb 25, 2022

github-actions bot commented Mar 28, 2022

uturunku1 commented Dec 10, 2021 •

edited by sebasslash

chrisarcand Jan 12, 2022 •

edited

uturunku1 Jan 13, 2022 •

edited