Address comments in PR 1861

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>
This commit is contained in:
Danail Branekov 2019-04-04 14:57:28 +03:00
parent feebfac358
commit c486e3c406
11 changed files with 381 additions and 134 deletions

23
init.go
View File

@ -1,14 +1,14 @@
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() {
@ -16,17 +16,14 @@ func 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()")
}
}

View File

@ -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"
@ -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 {
@ -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
@ -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) {
@ -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 {
@ -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),
@ -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 {
@ -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,

View File

@ -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",

View File

@ -1,70 +0,0 @@
package libcontainer
import (
"encoding/json"
"fmt"
"io"
"os"
"github.com/sirupsen/logrus"
)
func forwardLogs(p *os.File) {
defer p.Close()
type jsonLog struct {
Level string `json:"level"`
Msg string `json:"msg"`
}
dec := json.NewDecoder(p)
for {
var jl jsonLog
type logFuncTable map[logrus.Level]func(args ...interface{})
logMapping := logFuncTable{
logrus.PanicLevel: logrus.Panic,
logrus.FatalLevel: logrus.Fatal,
logrus.ErrorLevel: logrus.Error,
logrus.WarnLevel: logrus.Warn,
logrus.InfoLevel: logrus.Info,
logrus.DebugLevel: logrus.Debug,
}
err := dec.Decode(&jl)
if err != nil {
if err == io.EOF {
logrus.Debug("child pipe closed")
return
}
logrus.Errorf("json logs decoding error: %+v", err)
return
}
lvl, err := logrus.ParseLevel(jl.Level)
if err != nil {
fmt.Printf("parsing error\n")
}
if logMapping[lvl] != nil {
logMapping[lvl](jl.Msg)
}
}
}
func rawLogs(p *os.File) {
defer p.Close()
data := make([]byte, 128)
for {
_, err := p.Read(data)
if err != nil {
if err == io.EOF {
logrus.Debug("child pipe closed")
return
}
return
}
fmt.Printf("Read data: %s\n", string(data))
}
}

150
libcontainer/logs/logs.go Normal file
View File

@ -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
}

View File

@ -0,0 +1,158 @@
package logs
import (
"errors"
"io/ioutil"
"os"
"strings"
"testing"
"time"
)
func TestLoggingToFile(t *testing.T) {
logW, logFile, _ := runLogForwarding(t)
defer os.Remove(logFile)
defer logW.Close()
logToLogWriter(t, logW, `{"level": "info","msg":"kitten"}`)
logFileContent := waitForLogContent(t, logFile)
if !strings.Contains(string(logFileContent), "kitten") {
t.Fatalf("%s does not contain kitten", string(logFileContent))
}
}
func TestLogForwardingDoesNotStopOnJsonDecodeErr(t *testing.T) {
logW, logFile, _ := runLogForwarding(t)
defer os.Remove(logFile)
defer logW.Close()
logToLogWriter(t, logW, "invalid-json-with-kitten")
logFileContent := waitForLogContent(t, logFile)
if !strings.Contains(string(logFileContent), "failed to decode") {
t.Fatalf("%q does not contain decoding error", string(logFileContent))
}
truncateLogFile(t, logFile)
logToLogWriter(t, logW, `{"level": "info","msg":"puppy"}`)
logFileContent = waitForLogContent(t, logFile)
if !strings.Contains(string(logFileContent), "puppy") {
t.Fatalf("%s does not contain puppy", string(logFileContent))
}
}
func TestLogForwardingDoesNotStopOnLogLevelParsingErr(t *testing.T) {
logW, logFile, _ := runLogForwarding(t)
defer os.Remove(logFile)
defer logW.Close()
logToLogWriter(t, logW, `{"level": "alert","msg":"puppy"}`)
logFileContent := waitForLogContent(t, logFile)
if !strings.Contains(string(logFileContent), "failed to parse log level") {
t.Fatalf("%q does not contain log level parsing error", string(logFileContent))
}
truncateLogFile(t, logFile)
logToLogWriter(t, logW, `{"level": "info","msg":"puppy"}`)
logFileContent = waitForLogContent(t, logFile)
if !strings.Contains(string(logFileContent), "puppy") {
t.Fatalf("%s does not contain puppy", string(logFileContent))
}
}
func TestLogForwardingStopsAfterClosingTheWriter(t *testing.T) {
logW, logFile, doneForwarding := runLogForwarding(t)
defer os.Remove(logFile)
logToLogWriter(t, logW, `{"level": "info","msg":"sync"}`)
logFileContent := waitForLogContent(t, logFile)
if !strings.Contains(string(logFileContent), "sync") {
t.Fatalf("%q does not contain sync message", string(logFileContent))
}
logW.Close()
select {
case <-doneForwarding:
case <-time.After(10 * time.Second):
t.Fatal("log forwarding did not stop after closing the pipe")
}
}
func logToLogWriter(t *testing.T, logW *os.File, message string) {
_, err := logW.Write([]byte(message + "\n"))
if err != nil {
t.Fatalf("failed to write %q to log writer: %v", message, err)
}
}
func runLogForwarding(t *testing.T) (*os.File, string, chan struct{}) {
logR, logW, err := os.Pipe()
if err != nil {
t.Fatal(err)
}
tempFile, err := ioutil.TempFile("", "")
if err != nil {
t.Fatal(err)
}
logFile := tempFile.Name()
logConfig := &LoggingConfiguration{LogFormat: "json", LogFilePath: logFile}
return logW, logFile, startLogForwarding(t, logConfig, logR)
}
func startLogForwarding(t *testing.T, logConfig *LoggingConfiguration, logR *os.File) chan struct{} {
loggingConfigured = false
if err := ConfigureLogging(logConfig); err != nil {
t.Fatal(err)
}
doneForwarding := make(chan struct{})
go func() {
ForwardLogs(logR)
close(doneForwarding)
}()
return doneForwarding
}
func waitForLogContent(t *testing.T, logFile string) string {
startTime := time.Now()
for {
if time.Now().After(startTime.Add(10 * time.Second)) {
t.Fatal(errors.New("No content in log file after 10 seconds"))
break
}
fileContent, err := ioutil.ReadFile(logFile)
if err != nil {
t.Fatal(err)
}
if len(fileContent) == 0 {
continue
}
return string(fileContent)
}
return ""
}
func truncateLogFile(t *testing.T, logFile string) {
file, err := os.OpenFile(logFile, os.O_RDWR, 0666)
if err != nil {
t.Fatalf("failed to open log file: %v", err)
return
}
defer file.Close()
err = file.Truncate(0)
if err != nil {
t.Fatalf("failed to truncate log file: %v", err)
}
}

View File

@ -167,10 +167,15 @@ func TestNsenterIncorrectPathType(t *testing.T) {
func TestNsenterChildLogging(t *testing.T) {
args := []string{"nsenter-exec"}
parent, child, err := newPipe()
if err != nil {
t.Fatalf("failed to create exec pipe %v", err)
}
logread, logwrite, err := os.Pipe()
if err != nil {
t.Fatalf("failed to create pipe %v", err)
t.Fatalf("failed to create log pipe %v", err)
}
defer logread.Close()
defer logwrite.Close()
namespaces := []string{
// join pid ns of the current process

View File

@ -568,7 +568,7 @@ void write_log(loglevel_t level, const char *format, ...)
{
static const char *strlevel[] = {"panic", "fatal", "error", "warning", "info", "debug"};
static char jsonbuffer[1024];
int len;
int len, written;
va_list args;
if (logfd < 0 || level >= sizeof(strlevel) / sizeof(strlevel[0])) {
return;
@ -576,12 +576,17 @@ void write_log(loglevel_t level, const char *format, ...)
len = snprintf(jsonbuffer, sizeof(jsonbuffer),
"{\"level\":\"%s\", \"msg\": \"", strlevel[level]);
if (len < 0) return;
va_start(args, format);
len += vsnprintf(&jsonbuffer[len], sizeof(jsonbuffer) - len, format, args);
written = vsnprintf(&jsonbuffer[len], sizeof(jsonbuffer) - len, format, args);
if (written < 0) return;
len += written;
va_end(args);
len += snprintf(&jsonbuffer[len], sizeof(jsonbuffer) - len, "\"}");
written = snprintf(&jsonbuffer[len], sizeof(jsonbuffer) - len, "\"}\n");
if (written < 0) return;
len += written;
write(logfd, jsonbuffer, len);
}

View File

@ -13,6 +13,8 @@ import (
"strconv"
"syscall" // only for Signal
"github.com/opencontainers/runc/libcontainer/logs"
"github.com/opencontainers/runc/libcontainer/cgroups"
"github.com/opencontainers/runc/libcontainer/configs"
"github.com/opencontainers/runc/libcontainer/intelrdt"
@ -48,10 +50,10 @@ type parentProcess interface {
setExternalDescriptors(fds []string)
getChildLogs()
forwardChildLogs()
}
type pipePair struct {
type readWritePair struct {
r *os.File
w *os.File
}
@ -60,7 +62,7 @@ type setnsProcess struct {
cmd *exec.Cmd
parentPipe *os.File
childPipe *os.File
logPipe pipePair
logPipe readWritePair
cgroupPaths map[string]string
rootlessCgroups bool
intelRdtPath string
@ -215,15 +217,15 @@ func (p *setnsProcess) setExternalDescriptors(newFds []string) {
p.fds = newFds
}
func (p *setnsProcess) getChildLogs() {
go forwardLogs(p.logPipe.r)
func (p *setnsProcess) forwardChildLogs() {
go logs.ForwardLogs(p.logPipe.r)
}
type initProcess struct {
cmd *exec.Cmd
parentPipe *os.File
childPipe *os.File
logPipe pipePair
logPipe readWritePair
config *initConfig
manager cgroups.Manager
intelRdtManager intelrdt.Manager
@ -525,8 +527,8 @@ func (p *initProcess) setExternalDescriptors(newFds []string) {
p.fds = newFds
}
func (p *initProcess) getChildLogs() {
go forwardLogs(p.logPipe.r)
func (p *initProcess) forwardChildLogs() {
go logs.ForwardLogs(p.logPipe.r)
}
func getPipeFds(pid int) ([]string, error) {

View File

@ -76,7 +76,7 @@ func (p *restoredProcess) setExternalDescriptors(newFds []string) {
p.fds = newFds
}
func (p *restoredProcess) getChildLogs() {
func (p *restoredProcess) forwardChildLogs() {
}
// nonChildProcess represents a process where the calling process is not
@ -124,5 +124,5 @@ func (p *nonChildProcess) setExternalDescriptors(newFds []string) {
p.fds = newFds
}
func (p *nonChildProcess) getChildLogs() {
func (p *nonChildProcess) forwardChildLogs() {
}

42
main.go
View File

@ -2,6 +2,7 @@ package main
import (
"fmt"
"github.com/opencontainers/runc/libcontainer/logs"
"io"
"os"
"strings"
@ -133,32 +134,13 @@ func main() {
updateCommand,
}
app.Before = func(context *cli.Context) error {
// do nothing if logrus was already initialized in init.go
if logrus.StandardLogger().Out != logrus.New().Out {
return nil
}
if context.GlobalBool("debug") {
logrus.SetLevel(logrus.DebugLevel)
}
if path := context.GlobalString("log"); path != "" {
f, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_APPEND|os.O_SYNC, 0666)
loggingConfig, err := createLoggingConfiguration(context)
if err != nil {
return err
return fmt.Errorf("failed to create logging configuration: %v", err)
}
logrus.SetOutput(f)
}
switch context.GlobalString("log-format") {
case "text":
// retain logrus's default.
case "json":
logrus.SetFormatter(new(logrus.JSONFormatter))
default:
return fmt.Errorf("unknown log-format %q", context.GlobalString("log-format"))
}
return nil
return logs.ConfigureLogging(loggingConfig)
}
// If the command returns an error, cli takes upon itself to print
// the error on cli.ErrWriter and exit.
// Use our own writer here to ensure the log gets sent to the right location.
@ -176,3 +158,17 @@ func (f *FatalWriter) Write(p []byte) (n int, err error) {
logrus.Error(string(p))
return f.cliErrWriter.Write(p)
}
func createLoggingConfiguration(context *cli.Context) (*logs.LoggingConfiguration, error) {
config := logs.LoggingConfiguration{
IsDebug: context.GlobalBool("debug"),
LogFilePath: context.GlobalString("log"),
LogFormat: context.GlobalString("log-format"),
}
if envLogPipe, ok := os.LookupEnv("_LIBCONTAINER_LOGPIPE"); ok {
config.LogPipeFd = envLogPipe
}
return &config, nil
}