Merge pull request #12381 from hashicorp/b-hook-ui-fix

command: cancel still applying goroutine with channel

command/hook_ui.go

@@ -32,8 +32,12 @@ type UiHook struct {
 
 // uiResourceState tracks the state of a single resource
 type uiResourceState struct {
-	Op    uiResourceOp
+	Name       string
-	Start time.Time
+	ResourceId string
+	Op         uiResourceOp
+	Start      time.Time
+
+	DoneCh chan struct{} // To be used for cancellation
 }
 
 // uiResourceOp is an enum for operations on a resource
@@ -61,13 +65,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:
@@ -142,61 +139,74 @@ func (h *UiHook) PreApply(
 		stateIdSuffix,
 		attrString)))
 
-	// Set a timer to show an operation is still happening
+	uiState := uiResourceState{
-	time.AfterFunc(periodicUiTimer, func() { h.stillApplying(id, stateId) })
+		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, stateId string) {
+func (h *UiHook) stillApplying(state uiResourceState) {
-	// Grab the operation. We defer the lock here to avoid the "still..."
+	for {
-	// message showing up after a completion message.
+		select {
-	h.l.Lock()
+		case <-state.DoneCh:
-	defer h.l.Unlock()
+			return
-	state, ok := h.resources[id]
 
-	// If the resource is out of the map it means we're done with it
+		case <-time.After(periodicUiTimer):
-	if !ok {
+			// Timer up, show status
-		return
+		}
+
+		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 stateId != "" {
-		stateIdSuffix = fmt.Sprintf("ID: %s, ", truncateId(stateId, 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, stateId) })
 }
 
 func (h *UiHook) PostApply(
 	n *terraform.InstanceInfo,
 	s *terraform.InstanceState,
 	applyerr error) (terraform.HookAction, error) {
+
 	id := n.HumanId()
 
 	h.l.Lock()
 	state := h.resources[id]
+	if state.DoneCh != nil {
+		close(state.DoneCh)
+	}
+
 	delete(h.resources, id)
 	h.l.Unlock()
 
@@ -222,9 +232,11 @@ func (h *UiHook) PostApply(
 		return terraform.HookActionContinue, nil
 	}
 
-	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
 }