Skip to content

Commit 5e39470

Browse files
filanovfilanov
committed
imp: flow controller check bridge before adding flows
flow controller will check if relevant bridge was marked by rail cni befor trying to add flows it will avoid adding flows to a non relevant interfaces Signed-off-by: Michael Filanov <[email protected]>
1 parent 878f4d6 commit 5e39470

File tree

5 files changed

+111
-0
lines changed

5 files changed

+111
-0
lines changed

‎internal/controller/flow_controller.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,18 @@ func (r *FlowReconciler) handlePodFlows(ctx context.Context, pod *corev1.Pod, re
125125
continue
126126
}
127127

128+
isManaged, err := r.Flows.IsBridgeManagedByRailCNI(bridge, string(pod.UID))
129+
if err != nil {
130+
logr.Error(err, fmt.Sprintf("failed to check if bridge %s is managed by rail cni", bridge))
131+
errs = multierr.Append(errs, err)
132+
continue
133+
}
134+
135+
if !isManaged {
136+
logr.Info(fmt.Sprintf("skipping bridge [%s], not managed by rail cni", bridge))
137+
continue
138+
}
139+
128140
if err = r.Flows.AddPodRailFlows(cookie, rep, bridge, ns.IPs[0], ns.Mac); err != nil {
129141
logr.Error(err, fmt.Sprintf("failed to add flows to rail %s", ns.Interface))
130142
errs = multierr.Append(errs, err)

‎internal/controller/flow_controller_test.go‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,13 +200,15 @@ var _ = Describe("Pod Controller", func() {
200200
Execute("ovs-vsctl --no-heading --columns=name find Port external_ids:contIface=net1 external_ids:contPodUid="+string(pod.UID)).
201201
Return("pod-vf-1", nil).Times(1)
202202
execMock.EXPECT().Execute("ovs-vsctl iface-to-br pod-vf-1").Return("br-rail1", nil).Times(1)
203+
flowsMock.EXPECT().IsBridgeManagedByRailCNI(gomock.Any(), gomock.Any()).Return(true, nil).Times(1)
203204
flowsMock.EXPECT().AddPodRailFlows(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
204205

205206
// rail2
206207
execMock.EXPECT().
207208
Execute("ovs-vsctl --no-heading --columns=name find Port external_ids:contIface=net2 external_ids:contPodUid="+string(pod.UID)).
208209
Return("pod-vf-2", nil)
209210
execMock.EXPECT().Execute("ovs-vsctl iface-to-br pod-vf-2").Return("br-rail2", nil)
211+
flowsMock.EXPECT().IsBridgeManagedByRailCNI(gomock.Any(), gomock.Any()).Return(true, nil).Times(1)
210212
flowsMock.EXPECT().AddPodRailFlows(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
211213

212214
result, err := flowController.Reconcile(ctx, pod)
@@ -225,6 +227,7 @@ var _ = Describe("Pod Controller", func() {
225227
Execute("ovs-vsctl --no-heading --columns=name find Port external_ids:contIface=net2 external_ids:contPodUid="+string(pod.UID)).
226228
Return("pod-vf-2", nil)
227229
execMock.EXPECT().Execute("ovs-vsctl iface-to-br pod-vf-2").Return("br-rail2", nil)
230+
flowsMock.EXPECT().IsBridgeManagedByRailCNI(gomock.Any(), gomock.Any()).Return(true, nil).Times(1)
228231
flowsMock.EXPECT().AddPodRailFlows(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
229232

230233
result, err := flowController.Reconcile(ctx, pod)
@@ -245,6 +248,7 @@ var _ = Describe("Pod Controller", func() {
245248
Return("pod-vf-2", nil)
246249
execMock.EXPECT().Execute("ovs-vsctl iface-to-br pod-vf-2").Return("br-rail2", nil)
247250

251+
flowsMock.EXPECT().IsBridgeManagedByRailCNI(gomock.Any(), gomock.Any()).Return(true, nil).Times(1)
248252
flowsMock.EXPECT().AddPodRailFlows(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
249253
result, err := flowController.Reconcile(ctx, pod)
250254
Expect(err).Should(HaveOccurred())
@@ -257,6 +261,7 @@ var _ = Describe("Pod Controller", func() {
257261
Execute("ovs-vsctl --no-heading --columns=name find Port external_ids:contIface=net1 external_ids:contPodUid="+string(pod.UID)).
258262
Return("pod-vf-1", nil).Times(1)
259263
execMock.EXPECT().Execute("ovs-vsctl iface-to-br pod-vf-1").Return("br-rail1", nil).Times(1)
264+
flowsMock.EXPECT().IsBridgeManagedByRailCNI(gomock.Any(), gomock.Any()).Return(true, nil).Times(1)
260265
flowsMock.EXPECT().AddPodRailFlows(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
261266
Return(fmt.Errorf("failed to add flows to rail"))
262267

@@ -265,12 +270,55 @@ var _ = Describe("Pod Controller", func() {
265270
Execute("ovs-vsctl --no-heading --columns=name find Port external_ids:contIface=net2 external_ids:contPodUid="+string(pod.UID)).
266271
Return("pod-vf-2", nil)
267272
execMock.EXPECT().Execute("ovs-vsctl iface-to-br pod-vf-2").Return("br-rail2", nil)
273+
flowsMock.EXPECT().IsBridgeManagedByRailCNI(gomock.Any(), gomock.Any()).Return(true, nil).Times(1)
268274
flowsMock.EXPECT().AddPodRailFlows(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
269275

270276
result, err := flowController.Reconcile(ctx, pod)
271277
Expect(err).Should(HaveOccurred())
272278
Expect(result).To(Equal(reconcile.Result{}))
273279
})
274280

281+
It("skip when bridge is not managed by rail cni", func() {
282+
// rail1
283+
execMock.EXPECT().
284+
Execute("ovs-vsctl --no-heading --columns=name find Port external_ids:contIface=net1 external_ids:contPodUid="+string(pod.UID)).
285+
Return("pod-vf-1", nil).Times(1)
286+
execMock.EXPECT().Execute("ovs-vsctl iface-to-br pod-vf-1").Return("br-rail1", nil).Times(1)
287+
flowsMock.EXPECT().IsBridgeManagedByRailCNI(gomock.Any(), gomock.Any()).Return(false, nil).Times(1)
288+
289+
// rail2
290+
execMock.EXPECT().
291+
Execute("ovs-vsctl --no-heading --columns=name find Port external_ids:contIface=net2 external_ids:contPodUid="+string(pod.UID)).
292+
Return("pod-vf-2", nil)
293+
execMock.EXPECT().Execute("ovs-vsctl iface-to-br pod-vf-2").Return("br-rail2", nil)
294+
flowsMock.EXPECT().IsBridgeManagedByRailCNI(gomock.Any(), gomock.Any()).Return(true, nil).Times(1)
295+
flowsMock.EXPECT().AddPodRailFlows(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
296+
297+
result, err := flowController.Reconcile(ctx, pod)
298+
Expect(err).Should(Succeed())
299+
Expect(result).To(Equal(reconcile.Result{}))
300+
})
301+
302+
It("failed to check if bridge is managed by rail cni", func() {
303+
// rail1
304+
execMock.EXPECT().
305+
Execute("ovs-vsctl --no-heading --columns=name find Port external_ids:contIface=net1 external_ids:contPodUid="+string(pod.UID)).
306+
Return("pod-vf-1", nil).Times(1)
307+
execMock.EXPECT().Execute("ovs-vsctl iface-to-br pod-vf-1").Return("br-rail1", nil).Times(1)
308+
flowsMock.EXPECT().
309+
IsBridgeManagedByRailCNI(gomock.Any(), gomock.Any()).
310+
Return(false, fmt.Errorf("failed to check if bridge is managed by rail cni"))
311+
312+
// rail2
313+
execMock.EXPECT().
314+
Execute("ovs-vsctl --no-heading --columns=name find Port external_ids:contIface=net2 external_ids:contPodUid="+string(pod.UID)).
315+
Return("pod-vf-2", nil)
316+
execMock.EXPECT().Execute("ovs-vsctl iface-to-br pod-vf-2").Return("br-rail2", nil)
317+
flowsMock.EXPECT().IsBridgeManagedByRailCNI(gomock.Any(), gomock.Any()).Return(true, nil).Times(1)
318+
flowsMock.EXPECT().AddPodRailFlows(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
319+
result, err := flowController.Reconcile(ctx, pod)
320+
Expect(err).Should(HaveOccurred())
321+
Expect(result).To(Equal(reconcile.Result{}))
322+
})
275323
})
276324
})

‎internal/controller/flows.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ const (
3737
type FlowsAPI interface {
3838
AddPodRailFlows(cookie uint64, vf, bridge, podIP, podMAC string) error
3939
DeletePodRailFlows(cookie uint64, podID string) error
40+
IsBridgeManagedByRailCNI(bridge, podID string) (bool, error)
4041
}
4142

4243
var _ FlowsAPI = &Flows{}
@@ -141,6 +142,15 @@ func (f *Flows) DeletePodRailFlows(cookie uint64, podID string) error {
141142
return errs
142143
}
143144

145+
func (f *Flows) IsBridgeManagedByRailCNI(bridge, podID string) (bool, error) {
146+
out, err := f.Exec.Execute(fmt.Sprintf("ovs-vsctl br-get-external-id %s %s", bridge, podID))
147+
if err != nil {
148+
return false, fmt.Errorf("failed to get external id for bridge %s, output: %s, err: %v", bridge, out, err)
149+
}
150+
151+
return out == RailPodID, nil
152+
}
153+
144154
func (f *Flows) getTorMac(torIP string) (string, error) {
145155
// nsenter --target 1 --net -- arping 2.0.0.3 -c 1
146156
// nsenter --target 1 --net -- ip neighbor | grep 2.0.0.3 | awk '{print $5}'

‎internal/controller/flows_test.go‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,4 +295,30 @@ var _ = Describe("Flows", func() {
295295
Expect(err.Error()).Should(ContainSubstring("failed to delete flows"))
296296
})
297297
})
298+
299+
Context("IsBridgeManagedByRailCNI", func() {
300+
It("should return true if bridge is managed by rail cni", func() {
301+
execMock.EXPECT().Execute("ovs-vsctl br-get-external-id br-rail1 test-pod-uid").
302+
Return("rail_pod_id", nil)
303+
isManaged, err := flows.IsBridgeManagedByRailCNI("br-rail1", "test-pod-uid")
304+
Expect(err).Should(Succeed())
305+
Expect(isManaged).Should(BeTrue())
306+
})
307+
308+
It("should return false if bridge is not managed by rail cni", func() {
309+
execMock.EXPECT().Execute("ovs-vsctl br-get-external-id br-rail1 test-pod-uid").
310+
Return("", nil)
311+
isManaged, err := flows.IsBridgeManagedByRailCNI("br-rail1", "test-pod-uid")
312+
Expect(err).Should(Succeed())
313+
Expect(isManaged).Should(BeFalse())
314+
})
315+
316+
It("should return error if failed to get external id", func() {
317+
execMock.EXPECT().Execute("ovs-vsctl br-get-external-id br-rail1 test-pod-uid").
318+
Return("", fmt.Errorf("failed to get external id"))
319+
isManaged, err := flows.IsBridgeManagedByRailCNI("br-rail1", "test-pod-uid")
320+
Expect(err).Should(HaveOccurred())
321+
Expect(isManaged).Should(BeFalse())
322+
})
323+
})
298324
})

‎internal/controller/mock_flows.go‎

Lines changed: 15 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)