Skip to content

Commit b9af62d

Browse files
oilbeateroilbeater
committed
fix: use external_ids to identify vpc policy routes and static routes
Signed-off-by: Mengxin Liu <liumengxinfly@gmail.com>
1 parent f6c95e8 commit b9af62d

File tree

3 files changed

+76
-33
lines changed

3 files changed

+76
-33
lines changed

‎pkg/controller/vpc.go‎

Lines changed: 38 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
2525

2626
kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1"
27+
"github.com/kubeovn/kube-ovn/pkg/ovs"
2728
"github.com/kubeovn/kube-ovn/pkg/ovsdb/ovnnb"
2829
"github.com/kubeovn/kube-ovn/pkg/util"
2930
)
@@ -289,7 +290,23 @@ func (c *Controller) handleAddOrUpdateVpc(key string) error {
289290
return err
290291
}
291292

292-
learnFromARPRequest := true
293+
learnFromARPRequest := vpc.Spec.EnableExternal
294+
if !learnFromARPRequest {
295+
for _, subnetName := range vpc.Status.Subnets {
296+
subnet, err := c.subnetsLister.Get(subnetName)
297+
if err != nil {
298+
if k8serrors.IsNotFound(err) {
299+
continue
300+
}
301+
klog.Errorf("failed to get subnet %s for vpc %s: %v", subnetName, key, err)
302+
return err
303+
}
304+
if subnet.Spec.Vlan != "" && subnet.Spec.U2OInterconnection {
305+
learnFromARPRequest = true
306+
break
307+
}
308+
}
309+
}
293310

