Skip to content

Commit 4fa4a70

Browse files
author
Shetty Gaurav Jagadeesha Shetty
committed
feat: implement stable exit code mapping for CLI error categories
Replace the blanket os.Exit(1) with differentiated exit codes (0-5) derived from structured error classification, enabling automation tools and CI/CD pipelines to branch on specific failure types.
1 parent 765c061 commit 4fa4a70

File tree

11 files changed

+869
-96
lines changed

11 files changed

+869
-96
lines changed

cmd/lint/lint.go

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package lint
22

33
import (
4+
"errors"
45
"fmt"
56

67
"github.com/spf13/cobra"
@@ -91,7 +92,9 @@ func AddCommand(root *cobra.Command, flags *genericclioptions.ConfigFlags) {
9192
// Complete phase
9293
if err := command.Complete(); err != nil {
9394
if clierrors.WriteStructuredError(cmd.ErrOrStderr(), err, outputFormat) {
94-
return clierrors.NewAlreadyHandledError(err)
95+
return clierrors.NewAlreadyHandledError(
96+
clierrors.NewExitCodeError(clierrors.ExitCodeFromError(err), err),
97+
)
9598
}
9699

97100
if command.Verbose {
@@ -101,13 +104,17 @@ func AddCommand(root *cobra.Command, flags *genericclioptions.ConfigFlags) {
101104
clierrors.WriteTextError(cmd.ErrOrStderr(), err)
102105
}
103106

104-
return clierrors.NewAlreadyHandledError(err)
107+
return clierrors.NewAlreadyHandledError(
108+
clierrors.NewExitCodeError(clierrors.ExitCodeFromError(err), err),
109+
)
105110
}
106111

107112
// Validate phase
108113
if err := command.Validate(); err != nil {
109-
if clierrors.WriteStructuredError(cmd.ErrOrStderr(), err, outputFormat) {
110-
return clierrors.NewAlreadyHandledError(err)
114+
exitErr := clierrors.NewExitCodeError(clierrors.ExitValidation, err)
115+
116+
if clierrors.WriteStructuredError(cmd.ErrOrStderr(), exitErr, outputFormat) {
117+
return clierrors.NewAlreadyHandledError(exitErr) //nolint:wrapcheck
111118
}
112119

113120
if command.Verbose {
@@ -117,14 +124,21 @@ func AddCommand(root *cobra.Command, flags *genericclioptions.ConfigFlags) {
117124
clierrors.WriteTextError(cmd.ErrOrStderr(), err)
118125
}
119126

120-
return clierrors.NewAlreadyHandledError(err)
127+
return clierrors.NewAlreadyHandledError(exitErr) //nolint:wrapcheck
121128
}
122129

123130
// Run phase
124131
err := command.Run(cmd.Context())
125132
if err != nil {
133+
// Verdict errors (findings already rendered) propagate directly
134+
if errors.Is(err, clierrors.ErrAlreadyHandled) {
135+
return err //nolint:wrapcheck // already wrapped by NewAlreadyHandledError
136+
}
137+
126138
if clierrors.WriteStructuredError(cmd.ErrOrStderr(), err, outputFormat) {
127-
return clierrors.NewAlreadyHandledError(err)
139+
return clierrors.NewAlreadyHandledError(
140+
clierrors.NewExitCodeError(clierrors.ExitCodeFromError(err), err),
141+
)
128142
}
129143

130144
if command.Verbose {
@@ -134,7 +148,9 @@ func AddCommand(root *cobra.Command, flags *genericclioptions.ConfigFlags) {
134148
clierrors.WriteTextError(cmd.ErrOrStderr(), err)
135149
}
136150

137-
return clierrors.NewAlreadyHandledError(err)
151+
return clierrors.NewAlreadyHandledError(
152+
clierrors.NewExitCodeError(clierrors.ExitCodeFromError(err), err),
153+
)
138154
}
139155

140156
return nil

cmd/main.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,12 @@ func main() {
3131
lint.AddCommand(cmd, flags)
3232

3333
if err := cmd.Execute(); err != nil {
34+
exitCode := int(clierrors.ExitCodeFromError(err))
35+
3436
if !errors.Is(err, clierrors.ErrAlreadyHandled) {
35-
if _, writeErr := os.Stderr.WriteString(err.Error() + "\n"); writeErr != nil {
36-
os.Exit(1)
37-
}
37+
_, _ = os.Stderr.WriteString(err.Error() + "\n")
3838
}
3939

40-
os.Exit(1)
40+
os.Exit(exitCode)
4141
}
4242
}

docs/design.md

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,60 @@ kubectl odh lint --target-version 3.3.0
6565
kubectl odh version
6666
```
6767

68+
## Exit Codes
69+
70+
The CLI uses differentiated exit codes to help automation tools and CI/CD pipelines
71+
take appropriate action based on the type of outcome.
72+
73+
| Exit Code | Category | Description |
74+
|-----------|------------|----------------------------------------------------------|
75+
| 0 | Success | Process completed without issues. |
76+
| 1 | Error | General runtime or unexpected errors. |
77+
| 2 | Warning | Process finished, but advisory warnings were found. |
78+
| 3 | Validation | Invalid user input or configuration errors. |
79+
| 4 | Auth | Authentication or authorization failures. |
80+
| 5 | Connection | Network issues, timeouts, or service unavailability. |
81+
82+
### Precedence
83+
84+
When multiple issues occur, the exit code reflects the highest priority error:
85+
86+
1. Connection/Timeout (5) - Infrastructure failure
87+
2. Auth (4) - Security-related failures
88+
3. Validation (3) - Input-related failures
89+
4. Error (1) - Catch-all runtime errors
90+
5. Warning (2) - Only used if no higher-level errors exist
91+
92+
### Lint Command Exit Codes
93+
94+
The lint command maps finding impact levels to exit codes:
95+
96+
- **Prohibited or Blocking findings** → Exit 1 (upgrade cannot proceed)
97+
- **Advisory findings only** → Exit 2 (upgrade can proceed, review recommended)
98+
- **No findings** → Exit 0 (clean)
99+
100+
If a lint check fails to execute due to infrastructure issues (auth, connection),
101+
the corresponding exit code (4 or 5) takes precedence over finding-based exit codes.
102+
103+
### Structured Error Output
104+
105+
When using `-o json` or `-o yaml`, error responses include an `exitCode` field
106+
in the structured output, allowing automation tools to parse the exit code without
107+
relying on shell `$?`:
108+
109+
```json
110+
{
111+
"error": {
112+
"code": "AUTH_FAILED",
113+
"message": "token expired",
114+
"category": "authentication",
115+
"exitCode": 4,
116+
"retriable": false,
117+
"suggestion": "Refresh your kubeconfig credentials with 'oc login' or 'kubectl config'"
118+
}
119+
}
120+
```
121+
68122
## Key Architecture Decisions
69123

70124
### Core Principles

pkg/lint/command.go

Lines changed: 70 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,21 @@ import (
3838
trainingoperatorworkloads "github.com/opendatahub-io/odh-cli/pkg/lint/checks/workloads/trainingoperator"
3939
"github.com/opendatahub-io/odh-cli/pkg/resources"
4040
"github.com/opendatahub-io/odh-cli/pkg/util/client"
41+
clierrors "github.com/opendatahub-io/odh-cli/pkg/util/errors"
4142
"github.com/opendatahub-io/odh-cli/pkg/util/iostreams"
4243
"github.com/opendatahub-io/odh-cli/pkg/util/version"
4344
)
4445

4546
// Verify Command implements cmd.Command interface at compile time.
4647
var _ cmd.Command = (*Command)(nil)
4748

49+
const (
50+
msgProhibitedOrBlocking = "prohibited or blocking findings detected: upgrade cannot proceed"
51+
msgAdvisoryFindings = "advisory findings detected: review recommended before upgrade"
52+
msgInfrastructureErrors = "one or more checks failed due to infrastructure errors"
53+
msgCheckExecErrors = "check execution errors detected: %w"
54+
)
55+
4856
// Command contains the lint command configuration.
4957
type Command struct {
5058
*SharedOptions
@@ -248,8 +256,9 @@ func (c *Command) Run(ctx context.Context) error {
248256

249257
// Reject downgrades when explicit --target-version is provided
250258
if targetVersion.LT(*currentVersion) {
251-
return fmt.Errorf("target version %s is older than current version %s (downgrades not supported)",
252-
c.TargetVersion, currentVersion.String())
259+
return clierrors.NewExitCodeError(clierrors.ExitValidation,
260+
fmt.Errorf("target version %s is older than current version %s (downgrades not supported)",
261+
c.TargetVersion, currentVersion.String()))
253262
}
254263

255264
return c.runUpgradeMode(ctx, currentVersion)
@@ -315,8 +324,25 @@ func (c *Command) runUpgradeMode(ctx context.Context, currentVersion *semver.Ver
315324
resultsByGroup[group] = results
316325
}
317326

318-
// Flatten, strip nil results, and apply severity filter
327+
// Flatten results and compute the highest-priority exit code from execution
328+
// errors BEFORE filtering, so failures with Result == nil are not dropped.
319329
flatResults := FlattenResults(resultsByGroup)
330+
331+
execExitCode := clierrors.ExitSuccess
332+
333+
var firstExecErr error
334+
335+
for _, exec := range flatResults {
336+
if exec.Error != nil {
337+
code := clierrors.ExitCodeFromError(exec.Error)
338+
if clierrors.IsHigherPriority(code, execExitCode) {
339+
execExitCode = code
340+
firstExecErr = exec.Error
341+
}
342+
}
343+
}
344+
345+
// Strip nil results and apply severity filter for display/verdict
320346
flatResults = slices.DeleteFunc(flatResults, func(exec check.CheckExecution) bool {
321347
return exec.Result == nil
322348
})
@@ -327,13 +353,37 @@ func (c *Command) runUpgradeMode(ctx context.Context, currentVersion *semver.Ver
327353
return err
328354
}
329355

330-
// Print verdict and determine exit code
331-
return c.printVerdictAndExit(flatResults)
356+
// Print verdict and determine exit code from findings
357+
findingsErr := c.evaluateVerdict(flatResults)
358+
359+
// If check execution errors (e.g. auth, connection) have higher priority
360+
// than findings, propagate the infrastructure exit code instead.
361+
if execExitCode != clierrors.ExitSuccess {
362+
findingsExitCode := clierrors.ExitCodeFromError(findingsErr)
363+
if clierrors.IsHigherPriority(execExitCode, findingsExitCode) ||
364+
execExitCode == findingsExitCode {
365+
if findingsErr != nil {
366+
return clierrors.NewExitCodeError(execExitCode,
367+
fmt.Errorf(msgCheckExecErrors, firstExecErr))
368+
}
369+
370+
return clierrors.NewExitCodeError(execExitCode,
371+
fmt.Errorf(msgInfrastructureErrors+": %w", firstExecErr))
372+
}
373+
}
374+
375+
// Verdict errors are pure exit code signals — the findings have already
376+
// been rendered by formatAndOutputUpgradeResults above.
377+
if findingsErr != nil {
378+
return clierrors.NewAlreadyHandledError(findingsErr) //nolint:wrapcheck // wrapping is done by NewAlreadyHandledError
379+
}
380+
381+
return nil
332382
}
333383

334-
// printVerdictAndExit prints a prominent result verdict for table output and returns
335-
// an error if fail-on conditions are met (to control exit code).
336-
func (c *Command) printVerdictAndExit(results []check.CheckExecution) error {
384+
// evaluateVerdict prints a prominent result verdict for table output and returns
385+
// an error carrying the appropriate ExitCode when fail-on conditions are met.
386+
func (c *Command) evaluateVerdict(results []check.CheckExecution) error {
337387
var hasProhibited, hasBlocking, hasAdvisory bool
338388

339389
for _, exec := range results {
@@ -357,8 +407,18 @@ func (c *Command) printVerdictAndExit(results []check.CheckExecution) error {
357407
printVerdict(c.IO.Out(), hasProhibited, hasBlocking, hasAdvisory)
358408
}
359409

360-
if hasProhibited {
361-
return errors.New("prohibited findings detected: upgrade is not possible")
410+
if hasProhibited || hasBlocking {
411+
return clierrors.NewExitCodeError(
412+
clierrors.ExitError,
413+
errors.New(msgProhibitedOrBlocking),
414+
)
415+
}
416+
417+
if hasAdvisory {
418+
return clierrors.NewExitCodeError(
419+
clierrors.ExitWarning,
420+
errors.New(msgAdvisoryFindings),
421+
)
362422
}
363423

364424
return nil

0 commit comments

Comments
 (0)