Merge pull request #2528 from crosbymichael/shim-debug

dmcgowan · web-flow · commit 1ba4aa04b455 · 2018-08-07T14:14:01.000-07:00
Add shim log pipe for log forwarding to the daemon
diff --git a/cmd/containerd-shim-runc-v1/main.go b/cmd/containerd-shim-runc-v1/main.go
@@ -19,16 +19,10 @@
 package main
 
 import (
-	"fmt"
-	"os"
-
 	"github.com/containerd/containerd/runtime/v2/runc"
 	"github.com/containerd/containerd/runtime/v2/shim"
 )
 
 func main() {
-	if err := shim.Run(runc.New); err != nil {
-		fmt.Fprintf(os.Stderr, "containerd-shim-runc-v1: %s\n", err)
-		os.Exit(1)
-	}
+	shim.Run("io.containerd.runc.v1", runc.New)
 }
diff --git a/runtime/v2/README.md b/runtime/v2/README.md
@@ -157,6 +157,13 @@ If the shim collects Out of Memory events, it SHOULD also publish a `runtime.Tas
 
 If a shim does not or cannot implement an rpc call, it MUST return a `github.com/containerd/containerd/errdefs.ErrNotImplemented` error.
 
+#### Debugging and Shim Logs
+
+A fifo on unix or named pipe on Windows will be provided to the shim.
+It can be located inside the `cwd` of the shim named "log".
+The shims can use the existing `github.com/containerd/containerd/log` package to log debug messages.
+Messages will automatically be output in the containerd's daemon logs with the correct fields and runtime set.
+
 #### ttrpc
 
 [ttrpc](https://github.com/containerd/ttrpc) is the only currently supported protocol for shims.
diff --git a/runtime/v2/binary.go b/runtime/v2/binary.go
@@ -19,6 +19,8 @@ package v2
 import (
 	"bytes"
 	"context"
+	"io"
+	"os"
 	"strings"
 
 	eventstypes "github.com/containerd/containerd/api/events"
@@ -49,11 +51,36 @@ type binary struct {
 	rtTasks           *runtime.TaskList
 }
 
-func (b *binary) Start(ctx context.Context) (*shim, error) {
-	cmd, err := client.Command(ctx, b.runtime, b.containerdAddress, b.bundle.Path, "-id", b.bundle.ID, "start")
+func (b *binary) Start(ctx context.Context) (_ *shim, err error) {
+	cmd, err := client.Command(
+		ctx,
+		b.runtime,
+		b.containerdAddress,
+		b.bundle.Path,
+		"-id", b.bundle.ID,
+		"start",
+	)
 	if err != nil {
 		return nil, err
 	}
+	f, err := openShimLog(ctx, b.bundle)
+	if err != nil {
+		return nil, errors.Wrap(err, "open shim log pipe")
+	}
+	defer func() {
+		if err != nil {
+			f.Close()
+		}
+	}()
+	// open the log pipe and block until the writer is ready
+	// this helps with syncronization of the shim
+	// copy the shim's logs to containerd's output
+	go func() {
+		defer f.Close()
+		if _, err := io.Copy(os.Stderr, f); err != nil {
+			log.G(ctx).WithError(err).Error("copy shim log")
+		}
+	}()
 	out, err := cmd.CombinedOutput()
 	if err != nil {
 		return nil, errors.Wrapf(err, "%s", out)
diff --git a/runtime/v2/shim.go b/runtime/v2/shim.go
@@ -18,7 +18,9 @@ package v2
 
 import (
 	"context"
+	"io"
 	"io/ioutil"
+	"os"
 	"path/filepath"
 	"time"
 
@@ -54,6 +56,25 @@ func loadShim(ctx context.Context, bundle *Bundle, events *exchange.Exchange, rt
 	if err != nil {
 		return nil, err
 	}
+	f, err := openShimLog(ctx, bundle)
+	if err != nil {
+		return nil, errors.Wrap(err, "open shim log pipe")
+	}
+	defer func() {
+		if err != nil {
+			f.Close()
+		}
+	}()
+	// open the log pipe and block until the writer is ready
+	// this helps with syncronization of the shim
+	// copy the shim's logs to containerd's output
+	go func() {
+		defer f.Close()
+		if _, err := io.Copy(os.Stderr, f); err != nil {
+			log.G(ctx).WithError(err).Error("copy shim log")
+		}
+	}()
+
 	client := ttrpc.NewClient(conn)
 	client.OnClose(func() { conn.Close() })
 	s := &shim{
diff --git a/runtime/v2/shim/shim.go b/runtime/v2/shim/shim.go
@@ -27,6 +27,7 @@ import (
 	"time"
 
 	"github.com/containerd/containerd/events"
+	"github.com/containerd/containerd/log"
 	"github.com/containerd/containerd/namespaces"
 	shimapi "github.com/containerd/containerd/runtime/v2/task"
 	"github.com/containerd/ttrpc"
@@ -82,24 +83,38 @@ func setRuntime() {
 			debug.FreeOSMemory()
 		}
 	}()
-	if debugFlag {
-		f, err := os.OpenFile("shim.log", os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0600)
-		if err != nil {
-			fmt.Fprintf(os.Stderr, "open shim log %s", err)
-			os.Exit(1)
-		}
-		logrus.SetLevel(logrus.DebugLevel)
-		logrus.SetOutput(f)
-	}
 	if os.Getenv("GOMAXPROCS") == "" {
 		// If GOMAXPROCS hasn't been set, we default to a value of 2 to reduce
 		// the number of Go stacks present in the shim.
 		runtime.GOMAXPROCS(2)
 	}
 }
 
+func setLogger(ctx context.Context, id string) error {
+	logrus.SetFormatter(&logrus.TextFormatter{
+		TimestampFormat: log.RFC3339NanoFixed,
+		FullTimestamp:   true,
+	})
+	if debugFlag {
+		logrus.SetLevel(logrus.DebugLevel)
+	}
+	f, err := openLog(ctx, id)
+	if err != nil {
+		return err
+	}
+	logrus.SetOutput(f)
+	return nil
+}
+
 // Run initializes and runs a shim server
-func Run(initFunc Init) error {
+func Run(id string, initFunc Init) {
+	if err := run(id, initFunc); err != nil {
+		fmt.Fprintf(os.Stderr, "%s: %s\n", id, err)
+		os.Exit(1)
+	}
+}
+
+func run(id string, initFunc Init) error {
 	parseFlags()
 	setRuntime()
 
@@ -118,6 +133,8 @@ func Run(initFunc Init) error {
 		return fmt.Errorf("shim namespace cannot be empty")
 	}
 	ctx := namespaces.WithNamespace(context.Background(), namespaceFlag)
+	ctx = log.WithLogger(ctx, log.G(ctx).WithField("runtime", id))
+
 	service, err := initFunc(ctx, idFlag, publisher)
 	if err != nil {
 		return err
@@ -151,6 +168,9 @@ func Run(initFunc Init) error {
 		}
 		return nil
 	default:
+		if err := setLogger(ctx, idFlag); err != nil {
+			return err
+		}
 		client := NewShimClient(ctx, service, signals)
 		return client.Serve()
 	}
diff --git a/runtime/v2/shim/shim_unix.go b/runtime/v2/shim/shim_unix.go
@@ -21,6 +21,7 @@ package shim
 import (
 	"bytes"
 	"context"
+	"io"
 	"net"
 	"os"
 	"os/exec"
@@ -29,6 +30,7 @@ import (
 
 	"github.com/containerd/containerd/events"
 	"github.com/containerd/containerd/namespaces"
+	"github.com/containerd/fifo"
 	"github.com/containerd/typeurl"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -84,6 +86,10 @@ func handleSignals(logger *logrus.Entry, signals chan os.Signal) error {
 	}
 }
 
+func openLog(ctx context.Context, _ string) (io.Writer, error) {
+	return fifo.OpenFifo(context.Background(), "log", unix.O_WRONLY, 0700)
+}
+
 func (l *remoteEventsPublisher) Publish(ctx context.Context, topic string, event events.Event) error {
 	ns, _ := namespaces.Namespace(ctx)
 	encoded, err := typeurl.MarshalAny(event)
diff --git a/runtime/v2/shim/shim_windows.go b/runtime/v2/shim/shim_windows.go
@@ -21,6 +21,8 @@ package shim
 import (
 	"bytes"
 	"context"
+	"fmt"
+	"io"
 	"net"
 	"os"
 	"os/exec"
@@ -79,6 +81,14 @@ func handleSignals(logger *logrus.Entry, signals chan os.Signal) error {
 	}
 }
 
+func openLog(ctx context.Context, id string) (io.Writer, error) {
+	ns, err := namespaces.NamespaceRequired(ctx)
+	if err != nil {
+		return nil, err
+	}
+	return winio.DialPipe(fmt.Sprintf("\\\\.\\pipe\\containerd-shim-%s-%s-log", ns, id), nil)
+}
+
 func (l *remoteEventsPublisher) Publish(ctx context.Context, topic string, event events.Event) error {
 	ns, _ := namespaces.Namespace(ctx)
 	encoded, err := typeurl.MarshalAny(event)
diff --git a/runtime/v2/shim_unix.go b/runtime/v2/shim_unix.go
@@ -0,0 +1,32 @@
+// +build !windows
+
+/*
+   Copyright The containerd Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package v2
+
+import (
+	"context"
+	"io"
+	"path/filepath"
+
+	"github.com/containerd/fifo"
+	"golang.org/x/sys/unix"
+)
+
+func openShimLog(ctx context.Context, bundle *Bundle) (io.ReadCloser, error) {
+	return fifo.OpenFifo(ctx, filepath.Join(bundle.Path, "log"), unix.O_RDONLY|unix.O_CREAT|unix.O_NONBLOCK, 0700)
+}
diff --git a/runtime/v2/shim_windows.go b/runtime/v2/shim_windows.go
@@ -0,0 +1,42 @@
+/*
+   Copyright The containerd Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package v2
+
+import (
+	"context"
+	"fmt"
+	"io"
+
+	winio "github.com/Microsoft/go-winio"
+	"github.com/containerd/containerd/namespaces"
+)
+
+func openShimLog(ctx context.Context, bundle *Bundle) (io.ReadCloser, error) {
+	ns, err := namespaces.NamespaceRequired(ctx)
+	if err != nil {
+		return nil, err
+	}
+	l, err := winio.ListenPipe(fmt.Sprintf("\\\\.\\pipe\\containerd-shim-%s-%s-log", ns, bundle.ID), nil)
+	if err != nil {
+		return nil, err
+	}
+	c, err := l.Accept()
+	if err != nil {
+		l.Close()
+	}
+	return c, nil
+}