294311
if err = c.createVpcRouter(key, learnFromARPRequest); err != nil {
295312
klog.Errorf("failed to create vpc router for vpc %s: %v", key, err)
@@ -319,15 +336,18 @@ func (c *Controller) handleAddOrUpdateVpc(key string) error {
319336
}
320337

321338
// handle static route
339+
staticRouteExternalIDs := map[string]string{
340+
ovs.ExternalIDVendor: util.CniTypeName,
341+
ovs.ExternalIDVpcStaticRoute: "true",
342+
}
343+
322344
var (
323345
staticExistedRoutes []*ovnnb.LogicalRouterStaticRoute
324346
staticTargetRoutes []*kubeovnv1.StaticRoute
325347
staticRouteMapping map[string][]*kubeovnv1.StaticRoute
326-
externalIDs = map[string]string{"vendor": util.CniTypeName}
327348
)
328349

329-
// only manage static routes which are kube-ovn managed, by filtering for vendor util.CniTypeName
330-
staticExistedRoutes, err = c.OVNNbClient.ListLogicalRouterStaticRoutes(vpc.Name, nil, nil, "", externalIDs)
350+
staticExistedRoutes, err = c.OVNNbClient.ListLogicalRouterStaticRoutes(vpc.Name, nil, nil, "", staticRouteExternalIDs)
331351
if err != nil {
332352
klog.Errorf("failed to get vpc %s static route list, %v", vpc.Name, err)
333353
return err
@@ -464,15 +484,15 @@ func (c *Controller) handleAddOrUpdateVpc(key string) error {
464484
if item.BfdID != "" {
465485
klog.Infof("vpc %s add static ecmp route: %+v", vpc.Name, item)
466486
if err = c.OVNNbClient.AddLogicalRouterStaticRoute(
467-
vpc.Name, item.RouteTable, convertPolicy(item.Policy), item.CIDR, &item.BfdID, externalIDs, item.NextHopIP,
487+
vpc.Name, item.RouteTable, convertPolicy(item.Policy), item.CIDR, &item.BfdID, staticRouteExternalIDs, item.NextHopIP,
468488
); err != nil {
469489
klog.Errorf("failed to add bfd static route to vpc %s , %v", vpc.Name, err)
470490
return err
471491
}
472492
} else {
473493
klog.Infof("vpc %s add static route: %+v", vpc.Name, item)
474494
if err = c.OVNNbClient.AddLogicalRouterStaticRoute(
475-
vpc.Name, item.RouteTable, convertPolicy(item.Policy), item.CIDR, nil, externalIDs, item.NextHopIP,
495+
vpc.Name, item.RouteTable, convertPolicy(item.Policy), item.CIDR, nil, staticRouteExternalIDs, item.NextHopIP,
476496
); err != nil {
477497
klog.Errorf("failed to add normal static route to vpc %s , %v", vpc.Name, err)
478498
return err
@@ -481,6 +501,11 @@ func (c *Controller) handleAddOrUpdateVpc(key string) error {
481501
}
482502

483503
// handle policy route
504+
policyExternalIDs := map[string]string{
505+
ovs.ExternalIDVendor: util.CniTypeName,
506+
ovs.ExternalIDVpcPolicyRoute: "true",
507+
}
508+
484509
var (
485510
policyRouteExisted, policyRouteNeedDel, policyRouteNeedAdd []*kubeovnv1.PolicyRoute
486511
policyRouteLogical []*ovnnb.LogicalRouterPolicy
@@ -493,33 +518,30 @@ func (c *Controller) handleAddOrUpdateVpc(key string) error {
493518
policyRouteNeedDel, policyRouteNeedAdd = diffPolicyRouteWithExisted(policyRouteExisted, vpc.Spec.PolicyRoutes)
494519
} else {
495520
if vpc.Spec.PolicyRoutes == nil {
496-
// do not clean default vpc policy routes
497-
if err = c.OVNNbClient.ClearLogicalRouterPolicy(vpc.Name); err != nil {
498-
klog.Errorf("clean all vpc %s policy route failed, %v", vpc.Name, err)
521+
// only clean vpc spec managed policy routes, not those created by subnet or egress gateway controllers
522+
if err = c.OVNNbClient.DeleteLogicalRouterPolicies(vpc.Name, -1, policyExternalIDs); err != nil {
523+
klog.Errorf("clean vpc %s spec policy routes failed, %v", vpc.Name, err)
499524
return err
500525
}
501526
} else {
502-
policyRouteLogical, err = c.OVNNbClient.ListLogicalRouterPolicies(vpc.Name, -1, nil, true)
527+
policyRouteLogical, err = c.OVNNbClient.ListLogicalRouterPolicies(vpc.Name, -1, policyExternalIDs, false)
503528
if err != nil {
504529
klog.Errorf("failed to get vpc %s policy route list, %v", vpc.Name, err)
505530
return err
506531
}
507-
// diff vpc policy route
508532
policyRouteNeedDel, policyRouteNeedAdd = diffPolicyRouteWithLogical(policyRouteLogical, vpc.Spec.PolicyRoutes)
509533
}
510534
}
511-
// delete policies non-exist
512535
for _, item := range policyRouteNeedDel {
513536
klog.Infof("delete policy route for router: %s, priority: %d, match %s", vpc.Name, item.Priority, item.Match)
514537
if err = c.OVNNbClient.DeleteLogicalRouterPolicy(vpc.Name, item.Priority, item.Match); err != nil {
515538
klog.Errorf("del vpc %s policy route failed, %v", vpc.Name, err)
516539
return err
517540
}
518541
}
519-
// add new policies
520542
for _, item := range policyRouteNeedAdd {
521-
klog.Infof("add policy route for router: %s, match %s, action %s, nexthop %s, externalID %v", c.config.ClusterRouter, item.Match, string(item.Action), item.NextHopIP, externalIDs)
522-
if err = c.OVNNbClient.AddLogicalRouterPolicy(vpc.Name, item.Priority, item.Match, string(item.Action), []string{item.NextHopIP}, nil, externalIDs); err != nil {
543+
klog.Infof("add policy route for router: %s, match %s, action %s, nexthop %s, externalID %v", vpc.Name, item.Match, string(item.Action), item.NextHopIP, policyExternalIDs)
544+
if err = c.OVNNbClient.AddLogicalRouterPolicy(vpc.Name, item.Priority, item.Match, string(item.Action), []string{item.NextHopIP}, nil, policyExternalIDs); err != nil {
523545
klog.Errorf("add policy route to vpc %s failed, %v", vpc.Name, err)
524546
return err
525547
}
@@ -580,7 +602,7 @@ func (c *Controller) handleAddOrUpdateVpc(key string) error {
580602
if subnet.Spec.Vpc == key {
581603
// Accelerate subnet update when vpc config is updated.
582604
// In case VPC not set namespaces, subnet will backoff and may take long time to back to ready.
583-
if subnet.Status.IsNotReady() || subnet.Spec.U2OInterconnection {
605+
if subnet.Status.IsNotReady() {
584606
c.addOrUpdateSubnetQueue.Add(subnet.Name)
585607
}
586608
if vpc.Name != util.DefaultVpc && vpc.Spec.EnableBfd && subnet.Spec.EnableEcmp {

‎pkg/controller/vpc_test.go‎

Lines changed: 36 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"k8s.io/utils/keymutex"
1212

1313
kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1"
14+
"github.com/kubeovn/kube-ovn/pkg/ovs"
1415
"github.com/kubeovn/kube-ovn/pkg/ovsdb/ovnnb"
1516
"github.com/kubeovn/kube-ovn/pkg/util"
1617
)
@@ -24,11 +25,12 @@ func Test_handleAddOrUpdateVpc_staticRoutes(t *testing.T) {
2425
srcIPPolicy := ovnnb.LogicalRouterStaticRoutePolicySrcIP
2526
dstIPPolicy := ovnnb.LogicalRouterStaticRoutePolicyDstIP
2627

27-
// Internal static route created directly in OVN with kube-ovn vendor
28+
// Internal static route created directly in OVN with kube-ovn vendor and vpc-static-route label
2829
internalStaticRoute := &ovnnb.LogicalRouterStaticRoute{
2930
UUID: "internal-static-route-uuid",
3031
ExternalIDs: map[string]string{
31-
"vendor": util.CniTypeName,
32+
"vendor": util.CniTypeName,
33+
"vpc-static-route": "true",
3234
},
3335
IPPrefix: "10.0.0.0/24",
3436
Nexthop: "1.2.3.4",
@@ -40,7 +42,8 @@ func Test_handleAddOrUpdateVpc_staticRoutes(t *testing.T) {
4042
managedStaticRoute := &ovnnb.LogicalRouterStaticRoute{
4143
UUID: "managed-static-route-uuid",
4244
ExternalIDs: map[string]string{
43-
"vendor": util.CniTypeName,
45+
"vendor": util.CniTypeName,
46+
"vpc-static-route": "true",
4447
},
4548
IPPrefix: "192.168.0.0/24",
4649
Nexthop: "10.0.0.1",
@@ -89,11 +92,14 @@ func Test_handleAddOrUpdateVpc_staticRoutes(t *testing.T) {
8992
internalStaticRoute,
9093
}
9194

92-
externalIDs := map[string]string{"vendor": util.CniTypeName}
95+
staticRouteExternalIDs := map[string]string{
96+
ovs.ExternalIDVendor: util.CniTypeName,
97+
ovs.ExternalIDVpcStaticRoute: "true",
98+
}
9399

94100
mockOvnClient.EXPECT().CreateLogicalRouter(vpcName).Return(nil)
95101
mockOvnClient.EXPECT().UpdateLogicalRouter(gomock.Any(), gomock.Any()).Return(nil)
96-
mockOvnClient.EXPECT().ListLogicalRouterStaticRoutes(vpcName, nil, nil, "", externalIDs).Return(existingKubeOvnRoutes, nil)
102+
mockOvnClient.EXPECT().ListLogicalRouterStaticRoutes(vpcName, nil, nil, "", staticRouteExternalIDs).Return(existingKubeOvnRoutes, nil)
97103
mockOvnClient.EXPECT().GetLogicalRouter(vpcName, false).Return(&ovnnb.LogicalRouter{
98104
Name: vpcName,
99105
Nat: []string{},
@@ -105,10 +111,11 @@ func Test_handleAddOrUpdateVpc_staticRoutes(t *testing.T) {
105111
"dst-ip",
106112
"192.168.0.0/24",
107113
nil,
108-
externalIDs,
114+
staticRouteExternalIDs,
109115
"10.0.0.1",
110116
).Return(nil)
111-
mockOvnClient.EXPECT().ClearLogicalRouterPolicy(vpcName).Return(nil)
117+
policyExternalIDs := map[string]string{ovs.ExternalIDVendor: util.CniTypeName, ovs.ExternalIDVpcPolicyRoute: "true"}
118+
mockOvnClient.EXPECT().DeleteLogicalRouterPolicies(vpcName, -1, policyExternalIDs).Return(nil)
112119
mockOvnClient.EXPECT().ListLogicalSwitch(gomock.Any(), gomock.Any()).Return([]ovnnb.LogicalSwitch{}, nil).AnyTimes()
113120
mockOvnClient.EXPECT().ListLogicalRouter(gomock.Any(), gomock.Any()).Return([]ovnnb.LogicalRouter{}, nil).AnyTimes()
114121
mockOvnClient.EXPECT().DeleteLogicalRouterPort(fmt.Sprintf("bfd@%s", vpcName)).Return(nil)
@@ -158,17 +165,21 @@ func Test_handleAddOrUpdateVpc_staticRoutes(t *testing.T) {
158165
managedStaticRoute,
159166
}
160167

161-
externalIDs := map[string]string{"vendor": util.CniTypeName}
168+
staticRouteExternalIDs := map[string]string{
169+
ovs.ExternalIDVendor: util.CniTypeName,
170+
ovs.ExternalIDVpcStaticRoute: "true",
171+
}
162172

163173
mockOvnClient.EXPECT().CreateLogicalRouter(vpcName).Return(nil)
164174
mockOvnClient.EXPECT().UpdateLogicalRouter(gomock.Any(), gomock.Any()).Return(nil)
165-
mockOvnClient.EXPECT().ListLogicalRouterStaticRoutes(vpcName, nil, nil, "", externalIDs).Return(existingKubeOvnRoutes, nil)
175+
mockOvnClient.EXPECT().ListLogicalRouterStaticRoutes(vpcName, nil, nil, "", staticRouteExternalIDs).Return(existingKubeOvnRoutes, nil)
166176
mockOvnClient.EXPECT().GetLogicalRouter(vpcName, false).Return(&ovnnb.LogicalRouter{
167177
Name: vpcName,
168178
Nat: []string{},
169179
}, nil)
170180
mockOvnClient.EXPECT().DeleteLogicalRouterStaticRoute(vpcName, gomock.Any(), gomock.Any(), "10.0.0.0/24", "1.2.3.4").Return(nil)
171-
mockOvnClient.EXPECT().ClearLogicalRouterPolicy(vpcName).Return(nil)
181+
policyExternalIDs := map[string]string{ovs.ExternalIDVendor: util.CniTypeName, ovs.ExternalIDVpcPolicyRoute: "true"}
182+
mockOvnClient.EXPECT().DeleteLogicalRouterPolicies(vpcName, -1, policyExternalIDs).Return(nil)
172183
mockOvnClient.EXPECT().ListLogicalSwitch(gomock.Any(), gomock.Any()).Return([]ovnnb.LogicalSwitch{}, nil).AnyTimes()
173184
mockOvnClient.EXPECT().ListLogicalRouter(gomock.Any(), gomock.Any()).Return([]ovnnb.LogicalRouter{}, nil).AnyTimes()
174185
mockOvnClient.EXPECT().DeleteLogicalRouterPort(fmt.Sprintf("bfd@%s", vpcName)).Return(nil)
@@ -211,18 +222,22 @@ func Test_handleAddOrUpdateVpc_staticRoutes(t *testing.T) {
211222
managedStaticRoute,
212223
}
213224

214-
externalIDs := map[string]string{"vendor": util.CniTypeName}
225+
staticRouteExternalIDs := map[string]string{
226+
ovs.ExternalIDVendor: util.CniTypeName,
227+
ovs.ExternalIDVpcStaticRoute: "true",
228+
}
215229

216230
mockOvnClient.EXPECT().CreateLogicalRouter(vpcName).Return(nil)
217231
mockOvnClient.EXPECT().UpdateLogicalRouter(gomock.Any(), gomock.Any()).Return(nil)
218-
mockOvnClient.EXPECT().ListLogicalRouterStaticRoutes(vpcName, nil, nil, "", externalIDs).Return(existingKubeOvnRoutes, nil)
232+
mockOvnClient.EXPECT().ListLogicalRouterStaticRoutes(vpcName, nil, nil, "", staticRouteExternalIDs).Return(existingKubeOvnRoutes, nil)
219233
mockOvnClient.EXPECT().GetLogicalRouter(vpcName, false).Return(&ovnnb.LogicalRouter{
220234
Name: vpcName,
221235
Nat: []string{},
222236
}, nil)
223237
mockOvnClient.EXPECT().DeleteLogicalRouterStaticRoute(vpcName, gomock.Any(), gomock.Any(), "10.0.0.0/24", "1.2.3.4").Return(nil)
224238
mockOvnClient.EXPECT().DeleteLogicalRouterStaticRoute(vpcName, gomock.Any(), gomock.Any(), "192.168.0.0/24", "10.0.0.1").Return(nil)
225-
mockOvnClient.EXPECT().ClearLogicalRouterPolicy(vpcName).Return(nil)
239+
policyExternalIDs := map[string]string{ovs.ExternalIDVendor: util.CniTypeName, ovs.ExternalIDVpcPolicyRoute: "true"}
240+
mockOvnClient.EXPECT().DeleteLogicalRouterPolicies(vpcName, -1, policyExternalIDs).Return(nil)
226241
mockOvnClient.EXPECT().ListLogicalSwitch(gomock.Any(), gomock.Any()).Return([]ovnnb.LogicalSwitch{}, nil).AnyTimes()
227242
mockOvnClient.EXPECT().ListLogicalRouter(gomock.Any(), gomock.Any()).Return([]ovnnb.LogicalRouter{}, nil).AnyTimes()
228243
mockOvnClient.EXPECT().DeleteLogicalRouterPort(fmt.Sprintf("bfd@%s", vpcName)).Return(nil)
@@ -267,11 +282,14 @@ func Test_handleAddOrUpdateVpc_staticRoutes(t *testing.T) {
267282
err = fakeinformers.vpcInformer.Informer().GetStore().Add(vpc)
268283
require.NoError(t, err)
269284

270-
externalIDs := map[string]string{"vendor": util.CniTypeName}
285+
staticRouteExternalIDs := map[string]string{
286+
ovs.ExternalIDVendor: util.CniTypeName,
287+
ovs.ExternalIDVpcStaticRoute: "true",
288+
}
271289

272290
mockOvnClient.EXPECT().CreateLogicalRouter(vpcName).Return(nil)
273291
mockOvnClient.EXPECT().UpdateLogicalRouter(gomock.Any(), gomock.Any()).Return(nil)
274-
mockOvnClient.EXPECT().ListLogicalRouterStaticRoutes(vpcName, nil, nil, "", externalIDs).Return(nil, nil)
292+
mockOvnClient.EXPECT().ListLogicalRouterStaticRoutes(vpcName, nil, nil, "", staticRouteExternalIDs).Return(nil, nil)
275293
mockOvnClient.EXPECT().GetLogicalRouter(vpcName, false).Return(&ovnnb.LogicalRouter{
276294
Name: vpcName,
277295
Nat: []string{},
@@ -282,10 +300,11 @@ func Test_handleAddOrUpdateVpc_staticRoutes(t *testing.T) {
282300
"dst-ip",
283301
"192.168.0.0/24",
284302
nil,
285-
externalIDs,
303+
staticRouteExternalIDs,
286304
"10.0.0.1",
287305
).Return(nil)
288-
mockOvnClient.EXPECT().ClearLogicalRouterPolicy(vpcName).Return(nil)
306+
policyExternalIDs := map[string]string{ovs.ExternalIDVendor: util.CniTypeName, ovs.ExternalIDVpcPolicyRoute: "true"}
307+
mockOvnClient.EXPECT().DeleteLogicalRouterPolicies(vpcName, -1, policyExternalIDs).Return(nil)
289308
mockOvnClient.EXPECT().ListLogicalSwitch(gomock.Any(), gomock.Any()).Return([]ovnnb.LogicalSwitch{}, nil).AnyTimes()
290309
mockOvnClient.EXPECT().ListLogicalRouter(gomock.Any(), gomock.Any()).Return([]ovnnb.LogicalRouter{}, nil).AnyTimes()
291310
mockOvnClient.EXPECT().DeleteLogicalRouterPort(fmt.Sprintf("bfd@%s", vpcName)).Return(nil)

‎pkg/ovs/ovn.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ const (
5252

5353
ExternalIDVendor = "vendor"
5454
ExternalIDVpcEgressGateway = "vpc-egress-gateway"
55+
ExternalIDVpcPolicyRoute = "vpc-policy-route"
56+
ExternalIDVpcStaticRoute = "vpc-static-route"
5557
)
5658

5759
// NewLegacyClient init a legacy ovn client

0 commit comments

Comments
 (0)