Skip to content

Commit

Permalink
Merge pull request #435 from overmindtech/error-msg
Browse files Browse the repository at this point in the history 
Implement nicer message when risk calculation fails
  • Loading branch information
@DavidS-ovm
DavidS-ovm authored Jun 24, 2024
2 parents e97ec8f + c416543 commit 906d924
Show file tree
Hide file tree
Showing 2 changed files with 107 additions and 30 deletions.
101 changes: 84 additions & 17 deletions cmd/tea_submitplan.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package cmd

import (
"context"
"errors"
"fmt"
"os"
"os/exec"
Expand Down Expand Up @@ -238,8 +239,8 @@ func (m submitPlanModel) View() string {
}
}

if m.changeUrl != "" && m.riskTask.status != taskStatusDone && time.Since(m.risksStarted) > 1500*time.Millisecond {
bits = append(bits, fmt.Sprintf(" │ Check the blast radius graph while you wait:\n │ %v\n", m.changeUrl))
if m.changeUrl != "" && m.riskTask.status != taskStatusDone && m.riskTask.status != taskStatusError && time.Since(m.risksStarted) > 1500*time.Millisecond {
bits = append(bits, fmt.Sprintf(" │ Check the blast radius graph while you wait:\n │ %v\n", m.changeUrl))
}

return strings.Join(bits, "\n") + "\n"
Expand Down Expand Up @@ -277,6 +278,20 @@ func (m submitPlanModel) waitForSubmitPlanActivity() tea.Msg {
return <-m.processing
}

func (m submitPlanModel) risksError(msg string, err error) tea.Msg {
if m.changeUrl == "" {
if err == nil {
return fatalError{err: errors.New(msg)}
}
return fatalError{err: fmt.Errorf("%v: %w", msg, err)}
} else {
if err == nil {
return fatalError{err: fmt.Errorf("%v\nWe'll retry in the background. Find the results online when they're done: %v", msg, m.changeUrl)}
}
return fatalError{err: fmt.Errorf("%v: %w\nWe'll retry in the background. Find the results online when they're done: %v", msg, err, m.changeUrl)}
}
}

func (m submitPlanModel) submitPlanCmd() tea.Msg {
ctx := m.ctx
span := trace.SpanFromContext(ctx)
Expand Down Expand Up @@ -336,6 +351,8 @@ func (m submitPlanModel) submitPlanCmd() tea.Msg {
time.Sleep(time.Second)
m.processing <- submitPlanUpdateMsg{m.blastRadiusTask.FinishMsg()}

// update local copy
m.changeUrl = "https://example.com/changes/abc"
m.processing <- submitPlanUpdateMsg{changeUpdatedMsg{url: "https://example.com/changes/abc"}}
time.Sleep(time.Second)

Expand Down Expand Up @@ -401,6 +418,31 @@ func (m submitPlanModel) submitPlanCmd() tea.Msg {
}}
time.Sleep(1500 * time.Millisecond)

if viper.GetString("ovm-test-fake") == "risks-error" {
m.processing <- submitPlanUpdateMsg{changeUpdatedMsg{
url: "https://example.com/changes/abc",
riskMilestones: []*sdp.RiskCalculationStatus_ProgressMilestone{
{
Description: "fake done milestone",
Status: sdp.RiskCalculationStatus_ProgressMilestone_STATUS_DONE,
},
{
Description: "fake inprogress milestone",
Status: sdp.RiskCalculationStatus_ProgressMilestone_STATUS_DONE,
},
{
Description: "fake errored milestone",
Status: sdp.RiskCalculationStatus_ProgressMilestone_STATUS_ERROR,
},
},
risks: []*sdp.Risk{},
}}

m.processing <- submitPlanUpdateMsg{m.riskTask.UpdateStatusMsg(taskStatusError)}
m.processing <- submitPlanUpdateMsg{m.risksError("initial risk calculation errored", nil)}
close(m.processing)
return nil
}
high := uuid.New()
medium := uuid.New()
low := uuid.New()
Expand Down Expand Up @@ -464,8 +506,9 @@ func (m submitPlanModel) submitPlanCmd() tea.Msg {
planJson, err := tfPlanJsonCmd.Output()
if err != nil {
m.processing <- submitPlanUpdateMsg{m.removingSecretsTask.UpdateStatusMsg(taskStatusError)}
m.processing <- submitPlanUpdateMsg{m.risksError("failed to convert terraform plan to JSON", err)}
close(m.processing)
return fatalError{err: fmt.Errorf("processPlanCmd: failed to convert terraform plan to JSON: %w", err)}
return nil
}

m.processing <- submitPlanUpdateMsg{m.removingSecretsTask.UpdateStatusMsg(taskStatusDone)}
Expand All @@ -480,8 +523,9 @@ func (m submitPlanModel) submitPlanCmd() tea.Msg {
mappingResponse, err := mappedItemDiffsFromPlan(ctx, planJson, m.planFile, log.Fields{})
if err != nil {
m.processing <- submitPlanUpdateMsg{m.resourceExtractionTask.UpdateStatusMsg(taskStatusError)}
m.processing <- submitPlanUpdateMsg{m.risksError("failed to parse terraform plan", err)}
close(m.processing)
return fatalError{err: fmt.Errorf("processPlanCmd: failed to parse terraform plan: %w", err)}
return nil
}
m.processing <- submitPlanUpdateMsg{m.removingSecretsTask.UpdateTitleMsg(
fmt.Sprintf("Removed %v secrets", mappingResponse.RemovedSecrets),
Expand Down Expand Up @@ -512,17 +556,19 @@ func (m submitPlanModel) submitPlanCmd() tea.Msg {
ticketLink, err = getTicketLinkFromPlan(m.planFile)
if err != nil {
m.processing <- submitPlanUpdateMsg{m.uploadChangesTask.UpdateStatusMsg(taskStatusError)}
m.processing <- submitPlanUpdateMsg{m.risksError("failed to get ticket link from plan", err)}
close(m.processing)
return err
return nil
}
}

client := AuthenticatedChangesClient(ctx, m.oi)
changeUuid, err := getChangeUuid(ctx, m.oi, sdp.ChangeStatus_CHANGE_STATUS_DEFINING, ticketLink, false)
if err != nil {
m.processing <- submitPlanUpdateMsg{m.uploadChangesTask.UpdateStatusMsg(taskStatusError)}
m.processing <- submitPlanUpdateMsg{m.risksError("failed searching for existing changes", err)}
close(m.processing)
return fatalError{err: fmt.Errorf("processPlanCmd: failed searching for existing changes: %w", err)}
return nil
}

title := changeTitle(viper.GetString("title"))
Expand All @@ -534,8 +580,9 @@ func (m submitPlanModel) submitPlanCmd() tea.Msg {
tfPlanOutput, err := tfPlanTextCmd.Output()
if err != nil {
m.processing <- submitPlanUpdateMsg{m.removingSecretsTask.UpdateStatusMsg(taskStatusError)}
m.processing <- submitPlanUpdateMsg{m.risksError("failed to convert terraform plan to JSON", err)}
close(m.processing)
return fatalError{err: fmt.Errorf("processPlanCmd: failed to convert terraform plan to JSON: %w", err)}
return nil
}
codeChangesOutput := tryLoadText(ctx, viper.GetString("code-changes-diff"))

Expand All @@ -557,15 +604,17 @@ func (m submitPlanModel) submitPlanCmd() tea.Msg {
})
if err != nil {
m.processing <- submitPlanUpdateMsg{m.uploadChangesTask.UpdateStatusMsg(taskStatusError)}
m.processing <- submitPlanUpdateMsg{m.risksError("failed to create a new change", err)}
close(m.processing)
return fatalError{err: fmt.Errorf("processPlanCmd: failed to create a new change: %w", err)}
return nil
}

maybeChangeUuid := createResponse.Msg.GetChange().GetMetadata().GetUUIDParsed()
if maybeChangeUuid == nil {
m.processing <- submitPlanUpdateMsg{m.uploadChangesTask.UpdateStatusMsg(taskStatusError)}
m.processing <- submitPlanUpdateMsg{m.risksError("failed to read change id", err)}
close(m.processing)
return fatalError{err: fmt.Errorf("processPlanCmd: failed to read change id: %w", err)}
return nil
}

changeUuid = *maybeChangeUuid
Expand Down Expand Up @@ -597,8 +646,9 @@ func (m submitPlanModel) submitPlanCmd() tea.Msg {
})
if err != nil {
m.processing <- submitPlanUpdateMsg{m.uploadChangesTask.UpdateStatusMsg(taskStatusError)}
m.processing <- submitPlanUpdateMsg{m.risksError("failed to update change", err)}
close(m.processing)
return fatalError{err: fmt.Errorf("processPlanCmd: failed to update change: %w", err)}
return nil
}
}

Expand All @@ -619,8 +669,9 @@ func (m submitPlanModel) submitPlanCmd() tea.Msg {
})
if err != nil {
m.processing <- submitPlanUpdateMsg{m.blastRadiusTask.UpdateStatusMsg(taskStatusError)}
m.processing <- submitPlanUpdateMsg{m.risksError("failed to update planned changes", err)}
close(m.processing)
return fatalError{err: fmt.Errorf("processPlanCmd: failed to update planned changes: %w", err)}
return nil
}

last_log := time.Now()
Expand Down Expand Up @@ -657,21 +708,25 @@ func (m submitPlanModel) submitPlanCmd() tea.Msg {
}
if resultStream.Err() != nil {
m.processing <- submitPlanUpdateMsg{m.blastRadiusTask.UpdateStatusMsg(taskStatusError)}
m.processing <- submitPlanUpdateMsg{m.risksError("error streaming results", err)}
close(m.processing)
return fatalError{err: fmt.Errorf("processPlanCmd: error streaming results: %w", err)}
return nil
}
m.processing <- submitPlanUpdateMsg{m.blastRadiusTask.FinishMsg()}

changeUrl := *m.oi.FrontendUrl
changeUrl.Path = fmt.Sprintf("%v/changes/%v/blast-radius", changeUrl.Path, changeUuid)
log.WithField("change-url", changeUrl.String()).Info("Change ready")

m.processing <- submitPlanUpdateMsg{changeUpdatedMsg{url: changeUrl.String()}}
// update local copy
m.changeUrl = changeUrl.String()
m.processing <- submitPlanUpdateMsg{changeUpdatedMsg{url: m.changeUrl}}

///////////////////////////////////////////////////////////////////
// wait for risk calculation to happen
///////////////////////////////////////////////////////////////////
m.processing <- submitPlanUpdateMsg{m.riskTask.UpdateStatusMsg(taskStatusRunning)}
risksErrored := false
for {
riskRes, err := client.GetChangeRisks(ctx, &connect.Request[sdp.GetChangeRisksRequest]{
Msg: &sdp.GetChangeRisksRequest{
Expand All @@ -680,12 +735,13 @@ func (m submitPlanModel) submitPlanCmd() tea.Msg {
})
if err != nil {
m.processing <- submitPlanUpdateMsg{m.riskTask.UpdateStatusMsg(taskStatusError)}
m.processing <- submitPlanUpdateMsg{m.risksError("failed to get change risks", err)}
close(m.processing)
return fatalError{err: fmt.Errorf("processPlanCmd: failed to get change risks: %w", err)}
return nil
}

m.processing <- submitPlanUpdateMsg{changeUpdatedMsg{
url: changeUrl.String(),
url: m.changeUrl,
riskMilestones: riskRes.Msg.GetChangeRiskMetadata().GetRiskCalculationStatus().GetProgressMilestones(),
risks: riskRes.Msg.GetChangeRiskMetadata().GetRisks(),
}}
Expand All @@ -694,17 +750,28 @@ func (m submitPlanModel) submitPlanCmd() tea.Msg {
if status == sdp.RiskCalculationStatus_STATUS_UNSPECIFIED || status == sdp.RiskCalculationStatus_STATUS_INPROGRESS {
time.Sleep(time.Second)
// retry
} else if status == sdp.RiskCalculationStatus_STATUS_ERROR {
risksErrored = true
break
} else {
// it's done (or errored)
// it's done
break
}

if ctx.Err() != nil {
err := ctx.Err()
m.processing <- submitPlanUpdateMsg{m.riskTask.UpdateStatusMsg(taskStatusError)}
m.processing <- submitPlanUpdateMsg{m.risksError("context cancelled", err)}
close(m.processing)
return fatalError{err: fmt.Errorf("processPlanCmd: context cancelled: %w", ctx.Err())}
return nil
}
}

if risksErrored {
m.processing <- submitPlanUpdateMsg{m.riskTask.UpdateStatusMsg(taskStatusError)}
m.processing <- submitPlanUpdateMsg{m.risksError("initial risk calculation errored", nil)}
close(m.processing)
return nil
}

m.processing <- submitPlanUpdateMsg{m.riskTask.UpdateStatusMsg(taskStatusDone)}
Expand Down
36 changes: 23 additions & 13 deletions cmd/terraform_apply.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ import (

// terraformApplyCmd represents the `terraform apply` command
var terraformApplyCmd = &cobra.Command{
Use: "apply [overmind options...] -- [terraform options...]",
Short: "Runs `terraform apply` between two full system configuration snapshots for tracking. This will be automatically connected with the Change created by the `plan` command.",
Use: "apply [overmind options...] -- [terraform options...]",
Short: "Runs `terraform apply` between two full system configuration snapshots for tracking. This will be automatically connected with the Change created by the `plan` command.",
PreRun: PreRunSetup,
Run: CmdWrapper("apply", []string{"explore:read", "changes:write", "config:write", "request:receive"}, NewTfApplyModel),
Run: CmdWrapper("apply", []string{"explore:read", "changes:write", "config:write", "request:receive"}, NewTfApplyModel),
}

type tfApplyModel struct {
Expand Down Expand Up @@ -428,21 +428,24 @@ func (m tfApplyModel) startStartChangeCmd() tea.Cmd {
m.startingChange <- m.startingChangeSnapshot.ProgressMsg(fmt.Sprintf("progress %v", i), uint32(i), uint32(i))
time.Sleep(time.Second)
}
return m.startingChangeSnapshot.FinishMsg()
m.startingChange <- m.startingChangeSnapshot.FinishMsg()
return nil
}

var err error
ticketLink := viper.GetString("ticket-link")
if ticketLink == "" {
ticketLink, err = getTicketLinkFromPlan(m.planFile)
if err != nil {
return fatalError{err: err}
m.startingChange <- fatalError{err: err}
return nil
}
}

changeUuid, err := getChangeUuid(ctx, oi, sdp.ChangeStatus_CHANGE_STATUS_DEFINING, ticketLink, true)
if err != nil {
return fatalError{err: fmt.Errorf("failed to identify change: %w", err)}
m.startingChange <- fatalError{err: fmt.Errorf("failed to identify change: %w", err)}
return nil
}

m.startingChange <- changeIdentifiedMsg{uuid: changeUuid}
Expand All @@ -455,7 +458,8 @@ func (m tfApplyModel) startStartChangeCmd() tea.Cmd {
},
})
if err != nil {
return fatalError{err: fmt.Errorf("failed to start change: %w", err)}
m.startingChange <- fatalError{err: fmt.Errorf("failed to start change: %w", err)}
return nil
}

var msg *sdp.StartChangeResponse
Expand All @@ -480,10 +484,12 @@ func (m tfApplyModel) startStartChangeCmd() tea.Cmd {
m.startingChange <- m.startingChangeSnapshot.ProgressMsg(stateLabel, msg.GetNumItems(), msg.GetNumEdges())
}
if startStream.Err() != nil {
return fatalError{err: fmt.Errorf("failed to process start change: %w", startStream.Err())}
m.startingChange <- fatalError{err: fmt.Errorf("failed to process start change: %w", startStream.Err())}
return nil
}

return m.startingChangeSnapshot.FinishMsg()
m.endingChange <- m.startingChangeSnapshot.FinishMsg()
return nil
}
}

Expand All @@ -506,7 +512,8 @@ func (m tfApplyModel) startEndChangeCmd() tea.Cmd {
m.endingChange <- m.endingChangeSnapshot.ProgressMsg(fmt.Sprintf("progress %v", i), uint32(i), uint32(i))
time.Sleep(time.Second)
}
return m.endingChangeSnapshot.FinishMsg()
m.endingChange <- m.endingChangeSnapshot.FinishMsg()
return nil
}

m.endingChange <- m.endingChangeSnapshot.StartMsg()
Expand All @@ -518,7 +525,8 @@ func (m tfApplyModel) startEndChangeCmd() tea.Cmd {
},
})
if err != nil {
return fatalError{err: fmt.Errorf("failed to end change: %w", err)}
m.endingChange <- fatalError{err: fmt.Errorf("failed to end change: %w", err)}
return nil
}

var msg *sdp.EndChangeResponse
Expand All @@ -543,10 +551,12 @@ func (m tfApplyModel) startEndChangeCmd() tea.Cmd {
m.endingChange <- m.endingChangeSnapshot.ProgressMsg(stateLabel, msg.GetNumItems(), msg.GetNumEdges())
}
if endStream.Err() != nil {
return fatalError{err: fmt.Errorf("failed to process end change: %w", endStream.Err())}
m.endingChange <- fatalError{err: fmt.Errorf("failed to process end change: %w", endStream.Err())}
return nil
}

return m.endingChangeSnapshot.FinishMsg()
m.endingChange <- m.endingChangeSnapshot.FinishMsg()
return nil
}
}

Expand Down

0 comments on commit 906d924

Please sign in to comment.