fix: prevent subnet from getting permanently stuck when VLAN is not ready (#6352)

Fix two bugs that combine to cause underlay subnets to get permanently
stuck during controller startup when the VLAN is created after the subnet.

Bug 1: In handleAddOrUpdateSubnet, variable shadowing (err :=) and
overwriting (err =) in the VLAN/subnet validation error paths caused
patchSubnetStatus success to zero out the original validation error.
The handler returned nil, making the work queue forget the item instead
of retrying it. Fix by using a separate patchErr variable for the patch
call and using = instead of := for the error wrapping.

Bug 2: handleAddVlan did not re-enqueue subnets that reference the
newly created VLAN. Once a subnet's validation failed and was forgotten
by the queue, no event would trigger it to be reprocessed. Fix by
iterating over subnets at the end of handleAddVlan and adding those
referencing the VLAN back to the addOrUpdateSubnetQueue.

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

Author: oilbeater

pkg/controller/controller_test.go

@@ -28,6 +28,8 @@ import (
 	"k8s.io/client-go/informers"
 	coreinformers "k8s.io/client-go/informers/core/v1"
 	"k8s.io/client-go/kubernetes/fake"
+	"k8s.io/client-go/tools/record"
+	"k8s.io/utils/keymutex"
 
 	mockovs "github.com/kubeovn/kube-ovn/mocks/pkg/ovs"
 	kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1"
@@ -43,6 +45,7 @@ type fakeControllerInformers struct {
 	vpcNatGwInformer  kubeovninformer.VpcNatGatewayInformer
 	subnetInformer    kubeovninformer.SubnetInformer
 	ipInformer        kubeovninformer.IPInformer
+	vlanInformer      kubeovninformer.VlanInformer
 	serviceInformer   coreinformers.ServiceInformer
 	namespaceInformer coreinformers.NamespaceInformer
 	podInformer       coreinformers.PodInformer
@@ -60,6 +63,7 @@ func alwaysReady() bool { return true }
 type FakeControllerOptions struct {
 	Subnets            []*kubeovnv1.Subnet
 	IPs                []*kubeovnv1.IP
+	Vlans              []*kubeovnv1.Vlan
 	NetworkAttachments []*nadv1.NetworkAttachmentDefinition
 	Pods               []*corev1.Pod
 	Namespaces         []*corev1.Namespace
@@ -122,6 +126,13 @@ func newFakeControllerWithOptions(t *testing.T, opts *FakeControllerOptions) (*f
 			return nil, err
 		}
 	}
+	for _, vlan := range opts.Vlans {
+		_, err := kubeovnClient.KubeovnV1().Vlans().Create(
+			context.Background(), vlan, metav1.CreateOptions{})
+		if err != nil {
+			return nil, err
+		}
+	}
 
 	// Create informer factories
 	kubeInformerFactory := informers.NewSharedInformerFactory(kubeClient, 0)
@@ -137,12 +148,14 @@ func newFakeControllerWithOptions(t *testing.T, opts *FakeControllerOptions) (*f
 	subnetInformer := kubeovnInformerFactory.Kubeovn().V1().Subnets()
 	ipInformer := kubeovnInformerFactory.Kubeovn().V1().IPs()
 	vpcNatGwInformer := kubeovnInformerFactory.Kubeovn().V1().VpcNatGateways()
+	vlanInformer := kubeovnInformerFactory.Kubeovn().V1().Vlans()
 
 	fakeInformers := &fakeControllerInformers{
 		vpcInformer:       vpcInformer,
 		vpcNatGwInformer:  vpcNatGwInformer,
 		subnetInformer:    subnetInformer,
 		ipInformer:        ipInformer,
+		vlanInformer:      vlanInformer,
 		serviceInformer:   serviceInformer,
 		namespaceInformer: namespaceInformer,
 		podInformer:       podInformer,
@@ -162,10 +175,14 @@ func newFakeControllerWithOptions(t *testing.T, opts *FakeControllerOptions) (*f
 		subnetSynced:            alwaysReady,
 		ipsLister:               ipInformer.Lister(),
 		ipSynced:                alwaysReady,
+		vlansLister:             vlanInformer.Lister(),
 		netAttachLister:         nadInformer.Lister(),
 		netAttachSynced:         alwaysReady,
 		OVNNbClient:             mockOvnClient,
 		ipam:                    ovnipam.NewIPAM(),
+		recorder:                record.NewFakeRecorder(100),
+		subnetKeyMutex:          keymutex.NewHashed(0),
+		addOrUpdateSubnetQueue:  newTypedRateLimitingQueue[string]("AddOrUpdateSubnet", nil),
 		syncVirtualPortsQueue:   newTypedRateLimitingQueue[string]("SyncVirtualPort", nil),
 		updateSubnetStatusQueue: newTypedRateLimitingQueue[string]("UpdateSubnetStatus", nil),
 	}

pkg/controller/subnet.go

@@ -495,20 +495,20 @@ func (c *Controller) handleAddOrUpdateSubnet(key string) error {
 
 	err = c.validateSubnetVlan(subnet)
 	if err != nil {
-		err := fmt.Errorf("failed to validate vlan for subnet %s, %w", key, err)
+		err = fmt.Errorf("failed to validate vlan for subnet %s, %w", key, err)
 		klog.Error(err)
-		if err = c.patchSubnetStatus(subnet, "ValidateSubnetVlanFailed", err.Error()); err != nil {
-			klog.Error(err)
-			return err
+		if patchErr := c.patchSubnetStatus(subnet, "ValidateSubnetVlanFailed", err.Error()); patchErr != nil {
+			klog.Error(patchErr)
+			return patchErr
 		}
 		return err
 	}
 
 	if err = util.ValidateSubnet(*subnet); err != nil {
 		klog.Errorf("failed to validate subnet %s, %v", subnet.Name, err)
-		if err = c.patchSubnetStatus(subnet, "ValidateLogicalSwitchFailed", err.Error()); err != nil {
-			klog.Error(err)
-			return err
+		if patchErr := c.patchSubnetStatus(subnet, "ValidateLogicalSwitchFailed", err.Error()); patchErr != nil {
+			klog.Error(patchErr)
+			return patchErr
 		}
 		return err
 	}

pkg/controller/subnet_test.go

@@ -276,6 +276,34 @@ func Test_formatSubnet(t *testing.T) {
 	}
 }
 
+func Test_handleAddOrUpdateSubnet_vlanValidationError(t *testing.T) {
+	t.Parallel()
+
+	// Create a subnet that references a non-existent vlan
+	subnet := &kubeovnv1.Subnet{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "test-underlay",
+		},
+		Spec: kubeovnv1.SubnetSpec{
+			CIDRBlock: "10.0.0.0/24",
+			Gateway:   "10.0.0.1",
+			Vlan:      "non-existent-vlan",
+		},
+	}
+
+	fakeController, err := newFakeControllerWithOptions(t, &FakeControllerOptions{
+		Subnets: []*kubeovnv1.Subnet{subnet},
+	})
+	require.NoError(t, err)
+	ctrl := fakeController.fakeController
+
+	// handleAddOrUpdateSubnet should return an error when the vlan does not exist,
+	// so that the work queue retries the item instead of forgetting it
+	err = ctrl.handleAddOrUpdateSubnet("test-underlay")
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "failed to validate vlan")
+}
+
 func Test_isOvnSubnet(t *testing.T) {
 	t.Parallel()
 

pkg/controller/vlan.go

@@ -116,6 +116,14 @@ func (c *Controller) handleAddVlan(key string) error {
 		}
 	}
 
+	// re-enqueue subnets that reference this vlan, so they can proceed
+	// if they were previously blocked by the vlan not being ready
+	for _, subnet := range subnets {
+		if subnet.Spec.Vlan == vlan.Name {
+			c.addOrUpdateSubnetQueue.Add(subnet.Name)
+		}
+	}
+
 	return nil
 }