Skip to content

Commit

Permalink
Address comments in PR 1861
Browse files Browse the repository at this point in the history
Refactor configuring logging into a reusable component
so that it can be nicely used in both main() and init process init()

Co-authored-by: Georgi Sabev <georgethebeatle@gmail.com>
Co-authored-by: Giuseppe Capizzi <gcapizzi@pivotal.io>
Co-authored-by: Claudia Beresford <cberesford@pivotal.io>

Signed-off-by: Danail Branekov <danailster@gmail.com>
  • Loading branch information
danail-branekov committed Apr 4, 2019
1 parent feebfac commit fa7a3e6
Show file tree
Hide file tree
Showing 11 changed files with 381 additions and 134 deletions.
23 changes: 10 additions & 13 deletions init.go
@@ -1,32 +1,29 @@
package main

import (
"os"
"runtime"
"strconv"

"fmt"
"github.com/opencontainers/runc/libcontainer"
"github.com/opencontainers/runc/libcontainer/logs"
_ "github.com/opencontainers/runc/libcontainer/nsenter"
"github.com/sirupsen/logrus"
"github.com/urfave/cli"
"os"
"runtime"
)

func init() {
if len(os.Args) > 1 && os.Args[1] == "init" {
runtime.GOMAXPROCS(1)
runtime.LockOSThread()

// in child process, we need to retrieve the log pipe
envLogPipe := os.Getenv("_LIBCONTAINER_LOGPIPE")
logPipeFd, err := strconv.Atoi(envLogPipe)

err := logs.ConfigureLogging(&logs.LoggingConfiguration{
LogPipeFd: os.Getenv("_LIBCONTAINER_LOGPIPE"),
LogFormat: "json",
IsDebug: true,
})
if err != nil {
return
panic(fmt.Sprintf("libcontainer: failed to configure logging: %v", err))
}
logPipe := os.NewFile(uintptr(logPipeFd), "logpipe")
logrus.SetOutput(logPipe)
logrus.SetFormatter(&logrus.JSONFormatter{})
logrus.SetLevel(logrus.DebugLevel)
logrus.Debug("child process in init()")
}
}
Expand Down
27 changes: 14 additions & 13 deletions libcontainer/container_linux.go
Expand Up @@ -19,7 +19,7 @@ import (
"syscall" // only for SysProcAttr and Signal
"time"

"github.com/cyphar/filepath-securejoin"
securejoin "github.com/cyphar/filepath-securejoin"
"github.com/opencontainers/runc/libcontainer/cgroups"
"github.com/opencontainers/runc/libcontainer/configs"
"github.com/opencontainers/runc/libcontainer/intelrdt"
Expand Down Expand Up @@ -337,7 +337,7 @@ func (c *linuxContainer) start(process *Process) error {
if err != nil {
return newSystemErrorWithCause(err, "creating new parent process")
}
parent.getChildLogs()
parent.forwardChildLogs()
if err := parent.start(); err != nil {
// terminate the process to ensure that it properly is reaped.
if err := ignoreTerminateErrors(parent.terminate()); err != nil {
Expand Down Expand Up @@ -443,19 +443,20 @@ func (c *linuxContainer) newParentProcess(p *Process) (parentProcess, error) {
if err != nil {
return nil, newSystemErrorWithCause(err, "creating new init pipe")
}
messagePipe := readWritePair{parentPipe, childPipe}

r, w, err := os.Pipe()
if err != nil {
return nil, fmt.Errorf("Unable to create the log pipe: %s", err)
}
logPipe := pipePair{r, w}
logPipe := readWritePair{r, w}

cmd, err := c.commandTemplate(p, childPipe, logPipe.w)
if err != nil {
return nil, newSystemErrorWithCause(err, "creating new command template")
}
if !p.Init {
return c.newSetnsProcess(p, cmd, parentPipe, childPipe, &logPipe)
return c.newSetnsProcess(p, cmd, messagePipe, logPipe)
}

// We only set up fifoFd if we're not doing a `runc exec`. The historic
Expand All @@ -466,7 +467,7 @@ func (c *linuxContainer) newParentProcess(p *Process) (parentProcess, error) {
if err := c.includeExecFifo(cmd); err != nil {
return nil, newSystemErrorWithCause(err, "including execfifo in cmd.Exec setup")
}
return c.newInitProcess(p, cmd, parentPipe, childPipe, &logPipe)
return c.newInitProcess(p, cmd, messagePipe, logPipe)
}

func (c *linuxContainer) commandTemplate(p *Process, childPipe *os.File, logPipe *os.File) (*exec.Cmd, error) {
Expand Down Expand Up @@ -507,7 +508,7 @@ func (c *linuxContainer) commandTemplate(p *Process, childPipe *os.File, logPipe
return cmd, nil
}

func (c *linuxContainer) newInitProcess(p *Process, cmd *exec.Cmd, parentPipe, childPipe *os.File, logPipe *pipePair) (*initProcess, error) {
func (c *linuxContainer) newInitProcess(p *Process, cmd *exec.Cmd, initPipe, logPipe readWritePair) (*initProcess, error) {
cmd.Env = append(cmd.Env, "_LIBCONTAINER_INITTYPE="+string(initStandard))
nsMaps := make(map[configs.NamespaceType]string)
for _, ns := range c.config.Namespaces {
Expand All @@ -522,9 +523,9 @@ func (c *linuxContainer) newInitProcess(p *Process, cmd *exec.Cmd, parentPipe, c
}
init := &initProcess{
cmd: cmd,
childPipe: childPipe,
parentPipe: parentPipe,
logPipe: *logPipe,
childPipe: initPipe.w,
parentPipe: initPipe.r,
logPipe: logPipe,
manager: c.cgroupManager,
intelRdtManager: c.intelRdtManager,
config: c.newInitConfig(p),
Expand All @@ -537,7 +538,7 @@ func (c *linuxContainer) newInitProcess(p *Process, cmd *exec.Cmd, parentPipe, c
return init, nil
}

func (c *linuxContainer) newSetnsProcess(p *Process, cmd *exec.Cmd, parentPipe, childPipe *os.File, logPipe *pipePair) (*setnsProcess, error) {
func (c *linuxContainer) newSetnsProcess(p *Process, cmd *exec.Cmd, nsPipe, logPipe readWritePair) (*setnsProcess, error) {
cmd.Env = append(cmd.Env, "_LIBCONTAINER_INITTYPE="+string(initSetns))
state, err := c.currentState()
if err != nil {
Expand All @@ -554,9 +555,9 @@ func (c *linuxContainer) newSetnsProcess(p *Process, cmd *exec.Cmd, parentPipe,
cgroupPaths: c.cgroupManager.GetPaths(),
rootlessCgroups: c.config.RootlessCgroups,
intelRdtPath: state.IntelRdtPath,
childPipe: childPipe,
parentPipe: parentPipe,
logPipe: *logPipe,
childPipe: nsPipe.w,
parentPipe: nsPipe.r,
logPipe: logPipe,
config: c.newInitConfig(p),
process: p,
bootstrapData: data,
Expand Down
3 changes: 3 additions & 0 deletions libcontainer/container_linux_test.go
Expand Up @@ -114,6 +114,9 @@ func (m *mockProcess) externalDescriptors() []string {
func (m *mockProcess) setExternalDescriptors(newFds []string) {
}

func (m *mockProcess) forwardChildLogs() {
}

func TestGetContainerPids(t *testing.T) {
container := &linuxContainer{
id: "myid",
Expand Down
70 changes: 0 additions & 70 deletions libcontainer/logs.go

This file was deleted.

150 changes: 150 additions & 0 deletions libcontainer/logs/logs.go
@@ -0,0 +1,150 @@
package logs

import (
"bufio"
"encoding/json"
"fmt"
"io"
"os"
"strconv"

"github.com/sirupsen/logrus"
)

// loggingConfigured will be set once logging has been configured via invoking `ConfigureLogging`.
// Subsequent invocations of `ConfigureLogging` would be no-op
var loggingConfigured = false

type LoggingConfiguration struct {
IsDebug bool
LogFormat string
LogFilePath string
LogPipeFd string
}

func ForwardLogs(logPipe io.Reader) {
type jsonLog struct {
Level string `json:"level"`
Msg string `json:"msg"`
}

lineReader := bufio.NewReader(logPipe)
for {
line, err := lineReader.ReadBytes('\n')
if len(line) > 0 {
processEntry(line)
}
if err == io.EOF {
logrus.Debugf("log pipe has been closed: %+v", err)
return
}
if err != nil {
logrus.Errorf("log pipe read error: %+v", err)
}
}
}

func processEntry(text []byte) {
type jsonLog struct {
Level string `json:"level"`
Msg string `json:"msg"`
}

var jl jsonLog
if err := json.Unmarshal(text, &jl); err != nil {
logrus.Errorf("failed to decode %q to json: %+v", text, err)
return
}

lvl, err := logrus.ParseLevel(jl.Level)
if err != nil {
logrus.Errorf("failed to parse log level %q: %v\n", jl.Level, err)
return
}
log(lvl, jl.Msg)
}

func log(level logrus.Level, args ...interface{}) {
switch level {
case logrus.PanicLevel:
logrus.Panic(args...)
case logrus.FatalLevel:
logrus.Fatal(args...)
case logrus.ErrorLevel:
logrus.Error(args...)
case logrus.WarnLevel:
logrus.Warn(args...)
case logrus.InfoLevel:
logrus.Info(args...)
case logrus.DebugLevel:
logrus.Debug(args...)
default:
logrus.Warnf("Unsupported log level %v while trying to log '%#v'", level, args)
}
}

func ConfigureLogging(loggingConfig *LoggingConfiguration) error {
if loggingConfigured {
logrus.Debug("logging has been already configured")
return nil
}

configureLogLevel(loggingConfig)
if err := configureLogOutput(loggingConfig); err != nil {
return err
}
if err := configureLogFormat(loggingConfig); err != nil {
return err
}

loggingConfigured = true
return nil
}

func configureLogLevel(loggingConfig *LoggingConfiguration) {
if loggingConfig.IsDebug {
logrus.SetLevel(logrus.DebugLevel)
}
}

func configureLogOutput(loggingConfig *LoggingConfiguration) error {
if loggingConfig.LogFilePath != "" {
return configureLogFileOutput(loggingConfig.LogFilePath)
}

if loggingConfig.LogPipeFd != "" {
logPipeFdInt, err := strconv.Atoi(loggingConfig.LogPipeFd)
if err != nil {
return fmt.Errorf("failed to convert _LIBCONTAINER_LOGPIPE environment variable value %q to int: %v", loggingConfig.LogPipeFd, err)
}
configureLogPipeOutput(logPipeFdInt)
return nil
}

return nil
}

func configureLogPipeOutput(logPipeFd int) {
logrus.SetOutput(os.NewFile(uintptr(logPipeFd), "logpipe"))
}

func configureLogFileOutput(logFilePath string) error {
f, err := os.OpenFile(logFilePath, os.O_CREATE|os.O_WRONLY|os.O_APPEND|os.O_SYNC, 0644)
if err != nil {
return err
}
logrus.SetOutput(f)
return nil
}

func configureLogFormat(loggingConfig *LoggingConfiguration) error {
switch loggingConfig.LogFormat {
case "text":
// retain logrus's default.
case "json":
logrus.SetFormatter(new(logrus.JSONFormatter))
default:
return fmt.Errorf("unknown log-format %q", loggingConfig.LogFormat)
}
return nil
}

0 comments on commit fa7a3e6

Please sign in to comment.