From f0c4456e92ca07773ab0ceb477e3a03eda07c4ee Mon Sep 17 00:00:00 2001 From: Mengxin Liu Date: Sat, 21 Feb 2026 01:44:40 +0000 Subject: [PATCH] fix(controller): prevent NPE in isOvnSubnet when subnet is nil 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 #6309 Signed-off-by: Mengxin Liu Co-Authored-By: Claude Opus 4.6 Signed-off-by: Mengxin Liu --- pkg/controller/controller_test.go | 38 ++++++++++++++++------- pkg/controller/ip_test.go | 50 +++++++++++++++++++++++++++++++ pkg/controller/subnet.go | 2 +- pkg/controller/subnet_test.go | 43 ++++++++++++++++++++++++++ 4 files changed, 121 insertions(+), 12 deletions(-) create mode 100644 pkg/controller/ip_test.go diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index 5965e2c0710..d50e70e8b8b 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -34,6 +34,7 @@ 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" ) @@ -41,6 +42,7 @@ 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 @@ -110,6 +113,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.NewSharedInformerFactoryWithOptions(kubeClient, 0, @@ -140,12 +150,14 @@ func newFakeControllerWithOptions(t *testing.T, opts *FakeControllerOptions) (*f ) 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, @@ -156,17 +168,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{ diff --git a/pkg/controller/ip_test.go b/pkg/controller/ip_test.go new file mode 100644 index 00000000000..75ef9a6edab --- /dev/null +++ b/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()) +} diff --git a/pkg/controller/subnet.go b/pkg/controller/subnet.go index 4f5184938a7..f323f29df06 100644 --- a/pkg/controller/subnet.go +++ b/pkg/controller/subnet.go @@ -1882,7 +1882,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) { diff --git a/pkg/controller/subnet_test.go b/pkg/controller/subnet_test.go index 8671e91ef91..c3839c690ef 100644 --- a/pkg/controller/subnet_test.go +++ b/pkg/controller/subnet_test.go @@ -275,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)) + }) + } +}