From e21146d053ee579f90427a03ad4dd34c306bd7ec Mon Sep 17 00:00:00 2001 From: Isabelle Date: Wed, 29 Oct 2025 10:58:23 +0000 Subject: [PATCH 1/2] vpc controller: only manage static routes with kube-ovn as vendor Signed-off-by: isabelleatkins --- pkg/controller/controller_test.go | 5 + pkg/controller/vpc.go | 8 +- pkg/controller/vpc_test.go | 297 ++++++++++++++++++++++++++++++ 3 files changed, 307 insertions(+), 3 deletions(-) create mode 100644 pkg/controller/vpc_test.go diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index cf4be673f47..7a6bcdc5057 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -38,6 +38,7 @@ import ( type fakeControllerInformers struct { vpcInformer kubeovninformer.VpcInformer + vpcNatGwInformer kubeovninformer.VpcNatGatewayInformer subnetInformer kubeovninformer.SubnetInformer serviceInformer coreinformers.ServiceInformer namespaceInformer coreinformers.NamespaceInformer @@ -123,9 +124,11 @@ func newFakeControllerWithOptions(t *testing.T, opts *FakeControllerOptions) (*f kubeovnInformerFactory := kubeovninformerfactory.NewSharedInformerFactory(kubeovnClient, 0) vpcInformer := kubeovnInformerFactory.Kubeovn().V1().Vpcs() subnetInformer := kubeovnInformerFactory.Kubeovn().V1().Subnets() + vpcNatGwInformer := kubeovnInformerFactory.Kubeovn().V1().VpcNatGateways() fakeInformers := &fakeControllerInformers{ vpcInformer: vpcInformer, + vpcNatGwInformer: vpcNatGwInformer, subnetInformer: subnetInformer, serviceInformer: serviceInformer, namespaceInformer: namespaceInformer, @@ -155,6 +158,8 @@ func newFakeControllerWithOptions(t *testing.T, opts *FakeControllerOptions) (*f DefaultLogicalSwitch: "ovn-default", NodeSwitch: "join", KubeOvnClient: kubeovnClient, + KubeClient: kubeClient, + PodNamespace: "kube-system", AttachNetClient: nadClient, } diff --git a/pkg/controller/vpc.go b/pkg/controller/vpc.go index a75ce1bb114..82300b46372 100644 --- a/pkg/controller/vpc.go +++ b/pkg/controller/vpc.go @@ -339,9 +339,11 @@ func (c *Controller) handleAddOrUpdateVpc(key string) error { staticExistedRoutes []*ovnnb.LogicalRouterStaticRoute staticTargetRoutes []*kubeovnv1.StaticRoute staticRouteMapping map[string][]*kubeovnv1.StaticRoute + externalIDs = map[string]string{"vendor": util.CniTypeName} ) - staticExistedRoutes, err = c.OVNNbClient.ListLogicalRouterStaticRoutes(vpc.Name, nil, nil, "", nil) + // only manage static routes which are kube-ovn managed, by filtering for vendor util.CniTypeName + staticExistedRoutes, err = c.OVNNbClient.ListLogicalRouterStaticRoutes(vpc.Name, nil, nil, "", externalIDs) if err != nil { klog.Errorf("failed to get vpc %s static route list, %v", vpc.Name, err) return err @@ -498,7 +500,6 @@ func (c *Controller) handleAddOrUpdateVpc(key string) error { var ( policyRouteExisted, policyRouteNeedDel, policyRouteNeedAdd []*kubeovnv1.PolicyRoute policyRouteLogical []*ovnnb.LogicalRouterPolicy - externalIDs = map[string]string{"vendor": util.CniTypeName} ) if vpc.Name == c.config.ClusterRouter { @@ -948,10 +949,11 @@ func (c *Controller) batchDeletePolicyRouteFromVpc(name string, policies []*kube } func (c *Controller) addStaticRouteToVpc(name string, route *kubeovnv1.StaticRoute) error { + externalIDs := map[string]string{"vendor": util.CniTypeName} if route.BfdID != "" { klog.Infof("vpc %s add static ecmp route: %+v", name, route) if err := c.OVNNbClient.AddLogicalRouterStaticRoute( - name, route.RouteTable, convertPolicy(route.Policy), route.CIDR, &route.BfdID, nil, route.NextHopIP, + name, route.RouteTable, convertPolicy(route.Policy), route.CIDR, &route.BfdID, externalIDs, route.NextHopIP, ); err != nil { klog.Errorf("failed to add bfd static route to vpc %s , %v", name, err) return err diff --git a/pkg/controller/vpc_test.go b/pkg/controller/vpc_test.go new file mode 100644 index 00000000000..8aa65f44481 --- /dev/null +++ b/pkg/controller/vpc_test.go @@ -0,0 +1,297 @@ +package controller + +import ( + "context" + "fmt" + "testing" + + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1" + "github.com/kubeovn/kube-ovn/pkg/ovsdb/ovnnb" + "github.com/kubeovn/kube-ovn/pkg/util" + "k8s.io/utils/keymutex" +) + +func Test_handleAddOrUpdateVpc_staticRoutes(t *testing.T) { + t.Parallel() + + vpcName := "test-vpc" + + // Policy variables for taking pointers + srcIPPolicy := ovnnb.LogicalRouterStaticRoutePolicySrcIP + dstIPPolicy := ovnnb.LogicalRouterStaticRoutePolicyDstIP + + // Internal static route created directly in OVN with kube-ovn vendor + internalStaticRoute := &ovnnb.LogicalRouterStaticRoute{ + UUID: "internal-static-route-uuid", + ExternalIDs: map[string]string{ + "vendor": util.CniTypeName, + }, + IPPrefix: "10.0.0.0/24", + Nexthop: "1.2.3.4", + Policy: &srcIPPolicy, + RouteTable: util.MainRouteTable, + } + + // Static route that matches VPC spec + managedStaticRoute := &ovnnb.LogicalRouterStaticRoute{ + UUID: "managed-static-route-uuid", + ExternalIDs: map[string]string{ + "vendor": util.CniTypeName, + }, + IPPrefix: "192.168.0.0/24", + Nexthop: "10.0.0.1", + Policy: &dstIPPolicy, + RouteTable: util.MainRouteTable, + } + + t.Run("only try to manage static routes with vendor kube-ovn", func(t *testing.T) { + fakeController := newFakeController(t) + ctrl := fakeController.fakeController + fakeinformers := fakeController.fakeInformers + mockOvnClient := fakeController.mockOvnClient + + // Initialize mutexes + ctrl.vpcKeyMutex = keymutex.NewHashed(500) + + vpc := &kubeovnv1.Vpc{ + ObjectMeta: metav1.ObjectMeta{ + Name: vpcName, + }, + Spec: kubeovnv1.VpcSpec{ + StaticRoutes: []*kubeovnv1.StaticRoute{ + { + CIDR: "192.168.0.0/24", + NextHopIP: "10.0.0.1", + Policy: kubeovnv1.PolicyDst, + RouteTable: util.MainRouteTable, + }, + }, + EnableExternal: false, + PolicyRoutes: nil, + }, + Status: kubeovnv1.VpcStatus{ + Subnets: []string{}, + EnableExternal: false, + }, + } + + _, err := ctrl.config.KubeOvnClient.KubeovnV1().Vpcs().Create(context.Background(), vpc, metav1.CreateOptions{}) + require.NoError(t, err) + + err = fakeinformers.vpcInformer.Informer().GetStore().Add(vpc) + require.NoError(t, err) + + existingKubeOvnRoutes := []*ovnnb.LogicalRouterStaticRoute{ + internalStaticRoute, + } + + externalIDs := map[string]string{"vendor": util.CniTypeName} + + mockOvnClient.EXPECT().CreateLogicalRouter(vpcName).Return(nil) + mockOvnClient.EXPECT().UpdateLogicalRouter(gomock.Any(), gomock.Any()).Return(nil) + mockOvnClient.EXPECT().ListLogicalRouterStaticRoutes(vpcName, nil, nil, "", externalIDs).Return(existingKubeOvnRoutes, nil) + mockOvnClient.EXPECT().GetLogicalRouter(vpcName, false).Return(&ovnnb.LogicalRouter{ + Name: vpcName, + Nat: []string{}, + }, nil) + mockOvnClient.EXPECT().DeleteLogicalRouterStaticRoute(vpcName, gomock.Any(), gomock.Any(), "10.0.0.0/24", "1.2.3.4").Return(nil) + mockOvnClient.EXPECT().AddLogicalRouterStaticRoute( + vpcName, + util.MainRouteTable, + "dst-ip", + "192.168.0.0/24", + nil, + nil, + "10.0.0.1", + ).Return(nil) + mockOvnClient.EXPECT().ClearLogicalRouterPolicy(vpcName).Return(nil) + mockOvnClient.EXPECT().ListLogicalSwitch(gomock.Any(), gomock.Any()).Return([]ovnnb.LogicalSwitch{}, nil).AnyTimes() + mockOvnClient.EXPECT().ListLogicalRouter(gomock.Any(), gomock.Any()).Return([]ovnnb.LogicalRouter{}, nil).AnyTimes() + mockOvnClient.EXPECT().DeleteLogicalRouterPort(fmt.Sprintf("bfd@%s", vpcName)).Return(nil) + mockOvnClient.EXPECT().DeleteHAChassisGroup(fmt.Sprintf("bfd@%s", vpcName)).Return(nil) + err = ctrl.handleAddOrUpdateVpc(vpcName) + require.NoError(t, err) + }) + + t.Run("delete orphaned kube-ovn routes", func(t *testing.T) { + fakeController := newFakeController(t) + ctrl := fakeController.fakeController + fakeinformers := fakeController.fakeInformers + mockOvnClient := fakeController.mockOvnClient + + ctrl.vpcKeyMutex = keymutex.NewHashed(500) + + vpc := &kubeovnv1.Vpc{ + ObjectMeta: metav1.ObjectMeta{ + Name: vpcName, + }, + Spec: kubeovnv1.VpcSpec{ + StaticRoutes: []*kubeovnv1.StaticRoute{ + { + CIDR: "192.168.0.0/24", + NextHopIP: "10.0.0.1", + Policy: kubeovnv1.PolicyDst, + RouteTable: util.MainRouteTable, + }, + }, + EnableExternal: false, + PolicyRoutes: nil, + }, + Status: kubeovnv1.VpcStatus{ + Subnets: []string{}, + EnableExternal: false, + }, + } + + _, err := ctrl.config.KubeOvnClient.KubeovnV1().Vpcs().Create(context.Background(), vpc, metav1.CreateOptions{}) + require.NoError(t, err) + + err = fakeinformers.vpcInformer.Informer().GetStore().Add(vpc) + require.NoError(t, err) + + existingKubeOvnRoutes := []*ovnnb.LogicalRouterStaticRoute{ + internalStaticRoute, + managedStaticRoute, + } + + externalIDs := map[string]string{"vendor": util.CniTypeName} + + mockOvnClient.EXPECT().CreateLogicalRouter(vpcName).Return(nil) + mockOvnClient.EXPECT().UpdateLogicalRouter(gomock.Any(), gomock.Any()).Return(nil) + mockOvnClient.EXPECT().ListLogicalRouterStaticRoutes(vpcName, nil, nil, "", externalIDs).Return(existingKubeOvnRoutes, nil) + mockOvnClient.EXPECT().GetLogicalRouter(vpcName, false).Return(&ovnnb.LogicalRouter{ + Name: vpcName, + Nat: []string{}, + }, nil) + mockOvnClient.EXPECT().DeleteLogicalRouterStaticRoute(vpcName, gomock.Any(), gomock.Any(), "10.0.0.0/24", "1.2.3.4").Return(nil) + mockOvnClient.EXPECT().ClearLogicalRouterPolicy(vpcName).Return(nil) + mockOvnClient.EXPECT().ListLogicalSwitch(gomock.Any(), gomock.Any()).Return([]ovnnb.LogicalSwitch{}, nil).AnyTimes() + mockOvnClient.EXPECT().ListLogicalRouter(gomock.Any(), gomock.Any()).Return([]ovnnb.LogicalRouter{}, nil).AnyTimes() + mockOvnClient.EXPECT().DeleteLogicalRouterPort(fmt.Sprintf("bfd@%s", vpcName)).Return(nil) + mockOvnClient.EXPECT().DeleteHAChassisGroup(fmt.Sprintf("bfd@%s", vpcName)).Return(nil) + err = ctrl.handleAddOrUpdateVpc(vpcName) + require.NoError(t, err) + }) + + t.Run("handle empty VPC static routes", func(t *testing.T) { + fakeController := newFakeController(t) + ctrl := fakeController.fakeController + fakeinformers := fakeController.fakeInformers + mockOvnClient := fakeController.mockOvnClient + + ctrl.vpcKeyMutex = keymutex.NewHashed(500) + + vpcEmpty := &kubeovnv1.Vpc{ + ObjectMeta: metav1.ObjectMeta{ + Name: vpcName, + }, + Spec: kubeovnv1.VpcSpec{ + StaticRoutes: []*kubeovnv1.StaticRoute{}, + EnableExternal: false, + PolicyRoutes: nil, + }, + Status: kubeovnv1.VpcStatus{ + Subnets: []string{}, + EnableExternal: false, + }, + } + + _, err := ctrl.config.KubeOvnClient.KubeovnV1().Vpcs().Create(context.Background(), vpcEmpty, metav1.CreateOptions{}) + require.NoError(t, err) + + err = fakeinformers.vpcInformer.Informer().GetStore().Add(vpcEmpty) + require.NoError(t, err) + + existingKubeOvnRoutes := []*ovnnb.LogicalRouterStaticRoute{ + internalStaticRoute, + managedStaticRoute, + } + + externalIDs := map[string]string{"vendor": util.CniTypeName} + + mockOvnClient.EXPECT().CreateLogicalRouter(vpcName).Return(nil) + mockOvnClient.EXPECT().UpdateLogicalRouter(gomock.Any(), gomock.Any()).Return(nil) + mockOvnClient.EXPECT().ListLogicalRouterStaticRoutes(vpcName, nil, nil, "", externalIDs).Return(existingKubeOvnRoutes, nil) + mockOvnClient.EXPECT().GetLogicalRouter(vpcName, false).Return(&ovnnb.LogicalRouter{ + Name: vpcName, + Nat: []string{}, + }, nil) + mockOvnClient.EXPECT().DeleteLogicalRouterStaticRoute(vpcName, gomock.Any(), gomock.Any(), "10.0.0.0/24", "1.2.3.4").Return(nil) + mockOvnClient.EXPECT().DeleteLogicalRouterStaticRoute(vpcName, gomock.Any(), gomock.Any(), "192.168.0.0/24", "10.0.0.1").Return(nil) + mockOvnClient.EXPECT().ClearLogicalRouterPolicy(vpcName).Return(nil) + mockOvnClient.EXPECT().ListLogicalSwitch(gomock.Any(), gomock.Any()).Return([]ovnnb.LogicalSwitch{}, nil).AnyTimes() + mockOvnClient.EXPECT().ListLogicalRouter(gomock.Any(), gomock.Any()).Return([]ovnnb.LogicalRouter{}, nil).AnyTimes() + mockOvnClient.EXPECT().DeleteLogicalRouterPort(fmt.Sprintf("bfd@%s", vpcName)).Return(nil) + mockOvnClient.EXPECT().DeleteHAChassisGroup(fmt.Sprintf("bfd@%s", vpcName)).Return(nil) + err = ctrl.handleAddOrUpdateVpc(vpcName) + require.NoError(t, err) + }) + + t.Run("add static routes from VPC spec when none exist", func(t *testing.T) { + fakeController := newFakeController(t) + ctrl := fakeController.fakeController + fakeinformers := fakeController.fakeInformers + mockOvnClient := fakeController.mockOvnClient + + ctrl.vpcKeyMutex = keymutex.NewHashed(500) + + vpc := &kubeovnv1.Vpc{ + ObjectMeta: metav1.ObjectMeta{ + Name: vpcName, + }, + Spec: kubeovnv1.VpcSpec{ + StaticRoutes: []*kubeovnv1.StaticRoute{ + { + CIDR: "192.168.0.0/24", + NextHopIP: "10.0.0.1", + Policy: kubeovnv1.PolicyDst, + RouteTable: util.MainRouteTable, + }, + }, + EnableExternal: false, + PolicyRoutes: nil, + }, + Status: kubeovnv1.VpcStatus{ + Subnets: []string{}, + EnableExternal: false, + }, + } + + _, err := ctrl.config.KubeOvnClient.KubeovnV1().Vpcs().Create(context.Background(), vpc, metav1.CreateOptions{}) + require.NoError(t, err) + + err = fakeinformers.vpcInformer.Informer().GetStore().Add(vpc) + require.NoError(t, err) + + externalIDs := map[string]string{"vendor": util.CniTypeName} + + mockOvnClient.EXPECT().CreateLogicalRouter(vpcName).Return(nil) + mockOvnClient.EXPECT().UpdateLogicalRouter(gomock.Any(), gomock.Any()).Return(nil) + mockOvnClient.EXPECT().ListLogicalRouterStaticRoutes(vpcName, nil, nil, "", externalIDs).Return(nil, nil) + mockOvnClient.EXPECT().GetLogicalRouter(vpcName, false).Return(&ovnnb.LogicalRouter{ + Name: vpcName, + Nat: []string{}, + }, nil) + mockOvnClient.EXPECT().AddLogicalRouterStaticRoute( + vpcName, + util.MainRouteTable, + "dst-ip", + "192.168.0.0/24", + nil, + nil, + "10.0.0.1", + ).Return(nil) + mockOvnClient.EXPECT().ClearLogicalRouterPolicy(vpcName).Return(nil) + mockOvnClient.EXPECT().ListLogicalSwitch(gomock.Any(), gomock.Any()).Return([]ovnnb.LogicalSwitch{}, nil).AnyTimes() + mockOvnClient.EXPECT().ListLogicalRouter(gomock.Any(), gomock.Any()).Return([]ovnnb.LogicalRouter{}, nil).AnyTimes() + mockOvnClient.EXPECT().DeleteLogicalRouterPort(fmt.Sprintf("bfd@%s", vpcName)).Return(nil) + mockOvnClient.EXPECT().DeleteHAChassisGroup(fmt.Sprintf("bfd@%s", vpcName)).Return(nil) + err = ctrl.handleAddOrUpdateVpc(vpcName) + require.NoError(t, err) + }) +} From 7993e351b118c90e1770f018681059afd77cd964 Mon Sep 17 00:00:00 2001 From: isabelleatkins Date: Wed, 29 Oct 2025 15:07:16 +0000 Subject: [PATCH 2/2] VPC controller: add externalids with static routes across various locations Signed-off-by: isabelleatkins --- pkg/controller/vpc.go | 6 +++--- pkg/controller/vpc_test.go | 7 +++---- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/pkg/controller/vpc.go b/pkg/controller/vpc.go index 82300b46372..55e6049df5b 100644 --- a/pkg/controller/vpc.go +++ b/pkg/controller/vpc.go @@ -480,7 +480,7 @@ func (c *Controller) handleAddOrUpdateVpc(key string) error { if item.BfdID != "" { klog.Infof("vpc %s add static ecmp route: %+v", vpc.Name, item) if err = c.OVNNbClient.AddLogicalRouterStaticRoute( - vpc.Name, item.RouteTable, convertPolicy(item.Policy), item.CIDR, &item.BfdID, nil, item.NextHopIP, + vpc.Name, item.RouteTable, convertPolicy(item.Policy), item.CIDR, &item.BfdID, externalIDs, item.NextHopIP, ); err != nil { klog.Errorf("failed to add bfd static route to vpc %s , %v", vpc.Name, err) return err @@ -488,7 +488,7 @@ func (c *Controller) handleAddOrUpdateVpc(key string) error { } else { klog.Infof("vpc %s add static route: %+v", vpc.Name, item) if err = c.OVNNbClient.AddLogicalRouterStaticRoute( - vpc.Name, item.RouteTable, convertPolicy(item.Policy), item.CIDR, nil, nil, item.NextHopIP, + vpc.Name, item.RouteTable, convertPolicy(item.Policy), item.CIDR, nil, externalIDs, item.NextHopIP, ); err != nil { klog.Errorf("failed to add normal static route to vpc %s , %v", vpc.Name, err) return err @@ -961,7 +961,7 @@ func (c *Controller) addStaticRouteToVpc(name string, route *kubeovnv1.StaticRou } else { klog.Infof("vpc %s add static route: %+v", name, route) if err := c.OVNNbClient.AddLogicalRouterStaticRoute( - name, route.RouteTable, convertPolicy(route.Policy), route.CIDR, nil, nil, route.NextHopIP, + name, route.RouteTable, convertPolicy(route.Policy), route.CIDR, nil, externalIDs, route.NextHopIP, ); err != nil { klog.Errorf("failed to add normal static route to vpc %s , %v", name, err) return err diff --git a/pkg/controller/vpc_test.go b/pkg/controller/vpc_test.go index 8aa65f44481..33951528fea 100644 --- a/pkg/controller/vpc_test.go +++ b/pkg/controller/vpc_test.go @@ -7,13 +7,12 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/keymutex" kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1" "github.com/kubeovn/kube-ovn/pkg/ovsdb/ovnnb" "github.com/kubeovn/kube-ovn/pkg/util" - "k8s.io/utils/keymutex" ) func Test_handleAddOrUpdateVpc_staticRoutes(t *testing.T) { @@ -106,7 +105,7 @@ func Test_handleAddOrUpdateVpc_staticRoutes(t *testing.T) { "dst-ip", "192.168.0.0/24", nil, - nil, + externalIDs, "10.0.0.1", ).Return(nil) mockOvnClient.EXPECT().ClearLogicalRouterPolicy(vpcName).Return(nil) @@ -283,7 +282,7 @@ func Test_handleAddOrUpdateVpc_staticRoutes(t *testing.T) { "dst-ip", "192.168.0.0/24", nil, - nil, + externalIDs, "10.0.0.1", ).Return(nil) mockOvnClient.EXPECT().ClearLogicalRouterPolicy(vpcName).Return(nil)