OCPBUGS-105432: fix nil pointer panic in IRI controller informer race

pkg/controller/internalreleaseimage/internalreleaseimage_controller.go

@@ -111,6 +111,36 @@ func New(
 
 	ctrl.syncHandler = ctrl.syncInternalReleaseImage
 
+	// Assign the listers and their HasSynced functions BEFORE registering the
+	// event handlers below. AddEventHandler on an already-started informer
+	// replays synthetic Add events from a separate goroutine, which can invoke
+	// handlers (e.g. isControlPlaneNode/isNodeReady, which dereference
+	// ctrl.nodeLister) before these fields are assigned. Assigning the listers
+	// first eliminates that nil-pointer race.
+	ctrl.iriLister = iriInformer.Lister()
+	ctrl.iriListerSynced = iriInformer.Informer().HasSynced
+
+	ctrl.ccLister = ccInformer.Lister()
+	ctrl.ccListerSynced = ccInformer.Informer().HasSynced
+
+	ctrl.mcLister = mcInformer.Lister()
+	ctrl.mcListerSynced = mcInformer.Informer().HasSynced
+
+	ctrl.clusterVersionLister = clusterVersionInformer.Lister()
+	ctrl.clusterVersionListerSynced = clusterVersionInformer.Informer().HasSynced
+
+	ctrl.secretLister = secretInformer.Lister()
+	ctrl.secretListerSynced = secretInformer.Informer().HasSynced
+
+	ctrl.mcnLister = mcnInformer.Lister()
+	ctrl.mcnListerSynced = mcnInformer.Informer().HasSynced
+
+	ctrl.nodeLister = nodeInformer.Lister()
+	ctrl.nodeListerSynced = nodeInformer.Informer().HasSynced
+
+	ctrl.infraLister = infraInformer.Lister()
+	ctrl.infraListerSynced = infraInformer.Informer().HasSynced
+
 	iriInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
 		AddFunc:    ctrl.addInternalReleaseImage,
 		UpdateFunc: ctrl.updateInternalReleaseImage,
@@ -144,30 +174,6 @@ func New(
 		UpdateFunc: ctrl.updateNode,
 	})
 
-	ctrl.iriLister = iriInformer.Lister()
-	ctrl.iriListerSynced = iriInformer.Informer().HasSynced
-
-	ctrl.ccLister = ccInformer.Lister()
-	ctrl.ccListerSynced = ccInformer.Informer().HasSynced
-
-	ctrl.mcLister = mcInformer.Lister()
-	ctrl.mcListerSynced = mcInformer.Informer().HasSynced
-
-	ctrl.clusterVersionLister = clusterVersionInformer.Lister()
-	ctrl.clusterVersionListerSynced = clusterVersionInformer.Informer().HasSynced
-
-	ctrl.secretLister = secretInformer.Lister()
-	ctrl.secretListerSynced = secretInformer.Informer().HasSynced
-
-	ctrl.mcnLister = mcnInformer.Lister()
-	ctrl.mcnListerSynced = mcnInformer.Informer().HasSynced
-
-	ctrl.nodeLister = nodeInformer.Lister()
-	ctrl.nodeListerSynced = nodeInformer.Informer().HasSynced
-
-	ctrl.infraLister = infraInformer.Lister()
-	ctrl.infraListerSynced = infraInformer.Informer().HasSynced
-
 	return ctrl
 }
 
