Merge pull request #4315 from fuweid/fix-4294

restart plugin: support binary log uri

cio/io.go

@@ -245,19 +245,11 @@ func LogURI(uri *url.URL) Creator {
 // BinaryIO forwards container STDOUT|STDERR directly to a logging binary
 func BinaryIO(binary string, args map[string]string) Creator {
 	return func(_ string) (IO, error) {
-		binary = filepath.Clean(binary)
-		if !strings.HasPrefix(binary, "/") {
-			return nil, errors.New("absolute path needed")
+		uri, err := LogURIGenerator("binary", binary, args)
+		if err != nil {
+			return nil, err
 		}
-		uri := &url.URL{
-			Scheme: "binary",
-			Path:   binary,
-		}
-		q := uri.Query()
-		for k, v := range args {
-			q.Set(k, v)
-		}
-		uri.RawQuery = q.Encode()
+
 		res := uri.String()
 		return &logURI{
 			config: Config{
@@ -272,14 +264,11 @@ func BinaryIO(binary string, args map[string]string) Creator {
 // If the log file already exists, the logs will be appended to the file.
 func LogFile(path string) Creator {
 	return func(_ string) (IO, error) {
-		path = filepath.Clean(path)
-		if !strings.HasPrefix(path, "/") {
-			return nil, errors.New("absolute path needed")
-		}
-		uri := &url.URL{
-			Scheme: "file",
-			Path:   path,
+		uri, err := LogURIGenerator("file", path, nil)
+		if err != nil {
+			return nil, err
 		}
+
 		res := uri.String()
 		return &logURI{
 			config: Config{
@@ -290,6 +279,30 @@ func LogFile(path string) Creator {
 	}
 }
 
+// LogURIGenerator is the helper to generate log uri with specific scheme.
+func LogURIGenerator(scheme string, path string, args map[string]string) (*url.URL, error) {
+	path = filepath.Clean(path)
+	if !strings.HasPrefix(path, "/") {
+		return nil, errors.New("absolute path needed")
+	}
+
+	uri := &url.URL{
+		Scheme: scheme,
+		Path:   path,
+	}
+
+	if len(args) == 0 {
+		return uri, nil
+	}
+
+	q := uri.Query()
+	for k, v := range args {
+		q.Set(k, v)
+	}
+	uri.RawQuery = q.Encode()
+	return uri, nil
+}
+
 type logURI struct {
 	config Config
 }

cio/io_test.go

@@ -195,3 +195,47 @@ func TestLogFileFailOnRelativePath(t *testing.T) {
 	_, err := LogFile("./file.txt")("!")
 	assert.Error(t, err, "absolute path needed")
 }
+
+func TestLogURIGenerator(t *testing.T) {
+	for _, tc := range []struct {
+		scheme   string
+		path     string
+		args     map[string]string
+		expected string
+		err      string
+	}{
+		{
+			scheme:   "fifo",
+			path:     "/full/path/pipe.fifo",
+			expected: "fifo:///full/path/pipe.fifo",
+		},
+		{
+			scheme: "file",
+			path:   "/full/path/file.txt",
+			args: map[string]string{
+				"maxSize": "100MB",
+			},
+			expected: "file:///full/path/file.txt?maxSize=100MB",
+		},
+		{
+			scheme: "binary",
+			path:   "/full/path/bin",
+			args: map[string]string{
+				"id": "testing",
+			},
+			expected: "binary:///full/path/bin?id=testing",
+		},
+		{
+			scheme: "unknown",
+			path:   "nowhere",
+			err:    "absolute path needed",
+		},
+	} {
+		uri, err := LogURIGenerator(tc.scheme, tc.path, tc.args)
+		if err != nil {
+			assert.Error(t, err, tc.err)
+			continue
+		}
+		assert.Equal(t, tc.expected, uri.String())
+	}
+}

runtime/restart/monitor/change.go

@@ -18,10 +18,13 @@ package monitor
 
 import (
 	"context"
+	"net/url"
 	"syscall"
 
 	"github.com/containerd/containerd"
 	"github.com/containerd/containerd/cio"
+	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 )
 
 type stopChange struct {
@@ -34,14 +37,30 @@ func (s *stopChange) apply(ctx context.Context, client *containerd.Client) error
 
 type startChange struct {
 	container containerd.Container
+	logURI    string
+
+	// Deprecated(in release 1.5): but recognized now, prefer to use logURI
 	logPath string
 }
 
 func (s *startChange) apply(ctx context.Context, client *containerd.Client) error {
 	log := cio.NullIO
-	if s.logPath != "" {
+
+	if s.logURI != "" {
+		uri, err := url.Parse(s.logURI)
+		if err != nil {
+			return errors.Wrapf(err, "failed to parse %v into url", s.logURI)
+		}
+		log = cio.LogURI(uri)
+	} else if s.logPath != "" {
 		log = cio.LogFile(s.logPath)
 	}
+
+	if s.logURI != "" && s.logPath != "" {
+		logrus.Warnf("LogPathLabel=%v has been deprecated, using LogURILabel=%v",
+			s.logPath, s.logURI)
+	}
+
 	killTask(ctx, s.container)
 	task, err := s.container.NewTask(ctx, log)
 	if err != nil {

runtime/restart/monitor/monitor.go

@@ -200,6 +200,7 @@ func (m *monitor) monitor(ctx context.Context) ([]change, error) {
 			changes = append(changes, &startChange{
 				container: c,
 				logPath:   labels[restart.LogPathLabel],
+				logURI:    labels[restart.LogURILabel],
 			})
 		case containerd.Stopped:
 			changes = append(changes, &stopChange{

runtime/restart/restart.go

@@ -33,17 +33,53 @@ import (
 	"context"
 
 	"github.com/containerd/containerd"
+	"github.com/containerd/containerd/cio"
 	"github.com/containerd/containerd/containers"
 )
 
 const (
 	// StatusLabel sets the restart status label for a container
 	StatusLabel = "containerd.io/restart.status"
+	// LogURILabel sets the restart log uri label for a container
+	LogURILabel = "containerd.io/restart.loguri"
+
 	// LogPathLabel sets the restart log path label for a container
+	//
+	// Deprecated(in release 1.5): use LogURILabel
 	LogPathLabel = "containerd.io/restart.logpath"
 )
 
+// WithBinaryLogURI sets the binary-type log uri for a container.
+func WithBinaryLogURI(binary string, args map[string]string) func(context.Context, *containerd.Client, *containers.Container) error {
+	return func(_ context.Context, _ *containerd.Client, c *containers.Container) error {
+		uri, err := cio.LogURIGenerator("binary", binary, args)
+		if err != nil {
+			return err
+		}
+
+		ensureLabels(c)
+		c.Labels[LogURILabel] = uri.String()
+		return nil
+	}
+}
+
+// WithFileLogURI sets the file-type log uri for a container.
+func WithFileLogURI(path string) func(context.Context, *containerd.Client, *containers.Container) error {
+	return func(_ context.Context, _ *containerd.Client, c *containers.Container) error {
+		uri, err := cio.LogURIGenerator("file", path, nil)
+		if err != nil {
+			return err
+		}
+
+		ensureLabels(c)
+		c.Labels[LogURILabel] = uri.String()
+		return nil
+	}
+}
+
 // WithLogPath sets the log path for a container
+//
+// Deprecated(in release 1.5): use WithFileLogURI.
 func WithLogPath(path string) func(context.Context, *containerd.Client, *containers.Container) error {
 	return func(_ context.Context, _ *containerd.Client, c *containers.Container) error {
 		ensureLabels(c)
@@ -68,6 +104,7 @@ func WithNoRestarts(_ context.Context, _ *containerd.Client, c *containers.Conta
 	}
 	delete(c.Labels, StatusLabel)
 	delete(c.Labels, LogPathLabel)
+	delete(c.Labels, LogURILabel)
 	return nil
 }