From d656fa38ca32fc0e08f31b74169552f371bbd4e0 Mon Sep 17 00:00:00 2001 From: Wei Fu Date: Tue, 9 Jun 2020 14:34:44 +0800 Subject: [PATCH] restart plugin: support binary log uri Introduce LogURIGenerator helper function in cio package. It is used in the restart options, like WithBinaryLogURI and WithFileLogURI. And restart.LogPathLabel might be used in production and work well. In order to reduce breaking change, the LogPathLabel is still recognized if new LogURILabel is not set. In next release 1.5, the LogPathLabel will be removed. Signed-off-by: Wei Fu --- cio/io.go | 51 +++++++++++++++++++----------- cio/io_test.go | 44 ++++++++++++++++++++++++++ runtime/restart/monitor/change.go | 23 ++++++++++++-- runtime/restart/monitor/monitor.go | 1 + runtime/restart/restart.go | 37 ++++++++++++++++++++++ 5 files changed, 135 insertions(+), 21 deletions(-) diff --git a/cio/io.go b/cio/io.go index c7cf4f0bc..663799941 100644 --- a/cio/io.go +++ b/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 } diff --git a/cio/io_test.go b/cio/io_test.go index 1ca951c16..3f4ceb94c 100644 --- a/cio/io_test.go +++ b/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()) + } +} diff --git a/runtime/restart/monitor/change.go b/runtime/restart/monitor/change.go index f3e1a0b2d..7de6f67c3 100644 --- a/runtime/restart/monitor/change.go +++ b/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 - logPath string + 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 { diff --git a/runtime/restart/monitor/monitor.go b/runtime/restart/monitor/monitor.go index 7b293fead..3123f24a6 100644 --- a/runtime/restart/monitor/monitor.go +++ b/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{ diff --git a/runtime/restart/restart.go b/runtime/restart/restart.go index 47b98e003..54f56a47d 100644 --- a/runtime/restart/restart.go +++ b/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 }