@@ -384,6 +390,16 @@ func (ctrl *Controller) updateNode(_, cur interface{}) {
 // isControlPlaneNode checks if a node is a control plane node by checking its labels.
 // Returns true if the node has the master or control-plane role label.
 func (ctrl *Controller) isControlPlaneNode(nodeName string) bool {
+	// Defensive guard: the nodeLister may not be assigned yet if an event
+	// handler fires during controller construction. Treat the node as
+	// non-control-plane rather than panicking on a nil lister.
+	if ctrl.nodeLister == nil {
+		// Do not log the node name: node names can contain internal hostnames
+		// or other infrastructure identifiers. Log only the lister state.
+		klog.Warning("nodeLister not initialized yet; treating node as non-control-plane")
+		return false
+	}
+
 	node, err := ctrl.nodeLister.Get(nodeName)
 	if err != nil {
 		klog.V(4).Infof("Failed to get node %s: %v", nodeName, err)
@@ -403,6 +419,16 @@ func (ctrl *Controller) isControlPlaneNode(nodeName string) bool {
 
 // isNodeReady checks if a node is ready by examining its Ready condition.
 func (ctrl *Controller) isNodeReady(nodeName string) bool {
+	// Defensive guard: the nodeLister may not be assigned yet if an event
+	// handler fires during controller construction. Treat the node as not
+	// ready rather than panicking on a nil lister.
+	if ctrl.nodeLister == nil {
+		// Do not log the node name: node names can contain internal hostnames
+		// or other infrastructure identifiers. Log only the lister state.
+		klog.Warning("nodeLister not initialized yet; treating node as not ready")
+		return false
+	}
+
 	node, err := ctrl.nodeLister.Get(nodeName)
 	if err != nil {
 		klog.V(4).Infof("Failed to get node %s: %v", nodeName, err)

pkg/controller/internalreleaseimage/internalreleaseimage_controller_test.go

@@ -501,3 +501,81 @@ func TestTransformToAPIIntURL(t *testing.T) {
 		})
 	}
 }
+
+// TestNewWithAlreadyStartedInformers is a regression test for the nil-pointer
+// panic caused by an informer race in New(). When an informer is already
+// started, AddEventHandler replays the current cache contents as synthetic Add
+// events on a separate goroutine. If the listers were assigned after the event
+// handlers were registered, the replayed MachineConfigNode Add would invoke
+// isControlPlaneNode -> ctrl.nodeLister.Get on a nil nodeLister and panic.
+//
+// This test preloads and starts the informers (in particular mcnInformer and
+// nodeInformer) BEFORE calling New, exercising that ordering and asserting that
+// construction completes without panicking and that the replayed event is
+// handled.
+func TestNewWithAlreadyStartedInformers(t *testing.T) {
+	// A healthy MachineConfigNode plus its control-plane Node so that the
+	// replayed Add event drives addMachineConfigNode -> isControlPlaneNode,
+	// which dereferences the node lister.
+	mcfgClient := fake.NewSimpleClientset(mcn("master-0").build())
+	k8sClient := k8sfake.NewSimpleClientset(node("master-0").build())
+	configClient := fakeconfigv1client.NewSimpleClientset()
+
+	i := mcfginformers.NewSharedInformerFactory(mcfgClient, 0)
+	k := informers.NewSharedInformerFactory(k8sClient, 0)
+	ci := configinformers.NewSharedInformerFactory(configClient, 0)
+
+	iriInformer := i.Machineconfiguration().V1().InternalReleaseImages()
+	ccInformer := i.Machineconfiguration().V1().ControllerConfigs()
+	mcInformer := i.Machineconfiguration().V1().MachineConfigs()
+	cvInformer := ci.Config().V1().ClusterVersions()
+	secretInformer := k.Core().V1().Secrets()
+	mcnInformer := i.Machineconfiguration().V1().MachineConfigNodes()
+	nodeInformer := k.Core().V1().Nodes()
+	infraInformer := ci.Config().V1().Infrastructures()
+
+	// Instantiate each informer so the factories start and sync them below.
+	iriInformer.Informer()
+	ccInformer.Informer()
+	mcInformer.Informer()
+	cvInformer.Informer()
+	secretInformer.Informer()
+	mcnInformer.Informer()
+	nodeInformer.Informer()
+	infraInformer.Informer()
+
+	stopCh := make(chan struct{})
+	defer close(stopCh)
+
+	// Start and sync the informers BEFORE constructing the controller. This is
+	// the ordering that previously triggered the nil-pointer race.
+	i.Start(stopCh)
+	k.Start(stopCh)
+	ci.Start(stopCh)
+	i.WaitForCacheSync(stopCh)
+	k.WaitForCacheSync(stopCh)
+	ci.WaitForCacheSync(stopCh)
+
+	c := New(
+		iriInformer,
+		ccInformer,
+		mcInformer,
+		cvInformer,
+		secretInformer,
+		mcnInformer,
+		nodeInformer,
+		infraInformer,
+		k8sClient,
+		mcfgClient,
+	)
+	assert.NotNil(t, c)
+
+	// The replayed Add event for the control-plane MachineConfigNode must be
+	// handled by addMachineConfigNode without panicking, which enqueues the IRI
+	// singleton. Waiting for the enqueue proves the handler ran to completion
+	// against a non-nil node lister.
+	assert.Eventually(t, func() bool {
+		return c.queue.Len() > 0
+	}, 5*time.Second, 10*time.Millisecond,
+		"expected replayed MachineConfigNode Add to be handled and enqueue the IRI without panicking")
+}