libct/init: unify init, fix its error logic

This commit does two things:

1. Consolidate StartInitialization calling logic into Init().
2. Fix init error handling logic.

The main issues at hand are:
- the "unable to convert _LIBCONTAINER_INITPIPE" error from
  StartInitialization is never shown;
- errors from WriteSync and WriteJSON are never shown;
- the StartInit calling code is triplicated;
- using panic is questionable.

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.

Signed-off-by: Kir Kolyshkin <[email protected]>

Author: kolyshkin

init.go

@@ -2,7 +2,6 @@ package main
 
 import (
 	"os"
-	"runtime"
 
 	"github.com/opencontainers/runc/libcontainer"
 	_ "github.com/opencontainers/runc/libcontainer/nsenter"
@@ -12,14 +11,6 @@ func init() {
 	if len(os.Args) > 1 && os.Args[1] == "init" {
 		// This is the golang entry point for runc init, executed
 		// before main() but after libcontainer/nsenter's nsexec().
-		runtime.GOMAXPROCS(1)
-		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)
-		}
-		panic("libcontainer: container init failed to exec")
+		libcontainer.Init()
 	}
 }

libcontainer/README.md

@@ -23,22 +23,9 @@ 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/init.go)
+and [libcontainer/init_linux.go](https://github.com/opencontainers/runc/blob/master/libcontainer/init_linux.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:

libcontainer/init_linux.go

@@ -8,6 +8,7 @@ import (
 	"io"
 	"net"
 	"os"
+	"runtime"
 	"runtime/debug"
 	"strconv"
 	"strings"
@@ -84,10 +85,30 @@ type initConfig struct {
 	Cgroup2Path      string                `json:"cgroup2_path,omitempty"`
 }
 
-// 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.
-func StartInitialization() (retErr error) {
+// Init is part of "runc init" implementation.
+func Init() {
+	runtime.GOMAXPROCS(1)
+	runtime.LockOSThread()
+
+	if err := 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 to stderr here as a last resort.
+		//
+		// Do not use logrus as we are not sure if it has been
+		// set up yet, but most important, if the parent is
+		// alive (and its log forwarding is working).
+		fmt.Fprintln(os.Stderr, err)
+	}
+	// Normally, StartInitialization() never returns, meaning
+	// if we are here, it had failed.
+	os.Exit(1)
+}
+
+// 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")
 	pipefd, err := strconv.Atoi(envInitPipe)
@@ -98,16 +119,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.

libcontainer/integration/init_test.go

@@ -1,9 +1,7 @@
 package integration
 
 import (
-	"fmt"
 	"os"
-	"runtime"
 	"testing"
 
 	"github.com/opencontainers/runc/libcontainer"
@@ -14,20 +12,9 @@ import (
 
 // Same as ../../init.go but for libcontainer/integration.
 func init() {
-	if len(os.Args) < 2 || os.Args[1] != "init" {
-		return
+	if len(os.Args) > 1 && os.Args[1] == "init" {
+		libcontainer.Init()
 	}
-	// This is the golang entry point for runc init, executed
-	// before TestMain() but after libcontainer/nsenter's nsexec().
-	runtime.GOMAXPROCS(1)
-	runtime.LockOSThread()
-	if err := libcontainer.StartInitialization(); err != nil {
-		// logrus is not initialized
-		fmt.Fprintln(os.Stderr, err)
-	}
-	// Normally, StartInitialization() never returns, meaning
-	// if we are here, it had failed.
-	os.Exit(1)
 }
 
 func TestMain(m *testing.M) {