22522896888

Committed 28 Feb 2026 02:47PM UTC coverage: 22.949% (+0.06%) from 22.886%

Build # 22522896888

Build Type

push

github

Committed by

oilbeater

Commit Message

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 e9b65ce70)

Run Details

2 of 11 new or added lines in 2 files covered. (18.18%)

3 existing lines in 2 files now uncovered.

12395 of 54011 relevant lines covered (22.95%)

0.27 hits per line

Source File
Press 'n' to go to next uncovered line, 'b' for previous

0.0

/pkg/controller/vlan.go

package controller

import (
        "context"
        "fmt"
        "slices"

        k8serrors "k8s.io/apimachinery/pkg/api/errors"
        metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
        "k8s.io/apimachinery/pkg/labels"
        "k8s.io/client-go/tools/cache"
        "k8s.io/klog/v2"

        kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1"
        "github.com/kubeovn/kube-ovn/pkg/ovs"
        "github.com/kubeovn/kube-ovn/pkg/util"
)

func (c *Controller) enqueueAddVlan(obj any) {
        key := cache.MetaObjectToName(obj.(*kubeovnv1.Vlan)).String()
        klog.V(3).Infof("enqueue add vlan %s", key)
        c.addVlanQueue.Add(key)
}

func (c *Controller) enqueueUpdateVlan(_, newObj any) {
        key := cache.MetaObjectToName(newObj.(*kubeovnv1.Vlan)).String()
        klog.V(3).Infof("enqueue update vlan %s", key)
        c.updateVlanQueue.Add(key)
}

func (c *Controller) enqueueDelVlan(obj any) {
        var vlan *kubeovnv1.Vlan
        switch t := obj.(type) {
        case *kubeovnv1.Vlan:
                vlan = t
        case cache.DeletedFinalStateUnknown:
                v, ok := t.Obj.(*kubeovnv1.Vlan)
                if !ok {
                        klog.Warningf("unexpected object type: %T", t.Obj)
                        return
                }
                vlan = v
        default:
                klog.Warningf("unexpected type: %T", obj)
                return
        }

        key := cache.MetaObjectToName(vlan).String()
        klog.V(3).Infof("enqueue delete vlan %s", key)
        c.delVlanQueue.Add(key)
}

func (c *Controller) handleAddVlan(key string) error {
        c.vlanKeyMutex.LockKey(key)
        defer func() { _ = c.vlanKeyMutex.UnlockKey(key) }()
        klog.Infof("handle add vlan %s", key)

        cachedVlan, err := c.vlansLister.Get(key)
        if err != nil {
                if k8serrors.IsNotFound(err) {
                        return nil
                }
                klog.Error(err)
                return err
        }

        vlan := cachedVlan.DeepCopy()
        if vlan.Spec.Provider == "" {
                vlan.Spec.Provider = c.config.DefaultProviderName
                if vlan, err = c.config.KubeOvnClient.KubeovnV1().Vlans().Update(context.Background(), vlan, metav1.UpdateOptions{}); err != nil {
                        klog.Errorf("failed to update vlan %s, %v", vlan.Name, err)
                        return err
                }
        }

        subnets, err := c.subnetsLister.List(labels.Everything())
        if err != nil {
                klog.Errorf("failed to list subnets: %v", err)
                return err
        }

        var needUpdate bool
        for _, subnet := range subnets {
                if subnet.Spec.Vlan == vlan.Name && !slices.Contains(vlan.Status.Subnets, subnet.Name) {
                        vlan.Status.Subnets = append(vlan.Status.Subnets, subnet.Name)
                        needUpdate = true
                }
        }

        if needUpdate {
                vlan, err = c.config.KubeOvnClient.KubeovnV1().Vlans().UpdateStatus(context.Background(), vlan, metav1.UpdateOptions{})
                if err != nil {
                        klog.Errorf("failed to update status of vlan %s: %v", vlan.Name, err)
                        return err
                }
        }

        pn, err := c.providerNetworksLister.Get(vlan.Spec.Provider)
        if err != nil {
                klog.Errorf("failed to get provider network %s: %v", vlan.Spec.Provider, err)
                return err
        }

        if err = c.checkVlanConflict(vlan); err != nil {
                klog.Errorf("failed to check vlan %s: %v", vlan.Name, err)
                return err
        }

        if !slices.Contains(pn.Status.Vlans, vlan.Name) {
                newPn := pn.DeepCopy()
                newPn.Status.Vlans = append(newPn.Status.Vlans, vlan.Name)
                _, err = c.config.KubeOvnClient.KubeovnV1().ProviderNetworks().UpdateStatus(context.Background(), newPn, metav1.UpdateOptions{})
                if err != nil {
                        klog.Errorf("failed to update status of provider network %s: %v", pn.Name, err)
                        return err
                }
        }

        // 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
}

