Skip to content

Commit a146081

Browse files
georgethebeatleyulianedyalkova
georgethebeatle
and
yulianedyalkova
committed
Write logs to stderr by default
Minor refactoring to use the filePair struct for both init sock and log pipe Co-authored-by: Julia Nedialkova <[email protected]> Signed-off-by: Georgi Sabev <[email protected]>
1 parent 68b4ff5 commit a146081

File tree

7 files changed

+56
-55
lines changed

7 files changed

+56
-55
lines changed

‎libcontainer/container_linux.go

+16-18
Original file line numberDiff line numberDiff line change
@@ -439,24 +439,24 @@ func (c *linuxContainer) includeExecFifo(cmd *exec.Cmd) error {
439439
}
440440

441441
func (c *linuxContainer) newParentProcess(p *Process) (parentProcess, error) {
442-
parentPipe, childPipe, err := utils.NewSockPair("init")
442+
parentInitPipe, childInitPipe, err := utils.NewSockPair("init")
443443
if err != nil {
444444
return nil, newSystemErrorWithCause(err, "creating new init pipe")
445445
}
446-
messagePipe := readWritePair{parentPipe, childPipe}
446+
messageSockPair := filePair{parentInitPipe, childInitPipe}
447447

448-
r, w, err := os.Pipe()
448+
parentLogPipe, childLogPipe, err := os.Pipe()
449449
if err != nil {
450450
return nil, fmt.Errorf("Unable to create the log pipe: %s", err)
451451
}
452-
logPipe := readWritePair{r, w}
452+
logFilePair := filePair{parentLogPipe, childLogPipe}
453453

454-
cmd, err := c.commandTemplate(p, childPipe, logPipe.w)
454+
cmd, err := c.commandTemplate(p, childInitPipe, childLogPipe)
455455
if err != nil {
456456
return nil, newSystemErrorWithCause(err, "creating new command template")
457457
}
458458
if !p.Init {
459-
return c.newSetnsProcess(p, cmd, messagePipe, logPipe)
459+
return c.newSetnsProcess(p, cmd, messageSockPair, logFilePair)
460460
}
461461

462462
// We only set up fifoFd if we're not doing a `runc exec`. The historic
@@ -467,10 +467,10 @@ func (c *linuxContainer) newParentProcess(p *Process) (parentProcess, error) {
467467
if err := c.includeExecFifo(cmd); err != nil {
468468
return nil, newSystemErrorWithCause(err, "including execfifo in cmd.Exec setup")
469469
}
470-
return c.newInitProcess(p, cmd, messagePipe, logPipe)
470+
return c.newInitProcess(p, cmd, messageSockPair, logFilePair)
471471
}
472472

473-
func (c *linuxContainer) commandTemplate(p *Process, childPipe *os.File, logPipe *os.File) (*exec.Cmd, error) {
473+
func (c *linuxContainer) commandTemplate(p *Process, childInitPipe *os.File, childLogPipe *os.File) (*exec.Cmd, error) {
474474
cmd := exec.Command(c.initPath, c.initArgs[1:]...)
475475
cmd.Args[0] = c.initArgs[0]
476476
cmd.Stdin = p.Stdin
@@ -488,13 +488,13 @@ func (c *linuxContainer) commandTemplate(p *Process, childPipe *os.File, logPipe
488488
fmt.Sprintf("_LIBCONTAINER_CONSOLE=%d", stdioFdCount+len(cmd.ExtraFiles)-1),
489489
)
490490
}
491-
cmd.ExtraFiles = append(cmd.ExtraFiles, childPipe)
491+
cmd.ExtraFiles = append(cmd.ExtraFiles, childInitPipe)
492492
cmd.Env = append(cmd.Env,
493493
fmt.Sprintf("_LIBCONTAINER_INITPIPE=%d", stdioFdCount+len(cmd.ExtraFiles)-1),
494494
fmt.Sprintf("_LIBCONTAINER_STATEDIR=%s", c.root),
495495
)
496496

497-
cmd.ExtraFiles = append(cmd.ExtraFiles, logPipe)
497+
cmd.ExtraFiles = append(cmd.ExtraFiles, childLogPipe)
498498
cmd.Env = append(cmd.Env,
499499
fmt.Sprintf("_LIBCONTAINER_LOGPIPE=%d", stdioFdCount+len(cmd.ExtraFiles)-1),
500500
fmt.Sprintf("_LIBCONTAINER_LOGLEVEL=%s", p.LogLevel),
@@ -509,7 +509,7 @@ func (c *linuxContainer) commandTemplate(p *Process, childPipe *os.File, logPipe
509509
return cmd, nil
510510
}
511511

512-
func (c *linuxContainer) newInitProcess(p *Process, cmd *exec.Cmd, initPipe, logPipe readWritePair) (*initProcess, error) {
512+
func (c *linuxContainer) newInitProcess(p *Process, cmd *exec.Cmd, messageSockPair, logFilePair filePair) (*initProcess, error) {
513513
cmd.Env = append(cmd.Env, "_LIBCONTAINER_INITTYPE="+string(initStandard))
514514
nsMaps := make(map[configs.NamespaceType]string)
515515
for _, ns := range c.config.Namespaces {
@@ -524,9 +524,8 @@ func (c *linuxContainer) newInitProcess(p *Process, cmd *exec.Cmd, initPipe, log
524524
}
525525
init := &initProcess{
526526
cmd: cmd,
527-
childPipe: initPipe.w,
528-
parentPipe: initPipe.r,
529-
logPipe: logPipe,
527+
messageSockPair: messageSockPair,
528+
logFilePair: logFilePair,
530529
manager: c.cgroupManager,
531530
intelRdtManager: c.intelRdtManager,
532531
config: c.newInitConfig(p),
@@ -539,7 +538,7 @@ func (c *linuxContainer) newInitProcess(p *Process, cmd *exec.Cmd, initPipe, log
539538
return init, nil
540539
}
541540

542-
func (c *linuxContainer) newSetnsProcess(p *Process, cmd *exec.Cmd, nsPipe, logPipe readWritePair) (*setnsProcess, error) {
541+
func (c *linuxContainer) newSetnsProcess(p *Process, cmd *exec.Cmd, messageSockPair, logFilePair filePair) (*setnsProcess, error) {
543542
cmd.Env = append(cmd.Env, "_LIBCONTAINER_INITTYPE="+string(initSetns))
544543
state, err := c.currentState()
545544
if err != nil {
@@ -556,9 +555,8 @@ func (c *linuxContainer) newSetnsProcess(p *Process, cmd *exec.Cmd, nsPipe, logP
556555
cgroupPaths: c.cgroupManager.GetPaths(),
557556
rootlessCgroups: c.config.RootlessCgroups,
558557
intelRdtPath: state.IntelRdtPath,
559-
childPipe: nsPipe.w,
560-
parentPipe: nsPipe.r,
561-
logPipe: logPipe,
558+
messageSockPair: messageSockPair,
559+
logFilePair: logFilePair,
562560
config: c.newInitConfig(p),
563561
process: p,
564562
bootstrapData: data,

‎libcontainer/logs/logs.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import (
1313
)
1414

1515
var (
16-
mutex = &sync.Mutex{}
16+
configureMutex = sync.Mutex{}
1717
// loggingConfigured will be set once logging has been configured via invoking `ConfigureLogging`.
1818
// Subsequent invocations of `ConfigureLogging` would be no-op
1919
loggingConfigured = false
@@ -64,8 +64,8 @@ func processEntry(text []byte) {
6464
}
6565

6666
func ConfigureLogging(config Config) error {
67-
mutex.Lock()
68-
defer mutex.Unlock()
67+
configureMutex.Lock()
68+
defer configureMutex.Unlock()
6969

7070
if loggingConfigured {
7171
logrus.Debug("logging has already been configured")

‎libcontainer/nsenter/nsenter_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@ type pid struct {
2222
}
2323

2424
type logentry struct {
25-
Msg string
26-
Level string
25+
Msg string `json:"msg"`
26+
Level string `json:"level"`
2727
}
2828

2929
func TestNsenterValidPaths(t *testing.T) {

‎libcontainer/process_linux.go

+28-30
Original file line numberDiff line numberDiff line change
@@ -52,16 +52,15 @@ type parentProcess interface {
5252
forwardChildLogs()
5353
}
5454

55-
type readWritePair struct {
56-
r *os.File
57-
w *os.File
55+
type filePair struct {
56+
parent *os.File
57+
child *os.File
5858
}
5959

6060
type setnsProcess struct {
6161
cmd *exec.Cmd
62-
parentPipe *os.File
63-
childPipe *os.File
64-
logPipe readWritePair
62+
messageSockPair filePair
63+
logFilePair filePair
6564
cgroupPaths map[string]string
6665
rootlessCgroups bool
6766
intelRdtPath string
@@ -85,16 +84,16 @@ func (p *setnsProcess) signal(sig os.Signal) error {
8584
}
8685

8786
func (p *setnsProcess) start() (err error) {
88-
defer p.parentPipe.Close()
87+
defer p.messageSockPair.parent.Close()
8988
err = p.cmd.Start()
9089
// close the write-side of the pipes (controlled by child)
91-
p.childPipe.Close()
92-
p.logPipe.w.Close()
90+
p.messageSockPair.child.Close()
91+
p.logFilePair.child.Close()
9392
if err != nil {
9493
return newSystemErrorWithCause(err, "starting setns process")
9594
}
9695
if p.bootstrapData != nil {
97-
if _, err := io.Copy(p.parentPipe, p.bootstrapData); err != nil {
96+
if _, err := io.Copy(p.messageSockPair.parent, p.bootstrapData); err != nil {
9897
return newSystemErrorWithCause(err, "copying bootstrap data to pipe")
9998
}
10099
}
@@ -120,11 +119,11 @@ func (p *setnsProcess) start() (err error) {
120119
if err := setupRlimits(p.config.Rlimits, p.pid()); err != nil {
121120
return newSystemErrorWithCause(err, "setting rlimits for process")
122121
}
123-
if err := utils.WriteJSON(p.parentPipe, p.config); err != nil {
122+
if err := utils.WriteJSON(p.messageSockPair.parent, p.config); err != nil {
124123
return newSystemErrorWithCause(err, "writing config to pipe")
125124
}
126125

127-
ierr := parseSync(p.parentPipe, func(sync *syncT) error {
126+
ierr := parseSync(p.messageSockPair.parent, func(sync *syncT) error {
128127
switch sync.Type {
129128
case procReady:
130129
// This shouldn't happen.
@@ -137,7 +136,7 @@ func (p *setnsProcess) start() (err error) {
137136
}
138137
})
139138

140-
if err := unix.Shutdown(int(p.parentPipe.Fd()), unix.SHUT_WR); err != nil {
139+
if err := unix.Shutdown(int(p.messageSockPair.parent.Fd()), unix.SHUT_WR); err != nil {
141140
return newSystemErrorWithCause(err, "calling shutdown on init pipe")
142141
}
143142
// Must be done after Shutdown so the child will exit and we can wait for it.
@@ -163,7 +162,7 @@ func (p *setnsProcess) execSetns() error {
163162
return newSystemError(&exec.ExitError{ProcessState: status})
164163
}
165164
var pid *pid
166-
if err := json.NewDecoder(p.parentPipe).Decode(&pid); err != nil {
165+
if err := json.NewDecoder(p.messageSockPair.parent).Decode(&pid); err != nil {
167166
p.cmd.Wait()
168167
return newSystemErrorWithCause(err, "reading pid from init pipe")
169168
}
@@ -217,14 +216,13 @@ func (p *setnsProcess) setExternalDescriptors(newFds []string) {
217216
}
218217

219218
func (p *setnsProcess) forwardChildLogs() {
220-
go logs.ForwardLogs(p.logPipe.r)
219+
go logs.ForwardLogs(p.logFilePair.parent)
221220
}
222221

223222
type initProcess struct {
224223
cmd *exec.Cmd
225-
parentPipe *os.File
226-
childPipe *os.File
227-
logPipe readWritePair
224+
messageSockPair filePair
225+
logFilePair filePair
228226
config *initConfig
229227
manager cgroups.Manager
230228
intelRdtManager intelrdt.Manager
@@ -246,7 +244,7 @@ func (p *initProcess) externalDescriptors() []string {
246244
// getChildPid receives the final child's pid over the provided pipe.
247245
func (p *initProcess) getChildPid() (int, error) {
248246
var pid pid
249-
if err := json.NewDecoder(p.parentPipe).Decode(&pid); err != nil {
247+
if err := json.NewDecoder(p.messageSockPair.parent).Decode(&pid); err != nil {
250248
p.cmd.Wait()
251249
return -1, err
252250
}
@@ -282,12 +280,12 @@ func (p *initProcess) waitForChildExit(childPid int) error {
282280
}
283281

284282
func (p *initProcess) start() error {
285-
defer p.parentPipe.Close()
283+
defer p.messageSockPair.parent.Close()
286284
err := p.cmd.Start()
287285
p.process.ops = p
288286
// close the write-side of the pipes (controlled by child)
289-
p.childPipe.Close()
290-
p.logPipe.w.Close()
287+
p.messageSockPair.child.Close()
288+
p.logFilePair.child.Close()
291289
if err != nil {
292290
p.process.ops = nil
293291
return newSystemErrorWithCause(err, "starting init process command")
@@ -313,7 +311,7 @@ func (p *initProcess) start() error {
313311
}
314312
}()
315313

316-
if _, err := io.Copy(p.parentPipe, p.bootstrapData); err != nil {
314+
if _, err := io.Copy(p.messageSockPair.parent, p.bootstrapData); err != nil {
317315
return newSystemErrorWithCause(err, "copying bootstrap data to pipe")
318316
}
319317
childPid, err := p.getChildPid()
@@ -341,7 +339,7 @@ func (p *initProcess) start() error {
341339
}
342340
// Now it's time to setup cgroup namesapce
343341
if p.config.Config.Namespaces.Contains(configs.NEWCGROUP) && p.config.Config.Namespaces.PathOf(configs.NEWCGROUP) == "" {
344-
if _, err := p.parentPipe.Write([]byte{createCgroupns}); err != nil {
342+
if _, err := p.messageSockPair.parent.Write([]byte{createCgroupns}); err != nil {
345343
return newSystemErrorWithCause(err, "sending synchronization value to init process")
346344
}
347345
}
@@ -368,7 +366,7 @@ func (p *initProcess) start() error {
368366
sentResume bool
369367
)
370368

371-
ierr := parseSync(p.parentPipe, func(sync *syncT) error {
369+
ierr := parseSync(p.messageSockPair.parent, func(sync *syncT) error {
372370
switch sync.Type {
373371
case procReady:
374372
// set rlimits, this has to be done here because we lose permissions
@@ -404,7 +402,7 @@ func (p *initProcess) start() error {
404402
}
405403
}
406404
// Sync with child.
407-
if err := writeSync(p.parentPipe, procRun); err != nil {
405+
if err := writeSync(p.messageSockPair.parent, procRun); err != nil {
408406
return newSystemErrorWithCause(err, "writing syncT 'run'")
409407
}
410408
sentRun = true
@@ -433,7 +431,7 @@ func (p *initProcess) start() error {
433431
}
434432
}
435433
// Sync with child.
436-
if err := writeSync(p.parentPipe, procResume); err != nil {
434+
if err := writeSync(p.messageSockPair.parent, procResume); err != nil {
437435
return newSystemErrorWithCause(err, "writing syncT 'resume'")
438436
}
439437
sentResume = true
@@ -450,7 +448,7 @@ func (p *initProcess) start() error {
450448
if p.config.Config.Namespaces.Contains(configs.NEWNS) && !sentResume {
451449
return newSystemError(fmt.Errorf("could not synchronise after executing prestart hooks with container process"))
452450
}
453-
if err := unix.Shutdown(int(p.parentPipe.Fd()), unix.SHUT_WR); err != nil {
451+
if err := unix.Shutdown(int(p.messageSockPair.parent.Fd()), unix.SHUT_WR); err != nil {
454452
return newSystemErrorWithCause(err, "shutting down init pipe")
455453
}
456454

@@ -494,7 +492,7 @@ func (p *initProcess) sendConfig() error {
494492
// send the config to the container's init process, we don't use JSON Encode
495493
// here because there might be a problem in JSON decoder in some cases, see:
496494
// https://github.com/docker/docker/issues/14203#issuecomment-174177790
497-
return utils.WriteJSON(p.parentPipe, p.config)
495+
return utils.WriteJSON(p.messageSockPair.parent, p.config)
498496
}
499497

500498
func (p *initProcess) createNetworkInterfaces() error {
@@ -527,7 +525,7 @@ func (p *initProcess) setExternalDescriptors(newFds []string) {
527525
}
528526

529527
func (p *initProcess) forwardChildLogs() {
530-
go logs.ForwardLogs(p.logPipe.r)
528+
go logs.ForwardLogs(p.logFilePair.parent)
531529
}
532530

533531
func getPipeFds(pid int) ([]string, error) {

‎main.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ func main() {
8787
},
8888
cli.StringFlag{
8989
Name: "log",
90-
Value: "/dev/null",
90+
Value: "/dev/stderr",
9191
Usage: "set the log file path where internal debug information is written",
9292
},
9393
cli.StringFlag{

‎tests/integration/debug.bats

+5
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@ function teardown() {
1616
runc --debug run test_hello
1717
echo "${output}"
1818
[ "$status" -eq 0 ]
19+
20+
# check expected debug output was sent to stderr
21+
[[ "${output}" == *"level=debug"* ]]
22+
[[ "${output}" == *"nsexec started"* ]]
23+
[[ "${output}" == *"child process in init()"* ]]
1924
}
2025

2126
@test "global --debug to --log" {

‎tests/integration/helpers.bash

+1-1
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ function runc() {
6565

6666
# Raw wrapper for runc.
6767
function __runc() {
68-
"$RUNC" ${RUNC_USE_SYSTEMD+--systemd-cgroup} --log /proc/self/fd/2 --root "$ROOT" "$@"
68+
"$RUNC" ${RUNC_USE_SYSTEMD+--systemd-cgroup} --root "$ROOT" "$@"
6969
}
7070

7171
# Wrapper for runc spec, which takes only one argument (the bundle path).

0 commit comments

Comments
 (0)