Guest24897
/
DockerCLI
mirror of https://github.com/docker/cli.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #5236 from thaJeztah/cleanup_run_errors 

cli/command/container: remove reportError, and put StatusError to use
This commit is contained in:
Sebastiaan van Stijn 2024-07-22 17:56:16 +02:00 committed by GitHub
parent 6559d86217 90058df305
commit d5f90ed547
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 70 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
cli/command/container/create.go
View File

@ -92,8 +92,10 @@ func NewCreateCommand(dockerCli command.Cli) *cobra.Command {
func runCreate(ctx context.Context, dockerCli command.Cli, flags *pflag.FlagSet, options *createOptions, copts *containerOptions) error {
if err := validatePullOpt(options.pull); err != nil {
reportError(dockerCli.Err(), "create", err.Error(), true)
return cli.StatusError{StatusCode: 125}
return cli.StatusError{
Status: withHelp(err, "create").Error(),
StatusCode: 125,
}
}
proxyConfig := dockerCli.ConfigFile().ParseProxyConfig(dockerCli.Client().DaemonHost(), opts.ConvertKVStringsToMapWithNil(copts.env.GetAll()))
newEnv := []string{}
@ -107,12 +109,16 @@ func runCreate(ctx context.Context, dockerCli command.Cli, flags *pflag.FlagSet,
copts.env = *opts.NewListOptsRef(&newEnv, nil)
containerCfg, err := parse(flags, copts, dockerCli.ServerInfo().OSType)
if err != nil {
reportError(dockerCli.Err(), "create", err.Error(), true)
return cli.StatusError{StatusCode: 125}
return cli.StatusError{
Status: withHelp(err, "create").Error(),
StatusCode: 125,
}
}
if err = validateAPIVersion(containerCfg, dockerCli.Client().ClientVersion()); err != nil {
reportError(dockerCli.Err(), "create", err.Error(), true)
return cli.StatusError{StatusCode: 125}
return cli.StatusError{
Status: withHelp(err, "create").Error(),
StatusCode: 125,
}
}
id, err := createContainer(ctx, dockerCli, containerCfg, options)
if err != nil {

12
cli/command/container/create_test.go
View File

@ -189,8 +189,8 @@ func TestCreateContainerImagePullPolicyInvalid(t *testing.T) {
statusErr := cli.StatusError{}
assert.Check(t, errors.As(err, &statusErr))
assert.Equal(t, statusErr.StatusCode, 125)
assert.Check(t, is.Contains(dockerCli.ErrBuffer().String(), tc.ExpectedErrMsg))
assert.Check(t, is.Equal(statusErr.StatusCode, 125))
assert.Check(t, is.ErrorContains(err, tc.ExpectedErrMsg))
})
}
}
@ -285,7 +285,7 @@ func TestNewCreateCommandWithWarnings(t *testing.T) {
for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
cli := test.NewFakeCli(&fakeClient{
fakeCLI := test.NewFakeCli(&fakeClient{
createContainerFunc: func(config *container.Config,
hostConfig *container.HostConfig,
networkingConfig *network.NetworkingConfig,
@ -295,15 +295,15 @@ func TestNewCreateCommandWithWarnings(t *testing.T) {
return container.CreateResponse{}, nil
},
})
cmd := NewCreateCommand(cli)
cmd := NewCreateCommand(fakeCLI)
cmd.SetOut(io.Discard)
cmd.SetArgs(tc.args)
err := cmd.Execute()
assert.NilError(t, err)
if tc.warning {
golden.Assert(t, cli.ErrBuffer().String(), tc.name+".golden")
golden.Assert(t, fakeCLI.ErrBuffer().String(), tc.name+".golden")
} else {
assert.Equal(t, cli.ErrBuffer().String(), "")
assert.Equal(t, fakeCLI.ErrBuffer().String(), "")
}
})
}

78
cli/command/container/run.go
View File

@ -83,8 +83,10 @@ func NewRunCommand(dockerCli command.Cli) *cobra.Command {
func runRun(ctx context.Context, dockerCli command.Cli, flags *pflag.FlagSet, ropts *runOptions, copts *containerOptions) error {
if err := validatePullOpt(ropts.pull); err != nil {
reportError(dockerCli.Err(), "run", err.Error(), true)
return cli.StatusError{StatusCode: 125}
return cli.StatusError{
Status: withHelp(err, "run").Error(),
StatusCode: 125,
}
}
proxyConfig := dockerCli.ConfigFile().ParseProxyConfig(dockerCli.Client().DaemonHost(), opts.ConvertKVStringsToMapWithNil(copts.env.GetAll()))
newEnv := []string{}
@ -99,12 +101,16 @@ func runRun(ctx context.Context, dockerCli command.Cli, flags *pflag.FlagSet, ro
containerCfg, err := parse(flags, copts, dockerCli.ServerInfo().OSType)
// just in case the parse does not exit
if err != nil {
reportError(dockerCli.Err(), "run", err.Error(), true)
return cli.StatusError{StatusCode: 125}
return cli.StatusError{
Status: withHelp(err, "run").Error(),
StatusCode: 125,
}
}
if err = validateAPIVersion(containerCfg, dockerCli.CurrentVersion()); err != nil {
reportError(dockerCli.Err(), "run", err.Error(), true)
return cli.StatusError{StatusCode: 125}
return cli.StatusError{
Status: withHelp(err, "run").Error(),
StatusCode: 125,
}
}
return runContainer(ctx, dockerCli, ropts, copts, containerCfg)
}
@ -139,8 +145,7 @@ func runContainer(ctx context.Context, dockerCli command.Cli, runOpts *runOption
containerID, err := createContainer(ctx, dockerCli, containerCfg, &runOpts.createOptions)
if err != nil {
reportError(stderr, "run", err.Error(), true)
return runStartContainerErr(err)
return toStatusError(err)
}
if runOpts.sigProxy {
sigc := notifyAllSignals()
@ -204,12 +209,11 @@ func runContainer(ctx context.Context, dockerCli command.Cli, runOpts *runOption
<-errCh
}
reportError(stderr, "run", err.Error(), false)
if copts.autoRemove {
// wait container to be removed
<-statusChan
}
return runStartContainerErr(err)
return toStatusError(err)
}
if (config.AttachStdin || config.AttachStdout || config.AttachStderr) && config.Tty && dockerCli.Out().IsTerminal() {
@ -292,30 +296,40 @@ func attachContainer(ctx context.Context, dockerCli command.Cli, containerID str
return resp.Close, nil
}
// reportError is a utility method that prints a user-friendly message
// containing the error that occurred during parsing and a suggestion to get help
func reportError(stderr io.Writer, name string, str string, withHelp bool) {
str = strings.TrimSuffix(str, ".") + "."
if withHelp {
str += "\nSee 'docker " + name + " --help'."
}
_, _ = fmt.Fprintln(stderr, "docker:", str)
// withHelp decorates the error with a suggestion to use "--help".
func withHelp(err error, commandName string) error {
return fmt.Errorf("docker: %w\n\nRun 'docker %s --help' for more information", err, commandName)
}
// if container start fails with 'not found'/'no such' error, return 127
// if container start fails with 'permission denied' error, return 126
// return 125 for generic docker daemon failures
func runStartContainerErr(err error) error {
trimmedErr := strings.TrimPrefix(err.Error(), "Error response from daemon: ")
statusError := cli.StatusError{StatusCode: 125}
if strings.Contains(trimmedErr, "executable file not found") ||
strings.Contains(trimmedErr, "no such file or directory") ||
strings.Contains(trimmedErr, "system cannot find the file specified") {
statusError = cli.StatusError{StatusCode: 127}
} else if strings.Contains(trimmedErr, syscall.EACCES.Error()) ||
strings.Contains(trimmedErr, syscall.EISDIR.Error()) {
statusError = cli.StatusError{StatusCode: 126}
// toStatusError attempts to detect specific error-conditions to assign
// an appropriate exit-code for situations where the problem originates
// from the container. It returns [cli.StatusError] with the original
// error message and the Status field set as follows:
//
// - 125: for generic failures sent back from the daemon
// - 126: if container start fails with 'permission denied' error
// - 127: if container start fails with 'not found'/'no such' error
func toStatusError(err error) error {
// TODO(thaJeztah): some of these errors originate from the container: should we actually suggest "--help" for those?
errMsg := err.Error()
if strings.Contains(errMsg, "executable file not found") || strings.Contains(errMsg, "no such file or directory") || strings.Contains(errMsg, "system cannot find the file specified") {
return cli.StatusError{
Status: withHelp(err, "run").Error(),
StatusCode: 127,
}
}
return statusError
if strings.Contains(errMsg, syscall.EACCES.Error()) || strings.Contains(errMsg, syscall.EISDIR.Error()) {
return cli.StatusError{
Status: withHelp(err, "run").Error(),
StatusCode: 126,
}
}
return cli.StatusError{
Status: withHelp(err, "run").Error(),
StatusCode: 125,
}
}

10
cli/command/container/run_test.go
View File

@ -145,8 +145,10 @@ func TestRunCommandWithContentTrustErrors(t *testing.T) {
cmd.SetOut(io.Discard)
cmd.SetErr(io.Discard)
err := cmd.Execute()
assert.Assert(t, err != nil)
assert.Assert(t, is.Contains(fakeCLI.ErrBuffer().String(), tc.expectedError))
statusErr := cli.StatusError{}
assert.Check(t, errors.As(err, &statusErr))
assert.Check(t, is.Equal(statusErr.StatusCode, 125))
assert.Check(t, is.ErrorContains(err, tc.expectedError))
})
}
}
@ -179,8 +181,8 @@ func TestRunContainerImagePullPolicyInvalid(t *testing.T) {
statusErr := cli.StatusError{}
assert.Check(t, errors.As(err, &statusErr))
assert.Equal(t, statusErr.StatusCode, 125)
assert.Check(t, is.Contains(dockerCli.ErrBuffer().String(), tc.ExpectedErrMsg))
assert.Check(t, is.Equal(statusErr.StatusCode, 125))
assert.Check(t, is.ErrorContains(err, tc.ExpectedErrMsg))
})
}
}