From 14e621cf9192f4a8135bda270c60ec2e42c88de4 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 15 Nov 2023 13:05:22 +0100 Subject: [PATCH 1/6] services/server: gofumpt Signed-off-by: Sebastiaan van Stijn --- services/server/config/config_test.go | 16 ++++++++-------- services/server/server.go | 6 +++--- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/services/server/config/config_test.go b/services/server/config/config_test.go index e81800501..39d3a6616 100644 --- a/services/server/config/config_test.go +++ b/services/server/config/config_test.go @@ -86,7 +86,7 @@ func TestResolveImports(t *testing.T) { tempDir := t.TempDir() for _, filename := range []string{"config_1.toml", "config_2.toml", "test.toml"} { - err := os.WriteFile(filepath.Join(tempDir, filename), []byte(""), 0600) + err := os.WriteFile(filepath.Join(tempDir, filename), []byte(""), 0o600) assert.NoError(t, err) } @@ -118,7 +118,7 @@ root = "/var/lib/containerd" tempDir := t.TempDir() path := filepath.Join(tempDir, "config.toml") - err := os.WriteFile(path, []byte(data), 0600) + err := os.WriteFile(path, []byte(data), 0o600) assert.NoError(t, err) var out Config @@ -147,10 +147,10 @@ disabled_plugins = ["io.containerd.v1.xyz"] tempDir := t.TempDir() - err := os.WriteFile(filepath.Join(tempDir, "data1.toml"), []byte(data1), 0600) + err := os.WriteFile(filepath.Join(tempDir, "data1.toml"), []byte(data1), 0o600) assert.NoError(t, err) - err = os.WriteFile(filepath.Join(tempDir, "data2.toml"), []byte(data2), 0600) + err = os.WriteFile(filepath.Join(tempDir, "data2.toml"), []byte(data2), 0o600) assert.NoError(t, err) var out Config @@ -175,10 +175,10 @@ imports = ["data1.toml", "data2.toml"] ` tempDir := t.TempDir() - err := os.WriteFile(filepath.Join(tempDir, "data1.toml"), []byte(data1), 0600) + err := os.WriteFile(filepath.Join(tempDir, "data1.toml"), []byte(data1), 0o600) assert.NoError(t, err) - err = os.WriteFile(filepath.Join(tempDir, "data2.toml"), []byte(data2), 0600) + err = os.WriteFile(filepath.Join(tempDir, "data2.toml"), []byte(data2), 0o600) assert.NoError(t, err) var out Config @@ -207,7 +207,7 @@ version = 2 tempDir := t.TempDir() path := filepath.Join(tempDir, "config.toml") - err := os.WriteFile(path, []byte(data), 0600) + err := os.WriteFile(path, []byte(data), 0o600) assert.NoError(t, err) var out Config @@ -230,7 +230,7 @@ func TestDecodePluginInV1Config(t *testing.T) { ` path := filepath.Join(t.TempDir(), "config.toml") - err := os.WriteFile(path, []byte(data), 0600) + err := os.WriteFile(path, []byte(data), 0o600) assert.NoError(t, err) var out Config diff --git a/services/server/server.go b/services/server/server.go index bf775aa63..f013f3800 100644 --- a/services/server/server.go +++ b/services/server/server.go @@ -81,16 +81,16 @@ func CreateTopLevelDirectories(config *srvconfig.Config) error { return errors.New("root and state must be different paths") } - if err := sys.MkdirAllWithACL(config.Root, 0711); err != nil { + if err := sys.MkdirAllWithACL(config.Root, 0o711); err != nil { return err } - if err := sys.MkdirAllWithACL(config.State, 0711); err != nil { + if err := sys.MkdirAllWithACL(config.State, 0o711); err != nil { return err } if config.TempDir != "" { - if err := sys.MkdirAllWithACL(config.TempDir, 0711); err != nil { + if err := sys.MkdirAllWithACL(config.TempDir, 0o711); err != nil { return err } if runtime.GOOS == "windows" { From 09de4f1fcc72093e0234d37175bce13f9a1f1587 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 15 Nov 2023 13:06:52 +0100 Subject: [PATCH 2/6] services/server: rename var that collided with import Signed-off-by: Sebastiaan van Stijn --- services/server/server.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/services/server/server.go b/services/server/server.go index f013f3800..fb6a3bbb0 100644 --- a/services/server/server.go +++ b/services/server/server.go @@ -341,12 +341,12 @@ func New(ctx context.Context, config *srvconfig.Config) (*Server, error) { // recordConfigDeprecations attempts to record use of any deprecated config field. Failures are logged and ignored. func recordConfigDeprecations(ctx context.Context, config *srvconfig.Config, set *plugin.Set) { // record any detected deprecations without blocking server startup - plugin := set.Get(plugins.WarningPlugin, plugins.DeprecationsPlugin) - if plugin == nil { + p := set.Get(plugins.WarningPlugin, plugins.DeprecationsPlugin) + if p == nil { log.G(ctx).Warn("failed to find warning service to record deprecations") return } - instance, err := plugin.Instance() + instance, err := p.Instance() if err != nil { log.G(ctx).WithError(err).Warn("failed to load warning service to record deprecations") return From be22e12d567ed4cb12bf052088b76479686761fc Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 15 Nov 2023 13:23:32 +0100 Subject: [PATCH 3/6] services/server: use structured log for plugin ID These logs were already using structured logs, so include "id" as a field, which also prevents the id being quoted (and escaped when printing); time="2023-11-15T11:30:23.745574884Z" level=info msg="loading plugin \"io.containerd.internal.v1.shutdown\"..." runtime=io.containerd.runc.v2 type=io.containerd.internal.v1 time="2023-11-15T11:30:23.745612425Z" level=info msg="loading plugin \"io.containerd.ttrpc.v1.pause\"..." runtime=io.containerd.runc.v2 type=io.containerd.ttrpc.v1 time="2023-11-15T11:30:23.745620884Z" level=info msg="loading plugin \"io.containerd.event.v1.publisher\"..." runtime=io.containerd.runc.v2 type=io.containerd.event.v1 time="2023-11-15T11:30:23.745625925Z" level=info msg="loading plugin \"io.containerd.ttrpc.v1.task\"..." runtime=io.containerd.runc.v2 type=io.containerd.ttrpc.v1 Also updated some changed `WithError().WithField()` calls, to prevent some overhead. Signed-off-by: Sebastiaan van Stijn --- services/server/server.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/services/server/server.go b/services/server/server.go index fb6a3bbb0..c2258b4cd 100644 --- a/services/server/server.go +++ b/services/server/server.go @@ -247,7 +247,7 @@ func New(ctx context.Context, config *srvconfig.Config) (*Server, error) { for _, p := range loaded { id := p.URI() - log.G(ctx).WithField("type", p.Type).Infof("loading plugin %q...", id) + log.G(ctx).WithFields(log.Fields{"id": id, "type": p.Type}).Info("loading plugin") var mustSucceed int32 initContext := plugin.NewContext( @@ -281,9 +281,9 @@ func New(ctx context.Context, config *srvconfig.Config) (*Server, error) { instance, err := result.Instance() if err != nil { if plugin.IsSkipPlugin(err) { - log.G(ctx).WithError(err).WithField("type", p.Type).Infof("skip loading plugin %q...", id) + log.G(ctx).WithFields(log.Fields{"error": err, "id": id, "type": p.Type}).Info("skip loading plugin") } else { - log.G(ctx).WithError(err).Warnf("failed to load plugin %s", id) + log.G(ctx).WithFields(log.Fields{"error": err, "id": id, "type": p.Type}).Warn("failed to load plugin") } if _, ok := required[id]; ok { return nil, fmt.Errorf("load required plugin %s: %w", id, err) @@ -432,8 +432,7 @@ func (s *Server) Stop() { p := s.plugins[i] instance, err := p.Instance() if err != nil { - log.L.WithError(err).WithField("id", p.Registration.URI()). - Error("could not get plugin instance") + log.L.WithFields(log.Fields{"error": err, "id": p.Registration.URI()}).Error("could not get plugin instance") continue } closer, ok := instance.(io.Closer) @@ -441,8 +440,7 @@ func (s *Server) Stop() { continue } if err := closer.Close(); err != nil { - log.L.WithError(err).WithField("id", p.Registration.URI()). - Error("failed to close plugin") + log.L.WithFields(log.Fields{"error": err, "id": p.Registration.URI()}).Error("failed to close plugin") } } } @@ -523,7 +521,7 @@ func LoadPlugins(ctx context.Context, config *srvconfig.Config) ([]plugin.Regist if pp.Platform != "" { p, err = platforms.Parse(pp.Platform) if err != nil { - log.G(ctx).WithError(err).WithField("plugin", name).Warn("skipping proxy platform with bad platform") + log.G(ctx).WithFields(log.Fields{"error": err, "plugin": name}).Warn("skipping proxy platform with bad platform") } } else { p = platforms.DefaultSpec() From 0a59c33be56de8b76f39019f57711ebe592af0ac Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 15 Nov 2023 13:08:15 +0100 Subject: [PATCH 4/6] runtime/v2/shim: rename var that shadowed package var Signed-off-by: Sebastiaan van Stijn --- runtime/v2/shim/shim.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/runtime/v2/shim/shim.go b/runtime/v2/shim/shim.go index 18d28a145..74098dabf 100644 --- a/runtime/v2/shim/shim.go +++ b/runtime/v2/shim/shim.go @@ -318,8 +318,8 @@ func run(ctx context.Context, manager Manager, name string, config Config) error ) for _, p := range registry.Graph(func(*plugin.Registration) bool { return false }) { - id := p.URI() - log.G(ctx).WithField("type", p.Type).Infof("loading plugin %q...", id) + pID := p.URI() + log.G(ctx).WithField("type", p.Type).Infof("loading plugin %q...", pID) initContext := plugin.NewContext( ctx, @@ -353,14 +353,14 @@ func run(ctx context.Context, manager Manager, name string, config Config) error instance, err := result.Instance() if err != nil { if plugin.IsSkipPlugin(err) { - log.G(ctx).WithError(err).WithField("type", p.Type).Infof("skip loading plugin %q...", id) + log.G(ctx).WithError(err).WithField("type", p.Type).Infof("skip loading plugin %q...", pID) continue } - return fmt.Errorf("failed to load plugin %s: %w", id, err) + return fmt.Errorf("failed to load plugin %s: %w", pID, err) } if src, ok := instance.(TTRPCService); ok { - log.G(ctx).WithField("id", id).Debug("registering ttrpc service") + log.G(ctx).WithField("id", pID).Debug("registering ttrpc service") ttrpcServices = append(ttrpcServices, src) } From 71fd85f5ed82d1649ecd920559cbab18fd28553d Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 15 Nov 2023 13:02:57 +0100 Subject: [PATCH 5/6] runtime/v2/shim: run(): remove unused "name" argument Signed-off-by: Sebastiaan van Stijn --- runtime/v2/shim/shim.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/runtime/v2/shim/shim.go b/runtime/v2/shim/shim.go index 74098dabf..e66ca7f1d 100644 --- a/runtime/v2/shim/shim.go +++ b/runtime/v2/shim/shim.go @@ -189,13 +189,13 @@ func Run(ctx context.Context, manager Manager, opts ...BinaryOpts) { ctx = log.WithLogger(ctx, log.G(ctx).WithField("runtime", manager.Name())) - if err := run(ctx, manager, "", config); err != nil { + if err := run(ctx, manager, config); err != nil { fmt.Fprintf(os.Stderr, "%s: %s", manager.Name(), err) os.Exit(1) } } -func run(ctx context.Context, manager Manager, name string, config Config) error { +func run(ctx context.Context, manager Manager, config Config) error { parseFlags() if versionFlag { fmt.Printf("%s:\n", filepath.Base(os.Args[0])) From 1a1bd6d0a7ee1b569c7de0f4fd78b9e5a6728d4e Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 15 Nov 2023 13:18:33 +0100 Subject: [PATCH 6/6] runtime/v2/shim: use structured log for plugin ID These logs were already using structured logs, so include "id" as a field, which also prevents the id being quoted (and escaped when printing); time="2023-11-15T11:30:23.745574884Z" level=info msg="loading plugin \"io.containerd.internal.v1.shutdown\"..." runtime=io.containerd.runc.v2 type=io.containerd.internal.v1 time="2023-11-15T11:30:23.745612425Z" level=info msg="loading plugin \"io.containerd.ttrpc.v1.pause\"..." runtime=io.containerd.runc.v2 type=io.containerd.ttrpc.v1 time="2023-11-15T11:30:23.745620884Z" level=info msg="loading plugin \"io.containerd.event.v1.publisher\"..." runtime=io.containerd.runc.v2 type=io.containerd.event.v1 time="2023-11-15T11:30:23.745625925Z" level=info msg="loading plugin \"io.containerd.ttrpc.v1.task\"..." runtime=io.containerd.runc.v2 type=io.containerd.ttrpc.v1 Also updated some changed `WithError().WithField()` calls, to prevent some overhead. Signed-off-by: Sebastiaan van Stijn --- runtime/v2/shim/shim.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/runtime/v2/shim/shim.go b/runtime/v2/shim/shim.go index e66ca7f1d..87c053307 100644 --- a/runtime/v2/shim/shim.go +++ b/runtime/v2/shim/shim.go @@ -319,7 +319,7 @@ func run(ctx context.Context, manager Manager, config Config) error { for _, p := range registry.Graph(func(*plugin.Registration) bool { return false }) { pID := p.URI() - log.G(ctx).WithField("type", p.Type).Infof("loading plugin %q...", pID) + log.G(ctx).WithFields(log.Fields{"id": pID, "type": p.Type}).Info("loading plugin") initContext := plugin.NewContext( ctx, @@ -353,7 +353,7 @@ func run(ctx context.Context, manager Manager, config Config) error { instance, err := result.Instance() if err != nil { if plugin.IsSkipPlugin(err) { - log.G(ctx).WithError(err).WithField("type", p.Type).Infof("skip loading plugin %q...", pID) + log.G(ctx).WithFields(log.Fields{"id": pID, "type": p.Type, "error": err}).Info("skip loading plugin") continue } return fmt.Errorf("failed to load plugin %s: %w", pID, err)