Skip to content

Commit 52f15a4

Browse files
oilbeaterzbb88888
oilbeater
authored and
zbb88888
committed
refactor(controller): clean up redundant logic in vpc controller (kubeovn#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> (cherry picked from commit 436b519)
1 parent 3cc7064 commit 52f15a4

File tree

1 file changed

+42
-89
lines changed

1 file changed

+42
-89
lines changed

‎pkg/controller/vpc.go‎

Lines changed: 42 additions & 89 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)
@@ -758,7 +766,7 @@ func (c *Controller) handleUpdateVpcExternal(vpc *kubeovnv1.Vpc, custVpcEnableEx
758766
}
759767
}
760768

761-
if err := c.updateVpcExternalStatus(vpc.Name, vpc.Spec.EnableExternal); err != nil {
769+
if err := c.updateVpcExternalStatus(vpc.Name); err != nil {
762770
klog.Errorf("failed to update vpc external subnets status, %v", err)
763771
return err
764772
}
@@ -808,15 +816,15 @@ func (c *Controller) reconcileVpcBfdLRP(vpc *kubeovnv1.Vpc) (string, []string, e
808816
nodeNames := make([]string, 0, len(nodes))
809817
chassisCount = min(chassisCount, len(nodes))
810818
chassisNames := make([]string, 0, chassisCount)
811-
for _, nodes := range nodes[:chassisCount] {
812-
chassis, err := c.OVNSbClient.GetChassisByHost(nodes.Name)
819+
for _, node := range nodes[:chassisCount] {
820+
chassis, err := c.OVNSbClient.GetChassisByHost(node.Name)
813821
if err != nil {
814-
err = fmt.Errorf("failed to get chassis of node %s: %w", nodes.Name, err)
822+
err = fmt.Errorf("failed to get chassis of node %s: %w", node.Name, err)
815823
klog.Error(err)
816824
return portName, nil, err
817825
}
818826
chassisNames = append(chassisNames, chassis.Name)
819-
nodeNames = append(nodeNames, nodes.Name)
827+
nodeNames = append(nodeNames, node.Name)
820828
}
821829

822830
networks := strings.Split(vpc.Spec.BFDPort.IP, ",")
@@ -894,40 +902,14 @@ func (c *Controller) batchAddPolicyRouteToVpc(name string, policies []*kubeovnv1
894902
}
895903

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

925912
func (c *Controller) batchDeletePolicyRouteFromVpc(name string, policies []*kubeovnv1.PolicyRoute) error {
926-
var (
927-
vpc, cachedVpc *kubeovnv1.Vpc
928-
err error
929-
)
930-
931913
start := time.Now()
932914
routerPolicies := make([]*ovnnb.LogicalRouterPolicy, 0, len(policies))
933915
for _, policy := range policies {
@@ -937,26 +919,10 @@ func (c *Controller) batchDeletePolicyRouteFromVpc(name string, policies []*kube
937919
})
938920
}
939921

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

@@ -998,10 +964,6 @@ func (c *Controller) deleteStaticRouteFromVpc(name, table, cidr, nextHop string,
998964
}
999965

1000966
func (c *Controller) batchDeleteStaticRouteFromVpc(name string, staticRoutes []*kubeovnv1.StaticRoute) error {
1001-
var (
1002-
vpc, cachedVpc *kubeovnv1.Vpc
1003-
err error
1004-
)
1005967
start := time.Now()
1006968
routeCount := len(staticRoutes)
1007969
delRoutes := make([]*ovnnb.LogicalRouterStaticRoute, 0, routeCount)
@@ -1015,27 +977,11 @@ func (c *Controller) batchDeleteStaticRouteFromVpc(name string, staticRoutes []*
1015977
}
1016978
delRoutes = append(delRoutes, newRoute)
1017979
}
1018-
if err = c.OVNNbClient.BatchDeleteLogicalRouterStaticRoute(name, delRoutes); err != nil {
980+
if err := c.OVNNbClient.BatchDeleteLogicalRouterStaticRoute(name, delRoutes); err != nil {
1019981
klog.Errorf("batch del vpc %s static route %d failed, %v", name, routeCount, err)
1020982
return err
1021983
}
1022984
klog.V(3).Infof("take to %v batch delete static route from vpc %s static routes %d", time.Since(start), name, len(delRoutes))
1023-
1024-
cachedVpc, err = c.vpcsLister.Get(name)
1025-
if err != nil {
1026-
if k8serrors.IsNotFound(err) {
1027-
return nil
1028-
}
1029-
klog.Error(err)
1030-
return err
1031-
}
1032-
vpc = cachedVpc.DeepCopy()
1033-
// make sure custom policies not be deleted
1034-
_, err = c.config.KubeOvnClient.KubeovnV1().Vpcs().Update(context.Background(), vpc, metav1.UpdateOptions{})
1035-
if err != nil {
1036-
klog.Error(err)
1037-
return err
1038-
}
1039985
return nil
1040986
}
1041987

@@ -1075,9 +1021,11 @@ func diffPolicyRouteWithLogical(exists []*ovnnb.LogicalRouterPolicy, target []*k
10751021
key string
10761022
ok bool
10771023
)
1024+
klog.Infof("diffPolicyRouteWithLogical exists: %v, target: %v", exists, target)
10781025
existsMap = make(map[string]*kubeovnv1.PolicyRoute, len(exists))
10791026

10801027
for _, item := range exists {
1028+
klog.Infof("diffPolicyRouteWithLogical item: %+v", item)
10811029
if item.ExternalIDs["vpc-egress-gateway"] != "" || item.ExternalIDs["subnet"] != "" ||
10821030
item.ExternalIDs["isU2ORoutePolicy"] == "true" {
10831031
continue
@@ -1090,6 +1038,7 @@ func diffPolicyRouteWithLogical(exists []*ovnnb.LogicalRouterPolicy, target []*k
10901038
}
10911039
existsMap[getPolicyRouteItemKey(policy)] = policy
10921040
}
1041+
klog.Infof("diffPolicyRouteWithLogical existsMap: %v", existsMap)
10931042

10941043
for _, item := range target {
10951044
key = getPolicyRouteItemKey(item)
@@ -1101,9 +1050,12 @@ func diffPolicyRouteWithLogical(exists []*ovnnb.LogicalRouterPolicy, target []*k
11011050
}
11021051
}
11031052

1053+
klog.Infof("diffPolicyRouteWithLogical existsMap after delete: %v", existsMap)
1054+
11041055
for _, item := range existsMap {
11051056
dels = append(dels, item)
11061057
}
1058+
klog.Infof("diffPolicyRouteWithLogical dels: %v, adds: %v", dels, adds)
11071059
return dels, adds
11081060
}
11091061

@@ -1141,7 +1093,7 @@ func normalizePolicyRouteNextHops(nextHopIP string) string {
11411093
return strings.Join(nextHops, ",")
11421094
}
11431095

1144-
func diffStaticRoute(exist []*ovnnb.LogicalRouterStaticRoute, target []*kubeovnv1.StaticRoute) (routeNeedDel, routeNeedAdd []*kubeovnv1.StaticRoute, err error) {
1096+
func diffStaticRoute(exist []*ovnnb.LogicalRouterStaticRoute, target []*kubeovnv1.StaticRoute) (routeNeedDel, routeNeedAdd []*kubeovnv1.StaticRoute) {
11451097
existRouteMap := make(map[string]*kubeovnv1.StaticRoute, len(exist))
11461098
for _, item := range exist {
11471099
policy := kubeovnv1.PolicyDst
@@ -1172,7 +1124,7 @@ func diffStaticRoute(exist []*ovnnb.LogicalRouterStaticRoute, target []*kubeovnv
11721124
for _, item := range existRouteMap {
11731125
routeNeedDel = append(routeNeedDel, item)
11741126
}
1175-
return routeNeedDel, routeNeedAdd, err
1127+
return routeNeedDel, routeNeedAdd
11761128
}
11771129

11781130
func getStaticRouteItemKey(item *kubeovnv1.StaticRoute) string {
@@ -1452,14 +1404,15 @@ func (c *Controller) handleDeleteVpcStaticRoute(key string) error {
14521404
needUpdate := false
14531405
newStaticRoutes := make([]*kubeovnv1.StaticRoute, 0, len(vpc.Spec.StaticRoutes))
14541406
for _, route := range vpc.Spec.StaticRoutes {
1455-
if route.ECMPMode != util.StaticRouteBfdEcmp {
1456-
newStaticRoutes = append(newStaticRoutes, route)
1407+
if route.ECMPMode == util.StaticRouteBfdEcmp {
14571408
needUpdate = true
1409+
continue
14581410
}
1411+
newStaticRoutes = append(newStaticRoutes, route)
14591412
}
1460-
// keep non ecmp bfd routes
1461-
vpc.Spec.StaticRoutes = newStaticRoutes
1413+
// keep routes except bfd ecmp routes
14621414
if needUpdate {
1415+
vpc.Spec.StaticRoutes = newStaticRoutes
14631416
if _, err = c.config.KubeOvnClient.KubeovnV1().Vpcs().Update(context.Background(), vpc, metav1.UpdateOptions{}); err != nil {
14641417
klog.Errorf("failed to update vpc spec static route %s, %v", vpc.Name, err)
14651418
return err
@@ -1529,7 +1482,7 @@ func (c *Controller) getRouteTablesByVpc(vpc *kubeovnv1.Vpc) map[string][]*kubeo
15291482
return rtbs
15301483
}
15311484

1532-
func (c *Controller) updateVpcExternalStatus(key string, enableExternal bool) error {
1485+
func (c *Controller) updateVpcExternalStatus(key string) error {
15331486
cachedVpc, err := c.vpcsLister.Get(key)
15341487
if err != nil {
15351488
klog.Errorf("failed to get vpc %s, %v", key, err)
@@ -1539,7 +1492,7 @@ func (c *Controller) updateVpcExternalStatus(key string, enableExternal bool) er
15391492
vpc.Status.EnableExternal = vpc.Spec.EnableExternal
15401493
vpc.Status.EnableBfd = vpc.Spec.EnableBfd
15411494

1542-
if enableExternal {
1495+
if vpc.Spec.EnableExternal {
15431496
sort.Strings(vpc.Spec.ExtraExternalSubnets)
15441497
vpc.Status.ExtraExternalSubnets = vpc.Spec.ExtraExternalSubnets
15451498
} else {

0 commit comments

Comments
 (0)