Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cli: fix the process exit status codes #70673

Merged
merged 1 commit into from
Sep 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions pkg/cli/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -311,11 +311,14 @@ go_test(
"//pkg/base",
"//pkg/build",
"//pkg/cli/clicfg",
"//pkg/cli/clierror",
"//pkg/cli/clierrorplus",
"//pkg/cli/cliflags",
"//pkg/cli/clisqlcfg",
"//pkg/cli/clisqlclient",
"//pkg/cli/clisqlexec",
"//pkg/cli/democluster",
"//pkg/cli/exit",
"//pkg/gossip",
"//pkg/jobs",
"//pkg/jobs/jobspb",
Expand Down
15 changes: 10 additions & 5 deletions pkg/cli/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,16 +67,21 @@ func Main() {

// Finally, extract the error code, as optionally specified
// by the sub-command.
errCode = exit.UnspecifiedError()
var cliErr *clierror.Error
if errors.As(err, &cliErr) {
errCode = cliErr.GetExitCode()
}
errCode = getExitCode(err)
}

exit.WithCode(errCode)
}

func getExitCode(err error) (errCode exit.Code) {
errCode = exit.UnspecifiedError()
var cliErr *clierror.Error
if errors.As(err, &cliErr) {
errCode = cliErr.GetExitCode()
}
return errCode
}

func doMain(cmd *cobra.Command, cmdName string) error {
if cmd != nil {
// Apply the configuration defaults from environment variables.
Expand Down
27 changes: 27 additions & 0 deletions pkg/cli/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,13 @@ import (
"fmt"
"testing"

"github.com/cockroachdb/cockroach/pkg/cli/clierror"
"github.com/cockroachdb/cockroach/pkg/cli/clierrorplus"
"github.com/cockroachdb/cockroach/pkg/cli/exit"
"github.com/cockroachdb/cockroach/pkg/testutils"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/errors"
"github.com/spf13/cobra"
)

func TestCLITimeout(t *testing.T) {
Expand Down Expand Up @@ -72,3 +76,26 @@ func TestJunkPositionalArguments(t *testing.T) {
}
}
}

func Example_exitcode() {
c := NewCLITest(TestCLIParams{NoServer: true})
defer c.Cleanup()

testCmd := &cobra.Command{
Use: "test-exit-code",
RunE: clierrorplus.MaybeDecorateError(
func(_ *cobra.Command, _ []string) error {
return clierror.NewError(errors.New("err"), exit.Interrupted())
}),
}
cockroachCmd.AddCommand(testCmd)
defer cockroachCmd.RemoveCommand(testCmd)

c.reportExitCode = true
c.Run("test-exit-code")

// Output:
// test-exit-code
// ERROR: err
// exit code: 3
}
4 changes: 4 additions & 0 deletions pkg/cli/clierror/formatted_error.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ type formattedError struct {
showSeverity, verbose bool
}

func (f *formattedError) Unwrap() error {
return f.err
}

// Error implements the error interface.
func (f *formattedError) Error() string {
// If we're applying recursively, ignore what's there and display the original error.
Expand Down
4 changes: 2 additions & 2 deletions pkg/cli/interactive_tests/test_server_sig.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ eexpect "shutdown completed"
eexpect ":/# "
end_test

start_test "Check that Ctrl+C finishes with exit code 1. (#9051)"
start_test "Check that Ctrl+C finishes with exit code 3. (#9051+#70673)"
send "echo \$?\r"
eexpect "1\r\n"
eexpect "3\r\n"
eexpect ":/# "
end_test

Expand Down
26 changes: 21 additions & 5 deletions pkg/cli/testutils.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/cli/clierror"
"github.com/cockroachdb/cockroach/pkg/cli/cliflags"
"github.com/cockroachdb/cockroach/pkg/cli/clisqlexec"
"github.com/cockroachdb/cockroach/pkg/cli/exit"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/security"
"github.com/cockroachdb/cockroach/pkg/security/securitytest"
Expand Down Expand Up @@ -61,15 +62,23 @@ type TestCLI struct {
logScope *log.TestLogScope
// if true, doesn't print args during RunWithArgs.
omitArgs bool
// if true, prints the requested exit code during RunWithArgs.
reportExitCode bool
}

// TestCLIParams contains parameters used by TestCLI.
type TestCLIParams struct {
T *testing.T
Insecure bool
NoServer bool
StoreSpecs []base.StoreSpec
Locality roachpb.Locality
T *testing.T
Insecure bool
// NoServer, if true, starts the test without a DB server.
NoServer bool

// The store specifications for the in-memory server.
StoreSpecs []base.StoreSpec
// The locality tiers for the in-memory server.
Locality roachpb.Locality

// NoNodelocal, if true, disables node-local external I/O storage.
NoNodelocal bool
}

Expand Down Expand Up @@ -350,6 +359,13 @@ func (c TestCLI) RunWithArgs(origArgs []string) {
return Run(args)
}(); err != nil {
clierror.OutputError(os.Stdout, err, true /*showSeverity*/, false /*verbose*/)
if c.reportExitCode {
fmt.Fprintln(os.Stdout, "exit code:", getExitCode(err))
}
} else {
if c.reportExitCode {
fmt.Fprintln(os.Stdout, "exit code:", exit.Success())
}
}
}

Expand Down