Skip to content

Commit

Permalink
Ensure workflow status failure if command fails in backendless mode (#…
Browse files Browse the repository at this point in the history
…1760)

Fixes #1658

Error handling in backendless mode was incorrect, resulting in the
workflow exit status to be success even if the command execution failed.
This patch addresses this issue.

The following are some additional notes of this fix:

- When `allAppliesSuccessful` is false, we need to call
  `ReportErrorAndExit()` to ensure that the workflow fails with a
  non-zero exit status.
- Since `ReportErrorAndExit()` invokes `os.Exit()` immediately, it
  should be called after setting the status of the pull request.
- Contrary to intuition, `atLeastOneApply` is counted even if plan
  command, so we need to use `scheduler.IsPlanJobs()` to determine if
  the current workflow is plan or apply.
  • Loading branch information
minamijoyo authored Nov 4, 2024
1 parent bc9d12f commit 63ff5a2
Showing 1 changed file with 9 additions and 11 deletions.
20 changes: 9 additions & 11 deletions cli/pkg/github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,16 +276,14 @@ func GitHubCI(lock core_locking.Lock, policyCheckerProvider core_policy.PolicyCh
jobs = digger.SortedCommandsByDependency(jobs, &dependencyGraph)

allAppliesSuccessful, atLeastOneApply, err := digger.RunJobs(jobs, &githubPrService, &githubPrService, lock, reporter, planStorage, policyChecker, comment_updater.NoopCommentUpdater{}, backendApi, "", false, false, "0", currentDir)
if err != nil {
usage.ReportErrorAndExit(githubActor, fmt.Sprintf("Failed to run commands. %s", err), 8)
if !allAppliesSuccessful || err != nil {
// aggregate status checks: failure
if allAppliesSuccessful {
if atLeastOneApply {
githubPrService.SetStatus(prNumber, "failure", "digger/apply")
} else {
githubPrService.SetStatus(prNumber, "failure", "digger/plan")
}
if scheduler.IsPlanJobs(jobs) {
githubPrService.SetStatus(prNumber, "failure", "digger/plan")
} else {
githubPrService.SetStatus(prNumber, "failure", "digger/apply")
}
usage.ReportErrorAndExit(githubActor, fmt.Sprintf("Failed to run commands. %s", err), 8)
}

if diggerConfig.AutoMerge && allAppliesSuccessful && atLeastOneApply && coversAllImpactedProjects {
Expand All @@ -295,10 +293,10 @@ func GitHubCI(lock core_locking.Lock, policyCheckerProvider core_policy.PolicyCh

if allAppliesSuccessful {
// aggreate status checks: success
if atLeastOneApply {
githubPrService.SetStatus(prNumber, "success", "digger/apply")
} else {
if scheduler.IsPlanJobs(jobs) {
githubPrService.SetStatus(prNumber, "success", "digger/plan")
} else {
githubPrService.SetStatus(prNumber, "success", "digger/apply")
}
}

Expand Down

0 comments on commit 63ff5a2

Please sign in to comment.