Skip to content

Commit 436b519

Browse files
oilbeateroilbeater
authored
refactor(controller): clean up redundant logic in vpc controller (#6295)
* chore: remove unnecessary updates Signed-off-by: Mengxin Liu <liumengxinfly@gmail.com> * clean up vpc controller Signed-off-by: Mengxin Liu <liumengxinfly@gmail.com> --------- Signed-off-by: Mengxin Liu <liumengxinfly@gmail.com>
1 parent a0e4135 commit 436b519

File tree

1 file changed

+44
-92
lines changed

1 file changed

+44
-92
lines changed

‎pkg/controller/vpc.go‎

Lines changed: 44 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -468,21 +468,29 @@ func (c *Controller) handleAddOrUpdateVpc(key string) error {
468468
// Add static routes created by addCustomVPCStaticRouteForSubnet
469469
for _, subnet := range subnets {
470470
if subnet.Spec.Vpc == key {
471-
staticTargetRoutes = append(staticTargetRoutes, &kubeovnv1.StaticRoute{
472-
Policy: kubeovnv1.PolicySrc,
473-
CIDR: subnet.Spec.CIDRBlock,
474-
NextHopIP: subnet.Spec.Gateway,
475-
RouteTable: subnet.Spec.RouteTable,
476-
})
471+
v4Gw, v6Gw := util.SplitStringIP(subnet.Spec.Gateway)
472+
v4Cidr, v6Cidr := util.SplitStringIP(subnet.Spec.CIDRBlock)
473+
if v4Gw != "" && v4Cidr != "" {
474+
staticTargetRoutes = append(staticTargetRoutes, &kubeovnv1.StaticRoute{
475+
Policy: kubeovnv1.PolicySrc,
476+
CIDR: v4Cidr,
477+
NextHopIP: v4Gw,
478+
RouteTable: subnet.Spec.RouteTable,
479+
})
480+
}
481+
if v6Gw != "" && v6Cidr != "" {
482+
staticTargetRoutes = append(staticTargetRoutes, &kubeovnv1.StaticRoute{
483+
Policy: kubeovnv1.PolicySrc,
484+
CIDR: v6Cidr,
485+
NextHopIP: v6Gw,
486+
RouteTable: subnet.Spec.RouteTable,
487+
})
488+
}
477489
}
478490
}
479491
}
480492

481-
routeNeedDel, routeNeedAdd, err := diffStaticRoute(staticExistedRoutes, staticTargetRoutes)
482-
if err != nil {
483-
klog.Errorf("failed to diff vpc %s static route, %v", vpc.Name, err)
484-
return err
485-
}
493+
routeNeedDel, routeNeedAdd := diffStaticRoute(staticExistedRoutes, staticTargetRoutes)
486494

487495
for _, item := range routeNeedDel {
488496
klog.Infof("vpc %s del static route: %+v", vpc.Name, item)
@@ -608,7 +616,6 @@ func (c *Controller) handleAddOrUpdateVpc(key string) error {
608616
if subnet.Status.IsNotReady() {
609617
c.addOrUpdateSubnetQueue.Add(subnet.Name)
610618
}
611-
c.addOrUpdateSubnetQueue.Add(subnet.Name)
612619
if vpc.Name != util.DefaultVpc && vpc.Spec.EnableBfd && subnet.Spec.EnableEcmp {
613620
custVpcEnableExternalEcmp = true
614621
}
@@ -760,7 +767,7 @@ func (c *Controller) handleUpdateVpcExternal(vpc *kubeovnv1.Vpc, custVpcEnableEx
760767
}
761768
}
762769

763-
if err := c.updateVpcExternalStatus(vpc.Name, vpc.Spec.EnableExternal); err != nil {
770+
if err := c.updateVpcExternalStatus(vpc.Name); err != nil {
764771
klog.Errorf("failed to update vpc external subnets status, %v", err)
765772
return err
766773
}
@@ -810,15 +817,15 @@ func (c *Controller) reconcileVpcBfdLRP(vpc *kubeovnv1.Vpc) (string, []string, e
810817
nodeNames := make([]string, 0, len(nodes))
811818
chassisCount = min(chassisCount, len(nodes))
812819
chassisNames := make([]string, 0, chassisCount)
813-
for _, nodes := range nodes[:chassisCount] {
814-
chassis, err := c.OVNSbClient.GetChassisByHost(nodes.Name)
820+
for _, node := range nodes[:chassisCount] {
821+
chassis, err := c.OVNSbClient.GetChassisByHost(node.Name)
815822
if err != nil {
816-
err = fmt.Errorf("failed to get chassis of node %s: %w", nodes.Name, err)
823+
err = fmt.Errorf("failed to get chassis of node %s: %w", node.Name, err)
817824
klog.Error(err)
818825
return portName, nil, err
819826
}
820827
chassisNames = append(chassisNames, chassis.Name)
821-
nodeNames = append(nodeNames, nodes.Name)
828+
nodeNames = append(nodeNames, node.Name)
822829
}
823830

824831
networks := strings.Split(vpc.Spec.BFDPort.IP, ",")
@@ -896,40 +903,14 @@ func (c *Controller) batchAddPolicyRouteToVpc(name string, policies []*kubeovnv1
896903
}
897904

898905
func (c *Controller) deletePolicyRouteFromVpc(vpcName string, priority int, match string) error {
899-
var (
900-
vpc, cachedVpc *kubeovnv1.Vpc
901-
err error
902-
)
903-
904-
if err = c.OVNNbClient.DeleteLogicalRouterPolicy(vpcName, priority, match); err != nil {
905-
klog.Error(err)
906-
return err
907-
}
908-
909-
cachedVpc, err = c.vpcsLister.Get(vpcName)
910-
if err != nil {
911-
if k8serrors.IsNotFound(err) {
912-
return nil
913-
}
914-
klog.Error(err)
915-
return err
916-
}
917-
vpc = cachedVpc.DeepCopy()
918-
// make sure custom policies not be deleted
919-
_, err = c.config.KubeOvnClient.KubeovnV1().Vpcs().Update(context.Background(), vpc, metav1.UpdateOptions{})
920-
if err != nil {
906+
if err := c.OVNNbClient.DeleteLogicalRouterPolicy(vpcName, priority, match); err != nil {
921907
klog.Error(err)
922908
return err
923909
}
924910
return nil
925911
}
926912

927913
func (c *Controller) batchDeletePolicyRouteFromVpc(name string, policies []*kubeovnv1.PolicyRoute) error {
928-
var (
929-
vpc, cachedVpc *kubeovnv1.Vpc
930-
err error
931-
)
932-
933914
start := time.Now()
934915
routerPolicies := make([]*ovnnb.LogicalRouterPolicy, 0, len(policies))
935916
for _, policy := range policies {
@@ -939,26 +920,10 @@ func (c *Controller) batchDeletePolicyRouteFromVpc(name string, policies []*kube
939920
})
940921
}
941922

942-
if err = c.OVNNbClient.BatchDeleteLogicalRouterPolicy(name, routerPolicies); err != nil {
923+
if err := c.OVNNbClient.BatchDeleteLogicalRouterPolicy(name, routerPolicies); err != nil {
943924
return err
944925
}
945926
klog.V(3).Infof("take to %v batch delete policy route from vpc %s policies %d", time.Since(start), name, len(policies))
946-
947-
cachedVpc, err = c.vpcsLister.Get(name)
948-
if err != nil {
949-
if k8serrors.IsNotFound(err) {
950-
return nil
951-
}
952-
klog.Error(err)
953-
return err
954-
}
955-
vpc = cachedVpc.DeepCopy()
956-
// make sure custom policies not be deleted
957-
_, err = c.config.KubeOvnClient.KubeovnV1().Vpcs().Update(context.Background(), vpc, metav1.UpdateOptions{})
958-
if err != nil {
959-
klog.Error(err)
960-
return err
961-
}
962927
return nil
963928
}
964929

@@ -1000,10 +965,6 @@ func (c *Controller) deleteStaticRouteFromVpc(name, table, cidr, nextHop string,
1000965
}
1001966

1002967
func (c *Controller) batchDeleteStaticRouteFromVpc(name string, staticRoutes []*kubeovnv1.StaticRoute) error {
1003-
var (
1004-
vpc, cachedVpc *kubeovnv1.Vpc
1005-
err error
1006-
)
1007968
start := time.Now()
1008969
routeCount := len(staticRoutes)
1009970
delRoutes := make([]*ovnnb.LogicalRouterStaticRoute, 0, routeCount)
@@ -1017,27 +978,11 @@ func (c *Controller) batchDeleteStaticRouteFromVpc(name string, staticRoutes []*
1017978
}
1018979
delRoutes = append(delRoutes, newRoute)
1019980
}
1020-
if err = c.OVNNbClient.BatchDeleteLogicalRouterStaticRoute(name, delRoutes); err != nil {
981+
if err := c.OVNNbClient.BatchDeleteLogicalRouterStaticRoute(name, delRoutes); err != nil {
1021982
klog.Errorf("batch del vpc %s static route %d failed, %v", name, routeCount, err)
1022983
return err
1023984
}
1024985
klog.V(3).Infof("take to %v batch delete static route from vpc %s static routes %d", time.Since(start), name, len(delRoutes))
1025-
1026-
cachedVpc, err = c.vpcsLister.Get(name)
1027-
if err != nil {
1028-
if k8serrors.IsNotFound(err) {
1029-
return nil
1030-
}
1031-
klog.Error(err)
1032-
return err
1033-
}
1034-
vpc = cachedVpc.DeepCopy()
1035-
// make sure custom policies not be deleted
1036-
_, err = c.config.KubeOvnClient.KubeovnV1().Vpcs().Update(context.Background(), vpc, metav1.UpdateOptions{})
1037-
if err != nil {
1038-
klog.Error(err)
1039-
return err
1040-
}
1041986
return nil
1042987
}
1043988

@@ -1077,11 +1022,13 @@ func diffPolicyRouteWithLogical(exists []*ovnnb.LogicalRouterPolicy, target []*k
10771022
key string
10781023
ok bool
10791024
)
1025+
klog.Infof("diffPolicyRouteWithLogical exists: %v, target: %v", exists, target)
10801026
existsMap = make(map[string]*kubeovnv1.PolicyRoute, len(exists))
10811027

10821028
for _, item := range exists {
1083-
if item.ExternalIDs["vpc-egress-gateway"] != "" ||
1084-
item.ExternalIDs["isU2ORoutePolicy"] != "true" {
1029+
klog.Infof("diffPolicyRouteWithLogical item: %+v", item)
1030+
if item.ExternalIDs["vpc-egress-gateway"] != "" || item.ExternalIDs["subnet"] != "" ||
1031+
item.ExternalIDs["isU2ORoutePolicy"] == "true" {
10851032
continue
10861033
}
10871034
policy := &kubeovnv1.PolicyRoute{
@@ -1091,6 +1038,7 @@ func diffPolicyRouteWithLogical(exists []*ovnnb.LogicalRouterPolicy, target []*k
10911038
}
10921039
existsMap[getPolicyRouteItemKey(policy)] = policy
10931040
}
1041+
klog.Infof("diffPolicyRouteWithLogical existsMap: %v", existsMap)
10941042

10951043
for _, item := range target {
10961044
key = getPolicyRouteItemKey(item)
@@ -1102,17 +1050,20 @@ func diffPolicyRouteWithLogical(exists []*ovnnb.LogicalRouterPolicy, target []*k
11021050
}
11031051
}
11041052

1053+
klog.Infof("diffPolicyRouteWithLogical existsMap after delete: %v", existsMap)
1054+
11051055
for _, item := range existsMap {
11061056
dels = append(dels, item)
11071057
}
1058+
klog.Infof("diffPolicyRouteWithLogical dels: %v, adds: %v", dels, adds)
11081059
return dels, adds
11091060
}
11101061

11111062
func getPolicyRouteItemKey(item *kubeovnv1.PolicyRoute) (key string) {
11121063
return fmt.Sprintf("%d:%s:%s:%s", item.Priority, item.Match, item.Action, item.NextHopIP)
11131064
}
11141065

1115-
func diffStaticRoute(exist []*ovnnb.LogicalRouterStaticRoute, target []*kubeovnv1.StaticRoute) (routeNeedDel, routeNeedAdd []*kubeovnv1.StaticRoute, err error) {
1066+
func diffStaticRoute(exist []*ovnnb.LogicalRouterStaticRoute, target []*kubeovnv1.StaticRoute) (routeNeedDel, routeNeedAdd []*kubeovnv1.StaticRoute) {
11161067
existRouteMap := make(map[string]*kubeovnv1.StaticRoute, len(exist))
11171068
for _, item := range exist {
11181069
policy := kubeovnv1.PolicyDst
@@ -1143,7 +1094,7 @@ func diffStaticRoute(exist []*ovnnb.LogicalRouterStaticRoute, target []*kubeovnv
11431094
for _, item := range existRouteMap {
11441095
routeNeedDel = append(routeNeedDel, item)
11451096
}
1146-
return routeNeedDel, routeNeedAdd, err
1097+
return routeNeedDel, routeNeedAdd
11471098
}
11481099

11491100
func getStaticRouteItemKey(item *kubeovnv1.StaticRoute) string {
@@ -1423,14 +1374,15 @@ func (c *Controller) handleDeleteVpcStaticRoute(key string) error {
14231374
needUpdate := false
14241375
newStaticRoutes := make([]*kubeovnv1.StaticRoute, 0, len(vpc.Spec.StaticRoutes))
14251376
for _, route := range vpc.Spec.StaticRoutes {
1426-
if route.ECMPMode != util.StaticRouteBfdEcmp {
1427-
newStaticRoutes = append(newStaticRoutes, route)
1377+
if route.ECMPMode == util.StaticRouteBfdEcmp {
14281378
needUpdate = true
1379+
continue
14291380
}
1381+
newStaticRoutes = append(newStaticRoutes, route)
14301382
}
1431-
// keep non ecmp bfd routes
1432-
vpc.Spec.StaticRoutes = newStaticRoutes
1383+
// keep routes except bfd ecmp routes
14331384
if needUpdate {
1385+
vpc.Spec.StaticRoutes = newStaticRoutes
14341386
if _, err = c.config.KubeOvnClient.KubeovnV1().Vpcs().Update(context.Background(), vpc, metav1.UpdateOptions{}); err != nil {
14351387
klog.Errorf("failed to update vpc spec static route %s, %v", vpc.Name, err)
14361388
return err
@@ -1500,7 +1452,7 @@ func (c *Controller) getRouteTablesByVpc(vpc *kubeovnv1.Vpc) map[string][]*kubeo
15001452
return rtbs
15011453
}
15021454

1503-
func (c *Controller) updateVpcExternalStatus(key string, enableExternal bool) error {
1455+
func (c *Controller) updateVpcExternalStatus(key string) error {
15041456
cachedVpc, err := c.vpcsLister.Get(key)
15051457
if err != nil {
15061458
klog.Errorf("failed to get vpc %s, %v", key, err)
@@ -1510,7 +1462,7 @@ func (c *Controller) updateVpcExternalStatus(key string, enableExternal bool) er
15101462
vpc.Status.EnableExternal = vpc.Spec.EnableExternal
15111463
vpc.Status.EnableBfd = vpc.Spec.EnableBfd
15121464

1513-
if enableExternal {
1465+
if vpc.Spec.EnableExternal {
15141466
sort.Strings(vpc.Spec.ExtraExternalSubnets)
15151467
vpc.Status.ExtraExternalSubnets = vpc.Spec.ExtraExternalSubnets
15161468
} else {

0 commit comments

Comments
 (0)