Skip to content

Commit e21146d

Browse files
Isabelleisabelleatkins
Isabelle
authored and
isabelleatkins
committed
vpc controller: only manage static routes with kube-ovn as vendor
Signed-off-by: isabelleatkins <[email protected]>
1 parent bb30324 commit e21146d

File tree

3 files changed

+307
-3
lines changed

3 files changed

+307
-3
lines changed

‎pkg/controller/controller_test.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import (
3838

3939
type fakeControllerInformers struct {
4040
vpcInformer kubeovninformer.VpcInformer
41+
vpcNatGwInformer kubeovninformer.VpcNatGatewayInformer
4142
subnetInformer kubeovninformer.SubnetInformer
4243
serviceInformer coreinformers.ServiceInformer
4344
namespaceInformer coreinformers.NamespaceInformer
@@ -123,9 +124,11 @@ func newFakeControllerWithOptions(t *testing.T, opts *FakeControllerOptions) (*f
123124
kubeovnInformerFactory := kubeovninformerfactory.NewSharedInformerFactory(kubeovnClient, 0)
124125
vpcInformer := kubeovnInformerFactory.Kubeovn().V1().Vpcs()
125126
subnetInformer := kubeovnInformerFactory.Kubeovn().V1().Subnets()
127+
vpcNatGwInformer := kubeovnInformerFactory.Kubeovn().V1().VpcNatGateways()
126128

127129
fakeInformers := &fakeControllerInformers{
128130
vpcInformer: vpcInformer,
131+
vpcNatGwInformer: vpcNatGwInformer,
129132
subnetInformer: subnetInformer,
130133
serviceInformer: serviceInformer,
131134
namespaceInformer: namespaceInformer,
@@ -155,6 +158,8 @@ func newFakeControllerWithOptions(t *testing.T, opts *FakeControllerOptions) (*f
155158
DefaultLogicalSwitch: "ovn-default",
156159
NodeSwitch: "join",
157160
KubeOvnClient: kubeovnClient,
161+
KubeClient: kubeClient,
162+
PodNamespace: "kube-system",
158163
AttachNetClient: nadClient,
159164
}
160165

‎pkg/controller/vpc.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -339,9 +339,11 @@ func (c *Controller) handleAddOrUpdateVpc(key string) error {
339339
staticExistedRoutes []*ovnnb.LogicalRouterStaticRoute
340340
staticTargetRoutes []*kubeovnv1.StaticRoute
341341
staticRouteMapping map[string][]*kubeovnv1.StaticRoute
342+
externalIDs = map[string]string{"vendor": util.CniTypeName}
342343
)
343344

344-
staticExistedRoutes, err = c.OVNNbClient.ListLogicalRouterStaticRoutes(vpc.Name, nil, nil, "", nil)
345+
// only manage static routes which are kube-ovn managed, by filtering for vendor util.CniTypeName
346+
staticExistedRoutes, err = c.OVNNbClient.ListLogicalRouterStaticRoutes(vpc.Name, nil, nil, "", externalIDs)
345347
if err != nil {
346348
klog.Errorf("failed to get vpc %s static route list, %v", vpc.Name, err)
347349
return err
@@ -498,7 +500,6 @@ func (c *Controller) handleAddOrUpdateVpc(key string) error {
498500
var (
499501
policyRouteExisted, policyRouteNeedDel, policyRouteNeedAdd []*kubeovnv1.PolicyRoute
500502
policyRouteLogical []*ovnnb.LogicalRouterPolicy
501-
externalIDs = map[string]string{"vendor": util.CniTypeName}
502503
)
503504

504505
if vpc.Name == c.config.ClusterRouter {
@@ -948,10 +949,11 @@ func (c *Controller) batchDeletePolicyRouteFromVpc(name string, policies []*kube
948949
}
949950

950951
func (c *Controller) addStaticRouteToVpc(name string, route *kubeovnv1.StaticRoute) error {
952+
externalIDs := map[string]string{"vendor": util.CniTypeName}
951953
if route.BfdID != "" {
952954
klog.Infof("vpc %s add static ecmp route: %+v", name, route)
953955
if err := c.OVNNbClient.AddLogicalRouterStaticRoute(
954-
name, route.RouteTable, convertPolicy(route.Policy), route.CIDR, &route.BfdID, nil, route.NextHopIP,
956+
name, route.RouteTable, convertPolicy(route.Policy), route.CIDR, &route.BfdID, externalIDs, route.NextHopIP,
955957
); err != nil {
956958
klog.Errorf("failed to add bfd static route to vpc %s , %v", name, err)
957959
return err

‎pkg/controller/vpc_test.go‎

Lines changed: 297 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,297 @@
1+
package controller
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"testing"
7+
8+
"github.com/stretchr/testify/require"
9+
"go.uber.org/mock/gomock"
10+
11+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
12+
13+
kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1"
14+
"github.com/kubeovn/kube-ovn/pkg/ovsdb/ovnnb"
15+
"github.com/kubeovn/kube-ovn/pkg/util"
16+
"k8s.io/utils/keymutex"
17+
)
18+
19+
func Test_handleAddOrUpdateVpc_staticRoutes(t *testing.T) {
20+
t.Parallel()
21+
22+
vpcName := "test-vpc"
23+
24+
// Policy variables for taking pointers
25+
srcIPPolicy := ovnnb.LogicalRouterStaticRoutePolicySrcIP
26+
dstIPPolicy := ovnnb.LogicalRouterStaticRoutePolicyDstIP
27+
28+
// Internal static route created directly in OVN with kube-ovn vendor
29+
internalStaticRoute := &ovnnb.LogicalRouterStaticRoute{
30+
UUID: "internal-static-route-uuid",
31+
ExternalIDs: map[string]string{
32+
"vendor": util.CniTypeName,
33+
},
34+
IPPrefix: "10.0.0.0/24",
35+
Nexthop: "1.2.3.4",
36+
Policy: &srcIPPolicy,
37+
RouteTable: util.MainRouteTable,
38+
}
39+
40+
// Static route that matches VPC spec
41+
managedStaticRoute := &ovnnb.LogicalRouterStaticRoute{
42+
UUID: "managed-static-route-uuid",
43+
ExternalIDs: map[string]string{
44+
"vendor": util.CniTypeName,
45+
},
46+
IPPrefix: "192.168.0.0/24",
47+
Nexthop: "10.0.0.1",
48+
Policy: &dstIPPolicy,
49+
RouteTable: util.MainRouteTable,
50+
}
51+
52+
t.Run("only try to manage static routes with vendor kube-ovn", func(t *testing.T) {
53+
fakeController := newFakeController(t)
54+
ctrl := fakeController.fakeController
55+
fakeinformers := fakeController.fakeInformers
56+
mockOvnClient := fakeController.mockOvnClient
57+
58+
// Initialize mutexes
59+
ctrl.vpcKeyMutex = keymutex.NewHashed(500)
60+
61+
vpc := &kubeovnv1.Vpc{
62+
ObjectMeta: metav1.ObjectMeta{
63+
Name: vpcName,
64+
},
65+
Spec: kubeovnv1.VpcSpec{
66+
StaticRoutes: []*kubeovnv1.StaticRoute{
67+
{
68+
CIDR: "192.168.0.0/24",
69+
NextHopIP: "10.0.0.1",
70+
Policy: kubeovnv1.PolicyDst,
71+
RouteTable: util.MainRouteTable,
72+
},
73+
},
74+
EnableExternal: false,
75+
PolicyRoutes: nil,
76+
},
77+
Status: kubeovnv1.VpcStatus{
78+
Subnets: []string{},
79+
EnableExternal: false,
80+
},
81+
}
82+
83+
_, err := ctrl.config.KubeOvnClient.KubeovnV1().Vpcs().Create(context.Background(), vpc, metav1.CreateOptions{})
84+
require.NoError(t, err)
85+
86+
err = fakeinformers.vpcInformer.Informer().GetStore().Add(vpc)
87+
require.NoError(t, err)
88+
89+
existingKubeOvnRoutes := []*ovnnb.LogicalRouterStaticRoute{
90+
internalStaticRoute,
91+
}
92+
93+
externalIDs := map[string]string{"vendor": util.CniTypeName}
94+
95+
mockOvnClient.EXPECT().CreateLogicalRouter(vpcName).Return(nil)
96+
mockOvnClient.EXPECT().UpdateLogicalRouter(gomock.Any(), gomock.Any()).Return(nil)
97+
mockOvnClient.EXPECT().ListLogicalRouterStaticRoutes(vpcName, nil, nil, "", externalIDs).Return(existingKubeOvnRoutes, nil)
98+
mockOvnClient.EXPECT().GetLogicalRouter(vpcName, false).Return(&ovnnb.LogicalRouter{
99+
Name: vpcName,
100+
Nat: []string{},
101+
}, nil)
102+
mockOvnClient.EXPECT().DeleteLogicalRouterStaticRoute(vpcName, gomock.Any(), gomock.Any(), "10.0.0.0/24", "1.2.3.4").Return(nil)
103+
mockOvnClient.EXPECT().AddLogicalRouterStaticRoute(
104+
vpcName,
105+
util.MainRouteTable,
106+
"dst-ip",
107+
"192.168.0.0/24",
108+
nil,
109+
nil,
110+
"10.0.0.1",
111+
).Return(nil)
112+
mockOvnClient.EXPECT().ClearLogicalRouterPolicy(vpcName).Return(nil)
113+
mockOvnClient.EXPECT().ListLogicalSwitch(gomock.Any(), gomock.Any()).Return([]ovnnb.LogicalSwitch{}, nil).AnyTimes()
114+
mockOvnClient.EXPECT().ListLogicalRouter(gomock.Any(), gomock.Any()).Return([]ovnnb.LogicalRouter{}, nil).AnyTimes()
115+
mockOvnClient.EXPECT().DeleteLogicalRouterPort(fmt.Sprintf("bfd@%s", vpcName)).Return(nil)
116+
mockOvnClient.EXPECT().DeleteHAChassisGroup(fmt.Sprintf("bfd@%s", vpcName)).Return(nil)
117+
err = ctrl.handleAddOrUpdateVpc(vpcName)
118+
require.NoError(t, err)
119+
})
120+
121+
t.Run("delete orphaned kube-ovn routes", func(t *testing.T) {
122+
fakeController := newFakeController(t)
123+
ctrl := fakeController.fakeController
124+
fakeinformers := fakeController.fakeInformers
125+
mockOvnClient := fakeController.mockOvnClient
126+
127+
ctrl.vpcKeyMutex = keymutex.NewHashed(500)
128+
129+
vpc := &kubeovnv1.Vpc{
130+
ObjectMeta: metav1.ObjectMeta{
131+
Name: vpcName,
132+
},
133+
Spec: kubeovnv1.VpcSpec{
134+
StaticRoutes: []*kubeovnv1.StaticRoute{
135+
{
136+
CIDR: "192.168.0.0/24",
137+
NextHopIP: "10.0.0.1",
138+
Policy: kubeovnv1.PolicyDst,
139+
RouteTable: util.MainRouteTable,
140+
},
141+
},
142+
EnableExternal: false,
143+
PolicyRoutes: nil,
144+
},
145+
Status: kubeovnv1.VpcStatus{
146+
Subnets: []string{},
147+
EnableExternal: false,
148+
},
149+
}
150+
151+
_, err := ctrl.config.KubeOvnClient.KubeovnV1().Vpcs().Create(context.Background(), vpc, metav1.CreateOptions{})
152+
require.NoError(t, err)
153+
154+
err = fakeinformers.vpcInformer.Informer().GetStore().Add(vpc)
155+
require.NoError(t, err)
156+
157+
existingKubeOvnRoutes := []*ovnnb.LogicalRouterStaticRoute{
158+
internalStaticRoute,
159+
managedStaticRoute,
160+
}
161+
162+
externalIDs := map[string]string{"vendor": util.CniTypeName}
163+
164+
mockOvnClient.EXPECT().CreateLogicalRouter(vpcName).Return(nil)
165+
mockOvnClient.EXPECT().UpdateLogicalRouter(gomock.Any(), gomock.Any()).Return(nil)
166+
mockOvnClient.EXPECT().ListLogicalRouterStaticRoutes(vpcName, nil, nil, "", externalIDs).Return(existingKubeOvnRoutes, nil)
167+
mockOvnClient.EXPECT().GetLogicalRouter(vpcName, false).Return(&ovnnb.LogicalRouter{
168+
Name: vpcName,
169+
Nat: []string{},
170+
}, nil)
171+
mockOvnClient.EXPECT().DeleteLogicalRouterStaticRoute(vpcName, gomock.Any(), gomock.Any(), "10.0.0.0/24", "1.2.3.4").Return(nil)
172+
mockOvnClient.EXPECT().ClearLogicalRouterPolicy(vpcName).Return(nil)
173+
mockOvnClient.EXPECT().ListLogicalSwitch(gomock.Any(), gomock.Any()).Return([]ovnnb.LogicalSwitch{}, nil).AnyTimes()
174+
mockOvnClient.EXPECT().ListLogicalRouter(gomock.Any(), gomock.Any()).Return([]ovnnb.LogicalRouter{}, nil).AnyTimes()
175+
mockOvnClient.EXPECT().DeleteLogicalRouterPort(fmt.Sprintf("bfd@%s", vpcName)).Return(nil)
176+
mockOvnClient.EXPECT().DeleteHAChassisGroup(fmt.Sprintf("bfd@%s", vpcName)).Return(nil)
177+
err = ctrl.handleAddOrUpdateVpc(vpcName)
178+
require.NoError(t, err)
179+
})
180+
181+
t.Run("handle empty VPC static routes", func(t *testing.T) {
182+
fakeController := newFakeController(t)
183+
ctrl := fakeController.fakeController
184+
fakeinformers := fakeController.fakeInformers
185+
mockOvnClient := fakeController.mockOvnClient
186+
187+
ctrl.vpcKeyMutex = keymutex.NewHashed(500)
188+
189+
vpcEmpty := &kubeovnv1.Vpc{
190+
ObjectMeta: metav1.ObjectMeta{
191+
Name: vpcName,
192+
},
193+
Spec: kubeovnv1.VpcSpec{
194+
StaticRoutes: []*kubeovnv1.StaticRoute{},
195+
EnableExternal: false,
196+
PolicyRoutes: nil,
197+
},
198+
Status: kubeovnv1.VpcStatus{
199+
Subnets: []string{},
200+
EnableExternal: false,
201+
},
202+
}
203+
204+
_, err := ctrl.config.KubeOvnClient.KubeovnV1().Vpcs().Create(context.Background(), vpcEmpty, metav1.CreateOptions{})
205+
require.NoError(t, err)
206+
207+
err = fakeinformers.vpcInformer.Informer().GetStore().Add(vpcEmpty)
208+
require.NoError(t, err)
209+
210+
existingKubeOvnRoutes := []*ovnnb.LogicalRouterStaticRoute{
211+
internalStaticRoute,
212+
managedStaticRoute,
213+
}
214+
215+
externalIDs := map[string]string{"vendor": util.CniTypeName}
216+
217+
mockOvnClient.EXPECT().CreateLogicalRouter(vpcName).Return(nil)
218+
mockOvnClient.EXPECT().UpdateLogicalRouter(gomock.Any(), gomock.Any()).Return(nil)
219+
mockOvnClient.EXPECT().ListLogicalRouterStaticRoutes(vpcName, nil, nil, "", externalIDs).Return(existingKubeOvnRoutes, nil)
220+
mockOvnClient.EXPECT().GetLogicalRouter(vpcName, false).Return(&ovnnb.LogicalRouter{
221+
Name: vpcName,
222+
Nat: []string{},
223+
}, nil)
224+
mockOvnClient.EXPECT().DeleteLogicalRouterStaticRoute(vpcName, gomock.Any(), gomock.Any(), "10.0.0.0/24", "1.2.3.4").Return(nil)
225+
mockOvnClient.EXPECT().DeleteLogicalRouterStaticRoute(vpcName, gomock.Any(), gomock.Any(), "192.168.0.0/24", "10.0.0.1").Return(nil)
226+
mockOvnClient.EXPECT().ClearLogicalRouterPolicy(vpcName).Return(nil)
227+
mockOvnClient.EXPECT().ListLogicalSwitch(gomock.Any(), gomock.Any()).Return([]ovnnb.LogicalSwitch{}, nil).AnyTimes()
228+
mockOvnClient.EXPECT().ListLogicalRouter(gomock.Any(), gomock.Any()).Return([]ovnnb.LogicalRouter{}, nil).AnyTimes()
229+
mockOvnClient.EXPECT().DeleteLogicalRouterPort(fmt.Sprintf("bfd@%s", vpcName)).Return(nil)
230+
mockOvnClient.EXPECT().DeleteHAChassisGroup(fmt.Sprintf("bfd@%s", vpcName)).Return(nil)
231+
err = ctrl.handleAddOrUpdateVpc(vpcName)
232+
require.NoError(t, err)
233+
})
234+
235+
t.Run("add static routes from VPC spec when none exist", func(t *testing.T) {
236+
fakeController := newFakeController(t)
237+
ctrl := fakeController.fakeController
238+
fakeinformers := fakeController.fakeInformers
239+
mockOvnClient := fakeController.mockOvnClient
240+
241+
ctrl.vpcKeyMutex = keymutex.NewHashed(500)
242+
243+
vpc := &kubeovnv1.Vpc{
244+
ObjectMeta: metav1.ObjectMeta{
245+
Name: vpcName,
246+
},
247+
Spec: kubeovnv1.VpcSpec{
248+
StaticRoutes: []*kubeovnv1.StaticRoute{
249+
{
250+
CIDR: "192.168.0.0/24",
251+
NextHopIP: "10.0.0.1",
252+
Policy: kubeovnv1.PolicyDst,
253+
RouteTable: util.MainRouteTable,
254+
},
255+
},
256+
EnableExternal: false,
257+
PolicyRoutes: nil,
258+
},
259+
Status: kubeovnv1.VpcStatus{
260+
Subnets: []string{},
261+
EnableExternal: false,
262+
},
263+
}
264+
265+
_, err := ctrl.config.KubeOvnClient.KubeovnV1().Vpcs().Create(context.Background(), vpc, metav1.CreateOptions{})
266+
require.NoError(t, err)
267+
268+
err = fakeinformers.vpcInformer.Informer().GetStore().Add(vpc)
269+
require.NoError(t, err)
270+
271+
externalIDs := map[string]string{"vendor": util.CniTypeName}
272+
273+
mockOvnClient.EXPECT().CreateLogicalRouter(vpcName).Return(nil)
274+
mockOvnClient.EXPECT().UpdateLogicalRouter(gomock.Any(), gomock.Any()).Return(nil)
275+
mockOvnClient.EXPECT().ListLogicalRouterStaticRoutes(vpcName, nil, nil, "", externalIDs).Return(nil, nil)
276+
mockOvnClient.EXPECT().GetLogicalRouter(vpcName, false).Return(&ovnnb.LogicalRouter{
277+
Name: vpcName,
278+
Nat: []string{},
279+
}, nil)
280+
mockOvnClient.EXPECT().AddLogicalRouterStaticRoute(
281+
vpcName,
282+
util.MainRouteTable,
283+
"dst-ip",
284+
"192.168.0.0/24",
285+
nil,
286+
nil,
287+
"10.0.0.1",
288+
).Return(nil)
289+
mockOvnClient.EXPECT().ClearLogicalRouterPolicy(vpcName).Return(nil)
290+
mockOvnClient.EXPECT().ListLogicalSwitch(gomock.Any(), gomock.Any()).Return([]ovnnb.LogicalSwitch{}, nil).AnyTimes()
291+
mockOvnClient.EXPECT().ListLogicalRouter(gomock.Any(), gomock.Any()).Return([]ovnnb.LogicalRouter{}, nil).AnyTimes()
292+
mockOvnClient.EXPECT().DeleteLogicalRouterPort(fmt.Sprintf("bfd@%s", vpcName)).Return(nil)
293+
mockOvnClient.EXPECT().DeleteHAChassisGroup(fmt.Sprintf("bfd@%s", vpcName)).Return(nil)
294+
err = ctrl.handleAddOrUpdateVpc(vpcName)
295+
require.NoError(t, err)
296+
})
297+
}

0 commit comments

Comments
 (0)