From 8e1615a802a07e95b9d7b10618d7db9ff9ff6027 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Mon, 18 May 2020 15:12:44 -0700 Subject: [PATCH] backend/remote: Handle cost estimation skipped due to targeting The remote server might choose to skip running cost estimation for a targeted plan, in which case we'll show a note about it in the UI and then move on, rather than returning an "invalid status" error. This new status isn't yet available in the go-tfe library as a constant, so for now we have the string directly in our switch statement. This is a pragmatic way to expedite getting the "critical path" of this feature in place without blocking on changes to ancillary codebases. A subsequent commit should switch this over to tfe.CostEstimateSkippedDueToTargeting once that's available in a go-tfe release. --- backend/remote/backend_common.go | 4 ++++ backend/remote/backend_mock.go | 11 ++++++++++ backend/remote/backend_plan_test.go | 32 ++++++++++++++++++++++++++++- 3 files changed, 46 insertions(+), 1 deletion(-) diff --git a/backend/remote/backend_common.go b/backend/remote/backend_common.go index c92b80e92e..b83b9d8298 100644 --- a/backend/remote/backend_common.go +++ b/backend/remote/backend_common.go @@ -316,6 +316,10 @@ func (b *Remote) costEstimate(stopCtx, cancelCtx context.Context, op *backend.Op b.CLI.Output(b.Colorize().Color("Waiting for cost estimate to complete..." + elapsed + "\n")) } continue + case "skipped_due_to_targeting": // TEMP: not available in the go-tfe library yet; will update this to be tfe.CostEstimateSkippedDueToTargeting once that's available. + b.CLI.Output("Not available for this plan, because it was created with the -target option.") + b.CLI.Output("\n------------------------------------------------------------------------") + return nil case tfe.CostEstimateErrored: return fmt.Errorf(msgPrefix + " errored.") case tfe.CostEstimateCanceled: diff --git a/backend/remote/backend_mock.go b/backend/remote/backend_mock.go index a856b0612f..9b9b511268 100644 --- a/backend/remote/backend_mock.go +++ b/backend/remote/backend_mock.go @@ -696,6 +696,11 @@ type mockRuns struct { client *mockClient runs map[string]*tfe.Run workspaces map[string][]*tfe.Run + + // If modifyNewRun is non-nil, the create method will call it just before + // saving a new run in the runs map, so that a calling test can mimic + // side-effects that a real server might apply in certain situations. + modifyNewRun func(client *mockClient, options tfe.RunCreateOptions, run *tfe.Run) } func newMockRuns(client *mockClient) *mockRuns { @@ -780,6 +785,12 @@ func (m *mockRuns) Create(ctx context.Context, options tfe.RunCreateOptions) (*t w.CurrentRun = r } + if m.modifyNewRun != nil { + // caller-provided callback may modify the run in-place to mimic + // side-effects that a real server might take in some situations. + m.modifyNewRun(m.client, options, r) + } + m.runs[r.ID] = r m.workspaces[options.Workspace.ID] = append(m.workspaces[options.Workspace.ID], r) diff --git a/backend/remote/backend_plan_test.go b/backend/remote/backend_plan_test.go index 89cdc1ff18..dd247e0c6d 100644 --- a/backend/remote/backend_plan_test.go +++ b/backend/remote/backend_plan_test.go @@ -270,6 +270,29 @@ func TestRemote_planWithTarget(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() + // When the backend code creates a new run, we'll tweak it so that it + // has a cost estimation object with the "skipped_due_to_targeting" status, + // emulating how a real server is expected to behave in that case. + b.client.Runs.(*mockRuns).modifyNewRun = func(client *mockClient, options tfe.RunCreateOptions, run *tfe.Run) { + const fakeID = "fake" + // This is the cost estimate object embedded in the run itself which + // the backend will use to learn the ID to request from the cost + // estimates endpoint. It's pending to simulate what a freshly-created + // run is likely to look like. + run.CostEstimate = &tfe.CostEstimate{ + ID: fakeID, + Status: "pending", + } + // The backend will then use the main cost estimation API to retrieve + // the same ID indicated in the object above, where we'll then return + // the status "skipped_due_to_targeting" to trigger the special skip + // message in the backend output. + client.CostEstimates.estimations[fakeID] = &tfe.CostEstimate{ + ID: fakeID, + Status: "skipped_due_to_targeting", + } + } + op, configCleanup := testOperationPlan(t, "./testdata/plan") defer configCleanup() @@ -291,6 +314,13 @@ func TestRemote_planWithTarget(t *testing.T) { t.Fatalf("expected plan to be non-empty") } + // testBackendDefault above attached a "mock UI" to our backend, so we + // can retrieve its non-error output via the OutputWriter in-memory buffer. + gotOutput := b.CLI.(*cli.MockUi).OutputWriter.String() + if wantOutput := "Not available for this plan, because it was created with the -target option."; !strings.Contains(gotOutput, wantOutput) { + t.Errorf("missing message about skipped cost estimation\ngot:\n%s\nwant substring: %s", gotOutput, wantOutput) + } + // We should find a run inside the mock client that has the same // target address we requested above. runsAPI := b.client.Runs.(*mockRuns) @@ -303,7 +333,7 @@ func TestRemote_planWithTarget(t *testing.T) { } if !strings.Contains(run.Message, "using -target") { - t.Fatalf("incorrrect Message on the created run: %s", run.Message) + t.Errorf("incorrect Message on the created run: %s", run.Message) } } }