Merge pull request containerd#10401 from samuelkarp/nri-panic

cri: ensure NRI API never has nil CRI
oci-playground · Jul 1, 2024 · ebcbbe5 · ebcbbe5
2 parents 37e0f54 + 10aec35
commit ebcbbe5
Show file tree

Hide file tree

Showing 5 changed files with 13 additions and 13 deletions.
diff --git a/internal/cri/nri/nri_api_linux.go b/internal/cri/nri/nri_api_linux.go
@@ -46,9 +46,10 @@ type API struct {
 	nri nri.API
 }
 
-func NewAPI(nri nri.API) *API {
+func NewAPI(nri nri.API, cri CRIImplementation) *API {
 	return &API{
 		nri: nri,
+		cri: cri,
 	}
 }
 
@@ -58,12 +59,11 @@ func (a *API) IsDisabled() bool {
 
 func (a *API) IsEnabled() bool { return !a.IsDisabled() }
 
-func (a *API) Register(cri CRIImplementation) error {
+func (a *API) Register() error {
 	if a.IsDisabled() {
 		return nil
 	}
 
-	a.cri = cri
 	nri.RegisterDomain(a)
 
 	return a.nri.Start()

diff --git a/internal/cri/nri/nri_api_other.go b/internal/cri/nri/nri_api_other.go
@@ -37,11 +37,11 @@ import (
 type API struct {
 }
 
-func NewAPI(nri.API) *API {
+func NewAPI(nri.API, CRIImplementation) *API {
 	return nil
 }
 
-func (a *API) Register(CRIImplementation) error {
+func (a *API) Register() error {
 	return nil
 }
 

diff --git a/internal/cri/server/service.go b/internal/cri/server/service.go
@@ -35,6 +35,7 @@ import (
 	"k8s.io/kubelet/pkg/cri/streaming"
 
 	apitypes "github.com/containerd/containerd/api/types"
+
 	containerd "github.com/containerd/containerd/v2/client"
 	"github.com/containerd/containerd/v2/core/introspection"
 	_ "github.com/containerd/containerd/v2/core/runtime" // for typeurl init
@@ -50,6 +51,7 @@ import (
 	snapshotstore "github.com/containerd/containerd/v2/internal/cri/store/snapshot"
 	ctrdutil "github.com/containerd/containerd/v2/internal/cri/util"
 	"github.com/containerd/containerd/v2/internal/eventq"
+	nriservice "github.com/containerd/containerd/v2/internal/nri"
 	"github.com/containerd/containerd/v2/internal/registrar"
 	"github.com/containerd/containerd/v2/pkg/oci"
 	osinterface "github.com/containerd/containerd/v2/pkg/os"
@@ -164,7 +166,7 @@ type CRIServiceOptions struct {
 
 	StreamingConfig streaming.Config
 
-	NRI *nri.API
+	NRI nriservice.API
 
 	// SandboxControllers is a map of all the loaded sandbox controllers
 	SandboxControllers map[string]sandbox.Controller
@@ -236,7 +238,7 @@ func NewCRIService(options *CRIServiceOptions) (CRIService, runtime.RuntimeServi
 		}
 	}
 
-	c.nri = options.NRI
+	c.nri = nri.NewAPI(options.NRI, &criImplementation{c})
 
 	c.runtimeHandlers, err = c.introspectRuntimeHandlers(ctx)
 	if err != nil {
@@ -297,7 +299,7 @@ func (c *criService) Run(ready func()) error {
 	}()
 
 	// register CRI domain with NRI
-	if err := c.nri.Register(&criImplementation{c}); err != nil {
+	if err := c.nri.Register(); err != nil {
 		return fmt.Errorf("failed to set up NRI for CRI service: %w", err)
 	}
 

diff --git a/internal/nri/nri.go b/internal/nri/nri.go
@@ -38,7 +38,7 @@ type API interface {
 	// IsEnabled returns true if the NRI interface is enabled and initialized.
 	IsEnabled() bool
 
-	// Start start the NRI interface, allowing external NRI plugins to
+	// Start starts the NRI interface, allowing external NRI plugins to
 	// connect, register, and hook themselves into the lifecycle events
 	// of pods and containers.
 	Start() error

diff --git a/plugins/cri/cri.go b/plugins/cri/cri.go
@@ -32,7 +32,6 @@ import (
 	criconfig "github.com/containerd/containerd/v2/internal/cri/config"
 	"github.com/containerd/containerd/v2/internal/cri/constants"
 	"github.com/containerd/containerd/v2/internal/cri/instrument"
-	"github.com/containerd/containerd/v2/internal/cri/nri"
 	"github.com/containerd/containerd/v2/internal/cri/server"
 	nriservice "github.com/containerd/containerd/v2/internal/nri"
 	"github.com/containerd/containerd/v2/plugins"
@@ -212,7 +211,7 @@ func (c criGRPCServerWithTCP) RegisterTCP(s *grpc.Server) error {
 }
 
 // Get the NRI plugin, and set up our NRI API for it.
-func getNRIAPI(ic *plugin.InitContext) *nri.API {
+func getNRIAPI(ic *plugin.InitContext) nriservice.API {
 	const (
 		pluginType = plugins.NRIApiPlugin
 		pluginName = "nri"
@@ -234,8 +233,7 @@ func getNRIAPI(ic *plugin.InitContext) *nri.API {
 	}
 
 	log.G(ctx).Info("using experimental NRI integration - disable nri plugin to prevent this")
-
-	return nri.NewAPI(api)
+	return api
 }
 
 func getSandboxControllers(ic *plugin.InitContext) (map[string]sandbox.Controller, error) {