From 1941d45e327c54eb054d6f63d852dfebfe19c18b Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Thu, 6 Jan 2022 08:32:33 -0500 Subject: add OCI Runtime name to errors It would be easier to diagnose OCI runtime errors if the error actually had the name of the OCI runtime that produced the error. [NO NEW TESTS NEEDED] Signed-off-by: Daniel J Walsh --- libpod/oci_attach_linux.go | 2 +- libpod/oci_conmon_exec_linux.go | 14 +++++++------- libpod/oci_conmon_linux.go | 10 +++++----- libpod/oci_util.go | 14 +++++++------- test/system/030-run.bats | 4 ++-- 5 files changed, 22 insertions(+), 22 deletions(-) diff --git a/libpod/oci_attach_linux.go b/libpod/oci_attach_linux.go index 1f2a28ead..a382f6043 100644 --- a/libpod/oci_attach_linux.go +++ b/libpod/oci_attach_linux.go @@ -143,7 +143,7 @@ func (c *Container) attachToExec(streams *define.AttachStreams, keys *string, se } // 2: read from attachFd that the parent process has set up the console socket - if _, err := readConmonPipeData(attachFd, ""); err != nil { + if _, err := readConmonPipeData(c.ociRuntime.Name(), attachFd, ""); err != nil { return err } diff --git a/libpod/oci_conmon_exec_linux.go b/libpod/oci_conmon_exec_linux.go index 654306f92..e58d4d6f9 100644 --- a/libpod/oci_conmon_exec_linux.go +++ b/libpod/oci_conmon_exec_linux.go @@ -76,7 +76,7 @@ func (r *ConmonOCIRuntime) ExecContainer(c *Container, sessionID string, options return -1, nil, errors.Wrapf(err, "cannot run conmon") } - pid, err := readConmonPipeData(pipes.syncPipe, ociLog) + pid, err := readConmonPipeData(r.name, pipes.syncPipe, ociLog) return pid, attachChan, err } @@ -134,7 +134,7 @@ func (r *ConmonOCIRuntime) ExecContainerHTTP(ctr *Container, sessionID string, o conmonPipeDataChan := make(chan conmonPipeData) go func() { // attachToExec is responsible for closing pipes - attachChan <- attachExecHTTP(ctr, sessionID, req, w, streams, pipes, detachKeys, options.Terminal, cancel, hijackDone, holdConnOpen, execCmd, conmonPipeDataChan, ociLog, newSize) + attachChan <- attachExecHTTP(ctr, sessionID, req, w, streams, pipes, detachKeys, options.Terminal, cancel, hijackDone, holdConnOpen, execCmd, conmonPipeDataChan, ociLog, newSize, r.name) close(attachChan) }() @@ -176,7 +176,7 @@ func (r *ConmonOCIRuntime) ExecContainerDetached(ctr *Container, sessionID strin // Wait for Conmon to tell us we're ready to attach. // We aren't actually *going* to attach, but this means that we're good // to proceed. - if _, err := readConmonPipeData(pipes.attachPipe, ""); err != nil { + if _, err := readConmonPipeData(r.name, pipes.attachPipe, ""); err != nil { return -1, err } @@ -190,7 +190,7 @@ func (r *ConmonOCIRuntime) ExecContainerDetached(ctr *Container, sessionID strin return -1, errors.Wrapf(err, "cannot run conmon") } - pid, err := readConmonPipeData(pipes.syncPipe, ociLog) + pid, err := readConmonPipeData(r.name, pipes.syncPipe, ociLog) return pid, err } @@ -486,7 +486,7 @@ func (r *ConmonOCIRuntime) startExec(c *Container, sessionID string, options *Ex } // Attach to a container over HTTP -func attachExecHTTP(c *Container, sessionID string, r *http.Request, w http.ResponseWriter, streams *HTTPAttachStreams, pipes *execPipes, detachKeys []byte, isTerminal bool, cancel <-chan bool, hijackDone chan<- bool, holdConnOpen <-chan bool, execCmd *exec.Cmd, conmonPipeDataChan chan<- conmonPipeData, ociLog string, newSize *define.TerminalSize) (deferredErr error) { +func attachExecHTTP(c *Container, sessionID string, r *http.Request, w http.ResponseWriter, streams *HTTPAttachStreams, pipes *execPipes, detachKeys []byte, isTerminal bool, cancel <-chan bool, hijackDone chan<- bool, holdConnOpen <-chan bool, execCmd *exec.Cmd, conmonPipeDataChan chan<- conmonPipeData, ociLog string, newSize *define.TerminalSize, runtimeName string) (deferredErr error) { // NOTE: As you may notice, the attach code is quite complex. // Many things happen concurrently and yet are interdependent. // If you ever change this function, make sure to write to the @@ -519,7 +519,7 @@ func attachExecHTTP(c *Container, sessionID string, r *http.Request, w http.Resp } // 2: read from attachFd that the parent process has set up the console socket - if _, err := readConmonPipeData(pipes.attachPipe, ""); err != nil { + if _, err := readConmonPipeData(runtimeName, pipes.attachPipe, ""); err != nil { conmonPipeDataChan <- conmonPipeData{-1, err} return err } @@ -582,7 +582,7 @@ func attachExecHTTP(c *Container, sessionID string, r *http.Request, w http.Resp if err := execCmd.Wait(); err != nil { conmonPipeDataChan <- conmonPipeData{-1, err} } else { - pid, err := readConmonPipeData(pipes.syncPipe, ociLog) + pid, err := readConmonPipeData(runtimeName, pipes.syncPipe, ociLog) if err != nil { hijackWriteError(err, c.ID(), isTerminal, httpBuf) conmonPipeDataChan <- conmonPipeData{pid, err} diff --git a/libpod/oci_conmon_linux.go b/libpod/oci_conmon_linux.go index 5446a8f8a..3440507ed 100644 --- a/libpod/oci_conmon_linux.go +++ b/libpod/oci_conmon_linux.go @@ -1262,7 +1262,7 @@ func (r *ConmonOCIRuntime) createOCIContainer(ctr *Container, restoreOptions *Co return 0, err } - pid, err := readConmonPipeData(parentSyncPipe, ociLog) + pid, err := readConmonPipeData(r.name, parentSyncPipe, ociLog) if err != nil { if err2 := r.DeleteContainer(ctr); err2 != nil { logrus.Errorf("Removing container %s from runtime after creation failed", ctr.ID()) @@ -1564,7 +1564,7 @@ func readConmonPidFile(pidFile string) (int, error) { } // readConmonPipeData attempts to read a syncInfo struct from the pipe -func readConmonPipeData(pipe *os.File, ociLog string) (int, error) { +func readConmonPipeData(runtimeName string, pipe *os.File, ociLog string) (int, error) { // syncInfo is used to return data from monitor process to daemon type syncInfo struct { Data int `json:"data"` @@ -1600,7 +1600,7 @@ func readConmonPipeData(pipe *os.File, ociLog string) (int, error) { if err == nil { var ociErr ociError if err := json.Unmarshal(ociLogData, &ociErr); err == nil { - return -1, getOCIRuntimeError(ociErr.Msg) + return -1, getOCIRuntimeError(runtimeName, ociErr.Msg) } } } @@ -1613,13 +1613,13 @@ func readConmonPipeData(pipe *os.File, ociLog string) (int, error) { if err == nil { var ociErr ociError if err := json.Unmarshal(ociLogData, &ociErr); err == nil { - return ss.si.Data, getOCIRuntimeError(ociErr.Msg) + return ss.si.Data, getOCIRuntimeError(runtimeName, ociErr.Msg) } } } // If we failed to parse the JSON errors, then print the output as it is if ss.si.Message != "" { - return ss.si.Data, getOCIRuntimeError(ss.si.Message) + return ss.si.Data, getOCIRuntimeError(runtimeName, ss.si.Message) } return ss.si.Data, errors.Wrapf(define.ErrInternal, "container create failed") } diff --git a/libpod/oci_util.go b/libpod/oci_util.go index 6d99d5836..06d1dc652 100644 --- a/libpod/oci_util.go +++ b/libpod/oci_util.go @@ -136,7 +136,7 @@ func bindPort(protocol, hostIP string, port uint16, isV6 bool, sctpWarning *bool return file, nil } -func getOCIRuntimeError(runtimeMsg string) error { +func getOCIRuntimeError(name, runtimeMsg string) error { includeFullOutput := logrus.GetLevel() == logrus.DebugLevel if match := regexp.MustCompile("(?i).*permission denied.*|.*operation not permitted.*").FindString(runtimeMsg); match != "" { @@ -144,14 +144,14 @@ func getOCIRuntimeError(runtimeMsg string) error { if includeFullOutput { errStr = runtimeMsg } - return errors.Wrapf(define.ErrOCIRuntimePermissionDenied, "%s", strings.Trim(errStr, "\n")) + return errors.Wrapf(define.ErrOCIRuntimePermissionDenied, "%s: %s", name, strings.Trim(errStr, "\n")) } if match := regexp.MustCompile("(?i).*executable file not found in.*|.*no such file or directory.*").FindString(runtimeMsg); match != "" { errStr := match if includeFullOutput { errStr = runtimeMsg } - return errors.Wrapf(define.ErrOCIRuntimeNotFound, "%s", strings.Trim(errStr, "\n")) + return errors.Wrapf(define.ErrOCIRuntimeNotFound, "%s: %s", name, strings.Trim(errStr, "\n")) } if match := regexp.MustCompile("`/proc/[a-z0-9-].+/attr.*`").FindString(runtimeMsg); match != "" { errStr := match @@ -159,11 +159,11 @@ func getOCIRuntimeError(runtimeMsg string) error { errStr = runtimeMsg } if strings.HasSuffix(match, "/exec`") { - return errors.Wrapf(define.ErrSetSecurityAttribute, "%s", strings.Trim(errStr, "\n")) + return errors.Wrapf(define.ErrSetSecurityAttribute, "%s: %s", name, strings.Trim(errStr, "\n")) } else if strings.HasSuffix(match, "/current`") { - return errors.Wrapf(define.ErrGetSecurityAttribute, "%s", strings.Trim(errStr, "\n")) + return errors.Wrapf(define.ErrGetSecurityAttribute, "%s: %s", name, strings.Trim(errStr, "\n")) } - return errors.Wrapf(define.ErrSecurityAttribute, "%s", strings.Trim(errStr, "\n")) + return errors.Wrapf(define.ErrSecurityAttribute, "%s: %s", name, strings.Trim(errStr, "\n")) } - return errors.Wrapf(define.ErrOCIRuntime, "%s", strings.Trim(runtimeMsg, "\n")) + return errors.Wrapf(define.ErrOCIRuntime, "%s: %s", name, strings.Trim(runtimeMsg, "\n")) } diff --git a/test/system/030-run.bats b/test/system/030-run.bats index d81a0758c..317026710 100644 --- a/test/system/030-run.bats +++ b/test/system/030-run.bats @@ -14,8 +14,8 @@ load helpers # ...but check the configured runtime engine, and switch to crun as needed run_podman info --format '{{ .Host.OCIRuntime.Path }}' if expr "$output" : ".*/crun"; then - err_no_such_cmd="Error: executable file.* not found in \$PATH: No such file or directory: OCI runtime attempted to invoke a command that was not found" - err_no_exec_dir="Error: open executable: Operation not permitted: OCI permission denied" + err_no_such_cmd="Error:.*executable file.* not found in \$PATH: No such file or directory: OCI runtime attempted to invoke a command that was not found" + err_no_exec_dir="Error:.*open executable: Operation not permitted: OCI permission denied" fi tests=" -- cgit v1.2.3-54-g00ecf