fix(controller): prevent NPE in isOvnSubnet when subnet is nil (kubeovn#6312)

When an IP CR references a deleted subnet, handleUpdateIP calls
isOvnSubnet with a nil subnet pointer, causing a panic that puts
the controller into CrashLoopBackOff. Add a nil check so that
isOvnSubnet safely returns false for nil input.

Fixes kubeovn#6309

Signed-off-by: Mengxin Liu <liumengxinfly@gmail.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
(cherry picked from commit 3a28ee1)
(cherry picked from commit be347ca)

Author: 2 people

pkg/controller/controller_test.go

@@ -34,13 +34,15 @@ import (
 	kubeovnfake "github.com/kubeovn/kube-ovn/pkg/client/clientset/versioned/fake"
 	kubeovninformerfactory "github.com/kubeovn/kube-ovn/pkg/client/informers/externalversions"
 	kubeovninformer "github.com/kubeovn/kube-ovn/pkg/client/informers/externalversions/kubeovn/v1"
+	ovnipam "github.com/kubeovn/kube-ovn/pkg/ipam"
 	"github.com/kubeovn/kube-ovn/pkg/util"
 )
 
 type fakeControllerInformers struct {
 	vpcInformer       kubeovninformer.VpcInformer
 	vpcNatGwInformer  kubeovninformer.VpcNatGatewayInformer
 	subnetInformer    kubeovninformer.SubnetInformer
+	ipInformer        kubeovninformer.IPInformer
 	serviceInformer   coreinformers.ServiceInformer
 	namespaceInformer coreinformers.NamespaceInformer
 	podInformer       coreinformers.PodInformer
@@ -57,6 +59,7 @@ func alwaysReady() bool { return true }
 // FakeControllerOptions holds optional parameters for creating a fake controller
 type FakeControllerOptions struct {
 	Subnets            []*kubeovnv1.Subnet
+	IPs                []*kubeovnv1.IP
 	NetworkAttachments []*nadv1.NetworkAttachmentDefinition
 	Pods               []*corev1.Pod
 	Namespaces         []*corev1.Namespace
@@ -112,6 +115,13 @@ func newFakeControllerWithOptions(t *testing.T, opts *FakeControllerOptions) (*f
 			return nil, err
 		}
 	}
+	for _, ip := range opts.IPs {
+		_, err := kubeovnClient.KubeovnV1().IPs().Create(
+			context.Background(), ip, metav1.CreateOptions{})
+		if err != nil {
+			return nil, err
+		}
+	}
 
 	// Create informer factories
 	kubeInformerFactory := informers.NewSharedInformerFactory(kubeClient, 0)
@@ -125,12 +135,14 @@ func newFakeControllerWithOptions(t *testing.T, opts *FakeControllerOptions) (*f
 	kubeovnInformerFactory := kubeovninformerfactory.NewSharedInformerFactory(kubeovnClient, 0)
 	vpcInformer := kubeovnInformerFactory.Kubeovn().V1().Vpcs()
 	subnetInformer := kubeovnInformerFactory.Kubeovn().V1().Subnets()
+	ipInformer := kubeovnInformerFactory.Kubeovn().V1().IPs()
 	vpcNatGwInformer := kubeovnInformerFactory.Kubeovn().V1().VpcNatGateways()
 
 	fakeInformers := &fakeControllerInformers{
 		vpcInformer:       vpcInformer,
 		vpcNatGwInformer:  vpcNatGwInformer,
 		subnetInformer:    subnetInformer,
+		ipInformer:        ipInformer,
 		serviceInformer:   serviceInformer,
 		namespaceInformer: namespaceInformer,
 		podInformer:       podInformer,
@@ -141,17 +153,21 @@ func newFakeControllerWithOptions(t *testing.T, opts *FakeControllerOptions) (*f
 
 	// Create controller with all informers
 	ctrl := &Controller{
-		servicesLister:        serviceInformer.Lister(),
-		namespacesLister:      namespaceInformer.Lister(),
-		podsLister:            podInformer.Lister(),
-		vpcsLister:            vpcInformer.Lister(),
-		vpcSynced:             alwaysReady,
-		subnetsLister:         subnetInformer.Lister(),
-		subnetSynced:          alwaysReady,
-		netAttachLister:       nadInformer.Lister(),
-		netAttachSynced:       alwaysReady,
-		OVNNbClient:           mockOvnClient,
-		syncVirtualPortsQueue: newTypedRateLimitingQueue[string]("SyncVirtualPort", nil),
+		servicesLister:          serviceInformer.Lister(),
+		namespacesLister:        namespaceInformer.Lister(),
+		podsLister:              podInformer.Lister(),
+		vpcsLister:              vpcInformer.Lister(),
+		vpcSynced:               alwaysReady,
+		subnetsLister:           subnetInformer.Lister(),
+		subnetSynced:            alwaysReady,
+		ipsLister:               ipInformer.Lister(),
+		ipSynced:                alwaysReady,
+		netAttachLister:         nadInformer.Lister(),
+		netAttachSynced:         alwaysReady,
+		OVNNbClient:             mockOvnClient,
+		ipam:                    ovnipam.NewIPAM(),
+		syncVirtualPortsQueue:   newTypedRateLimitingQueue[string]("SyncVirtualPort", nil),
+		updateSubnetStatusQueue: newTypedRateLimitingQueue[string]("UpdateSubnetStatus", nil),
 	}
 
 	ctrl.config = &Configuration{

pkg/controller/ip.go

@@ -238,8 +238,12 @@ func (c *Controller) handleUpdateIP(key string) error {
 		klog.Infof("handle deleting ip %s", cachedIP.Name)
 		subnet, err := c.subnetsLister.Get(cachedIP.Spec.Subnet)
 		if err != nil {
-			klog.Errorf("failed to get subnet %s: %v", cachedIP.Spec.Subnet, err)
-			return err
+			if !k8serrors.IsNotFound(err) {
+				klog.Errorf("failed to get subnet %s: %v", cachedIP.Spec.Subnet, err)
+				return err
+			}
+			// subnet already deleted; isOvnSubnet(nil) returns false, skip OVN port cleanup
+			subnet = nil
 		}
 		portName := cachedIP.Name
 		if isOvnSubnet(subnet) {

pkg/controller/ip_test.go

@@ -0,0 +1,50 @@
+package controller
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1"
+	"github.com/kubeovn/kube-ovn/pkg/util"
+)
+
+func Test_handleUpdateIP_deletedSubnet(t *testing.T) {
+	t.Parallel()
+
+	now := metav1.Now()
+	ip := &kubeovnv1.IP{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:              "test-ip",
+			DeletionTimestamp: &now,
+			Finalizers:        []string{util.KubeOVNControllerFinalizer},
+		},
+		Spec: kubeovnv1.IPSpec{
+			Subnet:    "deleted-subnet",
+			Namespace: "default",
+			PodName:   "test-pod",
+		},
+	}
+
+	fakeCtrl, err := newFakeControllerWithOptions(t, &FakeControllerOptions{
+		IPs: []*kubeovnv1.IP{ip},
+	})
+	require.NoError(t, err)
+
+	ctrl := fakeCtrl.fakeController
+
+	// Shut down work queues to avoid goroutine leaks
+	t.Cleanup(func() {
+		ctrl.updateSubnetStatusQueue.ShutDown()
+		ctrl.syncVirtualPortsQueue.ShutDown()
+	})
+
+	// The subnet "deleted-subnet" does not exist in the fake client.
+	// This must not panic (previously caused NPE in isOvnSubnet).
+	err = ctrl.handleUpdateIP("test-ip")
+	require.NoError(t, err)
+
+	// Verify the subnet status update was enqueued
+	require.Equal(t, 1, ctrl.updateSubnetStatusQueue.Len())
+}

pkg/controller/subnet.go

@@ -1904,7 +1904,7 @@ func (c *Controller) releaseMcastQuerierIP(subnet *kubeovnv1.Subnet) (bool, erro
 }
 
 func isOvnSubnet(subnet *kubeovnv1.Subnet) bool {
-	return util.IsOvnProvider(subnet.Spec.Provider)
+	return subnet != nil && util.IsOvnProvider(subnet.Spec.Provider)
 }
 
 func formatExcludeIPRanges(subnet *kubeovnv1.Subnet) {

pkg/controller/subnet_test.go

@@ -12,6 +12,7 @@ import (
 
 	kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1"
 	"github.com/kubeovn/kube-ovn/pkg/ovsdb/ovnnb"
+	"github.com/kubeovn/kube-ovn/pkg/util"
 )
 
 func Test_reconcileVips(t *testing.T) {
@@ -274,3 +275,46 @@ func Test_formatSubnet(t *testing.T) {
 		})
 	}
 }
+
+func Test_isOvnSubnet(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name   string
+		subnet *kubeovnv1.Subnet
+		want   bool
+	}{
+		{
+			name:   "nil subnet returns false",
+			subnet: nil,
+			want:   false,
+		},
+		{
+			name: "empty provider defaults to OVN",
+			subnet: &kubeovnv1.Subnet{
+				Spec: kubeovnv1.SubnetSpec{Provider: ""},
+			},
+			want: true,
+		},
+		{
+			name: "explicit OVN provider",
+			subnet: &kubeovnv1.Subnet{
+				Spec: kubeovnv1.SubnetSpec{Provider: util.OvnProvider},
+			},
+			want: true,
+		},
+		{
+			name: "non-OVN provider",
+			subnet: &kubeovnv1.Subnet{
+				Spec: kubeovnv1.SubnetSpec{Provider: "external.provider"},
+			},
+			want: false,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			require.Equal(t, tc.want, isOvnSubnet(tc.subnet))
+		})
+	}
+}