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

fuweid · qiutongs · commit 4a5bc05aa00a · 2022-09-30T11:48:48.000Z
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 <fuweid89@gmail.com> (cherry picked from commit b297775) Signed-off-by: Qiutong Song <songqt01@gmail.com>
diff --git a/runtime/v2/shim/shim.go b/runtime/v2/shim/shim.go
@@ -413,10 +413,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 {
@@ -430,6 +429,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
`@@ -413,10 +413,9 @@ func run(ctx context.Context, manager Manager, initFunc Init, name string, confi`
`413`	`413`	`if err != nil {`
`414`	`414`	`if plugin.IsSkipPlugin(err) {`
`415`	`415`	`log.G(ctx).WithError(err).WithField("type", p.Type).Infof("skip loading plugin %q...", id)`
`416`		`- } else {`
`417`		`- log.G(ctx).WithError(err).Warnf("failed to load plugin %s", id)`
	`416`	`+ continue`
`418`	`417`	`}`
`419`		`- continue`
	`418`	`+ return fmt.Errorf("failed to load plugin %s: %w", id, err)`
`420`	`419`	`}`
`421`	`420`
`422`	`421`	`if src, ok := instance.(ttrpcService); ok {`
`@@ -430,6 +429,10 @@ func run(ctx context.Context, manager Manager, initFunc Init, name string, confi`
`430`	`429`	`}`
`431`	`430`	`}`
`432`	`431`
	`432`	`+ if len(ttrpcServices) == 0 {`
	`433`	`+ return fmt.Errorf("required that ttrpc service")`
	`434`	`+ }`
	`435`	`+`
`433`	`436`	`unaryInterceptor := chainUnaryServerInterceptors(ttrpcUnaryInterceptors...)`
`434`	`437`	`server, err := newServer(ttrpc.WithUnaryServerInterceptor(unaryInterceptor))`
`435`	`438`	`if err != nil {`