Merge pull request #9165 from dmcgowan/backport-1.7-readiness-hang-fix

fuweid · web-flow · commit 790f6d948c51 · 2023-09-30T15:32:54.000+08:00
[release/1.7] Require plugins to succeed after registering readiness
diff --git a/pkg/cri/cri.go b/pkg/cri/cri.go
@@ -54,7 +54,6 @@ func init() {
 }
 
 func initCRIService(ic *plugin.InitContext) (interface{}, error) {
-	ready := ic.RegisterReadiness()
 	ic.Meta.Platforms = []imagespec.Platform{platforms.DefaultSpec()}
 	ic.Meta.Exports = map[string]string{"CRIVersion": constants.CRIVersion, "CRIVersionAlpha": constants.CRIVersionAlpha}
 	ctx := ic.Context
@@ -99,6 +98,8 @@ func initCRIService(ic *plugin.InitContext) (interface{}, error) {
 		return nil, fmt.Errorf("failed to create CRI service: %w", err)
 	}
 
+	// RegisterReadiness() must be called after NewCRIService(): https://github.com/containerd/containerd/issues/9163
+	ready := ic.RegisterReadiness()
 	go func() {
 		if err := s.Run(ready); err != nil {
 			log.G(ctx).WithError(err).Fatal("Failed to run CRI service")
diff --git a/services/server/server.go b/services/server/server.go
@@ -31,6 +31,7 @@ import (
 	"path/filepath"
 	"runtime"
 	"sync"
+	"sync/atomic"
 	"time"
 
 	csapi "github.com/containerd/containerd/api/services/content/v1"
@@ -209,6 +210,7 @@ func New(ctx context.Context, config *srvconfig.Config) (*Server, error) {
 			reqID = p.ID
 		}
 		log.G(ctx).WithField("type", p.Type).Infof("loading plugin %q...", id)
+		var mustSucceed int32
 
 		initContext := plugin.NewContext(
 			ctx,
@@ -220,7 +222,10 @@ func New(ctx context.Context, config *srvconfig.Config) (*Server, error) {
 		initContext.Events = events
 		initContext.Address = config.GRPC.Address
 		initContext.TTRPCAddress = config.TTRPC.Address
-		initContext.RegisterReadiness = s.RegisterReadiness
+		initContext.RegisterReadiness = func() func() {
+			atomic.StoreInt32(&mustSucceed, 1)
+			return s.RegisterReadiness()
+		}
 
 		// load the plugin specific configuration if it is provided
 		if p.Config != nil {
@@ -245,6 +250,10 @@ func New(ctx context.Context, config *srvconfig.Config) (*Server, error) {
 			if _, ok := required[reqID]; ok {
 				return nil, fmt.Errorf("load required plugin %s: %w", id, err)
 			}
+			// If readiness was registered during initialization, the plugin cannot fail
+			if atomic.LoadInt32(&mustSucceed) != 0 {
+				return nil, fmt.Errorf("plugin failed after registering readiness %s: %w", id, err)
+			}
 			continue
 		}
 

Original file line number	Diff line number	Diff line change
`@@ -54,7 +54,6 @@ func init() {`
`54`	`54`	`}`
`55`	`55`
`56`	`56`	`func initCRIService(ic *plugin.InitContext) (interface{}, error) {`
`57`		`- ready := ic.RegisterReadiness()`
`58`	`57`	`ic.Meta.Platforms = []imagespec.Platform{platforms.DefaultSpec()}`
`59`	`58`	`ic.Meta.Exports = map[string]string{"CRIVersion": constants.CRIVersion, "CRIVersionAlpha": constants.CRIVersionAlpha}`
`60`	`59`	`ctx := ic.Context`
`@@ -99,6 +98,8 @@ func initCRIService(ic *plugin.InitContext) (interface{}, error) {`
`99`	`98`	`return nil, fmt.Errorf("failed to create CRI service: %w", err)`
`100`	`99`	`}`
`101`	`100`
	`101`	`+ // RegisterReadiness() must be called after NewCRIService(): https://github.com/containerd/containerd/issues/9163`
	`102`	`+ ready := ic.RegisterReadiness()`
`102`	`103`	`go func() {`
`103`	`104`	`if err := s.Run(ready); err != nil {`
`104`	`105`	`log.G(ctx).WithError(err).Fatal("Failed to run CRI service")`