From 17c9a403f217f4cbfcfa5c1ff13c14f43296f649 Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Thu, 2 Mar 2017 18:41:18 +0000 Subject: [PATCH 1/5] WIP --- command/hook_ui.go | 24 ++++++++++------ command/hook_ui_test.go | 62 +++++++++++++++++++++++++++++++++++++++++ terraform/resource.go | 13 +++++++++ 3 files changed, 90 insertions(+), 9 deletions(-) diff --git a/command/hook_ui.go b/command/hook_ui.go index 06f3cce52..60372123d 100644 --- a/command/hook_ui.go +++ b/command/hook_ui.go @@ -4,6 +4,7 @@ import ( "bufio" "bytes" "fmt" + "log" "sort" "strings" "sync" @@ -50,6 +51,7 @@ func (h *UiHook) PreApply( n *terraform.InstanceInfo, s *terraform.InstanceState, d *terraform.InstanceDiff) (terraform.HookAction, error) { + log.Printf("[DEBUG] PreApply arguments:\n%#v\n%#v\n%#v", n, s, d) h.once.Do(h.init) id := n.HumanId() @@ -129,9 +131,8 @@ func (h *UiHook) PreApply( attrString = "\n " + attrString } - var stateId, stateIdSuffix string + var stateIdSuffix string if s != nil && s.ID != "" { - stateId = s.ID stateIdSuffix = fmt.Sprintf(" (ID: %s)", truncateId(s.ID, maxIdLen)) } @@ -143,12 +144,12 @@ func (h *UiHook) PreApply( attrString))) // Set a timer to show an operation is still happening - time.AfterFunc(periodicUiTimer, func() { h.stillApplying(id, stateId) }) + time.AfterFunc(periodicUiTimer, func() { h.stillApplying(id, s) }) return terraform.HookActionContinue, nil } -func (h *UiHook) stillApplying(id, stateId string) { +func (h *UiHook) stillApplying(id string, s *terraform.InstanceState) { // Grab the operation. We defer the lock here to avoid the "still..." // message showing up after a completion message. h.l.Lock() @@ -173,8 +174,8 @@ func (h *UiHook) stillApplying(id, stateId string) { } var stateIdSuffix string - if stateId != "" { - stateIdSuffix = fmt.Sprintf("ID: %s, ", truncateId(stateId, maxIdLen)) + if s != nil && s.ID != "" { + stateIdSuffix = fmt.Sprintf("ID: %s, ", truncateId(s.ID, maxIdLen)) } h.ui.Output(h.Colorize.Color(fmt.Sprintf( @@ -186,13 +187,15 @@ func (h *UiHook) stillApplying(id, stateId string) { ))) // Reschedule - time.AfterFunc(periodicUiTimer, func() { h.stillApplying(id, stateId) }) + time.AfterFunc(periodicUiTimer, func() { h.stillApplying(id, s) }) } func (h *UiHook) PostApply( n *terraform.InstanceInfo, s *terraform.InstanceState, applyerr error) (terraform.HookAction, error) { + + log.Printf("[DEBUG] PostApply arguments:\n%#v\n%#v\n%#v", n, s, applyerr) id := n.HumanId() h.l.Lock() @@ -221,10 +224,13 @@ func (h *UiHook) PostApply( // Errors are collected and printed in ApplyCommand, no need to duplicate return terraform.HookActionContinue, nil } + log.Printf("[DEBUG] Printing out output: %#v", msg) - h.ui.Output(h.Colorize.Color(fmt.Sprintf( + colorized := h.Colorize.Color(fmt.Sprintf( "[reset][bold]%s: %s%s[reset]", - id, msg, stateIdSuffix))) + id, msg, stateIdSuffix)) + + h.ui.Output(colorized) return terraform.HookActionContinue, nil } diff --git a/command/hook_ui_test.go b/command/hook_ui_test.go index 1c6476efe..3ba7345b7 100644 --- a/command/hook_ui_test.go +++ b/command/hook_ui_test.go @@ -1,10 +1,72 @@ package command import ( + "bytes" "fmt" "testing" + "time" + + "github.com/hashicorp/terraform/terraform" + "github.com/mitchellh/cli" + "github.com/mitchellh/colorstring" ) +func TestUiHookPostApply_emptyState(t *testing.T) { + colorize := &colorstring.Colorize{ + Colors: colorstring.DefaultColors, + Disable: true, + Reset: true, + } + + ir := bytes.NewReader([]byte{}) + errBuf := bytes.NewBuffer([]byte{}) + outBuf := bytes.NewBuffer([]byte{}) + ui := cli.MockUi{ + InputReader: ir, + ErrorWriter: errBuf, + OutputWriter: outBuf, + } + h := &UiHook{ + Colorize: colorize, + Ui: &ui, + } + h.init() + h.resources = map[string]uiResourceState{ + "data.google_compute_zones.available": uiResourceState{ + Op: uiResourceDestroy, + Start: time.Now(), + }, + } + + mock := &terraform.MockInstanceInfo{ + terraform.InstanceInfo{ + Id: "data.google_compute_zones.available", + ModulePath: []string{"root"}, + Type: "google_compute_zones", + }, + } + n := mock.WithUniqueExtra("destroy") + action, err := h.PostApply(n, nil, nil) + if err != nil { + t.Fatal(err) + } + if action != terraform.HookActionContinue { + t.Fatal("Expected hook to continue, given: %#v", action) + } + + expectedOutput := "" + output := outBuf.String() + if output != expectedOutput { + t.Fatalf("Output didn't match.\nExpected: %q\nGiven: %q", expectedOutput, output) + } + + expectedErrOutput := "" + errOutput := errBuf.String() + if errOutput != expectedErrOutput { + t.Fatalf("Error output didn't match.\nExpected: %q\nGiven: %q", expectedErrOutput, errOutput) + } +} + func TestTruncateId(t *testing.T) { testCases := []struct { Input string diff --git a/terraform/resource.go b/terraform/resource.go index 0acf0beb2..b1f5822f6 100644 --- a/terraform/resource.go +++ b/terraform/resource.go @@ -72,6 +72,19 @@ type InstanceInfo struct { uniqueExtra string } +type MockInstanceInfo struct { + InstanceInfo +} + +func (m *MockInstanceInfo) WithUniqueExtra(extra string) *InstanceInfo { + return &InstanceInfo{ + Id: m.Id, + ModulePath: m.ModulePath, + Type: m.Type, + uniqueExtra: extra, + } +} + // HumanId is a unique Id that is human-friendly and useful for UI elements. func (i *InstanceInfo) HumanId() string { if i == nil { From 866de2776ea34ad70e68fb1512d7ac8c0a8cb67b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 2 Mar 2017 11:15:02 -0800 Subject: [PATCH 2/5] command: trigger still applying cancellations from a channel --- command/hook_ui.go | 108 +++++++++++++++++++++++++-------------------- 1 file changed, 59 insertions(+), 49 deletions(-) diff --git a/command/hook_ui.go b/command/hook_ui.go index 60372123d..df5b6c09a 100644 --- a/command/hook_ui.go +++ b/command/hook_ui.go @@ -33,8 +33,12 @@ type UiHook struct { // uiResourceState tracks the state of a single resource type uiResourceState struct { - Op uiResourceOp - Start time.Time + Name string + ResourceId string + Op uiResourceOp + Start time.Time + + DoneCh chan struct{} // To be used for cancellation } // uiResourceOp is an enum for operations on a resource @@ -63,13 +67,6 @@ func (h *UiHook) PreApply( op = uiResourceCreate } - h.l.Lock() - h.resources[id] = uiResourceState{ - Op: op, - Start: time.Now().Round(time.Second), - } - h.l.Unlock() - var operation string switch op { case uiResourceModify: @@ -131,8 +128,9 @@ func (h *UiHook) PreApply( attrString = "\n " + attrString } - var stateIdSuffix string + var stateId, stateIdSuffix string if s != nil && s.ID != "" { + stateId = s.ID stateIdSuffix = fmt.Sprintf(" (ID: %s)", truncateId(s.ID, maxIdLen)) } @@ -143,51 +141,59 @@ func (h *UiHook) PreApply( stateIdSuffix, attrString))) - // Set a timer to show an operation is still happening - time.AfterFunc(periodicUiTimer, func() { h.stillApplying(id, s) }) + uiState := uiResourceState{ + Name: id, + ResourceId: stateId, + Op: op, + Start: time.Now().Round(time.Second), + DoneCh: make(chan struct{}), + } + + h.l.Lock() + h.resources[id] = uiState + h.l.Unlock() + + // Start goroutine that shows progress + go h.stillApplying(uiState) return terraform.HookActionContinue, nil } -func (h *UiHook) stillApplying(id string, s *terraform.InstanceState) { - // Grab the operation. We defer the lock here to avoid the "still..." - // message showing up after a completion message. - h.l.Lock() - defer h.l.Unlock() - state, ok := h.resources[id] +func (h *UiHook) stillApplying(state uiResourceState) { + for { + select { + case <-state.DoneCh: + return - // If the resource is out of the map it means we're done with it - if !ok { - return + case <-time.After(periodicUiTimer): + // Timer up, show status + } + + var msg string + switch state.Op { + case uiResourceModify: + msg = "Still modifying..." + case uiResourceDestroy: + msg = "Still destroying..." + case uiResourceCreate: + msg = "Still creating..." + case uiResourceUnknown: + return + } + + idSuffix := "" + if v := state.ResourceId; v != "" { + idSuffix = fmt.Sprintf("ID: %s, ", truncateId(v, maxIdLen)) + } + + h.ui.Output(h.Colorize.Color(fmt.Sprintf( + "[reset][bold]%s: %s (%s%s elapsed)[reset]", + state.Name, + msg, + idSuffix, + time.Now().Round(time.Second).Sub(state.Start), + ))) } - - var msg string - switch state.Op { - case uiResourceModify: - msg = "Still modifying..." - case uiResourceDestroy: - msg = "Still destroying..." - case uiResourceCreate: - msg = "Still creating..." - case uiResourceUnknown: - return - } - - var stateIdSuffix string - if s != nil && s.ID != "" { - stateIdSuffix = fmt.Sprintf("ID: %s, ", truncateId(s.ID, maxIdLen)) - } - - h.ui.Output(h.Colorize.Color(fmt.Sprintf( - "[reset][bold]%s: %s (%s%s elapsed)[reset]", - id, - msg, - stateIdSuffix, - time.Now().Round(time.Second).Sub(state.Start), - ))) - - // Reschedule - time.AfterFunc(periodicUiTimer, func() { h.stillApplying(id, s) }) } func (h *UiHook) PostApply( @@ -200,6 +206,10 @@ func (h *UiHook) PostApply( h.l.Lock() state := h.resources[id] + if state.DoneCh != nil { + close(state.DoneCh) + } + delete(h.resources, id) h.l.Unlock() From 5086f9f5683307895dfc4d2e8f98cf0dc5e4ed1e Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 2 Mar 2017 11:15:38 -0800 Subject: [PATCH 3/5] command: remove log --- command/hook_ui.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/command/hook_ui.go b/command/hook_ui.go index df5b6c09a..1ebcc578d 100644 --- a/command/hook_ui.go +++ b/command/hook_ui.go @@ -4,7 +4,6 @@ import ( "bufio" "bytes" "fmt" - "log" "sort" "strings" "sync" @@ -55,7 +54,6 @@ func (h *UiHook) PreApply( n *terraform.InstanceInfo, s *terraform.InstanceState, d *terraform.InstanceDiff) (terraform.HookAction, error) { - log.Printf("[DEBUG] PreApply arguments:\n%#v\n%#v\n%#v", n, s, d) h.once.Do(h.init) id := n.HumanId() @@ -201,7 +199,6 @@ func (h *UiHook) PostApply( s *terraform.InstanceState, applyerr error) (terraform.HookAction, error) { - log.Printf("[DEBUG] PostApply arguments:\n%#v\n%#v\n%#v", n, s, applyerr) id := n.HumanId() h.l.Lock() @@ -234,7 +231,6 @@ func (h *UiHook) PostApply( // Errors are collected and printed in ApplyCommand, no need to duplicate return terraform.HookActionContinue, nil } - log.Printf("[DEBUG] Printing out output: %#v", msg) colorized := h.Colorize.Color(fmt.Sprintf( "[reset][bold]%s: %s%s[reset]", From 0224a12a00f58d0ffa8a1a949842ef01dc1c75b9 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 2 Mar 2017 11:19:54 -0800 Subject: [PATCH 4/5] command: fix go vet --- command/hook_ui_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/hook_ui_test.go b/command/hook_ui_test.go index 3ba7345b7..c80ac6850 100644 --- a/command/hook_ui_test.go +++ b/command/hook_ui_test.go @@ -51,7 +51,7 @@ func TestUiHookPostApply_emptyState(t *testing.T) { t.Fatal(err) } if action != terraform.HookActionContinue { - t.Fatal("Expected hook to continue, given: %#v", action) + t.Fatalf("Expected hook to continue, given: %#v", action) } expectedOutput := "" From d1b930d5b551ed7c809172df4637facfc2bd8dd4 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 2 Mar 2017 11:21:48 -0800 Subject: [PATCH 5/5] command: remove unused test --- command/hook_ui_test.go | 62 ----------------------------------------- terraform/resource.go | 13 --------- 2 files changed, 75 deletions(-) diff --git a/command/hook_ui_test.go b/command/hook_ui_test.go index c80ac6850..1c6476efe 100644 --- a/command/hook_ui_test.go +++ b/command/hook_ui_test.go @@ -1,72 +1,10 @@ package command import ( - "bytes" "fmt" "testing" - "time" - - "github.com/hashicorp/terraform/terraform" - "github.com/mitchellh/cli" - "github.com/mitchellh/colorstring" ) -func TestUiHookPostApply_emptyState(t *testing.T) { - colorize := &colorstring.Colorize{ - Colors: colorstring.DefaultColors, - Disable: true, - Reset: true, - } - - ir := bytes.NewReader([]byte{}) - errBuf := bytes.NewBuffer([]byte{}) - outBuf := bytes.NewBuffer([]byte{}) - ui := cli.MockUi{ - InputReader: ir, - ErrorWriter: errBuf, - OutputWriter: outBuf, - } - h := &UiHook{ - Colorize: colorize, - Ui: &ui, - } - h.init() - h.resources = map[string]uiResourceState{ - "data.google_compute_zones.available": uiResourceState{ - Op: uiResourceDestroy, - Start: time.Now(), - }, - } - - mock := &terraform.MockInstanceInfo{ - terraform.InstanceInfo{ - Id: "data.google_compute_zones.available", - ModulePath: []string{"root"}, - Type: "google_compute_zones", - }, - } - n := mock.WithUniqueExtra("destroy") - action, err := h.PostApply(n, nil, nil) - if err != nil { - t.Fatal(err) - } - if action != terraform.HookActionContinue { - t.Fatalf("Expected hook to continue, given: %#v", action) - } - - expectedOutput := "" - output := outBuf.String() - if output != expectedOutput { - t.Fatalf("Output didn't match.\nExpected: %q\nGiven: %q", expectedOutput, output) - } - - expectedErrOutput := "" - errOutput := errBuf.String() - if errOutput != expectedErrOutput { - t.Fatalf("Error output didn't match.\nExpected: %q\nGiven: %q", expectedErrOutput, errOutput) - } -} - func TestTruncateId(t *testing.T) { testCases := []struct { Input string diff --git a/terraform/resource.go b/terraform/resource.go index b1f5822f6..0acf0beb2 100644 --- a/terraform/resource.go +++ b/terraform/resource.go @@ -72,19 +72,6 @@ type InstanceInfo struct { uniqueExtra string } -type MockInstanceInfo struct { - InstanceInfo -} - -func (m *MockInstanceInfo) WithUniqueExtra(extra string) *InstanceInfo { - return &InstanceInfo{ - Id: m.Id, - ModulePath: m.ModulePath, - Type: m.Type, - uniqueExtra: extra, - } -} - // HumanId is a unique Id that is human-friendly and useful for UI elements. func (i *InstanceInfo) HumanId() string { if i == nil {