runtime/v2/shim: return if error in load plugin

fuweid · fuweid · commit b297775eafd8 · 2022-07-22T23:25:40.000+08:00
If there is any unskipable error during setuping shim plugins, we should
fail return error to prevent from leaky shim instance. For example,
there is error during init task plugin, the shim ttrpc server will not
contain any shim API method. The any call to the shim will receive that

  failed to create shim task: service containerd.task.v2.Task: not implemented

Then containerd can't use `Shutdown` to let the shim close. The shim
will be leaky. And also fail return if there is no ttrpc service.

Signed-off-by: Wei Fu &lt;fuweid89@gmail.com&gt;
diff --git a/runtime/v2/shim/shim.go b/runtime/v2/shim/shim.go
@@ -417,10 +417,9 @@ func run(ctx context.Context, manager Manager, initFunc Init, name string, confi
 		if err != nil {
 			if plugin.IsSkipPlugin(err) {
 				log.G(ctx).WithError(err).WithField("type", p.Type).Infof("skip loading plugin %q...", id)
-			} else {
-				log.G(ctx).WithError(err).Warnf("failed to load plugin %s", id)
+				continue
 			}
-			continue
+			return fmt.Errorf("failed to load plugin %s: %w", id, err)
 		}
 
 		if src, ok := instance.(ttrpcService); ok {
@@ -434,6 +433,10 @@ func run(ctx context.Context, manager Manager, initFunc Init, name string, confi
 		}
 	}
 
+	if len(ttrpcServices) == 0 {
+		return fmt.Errorf("required that ttrpc service")
+	}
+
 	unaryInterceptor := chainUnaryServerInterceptors(ttrpcUnaryInterceptors...)
 	server, err := newServer(ttrpc.WithUnaryServerInterceptor(unaryInterceptor))
 	if err != nil {

Original file line number	Diff line number	Diff line change
`@@ -417,10 +417,9 @@ func run(ctx context.Context, manager Manager, initFunc Init, name string, confi`
`417`	`417`	`if err != nil {`
`418`	`418`	`if plugin.IsSkipPlugin(err) {`
`419`	`419`	`log.G(ctx).WithError(err).WithField("type", p.Type).Infof("skip loading plugin %q...", id)`
`420`		`- } else {`
`421`		`- log.G(ctx).WithError(err).Warnf("failed to load plugin %s", id)`
	`420`	`+ continue`
`422`	`421`	`}`
`423`		`- continue`
	`422`	`+ return fmt.Errorf("failed to load plugin %s: %w", id, err)`
`424`	`423`	`}`
`425`	`424`
`426`	`425`	`if src, ok := instance.(ttrpcService); ok {`
`@@ -434,6 +433,10 @@ func run(ctx context.Context, manager Manager, initFunc Init, name string, confi`
`434`	`433`	`}`
`435`	`434`	`}`
`436`	`435`
	`436`	`+ if len(ttrpcServices) == 0 {`
	`437`	`+ return fmt.Errorf("required that ttrpc service")`
	`438`	`+ }`
	`439`	`+`
`437`	`440`	`unaryInterceptor := chainUnaryServerInterceptors(ttrpcUnaryInterceptors...)`
`438`	`441`	`server, err := newServer(ttrpc.WithUnaryServerInterceptor(unaryInterceptor))`
`439`	`442`	`if err != nil {`