func (c *Controller) checkVlanConflict(vlan *kubeovnv1.Vlan) error {
        if vlan.Spec.ID == 0 {
                // no conflict if vlan id is 0
                return nil
        }
        // todo: check if vlan conflict in webhook
        vlans, err := c.vlansLister.List(labels.Everything())
        if err != nil {
                klog.Errorf("failed to list vlans: %v", err)
                return err
        }
        // check if new vlan conflict with old vlan
        var conflict bool
        var conflictErr error
        for _, v := range vlans {
                // different provider allow to have same vlan
                if vlan.Spec.Provider == v.Spec.Provider && vlan.Spec.ID == v.Spec.ID && vlan.Name != v.Name {
                        conflictErr = fmt.Errorf("provider %s new vlan %s conflict with old vlan %s", vlan.Spec.Provider, vlan.Name, v.Name)
                        klog.Error(conflictErr)
                        conflict = true
                }
        }
        if vlan.Status.Conflict != conflict {
                vlan.Status.Conflict = conflict
                vlan, err = c.config.KubeOvnClient.KubeovnV1().Vlans().UpdateStatus(context.Background(), vlan, metav1.UpdateOptions{})
                if err != nil {
                        klog.Errorf("failed to update conflict status of vlan %s: %v", vlan.Name, err)
                        return err
                }
        }
        return conflictErr
}

func (c *Controller) handleUpdateVlan(key string) error {
        c.vlanKeyMutex.LockKey(key)
        defer func() { _ = c.vlanKeyMutex.UnlockKey(key) }()
        klog.Infof("handle update vlan %s", key)

        vlan, err := c.vlansLister.Get(key)
        if err != nil {
                if k8serrors.IsNotFound(err) {
                        return nil
                }
                klog.Error(err)
                return err
        }

        if vlan.Spec.Provider == "" {
                newVlan := vlan.DeepCopy()
                newVlan.Spec.Provider = c.config.DefaultProviderName
                if vlan, err = c.config.KubeOvnClient.KubeovnV1().Vlans().Update(context.Background(), newVlan, metav1.UpdateOptions{}); err != nil {
                        klog.Errorf("failed to update vlan %s: %v", vlan.Name, err)
                        return err
                }
        }
        newVlan := vlan.DeepCopy()
        if err = c.checkVlanConflict(newVlan); err != nil {
                klog.Errorf("failed to check vlan %s: %v", vlan.Name, err)
                return err
        }
        subnets, err := c.subnetsLister.List(labels.Everything())
        if err != nil {
                klog.Errorf("failed to list subnets: %v", err)
                return err
        }
        for _, subnet := range subnets {
                if subnet.Spec.Vlan == vlan.Name {
                        if err = c.setLocalnetTag(subnet.Name, vlan.Spec.ID); err != nil {
                                klog.Error(err)
                                return err
                        }
                }
        }

        return nil
}

func (c *Controller) handleDelVlan(key string) error {
        c.vlanKeyMutex.LockKey(key)
        defer func() { _ = c.vlanKeyMutex.UnlockKey(key) }()
        klog.Infof("handle delete vlan %s", key)

        subnet, err := c.subnetsLister.List(labels.Everything())
        if err != nil {
                klog.Errorf("failed to list subnets: %v", err)
                return err
        }

        for _, s := range subnet {
                if s.Spec.Vlan == key {
                        c.addOrUpdateSubnetQueue.Add(s.Name)
                }
        }

        providerNetworks, err := c.providerNetworksLister.List(labels.Everything())
        if err != nil && !k8serrors.IsNotFound(err) {
                klog.Errorf("failed to list provider networks: %v", err)
                return err
        }

        for _, pn := range providerNetworks {
                if err = c.updateProviderNetworkStatusForVlanDeletion(pn, key); err != nil {
                        klog.Error(err)
                        return err
                }
        }

        return nil
}

