Skip to content
Draft
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
17 changes: 9 additions & 8 deletions libcontainer/container_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -338,8 +338,6 @@ func (c *Container) start(process *Process) (retErr error) {
// We do not need the cloned binaries once the process is spawned.
defer process.closeClonedExes()

logsDone := parent.forwardChildLogs()

// Before starting "runc init", mark all non-stdio open files as O_CLOEXEC
// to make sure we don't leak any files into "runc init". Any files to be
// passed to "runc init" through ExtraFiles will get dup2'd by the Go
Expand All @@ -349,21 +347,24 @@ func (c *Container) start(process *Process) (retErr error) {
if err := utils.CloseExecFrom(3); err != nil {
return fmt.Errorf("unable to mark non-stdio fds as cloexec: %w", err)
}
if err := parent.start(); err != nil {
return fmt.Errorf("unable to start container process: %w", err)
}

if logsDone != nil {
if logsDone := parent.forwardChildLogs(); logsDone != nil {
defer func() {
// Wait for log forwarder to finish. This depends on
// runc init closing the _LIBCONTAINER_LOGPIPE log fd.
err := <-logsDone
if err != nil && retErr == nil {
retErr = fmt.Errorf("unable to forward init logs: %w", err)
// An error from child logs is nsexec FATAL messages.
// Always append those to the original error.
if err != nil {
retErr = fmt.Errorf("%w; runc init error(s): %w ", retErr, err)
}
}()
}

if err := parent.start(); err != nil {
return fmt.Errorf("unable to start container process: %w", err)
}

if process.Init {
c.fifo.Close()
if c.config.HasHook(configs.Poststart) {
Expand Down
35 changes: 27 additions & 8 deletions libcontainer/logs/logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,16 @@ package logs

import (
"bufio"
"bytes"
"encoding/json"
"errors"
"io"

"github.com/sirupsen/logrus"
)

var fatalsSep = []byte("; ")

func ForwardLogs(logPipe io.ReadCloser) chan error {
done := make(chan error, 1)
s := bufio.NewScanner(logPipe)
Expand All @@ -25,24 +29,33 @@ func ForwardLogs(logPipe io.ReadCloser) chan error {
}

go func() {
fatals := []byte{}
for s.Scan() {
processEntry(s.Bytes(), logger)
fatals = processEntry(s.Bytes(), logger, fatals)
}
if err := s.Err(); err != nil {
logrus.Errorf("error reading from log source: %v", err)
}
if err := logPipe.Close(); err != nil {
logrus.Errorf("error closing log source: %v", err)
}
// The only error we want to return is when reading from
// logPipe has failed.
done <- s.Err()
// The only error we return is fatal messages from runc init.
var err error
if len(fatals) > 0 {
err = errors.New(string(bytes.TrimSuffix(fatals, fatalsSep)))
}
done <- err
close(done)
}()

return done
}

func processEntry(text []byte, logger *logrus.Logger) {
// processEntry parses the error and either logs it via the standard logger or,
// if this is a fatal error, appends its text to fatals.
func processEntry(text []byte, logger *logrus.Logger, fatals []byte) []byte {
if len(text) == 0 {
return
return fatals
}

var jl struct {
Expand All @@ -51,8 +64,14 @@ func processEntry(text []byte, logger *logrus.Logger) {
}
if err := json.Unmarshal(text, &jl); err != nil {
logrus.Errorf("failed to decode %q to json: %v", text, err)
return
return fatals
}

logger.Log(jl.Level, jl.Msg)
if jl.Level == logrus.FatalLevel {
fatals = append(fatals, jl.Msg...)
fatals = append(fatals, fatalsSep...)
} else {
logger.Log(jl.Level, jl.Msg)
}
return fatals
}
10 changes: 10 additions & 0 deletions libcontainer/nsenter/log.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,15 @@ extern int logfd;
exit(1); \
} while(0)

/* bailx is the same as bail, except it does not add ": %m" (errno). */
#define bailx(fmt, ...) \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice!

I think the first three commits are bug fixes — could you please move them to a separate PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see #4951

do { \
if (logfd < 0) \
fprintf(stderr, "FATAL: " fmt "\n", ##__VA_ARGS__); \
else \
write_log(FATAL, fmt, ##__VA_ARGS__); \
exit(1); \
} while(0)


#endif /* NSENTER_LOG_H */
Loading