Skip to content

Commit 2346e35

Browse files
authored
Add tests for VIP finalizer handling and subnet status updates (#6068)
* Add tests for VIP finalizer handling and subnet status updates - Introduced a new test to verify that the subnet status is correctly updated when a VIP is created and deleted, ensuring that finalizers are properly handled. - Added checks for both IPv4 and IPv6 protocols, including dual stack scenarios, to confirm that available and using IP counts and ranges are updated as expected. - Enhanced the existing VIP creation test to wait for the finalizer to be added before proceeding with subnet status verification. - Updated sleep durations to ensure sufficient time for status updates after VIP operations. Signed-off-by: zbb88888 <jmdxjsjgcxy@gmail.com> * fix after review Signed-off-by: zbb88888 <jmdxjsjgcxy@gmail.com> --------- Signed-off-by: zbb88888 <jmdxjsjgcxy@gmail.com>
1 parent 14de682 commit 2346e35

File tree

15 files changed

+2142
-993
lines changed

15 files changed

+2142
-993
lines changed

makefiles/e2e.mk

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -205,15 +205,31 @@ vpc-egress-gateway-e2e:
205205
ginkgo $(GINKGO_OUTPUT_OPT) $(GINKGO_PARALLEL_OPT) --randomize-all -v --timeout=30m \
206206
--focus=CNI:Kube-OVN ./test/e2e/vpc-egress-gateway/vpc-egress-gateway.test -- $(TEST_BIN_ARGS)
207207

208-
.PHONY: iptables-vpc-nat-gw-conformance-e2e
209-
iptables-vpc-nat-gw-conformance-e2e:
208+
.PHONY: iptables-eip-conformance-e2e
209+
iptables-eip-conformance-e2e:
210210
ginkgo build $(E2E_BUILD_FLAGS) ./test/e2e/iptables-vpc-nat-gw
211211
E2E_BRANCH=$(E2E_BRANCH) \
212212
E2E_IP_FAMILY=$(E2E_IP_FAMILY) \
213213
E2E_NETWORK_MODE=$(E2E_NETWORK_MODE) \
214214
ginkgo $(GINKGO_OUTPUT_OPT) $(GINKGO_PARALLEL_OPT) --randomize-all -v \
215215
--focus=CNI:Kube-OVN ./test/e2e/iptables-vpc-nat-gw/iptables-vpc-nat-gw.test -- $(TEST_BIN_ARGS)
216216

217+
.PHONY: iptables-eip-qos-conformance-e2e
218+
iptables-eip-qos-conformance-e2e:
219+
ginkgo build $(E2E_BUILD_FLAGS) ./test/e2e/iptables-eip-qos
220+
E2E_BRANCH=$(E2E_BRANCH) \
221+
E2E_IP_FAMILY=$(E2E_IP_FAMILY) \
222+
E2E_NETWORK_MODE=$(E2E_NETWORK_MODE) \
223+
ginkgo $(GINKGO_OUTPUT_OPT) --randomize-all -v \
224+
--focus=CNI:Kube-OVN ./test/e2e/iptables-eip-qos/iptables-eip-qos.test -- $(TEST_BIN_ARGS)
225+
226+
.PHONY: iptables-vpc-nat-gw-conformance-e2e
227+
iptables-vpc-nat-gw-conformance-e2e:
228+
$(MAKE) iptables-eip-conformance-e2e
229+
$(MAKE) iptables-eip-qos-conformance-e2e
230+
231+
232+
217233
.PHONY: ovn-vpc-nat-gw-conformance-e2e
218234
ovn-vpc-nat-gw-conformance-e2e:
219235
ginkgo build $(E2E_BUILD_FLAGS) ./test/e2e/ovn-vpc-nat-gw

pkg/controller/controller.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ type Controller struct {
159159
addIptablesEipQueue workqueue.TypedRateLimitingInterface[string]
160160
updateIptablesEipQueue workqueue.TypedRateLimitingInterface[string]
161161
resetIptablesEipQueue workqueue.TypedRateLimitingInterface[string]
162-
delIptablesEipQueue workqueue.TypedRateLimitingInterface[string]
162+
delIptablesEipQueue workqueue.TypedRateLimitingInterface[*kubeovnv1.IptablesEIP]
163163

164164
iptablesFipsLister kubeovnlister.IptablesFIPRuleLister
165165
iptablesFipSynced cache.InformerSynced
@@ -184,7 +184,7 @@ type Controller struct {
184184
addOvnEipQueue workqueue.TypedRateLimitingInterface[string]
185185
updateOvnEipQueue workqueue.TypedRateLimitingInterface[string]
186186
resetOvnEipQueue workqueue.TypedRateLimitingInterface[string]
187-
delOvnEipQueue workqueue.TypedRateLimitingInterface[string]
187+
delOvnEipQueue workqueue.TypedRateLimitingInterface[*kubeovnv1.OvnEip]
188188

189189
ovnFipsLister kubeovnlister.OvnFipLister
190190
ovnFipSynced cache.InformerSynced
@@ -472,7 +472,7 @@ func Run(ctx context.Context, config *Configuration) {
472472
addIptablesEipQueue: newTypedRateLimitingQueue("AddIptablesEip", custCrdRateLimiter),
473473
updateIptablesEipQueue: newTypedRateLimitingQueue("UpdateIptablesEip", custCrdRateLimiter),
474474
resetIptablesEipQueue: newTypedRateLimitingQueue("ResetIptablesEip", custCrdRateLimiter),
475-
delIptablesEipQueue: newTypedRateLimitingQueue("DeleteIptablesEip", custCrdRateLimiter),
475+
delIptablesEipQueue: newTypedRateLimitingQueue[*kubeovnv1.IptablesEIP]("DeleteIptablesEip", nil),
476476

477477
iptablesFipsLister: iptablesFipInformer.Lister(),
478478
iptablesFipSynced: iptablesFipInformer.Informer().HasSynced,
@@ -563,7 +563,7 @@ func Run(ctx context.Context, config *Configuration) {
563563
addOvnEipQueue: newTypedRateLimitingQueue("AddOvnEip", custCrdRateLimiter),
564564
updateOvnEipQueue: newTypedRateLimitingQueue("UpdateOvnEip", custCrdRateLimiter),
565565
resetOvnEipQueue: newTypedRateLimitingQueue("ResetOvnEip", custCrdRateLimiter),
566-
delOvnEipQueue: newTypedRateLimitingQueue("DeleteOvnEip", custCrdRateLimiter),
566+
delOvnEipQueue: newTypedRateLimitingQueue[*kubeovnv1.OvnEip]("DeleteOvnEip", nil),
567567

568568
ovnFipsLister: ovnFipInformer.Lister(),
569569
ovnFipSynced: ovnFipInformer.Informer().HasSynced,

pkg/controller/ippool.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ func (c *Controller) enqueueUpdateIPPool(oldObj, newObj any) {
5252
newIPPool := newObj.(*kubeovnv1.IPPool)
5353
if !newIPPool.DeletionTimestamp.IsZero() {
5454
klog.V(3).Infof("enqueue delete ippool %s due to deletion timestamp", cache.MetaObjectToName(newIPPool).String())
55-
c.deleteIPPoolQueue.Add(newIPPool.DeepCopy())
55+
c.deleteIPPoolQueue.Add(newIPPool)
5656
return
5757
}
5858
if !slices.Equal(oldIPPool.Spec.Namespaces, newIPPool.Spec.Namespaces) ||

pkg/controller/ovn_dnat.go

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,9 @@ func (c *Controller) enqueueUpdateOvnDnatRule(oldObj, newObj any) {
3434
// avoid delete twice
3535
return
3636
}
37-
klog.Infof("enqueue del ovn dnat %s", key)
38-
c.delOvnDnatRuleQueue.Add(key)
37+
// DNAT with finalizer should be handled in updateOvnDnatRuleQueue
38+
klog.Infof("enqueue update (deleting) ovn dnat %s", key)
39+
c.updateOvnDnatRuleQueue.Add(key)
3940
return
4041
}
4142
oldDnat := oldObj.(*kubeovnv1.OvnDnatRule)
@@ -297,6 +298,48 @@ func (c *Controller) handleUpdateOvnDnatRule(key string) error {
297298
klog.Error(err)
298299
return err
299300
}
301+
302+
// Handle deletion first (for DNATs with finalizers)
303+
if !cachedDnat.DeletionTimestamp.IsZero() {
304+
klog.Infof("handle deleting ovn dnat %s", key)
305+
if cachedDnat.Status.Vpc == "" {
306+
// Already cleaned, just remove finalizer
307+
if err = c.handleDelOvnDnatFinalizer(cachedDnat); err != nil {
308+
klog.Errorf("failed to remove finalizer for ovn dnat %s, %v", cachedDnat.Name, err)
309+
return err
310+
}
311+
return nil
312+
}
313+
314+
// ovn delete dnat
315+
if cachedDnat.Status.V4Eip != "" && cachedDnat.Status.ExternalPort != "" {
316+
if err = c.DelDnatRule(cachedDnat.Status.Vpc, cachedDnat.Name,
317+
cachedDnat.Status.V4Eip, cachedDnat.Status.ExternalPort); err != nil {
318+
klog.Errorf("failed to delete v4 dnat %s, %v", key, err)
319+
return err
320+
}
321+
}
322+
if cachedDnat.Status.V6Eip != "" && cachedDnat.Status.ExternalPort != "" {
323+
if err = c.DelDnatRule(cachedDnat.Status.Vpc, cachedDnat.Name,
324+
cachedDnat.Status.V6Eip, cachedDnat.Status.ExternalPort); err != nil {
325+
klog.Errorf("failed to delete v6 dnat %s, %v", key, err)
326+
return err
327+
}
328+
}
329+
330+
// Remove finalizer
331+
if err = c.handleDelOvnDnatFinalizer(cachedDnat); err != nil {
332+
klog.Errorf("failed to remove finalizer for ovn dnat %s, %v", cachedDnat.Name, err)
333+
return err
334+
}
335+
336+
// Reset eip
337+
if cachedDnat.Spec.OvnEip != "" {
338+
c.resetOvnEipQueue.Add(cachedDnat.Spec.OvnEip)
339+
}
340+
return nil
341+
}
342+
300343
if !cachedDnat.Status.Ready {
301344
// create dnat only in add process, just check to error out here
302345
klog.Infof("wait ovn dnat %s to be ready only in the handle add process", cachedDnat.Name)
@@ -617,6 +660,7 @@ func (c *Controller) handleAddOvnDnatFinalizer(cachedDnat *kubeovnv1.OvnDnatRule
617660
err error
618661
)
619662

663+
controllerutil.RemoveFinalizer(newDnat, util.DepreciatedFinalizerName)
620664
controllerutil.AddFinalizer(newDnat, util.KubeOVNControllerFinalizer)
621665
if patch, err = util.GenerateMergePatchPayload(cachedDnat, newDnat); err != nil {
622666
klog.Errorf("failed to generate patch payload for ovn dnat '%s', %v", cachedDnat.Name, err)
@@ -655,5 +699,12 @@ func (c *Controller) handleDelOvnDnatFinalizer(cachedDnat *kubeovnv1.OvnDnatRule
655699
klog.Errorf("failed to remove finalizer from ovn dnat '%s', %v", cachedDnat.Name, err)
656700
return err
657701
}
702+
703+
// Trigger associated EIP to recheck if it can be deleted now
704+
if cachedDnat.Spec.OvnEip != "" {
705+
klog.Infof("triggering eip %s update after dnat %s deletion", cachedDnat.Spec.OvnEip, cachedDnat.Name)
706+
c.updateOvnEipQueue.Add(cachedDnat.Spec.OvnEip)
707+
}
708+
658709
return nil
659710
}

0 commit comments

Comments
 (0)