Skip to content

Commit

Permalink
init: wait for log forwarder to finish
Browse files Browse the repository at this point in the history
Since the extended logging was added to runc, some test cases from
debug.bats, notably those that write to a log file, started failing
sometimes in our CI, not showing the complete logs as expected, but
only the first few lines.

Presumably this happens because the binary exits before logrus forwarder
has a chance to finish forwarding/writing the logs to the file. To fix,
add a channel to signal that ForwardLogs has finished, and use it from
Finish().

Signed-off-by: Kir Kolyshkin <[email protected]>
  • Loading branch information
kolyshkin committed Aug 9, 2020
1 parent 0a429ce commit f9b4cf0
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 1 deletion.
20 changes: 19 additions & 1 deletion libcontainer/logs/logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ var (
// loggingConfigured will be set once logging has been configured via invoking `ConfigureLogging`.
// Subsequent invocations of `ConfigureLogging` would be no-op
loggingConfigured = false

inputPipe io.ReadCloser
forwardDone chan struct{}
)

type Config struct {
Expand All @@ -25,7 +28,12 @@ type Config struct {
LogPipeFd int
}

func ForwardLogs(logPipe io.Reader) {
func ForwardLogs(logPipe io.ReadCloser) {
configureMutex.Lock()
forwardDone = make(chan struct{})
inputPipe = logPipe
configureMutex.Unlock()

s := bufio.NewScanner(logPipe)
for s.Scan() {
processEntry(s.Bytes())
Expand All @@ -35,6 +43,7 @@ func ForwardLogs(logPipe io.Reader) {
} else {
logrus.Debugf("log pipe closed")
}
close(forwardDone)
}

func processEntry(text []byte) {
Expand All @@ -61,6 +70,15 @@ func processEntry(text []byte) {
logrus.StandardLogger().Logf(lvl, jl.Msg)
}

func Finish() {
configureMutex.Lock()
defer configureMutex.Unlock()
if inputPipe != nil {
inputPipe.Close()
<-forwardDone
}
}

func ConfigureLogging(config Config) error {
configureMutex.Lock()
defer configureMutex.Unlock()
Expand Down
2 changes: 2 additions & 0 deletions libcontainer/setns_init_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"github.com/opencontainers/runc/libcontainer/apparmor"
"github.com/opencontainers/runc/libcontainer/keys"
"github.com/opencontainers/runc/libcontainer/logs"
"github.com/opencontainers/runc/libcontainer/seccomp"
"github.com/opencontainers/runc/libcontainer/system"
"github.com/opencontainers/selinux/go-selinux"
Expand Down Expand Up @@ -87,5 +88,6 @@ func (l *linuxSetnsInit) Init() error {
return newSystemErrorWithCause(err, "init seccomp")
}
}
logs.Finish()
return system.Execv(l.config.Args[0], l.config.Args[0:], os.Environ())
}
5 changes: 5 additions & 0 deletions libcontainer/standard_init_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@ import (
"github.com/opencontainers/runc/libcontainer/apparmor"
"github.com/opencontainers/runc/libcontainer/configs"
"github.com/opencontainers/runc/libcontainer/keys"
"github.com/opencontainers/runc/libcontainer/logs"
"github.com/opencontainers/runc/libcontainer/seccomp"
"github.com/opencontainers/runc/libcontainer/system"
"github.com/opencontainers/runtime-spec/specs-go"
"github.com/opencontainers/selinux/go-selinux"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"golang.org/x/sys/unix"
)

Expand Down Expand Up @@ -180,7 +182,10 @@ func (l *linuxStandardInit) Init() error {
return err
}
// Close the pipe to signal that we have completed our init.
logrus.Debugf("init: closing the pipe to signal completion")
logs.Finish() // no more logrus below
l.pipe.Close()

// Wait for the FIFO to be opened on the other side before exec-ing the
// user process. We open it through /proc/self/fd/$fd, because the fd that
// was given to us was an O_PATH fd to the fifo itself. Linux allows us to
Expand Down

0 comments on commit f9b4cf0

Please sign in to comment.