func (c *Controller) updateProviderNetworkStatusForVlanDeletion(pn *kubeovnv1.ProviderNetwork, vlan string) error {
        if !slices.Contains(pn.Status.Vlans, vlan) {
                return nil
        }

        newPn := pn.DeepCopy()
        newPn.Status.Vlans = util.RemoveString(newPn.Status.Vlans, vlan)
        _, err := c.config.KubeOvnClient.KubeovnV1().ProviderNetworks().UpdateStatus(context.Background(), newPn, metav1.UpdateOptions{})
        if err != nil {
                klog.Errorf("failed to update status of provider network %s: %v", pn.Name, err)
                return err
        }
        return nil
}

func (c *Controller) setLocalnetTag(subnet string, vlanID int) error {
        localnetPort := ovs.GetLocalnetName(subnet)
        if err := c.OVNNbClient.SetLogicalSwitchPortVlanTag(localnetPort, vlanID); err != nil {
                klog.Errorf("set localnet port %s vlan tag %d: %v", localnetPort, vlanID, err)
                return err
        }

        return nil
}

func (c *Controller) delLocalnet(subnet string) error {
        localnetPort := ovs.GetLocalnetName(subnet)
        if err := c.OVNNbClient.DeleteLogicalSwitchPort(localnetPort); err != nil {
                klog.Errorf("delete localnet port %s: %v", localnetPort, err)
                return err
        }

        return nil
}

