Skip to content

Commit

Permalink
libct/init: fix and document init error logic
Browse files Browse the repository at this point in the history
Currently, the code has a few issues:
1. The "unable to convert _LIBCONTAINER_INITPIPE" error from
   StartInitialization is never shown.
2. Errors from WriteSync and WriteJSON are never shown.
3. It's complicated.

Generally, our goals are:
 - if there's any error, do our best to show it;
 - but only show each error once;
 - simplify the code, unify init implementations.

So:
 - move error printing from StartInit to its callers;
 - do not return error from StartInit if we sent it to the parent
   process;
 - print Write* errors to stderr;

Also modify, document, and unify Start Initialization callers:
 - always call os.Exit(1), since normally StartInitialization does not
   return. If it did, it's an error.
 - do not panic (no one needs a long, ugly, and useless backtrace here);
 - add some inline comments to document all this;
 - make sure init.go and libct/int/init_test.go are in sync;
 - libcontainer/README.md: drop an example implementation, instead
   refer to init.go as an always-up-to-date example.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
  • Loading branch information
kolyshkin committed May 3, 2023
1 parent 6dbbbf6 commit 3ae62ab
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 24 deletions.
12 changes: 8 additions & 4 deletions init.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package main

import (
"fmt"
"os"
"runtime"

Expand All @@ -16,10 +17,13 @@ func init() {
runtime.LockOSThread()

if err := libcontainer.StartInitialization(); err != nil {
// as the error is sent back to the parent there is no need to log
// or write it to stderr because the parent process will handle this
os.Exit(1)
// If the error is returned, it was not communicated
// back to the parent (which is not a common case),
// so print it here as a last resort.
fmt.Fprintln(os.Stderr, err)
}
panic("libcontainer: container init failed to exec")
// Normally, StartInitialization() never returns, meaning
// if we are here, it had failed.
os.Exit(1)
}
}
18 changes: 2 additions & 16 deletions libcontainer/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,22 +23,8 @@ function as the entry of "bootstrap".
In addition to the go init function the early stage bootstrap is handled by importing
[nsenter](https://github.com/opencontainers/runc/blob/master/libcontainer/nsenter/README.md).

```go
import (
_ "github.com/opencontainers/runc/libcontainer/nsenter"
)

func init() {
if len(os.Args) > 1 && os.Args[1] == "init" {
runtime.GOMAXPROCS(1)
runtime.LockOSThread()
if err := libcontainer.StartInitialization(); err != nil {
logrus.Fatal(err)
}
panic("--this line should have never been executed, congratulations--")
}
}
```
For details on how runc implements such "init", see
[init.go]((https://github.com/opencontainers/runc/blob/master/libcontainer/init.go).

Then to create a container you first have to create a configuration
struct describing how the container is to be created. A sample would look similar to this:
Expand Down
14 changes: 10 additions & 4 deletions libcontainer/init_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ type initConfig struct {
// StartInitialization loads a container by opening the pipe fd from the parent
// to read the configuration and state. This is a low level implementation
// detail of the reexec and should not be consumed externally.
//
// Normally, this function does not return. If it returns, with or without an
// error, it means the initialization has failed. If the error is returned,
// it means the error can not be communicated back to the parent.
func StartInitialization() (retErr error) {
// Get the INITPIPE.
envInitPipe := os.Getenv("_LIBCONTAINER_INITPIPE")
Expand All @@ -85,16 +89,18 @@ func StartInitialization() (retErr error) {
defer pipe.Close()

defer func() {
// We have an error during the initialization of the container's init,
// send it back to the parent process in the form of an initError.
// If this defer is ever called, this means initialization has failed.
// Send the error back to the parent process in the form of an initError.
if err := writeSync(pipe, procError); err != nil {
fmt.Fprintln(os.Stderr, retErr)
fmt.Fprintln(os.Stderr, err)
return
}
if err := utils.WriteJSON(pipe, &initError{Message: retErr.Error()}); err != nil {
fmt.Fprintln(os.Stderr, retErr)
fmt.Fprintln(os.Stderr, err)
return
}
// The error is sent, no need to also return it (or it will be reported twice).
retErr = nil
}()

// Set up logging. This is used rarely, and mostly for init debugging.
Expand Down
3 changes: 3 additions & 0 deletions libcontainer/integration/init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ func init() {
runtime.GOMAXPROCS(1)
runtime.LockOSThread()
if err := libcontainer.StartInitialization(); err != nil {
// If the error is returned, it was not communicated
// back to the parent (which is not a common case),
// so print it here as a last resort.
fmt.Fprintln(os.Stderr, err)
}
// Normally, StartInitialization() never returns, meaning
Expand Down

0 comments on commit 3ae62ab

Please sign in to comment.