From a7b7cd29fcea1192c8151da9de319d0ea0b08ef0 Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Thu, 11 Feb 2021 20:52:10 -0500 Subject: [PATCH] cli: Migrate Terraform UI hook to command views Move the code which renders Terraform hook callbacks as UI into the views package, backed by a views.View instead of a cli.Ui. Update test setup accordingly. To allow commands to control this hook, we add a hooks member on the backend Operation struct. This supersedes the hooks in the Terraform context, which is not directly controlled by the command logic. This commit should not change how Terraform works, and is refactoring in preparation for more changes which move UI code out of the backend. --- backend/backend.go | 4 ++ backend/local/backend_apply.go | 7 +-- backend/local/backend_local.go | 1 + command/apply.go | 3 +- command/apply_test.go | 2 + command/import.go | 1 + command/import_test.go | 38 ++++++++++++ command/meta.go | 22 +++---- command/plan.go | 2 + command/plan_test.go | 52 ++++++++++++++++- command/refresh.go | 2 + command/refresh_test.go | 4 +- command/show_test.go | 2 + command/{ => views}/hook_ui.go | 90 ++++++++++++----------------- command/{ => views}/hook_ui_test.go | 61 +++++++------------ 15 files changed, 177 insertions(+), 114 deletions(-) rename command/{ => views}/hook_ui.go (83%) rename command/{ => views}/hook_ui_test.go (88%) diff --git a/backend/backend.go b/backend/backend.go index 73d529ca1a..bb50b93089 100644 --- a/backend/backend.go +++ b/backend/backend.go @@ -183,6 +183,10 @@ type Operation struct { // configuration from ConfigDir. ConfigLoader *configload.Loader + // Hooks can be used to perform actions triggered by various events during + // the operation's lifecycle. + Hooks []terraform.Hook + // Plan is a plan that was passed as an argument. This is valid for // plan and apply arguments but may not work for all backends. PlanFile *planfile.Reader diff --git a/backend/local/backend_apply.go b/backend/local/backend_apply.go index 936f9fe99b..4f56b65153 100644 --- a/backend/local/backend_apply.go +++ b/backend/local/backend_apply.go @@ -40,12 +40,7 @@ func (b *Local) opApply( } stateHook := new(StateHook) - if b.ContextOpts == nil { - b.ContextOpts = new(terraform.ContextOpts) - } - old := b.ContextOpts.Hooks - defer func() { b.ContextOpts.Hooks = old }() - b.ContextOpts.Hooks = append(b.ContextOpts.Hooks, stateHook) + op.Hooks = append(op.Hooks, stateHook) // Get our context tfCtx, _, opState, contextDiags := b.context(op) diff --git a/backend/local/backend_local.go b/backend/local/backend_local.go index 1b67939af0..014b157a0a 100644 --- a/backend/local/backend_local.go +++ b/backend/local/backend_local.go @@ -77,6 +77,7 @@ func (b *Local) context(op *backend.Operation) (*terraform.Context, *configload. opts.Destroy = op.Destroy opts.Targets = op.Targets opts.UIInput = op.UIIn + opts.Hooks = op.Hooks opts.SkipRefresh = op.Type != backend.OperationTypeRefresh && !op.PlanRefresh if opts.SkipRefresh { diff --git a/command/apply.go b/command/apply.go index 96ced305cb..df8a726ba6 100644 --- a/command/apply.go +++ b/command/apply.go @@ -8,6 +8,7 @@ import ( "github.com/hashicorp/terraform/command/arguments" "github.com/hashicorp/terraform/command/views" "github.com/hashicorp/terraform/plans/planfile" + "github.com/hashicorp/terraform/terraform" "github.com/hashicorp/terraform/tfdiags" ) @@ -110,7 +111,6 @@ func (c *ApplyCommand) Run(args []string) int { // Set up our count hook that keeps track of resource changes countHook := new(CountHook) - c.ExtraHooks = append(c.ExtraHooks, countHook) // Load the backend var be backend.Enhanced @@ -171,6 +171,7 @@ func (c *ApplyCommand) Run(args []string) int { opReq.AutoApprove = autoApprove opReq.ConfigDir = configPath opReq.Destroy = c.Destroy + opReq.Hooks = []terraform.Hook{countHook, c.uiHook()} opReq.PlanFile = planFile opReq.PlanRefresh = refresh opReq.ShowDiagnostics = c.showDiagnostics diff --git a/command/apply_test.go b/command/apply_test.go index 2980bd61d6..8f11d51bf0 100644 --- a/command/apply_test.go +++ b/command/apply_test.go @@ -941,10 +941,12 @@ func TestApply_planNoModuleFiles(t *testing.T) { p := applyFixtureProvider() planPath := applyFixturePlanFile(t) + view, _ := testView(t) apply := &ApplyCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: new(cli.MockUi), + View: view, }, } args := []string{ diff --git a/command/import.go b/command/import.go index 275349408d..ba49e630a0 100644 --- a/command/import.go +++ b/command/import.go @@ -189,6 +189,7 @@ func (c *ImportCommand) Run(args []string) int { c.showDiagnostics(diags) return 1 } + opReq.Hooks = []terraform.Hook{c.uiHook()} { var moreDiags tfdiags.Diagnostics opReq.Variables, moreDiags = c.collectVariableValues() diff --git a/command/import_test.go b/command/import_test.go index 354b205363..e3840b8df3 100644 --- a/command/import_test.go +++ b/command/import_test.go @@ -24,10 +24,12 @@ func TestImport(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ImportCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -77,10 +79,12 @@ func TestImport_providerConfig(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ImportCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -170,9 +174,11 @@ func TestImport_remoteState(t *testing.T) { // init our backend ui := cli.NewMockUi() + view, _ := testView(t) m := Meta{ testingOverrides: metaOverridesForProvider(testProvider()), Ui: ui, + View: view, ProviderSource: providerSource, } @@ -192,6 +198,7 @@ func TestImport_remoteState(t *testing.T) { Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -280,9 +287,11 @@ func TestImport_initializationErrorShouldUnlock(t *testing.T) { // init our backend ui := cli.NewMockUi() + view, _ := testView(t) m := Meta{ testingOverrides: metaOverridesForProvider(testProvider()), Ui: ui, + View: view, ProviderSource: providerSource, } @@ -305,6 +314,7 @@ func TestImport_initializationErrorShouldUnlock(t *testing.T) { Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -339,10 +349,12 @@ func TestImport_providerConfigWithVar(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ImportCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -417,10 +429,12 @@ func TestImport_providerConfigWithDataSource(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ImportCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -480,10 +494,12 @@ func TestImport_providerConfigWithVarDefault(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ImportCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -557,10 +573,12 @@ func TestImport_providerConfigWithVarFile(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ImportCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -635,10 +653,12 @@ func TestImport_allowMissingResourceConfig(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ImportCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -690,10 +710,12 @@ func TestImport_emptyConfig(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ImportCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -720,10 +742,12 @@ func TestImport_missingResourceConfig(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ImportCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -750,10 +774,12 @@ func TestImport_missingModuleConfig(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ImportCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -801,9 +827,11 @@ func TestImportModuleVarFile(t *testing.T) { // init to install the module ui := new(cli.MockUi) + view, _ := testView(t) m := Meta{ testingOverrides: metaOverridesForProvider(testProvider()), Ui: ui, + View: view, ProviderSource: providerSource, } @@ -820,6 +848,7 @@ func TestImportModuleVarFile(t *testing.T) { Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } args := []string{ @@ -873,9 +902,11 @@ func TestImportModuleInputVariableEvaluation(t *testing.T) { // init to install the module ui := new(cli.MockUi) + view, _ := testView(t) m := Meta{ testingOverrides: metaOverridesForProvider(testProvider()), Ui: ui, + View: view, ProviderSource: providerSource, } @@ -892,6 +923,7 @@ func TestImportModuleInputVariableEvaluation(t *testing.T) { Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } args := []string{ @@ -912,10 +944,12 @@ func TestImport_dataResource(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ImportCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -942,10 +976,12 @@ func TestImport_invalidResourceAddr(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ImportCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -972,10 +1008,12 @@ func TestImport_targetIsModule(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &ImportCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } diff --git a/command/meta.go b/command/meta.go index 4f01af630d..9f0ad8da68 100644 --- a/command/meta.go +++ b/command/meta.go @@ -21,6 +21,7 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/backend" "github.com/hashicorp/terraform/backend/local" + "github.com/hashicorp/terraform/command/arguments" "github.com/hashicorp/terraform/command/format" "github.com/hashicorp/terraform/command/views" "github.com/hashicorp/terraform/command/webbrowser" @@ -69,9 +70,6 @@ type Meta struct { GlobalPluginDirs []string // Additional paths to search for plugins Ui cli.Ui // Ui for output - // ExtraHooks are extra hooks to add to the context. - ExtraHooks []terraform.Hook - // Services provides access to remote endpoint information for // "terraform-native' services running at a specific user-facing hostname. Services *disco.Disco @@ -434,8 +432,6 @@ func (m *Meta) contextOpts() (*terraform.ContextOpts, error) { } var opts terraform.ContextOpts - opts.Hooks = []terraform.Hook{m.uiHook()} - opts.Hooks = append(opts.Hooks, m.ExtraHooks...) opts.Targets = m.targets opts.UIInput = m.UIInput() @@ -621,15 +617,21 @@ func (m *Meta) process(args []string) []string { }, } + // Reconfigure the view. This is necessary for commands which use both + // views.View and cli.Ui during the migration phase. + if m.View != nil { + m.View.Configure(&arguments.View{ + CompactWarnings: m.compactWarnings, + NoColor: !m.Color, + }) + } + return args } // uiHook returns the UiHook to use with the context. -func (m *Meta) uiHook() *UiHook { - return &UiHook{ - Colorize: m.Colorize(), - Ui: m.Ui, - } +func (m *Meta) uiHook() *views.UiHook { + return views.NewUiHook(m.View) } // confirm asks a yes/no confirmation. diff --git a/command/plan.go b/command/plan.go index 38e2b77d0a..c81ea75ccb 100644 --- a/command/plan.go +++ b/command/plan.go @@ -7,6 +7,7 @@ import ( "github.com/hashicorp/terraform/backend" "github.com/hashicorp/terraform/configs" "github.com/hashicorp/terraform/plans" + "github.com/hashicorp/terraform/terraform" "github.com/hashicorp/terraform/tfdiags" ) @@ -82,6 +83,7 @@ func (c *PlanCommand) Run(args []string) int { opReq := c.Operation(b) opReq.ConfigDir = configPath opReq.Destroy = destroy + opReq.Hooks = []terraform.Hook{c.uiHook()} opReq.PlanOutPath = outPath opReq.PlanRefresh = refresh opReq.ShowDiagnostics = c.showDiagnostics diff --git a/command/plan_test.go b/command/plan_test.go index c9c09476e1..07ab17d612 100644 --- a/command/plan_test.go +++ b/command/plan_test.go @@ -32,10 +32,12 @@ func TestPlan(t *testing.T) { p := planFixtureProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &PlanCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -59,10 +61,12 @@ func TestPlan_lockedState(t *testing.T) { p := planFixtureProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &PlanCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -85,10 +89,12 @@ func TestPlan_plan(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &PlanCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -126,10 +132,12 @@ func TestPlan_destroy(t *testing.T) { p := planFixtureProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &PlanCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -158,10 +166,12 @@ func TestPlan_noState(t *testing.T) { p := planFixtureProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &PlanCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -193,10 +203,12 @@ func TestPlan_outPath(t *testing.T) { p := planFixtureProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &PlanCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -246,10 +258,12 @@ func TestPlan_outPathNoChange(t *testing.T) { p := planFixtureProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &PlanCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -324,10 +338,12 @@ func TestPlan_outBackend(t *testing.T) { } } ui := cli.NewMockUi() + view, _ := testView(t) c := &PlanCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -376,10 +392,12 @@ func TestPlan_refreshFalse(t *testing.T) { p := planFixtureProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &PlanCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -407,10 +425,12 @@ func TestPlan_state(t *testing.T) { p := planFixtureProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &PlanCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -450,10 +470,12 @@ func TestPlan_stateDefault(t *testing.T) { p := planFixtureProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &PlanCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -505,10 +527,12 @@ func TestPlan_validate(t *testing.T) { } } ui := new(cli.MockUi) + view, _ := testView(t) c := &PlanCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -532,10 +556,12 @@ func TestPlan_vars(t *testing.T) { p := planVarsFixtureProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &PlanCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -577,10 +603,12 @@ func TestPlan_varsUnset(t *testing.T) { p := planVarsFixtureProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &PlanCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -640,10 +668,12 @@ func TestPlan_providerArgumentUnset(t *testing.T) { }, } ui := new(cli.MockUi) + view, _ := testView(t) c := &PlanCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -667,10 +697,12 @@ func TestPlan_varFile(t *testing.T) { p := planVarsFixtureProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &PlanCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -707,10 +739,12 @@ func TestPlan_varFileDefault(t *testing.T) { p := planVarsFixtureProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &PlanCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -745,10 +779,12 @@ func TestPlan_varFileWithDecls(t *testing.T) { p := planVarsFixtureProvider() ui := cli.NewMockUi() + view, _ := testView(t) c := &PlanCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -773,10 +809,12 @@ func TestPlan_detailedExitcode(t *testing.T) { p := planFixtureProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &PlanCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -794,10 +832,12 @@ func TestPlan_detailedExitcode_emptyDiff(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &PlanCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -819,10 +859,12 @@ func TestPlan_shutdown(t *testing.T) { p := testProvider() ui := new(cli.MockUi) + view, _ := testView(t) c := &PlanCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, ShutdownCh: shutdownCh, }, } @@ -884,10 +926,12 @@ func TestPlan_init_required(t *testing.T) { defer testChdir(t, td)() ui := new(cli.MockUi) + view, _ := testView(t) c := &PlanCommand{ Meta: Meta{ // Running plan without setting testingOverrides is similar to plan without init - Ui: ui, + Ui: ui, + View: view, }, } @@ -927,10 +971,12 @@ func TestPlan_targeted(t *testing.T) { } ui := new(cli.MockUi) + view, _ := testView(t) c := &PlanCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, }, } @@ -961,9 +1007,11 @@ func TestPlan_targetFlagsDiags(t *testing.T) { defer testChdir(t, td)() ui := new(cli.MockUi) + view, _ := testView(t) c := &PlanCommand{ Meta: Meta{ - Ui: ui, + Ui: ui, + View: view, }, } diff --git a/command/refresh.go b/command/refresh.go index 00b4d9d77b..df70252fa8 100644 --- a/command/refresh.go +++ b/command/refresh.go @@ -7,6 +7,7 @@ import ( "github.com/hashicorp/terraform/backend" "github.com/hashicorp/terraform/command/arguments" "github.com/hashicorp/terraform/command/views" + "github.com/hashicorp/terraform/terraform" "github.com/hashicorp/terraform/tfdiags" ) @@ -75,6 +76,7 @@ func (c *RefreshCommand) Run(args []string) int { // Build the operation opReq := c.Operation(b) opReq.ConfigDir = configPath + opReq.Hooks = []terraform.Hook{c.uiHook()} opReq.ShowDiagnostics = c.showDiagnostics opReq.Type = backend.OperationTypeRefresh diff --git a/command/refresh_test.go b/command/refresh_test.go index 2175b03233..1541c347d4 100644 --- a/command/refresh_test.go +++ b/command/refresh_test.go @@ -791,7 +791,7 @@ func TestRefresh_targeted(t *testing.T) { } ui := new(cli.MockUi) - view, _ := testView(t) + view, done := testView(t) c := &RefreshCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), @@ -808,7 +808,7 @@ func TestRefresh_targeted(t *testing.T) { t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) } - got := ui.OutputWriter.String() + got := done(t).Stdout() if want := "test_instance.foo: Refreshing"; !strings.Contains(got, want) { t.Fatalf("expected output to contain %q, got:\n%s", want, got) } diff --git a/command/show_test.go b/command/show_test.go index aec6e22ec8..7acffe060e 100644 --- a/command/show_test.go +++ b/command/show_test.go @@ -255,9 +255,11 @@ func TestShow_json_output(t *testing.T) { p := showFixtureProvider() ui := new(cli.MockUi) + view, _ := testView(t) m := Meta{ testingOverrides: metaOverridesForProvider(p), Ui: ui, + View: view, ProviderSource: providerSource, } diff --git a/command/hook_ui.go b/command/views/hook_ui.go similarity index 83% rename from command/hook_ui.go rename to command/views/hook_ui.go index b6a2e52f21..9427511d35 100644 --- a/command/hook_ui.go +++ b/command/views/hook_ui.go @@ -1,4 +1,4 @@ -package command +package views import ( "bufio" @@ -9,8 +9,6 @@ import ( "time" "unicode" - "github.com/mitchellh/cli" - "github.com/mitchellh/colorstring" "github.com/zclconf/go-cty/cty" "github.com/hashicorp/terraform/addrs" @@ -24,17 +22,24 @@ import ( const defaultPeriodicUiTimer = 10 * time.Second const maxIdLen = 80 +func NewUiHook(view *View) *UiHook { + return &UiHook{ + view: view, + periodicUiTimer: defaultPeriodicUiTimer, + resources: make(map[string]uiResourceState), + } +} + type UiHook struct { terraform.NilHook - Colorize *colorstring.Colorize - Ui cli.Ui - PeriodicUiTimer time.Duration + view *View + viewLock sync.Mutex - l sync.Mutex - once sync.Once - resources map[string]uiResourceState - ui cli.Ui + periodicUiTimer time.Duration + + resources map[string]uiResourceState + resourcesLock sync.Mutex } var _ terraform.Hook = (*UiHook)(nil) @@ -63,8 +68,6 @@ const ( ) func (h *UiHook) PreApply(addr addrs.AbsResourceInstance, gen states.Generation, action plans.Action, priorState, plannedNewState cty.Value) (terraform.HookAction, error) { - h.once.Do(h.init) - dispAddr := addr.String() if gen != states.CurrentGen { dispAddr = fmt.Sprintf("%s (%s)", dispAddr, gen) @@ -89,7 +92,7 @@ func (h *UiHook) PreApply(addr addrs.AbsResourceInstance, gen states.Generation, default: // We don't expect any other actions in here, so anything else is a // bug in the caller but we'll ignore it in order to be robust. - h.ui.Output(fmt.Sprintf("(Unknown action %s for %s)", action, dispAddr)) + h.println(fmt.Sprintf("(Unknown action %s for %s)", action, dispAddr)) return terraform.HookActionContinue, nil } @@ -103,7 +106,7 @@ func (h *UiHook) PreApply(addr addrs.AbsResourceInstance, gen states.Generation, idValue = "" } - h.ui.Output(h.Colorize.Color(fmt.Sprintf( + h.println(h.view.colorize.Color(fmt.Sprintf( "[reset][bold]%s: %s%s[reset]", dispAddr, operation, @@ -121,9 +124,9 @@ func (h *UiHook) PreApply(addr addrs.AbsResourceInstance, gen states.Generation, done: make(chan struct{}), } - h.l.Lock() + h.resourcesLock.Lock() h.resources[key] = uiState - h.l.Unlock() + h.resourcesLock.Unlock() // Start goroutine that shows progress go h.stillApplying(uiState) @@ -138,7 +141,7 @@ func (h *UiHook) stillApplying(state uiResourceState) { case <-state.DoneCh: return - case <-time.After(h.PeriodicUiTimer): + case <-time.After(h.periodicUiTimer): // Timer up, show status } @@ -161,7 +164,7 @@ func (h *UiHook) stillApplying(state uiResourceState) { idSuffix = fmt.Sprintf("%s=%s, ", state.IDKey, truncateId(state.IDValue, maxIdLen)) } - h.ui.Output(h.Colorize.Color(fmt.Sprintf( + h.println(h.view.colorize.Color(fmt.Sprintf( "[reset][bold]%s: %s [%s%s elapsed][reset]", state.DispAddr, msg, @@ -172,17 +175,16 @@ func (h *UiHook) stillApplying(state uiResourceState) { } func (h *UiHook) PostApply(addr addrs.AbsResourceInstance, gen states.Generation, newState cty.Value, applyerr error) (terraform.HookAction, error) { - id := addr.String() - h.l.Lock() + h.resourcesLock.Lock() state := h.resources[id] if state.DoneCh != nil { close(state.DoneCh) } delete(h.resources, id) - h.l.Unlock() + h.resourcesLock.Unlock() var stateIdSuffix string if k, v := format.ObjectValueID(newState); k != "" && v != "" { @@ -208,21 +210,17 @@ func (h *UiHook) PostApply(addr addrs.AbsResourceInstance, gen states.Generation return terraform.HookActionContinue, nil } - colorized := h.Colorize.Color(fmt.Sprintf( + colorized := h.view.colorize.Color(fmt.Sprintf( "[reset][bold]%s: %s after %s%s[reset]", addr, msg, time.Now().Round(time.Second).Sub(state.Start), stateIdSuffix)) - h.ui.Output(colorized) + h.println(colorized) return terraform.HookActionContinue, nil } -func (h *UiHook) PreDiff(addr addrs.AbsResourceInstance, gen states.Generation, priorState, proposedNewState cty.Value) (terraform.HookAction, error) { - return terraform.HookActionContinue, nil -} - func (h *UiHook) PreProvisionInstanceStep(addr addrs.AbsResourceInstance, typeName string) (terraform.HookAction, error) { - h.ui.Output(h.Colorize.Color(fmt.Sprintf( + h.println(h.view.colorize.Color(fmt.Sprintf( "[reset][bold]%s: Provisioning with '%s'...[reset]", addr, typeName, ))) @@ -231,7 +229,7 @@ func (h *UiHook) PreProvisionInstanceStep(addr addrs.AbsResourceInstance, typeNa func (h *UiHook) ProvisionOutput(addr addrs.AbsResourceInstance, typeName string, msg string) { var buf bytes.Buffer - buf.WriteString(h.Colorize.Color("[reset]")) + buf.WriteString(h.view.colorize.Color("[reset]")) prefix := fmt.Sprintf("%s (%s): ", addr, typeName) s := bufio.NewScanner(strings.NewReader(msg)) @@ -243,27 +241,23 @@ func (h *UiHook) ProvisionOutput(addr addrs.AbsResourceInstance, typeName string } } - h.ui.Output(strings.TrimSpace(buf.String())) + h.println(strings.TrimSpace(buf.String())) } func (h *UiHook) PreRefresh(addr addrs.AbsResourceInstance, gen states.Generation, priorState cty.Value) (terraform.HookAction, error) { - h.once.Do(h.init) - var stateIdSuffix string if k, v := format.ObjectValueID(priorState); k != "" && v != "" { stateIdSuffix = fmt.Sprintf(" [%s=%s]", k, v) } - h.ui.Output(h.Colorize.Color(fmt.Sprintf( + h.println(h.view.colorize.Color(fmt.Sprintf( "[reset][bold]%s: Refreshing state...%s", addr, stateIdSuffix))) return terraform.HookActionContinue, nil } func (h *UiHook) PreImportState(addr addrs.AbsResourceInstance, importID string) (terraform.HookAction, error) { - h.once.Do(h.init) - - h.ui.Output(h.Colorize.Color(fmt.Sprintf( + h.println(h.view.colorize.Color(fmt.Sprintf( "[reset][bold]%s: Importing from ID %q...", addr, importID, ))) @@ -271,12 +265,10 @@ func (h *UiHook) PreImportState(addr addrs.AbsResourceInstance, importID string) } func (h *UiHook) PostImportState(addr addrs.AbsResourceInstance, imported []providers.ImportedResource) (terraform.HookAction, error) { - h.once.Do(h.init) - - h.ui.Output(h.Colorize.Color(fmt.Sprintf( + h.println(h.view.colorize.Color(fmt.Sprintf( "[reset][bold][green]%s: Import prepared!", addr))) for _, s := range imported { - h.ui.Output(h.Colorize.Color(fmt.Sprintf( + h.println(h.view.colorize.Color(fmt.Sprintf( "[reset][green] Prepared %s for import", s.TypeName, ))) @@ -285,19 +277,11 @@ func (h *UiHook) PostImportState(addr addrs.AbsResourceInstance, imported []prov return terraform.HookActionContinue, nil } -func (h *UiHook) init() { - if h.Colorize == nil { - panic("colorize not given") - } - if h.PeriodicUiTimer == 0 { - h.PeriodicUiTimer = defaultPeriodicUiTimer - } - - h.resources = make(map[string]uiResourceState) - - // Wrap the ui so that it is safe for concurrency regardless of the - // underlying reader/writer that is in place. - h.ui = &cli.ConcurrentUi{Ui: h.Ui} +// Wrap calls to the view so that concurrent calls do not interleave println. +func (h *UiHook) println(s string) { + h.viewLock.Lock() + defer h.viewLock.Unlock() + h.view.streams.Println(s) } // scanLines is basically copied from the Go standard library except diff --git a/command/hook_ui_test.go b/command/views/hook_ui_test.go similarity index 88% rename from command/hook_ui_test.go rename to command/views/hook_ui_test.go index 03b62add01..5e4544e8f0 100644 --- a/command/hook_ui_test.go +++ b/command/views/hook_ui_test.go @@ -1,4 +1,4 @@ -package command +package views import ( "fmt" @@ -6,28 +6,20 @@ import ( "testing" "time" - "github.com/mitchellh/cli" - "github.com/mitchellh/colorstring" "github.com/zclconf/go-cty/cty" "github.com/hashicorp/terraform/addrs" + "github.com/hashicorp/terraform/internal/terminal" "github.com/hashicorp/terraform/plans" "github.com/hashicorp/terraform/states" "github.com/hashicorp/terraform/terraform" ) func TestUiHookPreApply_periodicTimer(t *testing.T) { - ui := cli.NewMockUi() - h := &UiHook{ - Colorize: &colorstring.Colorize{ - Colors: colorstring.DefaultColors, - Disable: true, - Reset: true, - }, - Ui: ui, - PeriodicUiTimer: 1 * time.Second, - } - h.init() + streams, done := terminal.StreamsForTesting(t) + view := NewView(streams) + h := NewUiHook(view) + h.periodicUiTimer = 1 * time.Second h.resources = map[string]uiResourceState{ "data.aws_availability_zones.available": uiResourceState{ Op: uiResourceDestroy, @@ -75,29 +67,23 @@ data.aws_availability_zones.available: Still destroying... [id=2017-03-05 10:56: data.aws_availability_zones.available: Still destroying... [id=2017-03-05 10:56:59.298784526 +0000 UTC, 2s elapsed] data.aws_availability_zones.available: Still destroying... [id=2017-03-05 10:56:59.298784526 +0000 UTC, 3s elapsed] ` - output := ui.OutputWriter.String() + result := done(t) + output := result.Stdout() if output != expectedOutput { t.Fatalf("Output didn't match.\nExpected: %q\nGiven: %q", expectedOutput, output) } expectedErrOutput := "" - errOutput := ui.ErrorWriter.String() + errOutput := result.Stderr() if errOutput != expectedErrOutput { t.Fatalf("Error output didn't match.\nExpected: %q\nGiven: %q", expectedErrOutput, errOutput) } } func TestUiHookPreApply_destroy(t *testing.T) { - ui := cli.NewMockUi() - h := &UiHook{ - Colorize: &colorstring.Colorize{ - Colors: colorstring.DefaultColors, - Disable: true, - Reset: true, - }, - Ui: ui, - } - h.init() + streams, done := terminal.StreamsForTesting(t) + view := NewView(streams) + h := NewUiHook(view) h.resources = map[string]uiResourceState{ "data.aws_availability_zones.available": uiResourceState{ Op: uiResourceDestroy, @@ -138,30 +124,24 @@ func TestUiHookPreApply_destroy(t *testing.T) { close(uiState.DoneCh) <-uiState.done + result := done(t) expectedOutput := "data.aws_availability_zones.available: Destroying... [id=2017-03-05 10:56:59.298784526 +0000 UTC]\n" - output := ui.OutputWriter.String() + output := result.Stdout() if output != expectedOutput { t.Fatalf("Output didn't match.\nExpected: %q\nGiven: %q", expectedOutput, output) } expectedErrOutput := "" - errOutput := ui.ErrorWriter.String() + errOutput := result.Stderr() if errOutput != expectedErrOutput { t.Fatalf("Error output didn't match.\nExpected: %q\nGiven: %q", expectedErrOutput, errOutput) } } func TestUiHookPostApply_emptyState(t *testing.T) { - ui := cli.NewMockUi() - h := &UiHook{ - Colorize: &colorstring.Colorize{ - Colors: colorstring.DefaultColors, - Disable: true, - Reset: true, - }, - Ui: ui, - } - h.init() + streams, done := terminal.StreamsForTesting(t) + view := NewView(streams) + h := NewUiHook(view) h.resources = map[string]uiResourceState{ "data.google_compute_zones.available": uiResourceState{ Op: uiResourceDestroy, @@ -187,15 +167,16 @@ func TestUiHookPostApply_emptyState(t *testing.T) { if action != terraform.HookActionContinue { t.Fatalf("Expected hook to continue, given: %#v", action) } + result := done(t) expectedRegexp := "^data.google_compute_zones.available: Destruction complete after -?[a-z0-9ยต.]+\n$" - output := ui.OutputWriter.String() + output := result.Stdout() if matched, _ := regexp.MatchString(expectedRegexp, output); !matched { t.Fatalf("Output didn't match regexp.\nExpected: %q\nGiven: %q", expectedRegexp, output) } expectedErrOutput := "" - errOutput := ui.ErrorWriter.String() + errOutput := result.Stderr() if errOutput != expectedErrOutput { t.Fatalf("Error output didn't match.\nExpected: %q\nGiven: %q", expectedErrOutput, errOutput) }