1	package controller
2
3	import (
4	"context"
5	"fmt"
6	"slices"
7
8	k8serrors "k8s.io/apimachinery/pkg/api/errors"
9	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
10	"k8s.io/apimachinery/pkg/labels"
11	"k8s.io/client-go/tools/cache"
12	"k8s.io/klog/v2"
13
14	kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1"
15	"github.com/kubeovn/kube-ovn/pkg/ovs"
16	"github.com/kubeovn/kube-ovn/pkg/util"
17	)
18
19	func (c *Controller) enqueueAddVlan(obj any) {	×
20	key := cache.MetaObjectToName(obj.(*kubeovnv1.Vlan)).String()	×
21	klog.V(3).Infof("enqueue add vlan %s", key)	×
22	c.addVlanQueue.Add(key)	×
23	}	×
24
25	func (c *Controller) enqueueUpdateVlan(_, newObj any) {	×
26	key := cache.MetaObjectToName(newObj.(*kubeovnv1.Vlan)).String()	×
27	klog.V(3).Infof("enqueue update vlan %s", key)	×
28	c.updateVlanQueue.Add(key)	×
29	}	×
30
31	func (c *Controller) enqueueDelVlan(obj any) {	×
32	var vlan *kubeovnv1.Vlan	×
33	switch t := obj.(type) {	×
34	case *kubeovnv1.Vlan:	×
35	vlan = t	×
36	case cache.DeletedFinalStateUnknown:	×
37	v, ok := t.Obj.(*kubeovnv1.Vlan)	×
38	if !ok {	×
39	klog.Warningf("unexpected object type: %T", t.Obj)	×
40	return	×
41	}	×
42	vlan = v	×
43	default:	×
44	klog.Warningf("unexpected type: %T", obj)	×
45	return	×
46	}
47
48	key := cache.MetaObjectToName(vlan).String()	×
49	klog.V(3).Infof("enqueue delete vlan %s", key)	×
50	c.delVlanQueue.Add(key)	×
51	}
52
53	func (c *Controller) handleAddVlan(key string) error {	×
54	c.vlanKeyMutex.LockKey(key)	×
55	defer func() { _ = c.vlanKeyMutex.UnlockKey(key) }()	×
56	klog.Infof("handle add vlan %s", key)	×
57		×
58	cachedVlan, err := c.vlansLister.Get(key)	×
59	if err != nil {	×
60	if k8serrors.IsNotFound(err) {	×
61	return nil	×
62	}	×
63	klog.Error(err)	×
64	return err	×
65	}
66
67	vlan := cachedVlan.DeepCopy()	×
68	if vlan.Spec.Provider == "" {	×
69	vlan.Spec.Provider = c.config.DefaultProviderName	×
70	if vlan, err = c.config.KubeOvnClient.KubeovnV1().Vlans().Update(context.Background(), vlan, metav1.UpdateOptions{}); err != nil {	×
71	klog.Errorf("failed to update vlan %s, %v", vlan.Name, err)	×
72	return err	×
73	}	×
74	}
75
76	subnets, err := c.subnetsLister.List(labels.Everything())	×
77	if err != nil {	×
78	klog.Errorf("failed to list subnets: %v", err)	×
79	return err	×
80	}	×
81
82	var needUpdate bool	×
83	for _, subnet := range subnets {	×
84	if subnet.Spec.Vlan == vlan.Name && !slices.Contains(vlan.Status.Subnets, subnet.Name) {	×
85	vlan.Status.Subnets = append(vlan.Status.Subnets, subnet.Name)	×
86	needUpdate = true	×
87	}	×
88	}
89
90	if needUpdate {	×
91	vlan, err = c.config.KubeOvnClient.KubeovnV1().Vlans().UpdateStatus(context.Background(), vlan, metav1.UpdateOptions{})	×
92	if err != nil {	×
93	klog.Errorf("failed to update status of vlan %s: %v", vlan.Name, err)	×
94	return err	×
95	}	×
96	}
97
98	pn, err := c.providerNetworksLister.Get(vlan.Spec.Provider)	×
99	if err != nil {	×
100	klog.Errorf("failed to get provider network %s: %v", vlan.Spec.Provider, err)	×
101	return err	×
102	}	×
103
104	if err = c.checkVlanConflict(vlan); err != nil {	×
105	klog.Errorf("failed to check vlan %s: %v", vlan.Name, err)	×
106	return err	×
107	}	×
108
109	if !slices.Contains(pn.Status.Vlans, vlan.Name) {	×
110	newPn := pn.DeepCopy()	×
111	newPn.Status.Vlans = append(newPn.Status.Vlans, vlan.Name)	×
112	_, err = c.config.KubeOvnClient.KubeovnV1().ProviderNetworks().UpdateStatus(context.Background(), newPn, metav1.UpdateOptions{})	×
113	if err != nil {	×
114	klog.Errorf("failed to update status of provider network %s: %v", pn.Name, err)	×
115	return err	×
116	}	×
117	}
118
119	// re-enqueue subnets that reference this vlan, so they can proceed
120	// if they were previously blocked by the vlan not being ready
NEW 121	for _, subnet := range subnets {	×
NEW 122	if subnet.Spec.Vlan == vlan.Name {	×
NEW 123	c.addOrUpdateSubnetQueue.Add(subnet.Name)	×
NEW 124	}	×
125	}
126
UNCOV 127	return nil	×
128	}
129
130	func (c Controller) checkVlanConflict(vlan kubeovnv1.Vlan) error {	×
131	if vlan.Spec.ID == 0 {	×
132	// no conflict if vlan id is 0	×
133	return nil	×
134	}	×
135	// todo: check if vlan conflict in webhook
136	vlans, err := c.vlansLister.List(labels.Everything())	×
137	if err != nil {	×
138	klog.Errorf("failed to list vlans: %v", err)	×
139	return err	×
140	}	×
141	// check if new vlan conflict with old vlan
142	var conflict bool	×
143	var conflictErr error	×
144	for _, v := range vlans {	×
145	// different provider allow to have same vlan	×
146	if vlan.Spec.Provider == v.Spec.Provider && vlan.Spec.ID == v.Spec.ID && vlan.Name != v.Name {	×
147	conflictErr = fmt.Errorf("provider %s new vlan %s conflict with old vlan %s", vlan.Spec.Provider, vlan.Name, v.Name)	×
148	klog.Error(conflictErr)	×
149	conflict = true	×
150	}	×
151	}
152	if vlan.Status.Conflict != conflict {	×
153	vlan.Status.Conflict = conflict	×
154	vlan, err = c.config.KubeOvnClient.KubeovnV1().Vlans().UpdateStatus(context.Background(), vlan, metav1.UpdateOptions{})	×
155	if err != nil {	×
156	klog.Errorf("failed to update conflict status of vlan %s: %v", vlan.Name, err)	×
157	return err	×
158	}	×
159	}
160	return conflictErr	×
161	}
162
163	func (c *Controller) handleUpdateVlan(key string) error {	×
164	c.vlanKeyMutex.LockKey(key)	×
165	defer func() { _ = c.vlanKeyMutex.UnlockKey(key) }()	×
166	klog.Infof("handle update vlan %s", key)	×
167		×
168	vlan, err := c.vlansLister.Get(key)	×
169	if err != nil {	×
170	if k8serrors.IsNotFound(err) {	×
171	return nil	×
172	}	×
173	klog.Error(err)	×
174	return err	×
175	}
176
177	if vlan.Spec.Provider == "" {	×
178	newVlan := vlan.DeepCopy()	×
179	newVlan.Spec.Provider = c.config.DefaultProviderName	×
180	if vlan, err = c.config.KubeOvnClient.KubeovnV1().Vlans().Update(context.Background(), newVlan, metav1.UpdateOptions{}); err != nil {	×
181	klog.Errorf("failed to update vlan %s: %v", vlan.Name, err)	×
182	return err	×
183	}	×
184	}
185	newVlan := vlan.DeepCopy()	×
186	if err = c.checkVlanConflict(newVlan); err != nil {	×
187	klog.Errorf("failed to check vlan %s: %v", vlan.Name, err)	×
188	return err	×
189	}	×
190	subnets, err := c.subnetsLister.List(labels.Everything())	×
191	if err != nil {	×
192	klog.Errorf("failed to list subnets: %v", err)	×
193	return err	×
194	}	×
195	for _, subnet := range subnets {	×
196	if subnet.Spec.Vlan == vlan.Name {	×
197	if err = c.setLocalnetTag(subnet.Name, vlan.Spec.ID); err != nil {	×
198	klog.Error(err)	×
199	return err	×
200	}	×
201	}
202	}
203
204	return nil	×
205	}
206
207	func (c *Controller) handleDelVlan(key string) error {	×
208	c.vlanKeyMutex.LockKey(key)	×
209	defer func() { _ = c.vlanKeyMutex.UnlockKey(key) }()	×
210	klog.Infof("handle delete vlan %s", key)	×
211		×
212	subnet, err := c.subnetsLister.List(labels.Everything())	×
213	if err != nil {	×
214	klog.Errorf("failed to list subnets: %v", err)	×
215	return err	×
216	}	×
217
218	for _, s := range subnet {	×
219	if s.Spec.Vlan == key {	×
220	c.addOrUpdateSubnetQueue.Add(s.Name)	×
221	}	×
222	}
223
224	providerNetworks, err := c.providerNetworksLister.List(labels.Everything())	×
225	if err != nil && !k8serrors.IsNotFound(err) {	×
226	klog.Errorf("failed to list provider networks: %v", err)	×
227	return err	×
228	}	×
229
230	for _, pn := range providerNetworks {	×
231	if err = c.updateProviderNetworkStatusForVlanDeletion(pn, key); err != nil {	×
232	klog.Error(err)	×
233	return err	×
234	}	×
235	}
236
237	return nil	×
238	}
239
240	func (c Controller) updateProviderNetworkStatusForVlanDeletion(pn kubeovnv1.ProviderNetwork, vlan string) error {	×
241	if !slices.Contains(pn.Status.Vlans, vlan) {	×
242	return nil	×
243	}	×
244
245	newPn := pn.DeepCopy()	×
246	newPn.Status.Vlans = util.RemoveString(newPn.Status.Vlans, vlan)	×
247	_, err := c.config.KubeOvnClient.KubeovnV1().ProviderNetworks().UpdateStatus(context.Background(), newPn, metav1.UpdateOptions{})	×
248	if err != nil {	×
249	klog.Errorf("failed to update status of provider network %s: %v", pn.Name, err)	×
250	return err	×
251	}	×
252	return nil	×
253	}
254
255	func (c *Controller) setLocalnetTag(subnet string, vlanID int) error {	×
256	localnetPort := ovs.GetLocalnetName(subnet)	×
257	if err := c.OVNNbClient.SetLogicalSwitchPortVlanTag(localnetPort, vlanID); err != nil {	×
258	klog.Errorf("set localnet port %s vlan tag %d: %v", localnetPort, vlanID, err)	×
259	return err	×
260	}	×
261
262	return nil	×
263	}
264
265	func (c *Controller) delLocalnet(subnet string) error {	×
266	localnetPort := ovs.GetLocalnetName(subnet)	×
267	if err := c.OVNNbClient.DeleteLogicalSwitchPort(localnetPort); err != nil {	×
268	klog.Errorf("delete localnet port %s: %v", localnetPort, err)	×
269	return err	×
270	}	×
271
272	return nil	×
273	}

kubeovn / kube-ovn / 22522896888

Source File Press 'n' to go to next uncovered line, 'b' for previous

Source File
Press 'n' to go to next uncovered line, 'b' for previous