Skip to content

Commit ade593c

Browse files
danwinshipdanwinship
committed
Make getPodDetailsBySandboxID a method of ovsController
getPodDetailsBySandboxID() was written to have the list of flows passed into it, for ease of testing. But that requires the caller to know what flows it is looking for, and anyway, ovsController is already mockable anyway so we can test the function just as easily as an ovsController method.
1 parent 0895dfe commit ade593c

File tree

2 files changed

+16
-73
lines changed

2 files changed

+16
-73
lines changed

‎pkg/network/node/ovscontroller.go‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -338,11 +338,15 @@ func (oc *ovsController) SetPodBandwidth(hostVeth string, ingressBPS, egressBPS
338338
return nil
339339
}
340340

341-
func getPodDetailsBySandboxID(flows []string, sandboxID string) (int, string, string, string, error) {
341+
func (oc *ovsController) getPodDetailsBySandboxID(sandboxID string) (int, string, string, string, error) {
342342
note, err := getPodNote(sandboxID)
343343
if err != nil {
344344
return 0, "", "", "", err
345345
}
346+
flows, err := oc.ovs.DumpFlows()
347+
if err != nil {
348+
return 0, "", "", "", err
349+
}
346350

347351
// Find the table=20 flow with the given note and extract the podIP, ofport, and MAC from them
348352
for _, flow := range flows {
@@ -381,11 +385,7 @@ func getPodDetailsBySandboxID(flows []string, sandboxID string) (int, string, st
381385
}
382386

383387
func (oc *ovsController) UpdatePod(sandboxID string, vnid uint32) error {
384-
flows, err := oc.ovs.DumpFlows()
385-
if err != nil {
386-
return err
387-
}
388-
ofport, podIP, podMAC, note, err := getPodDetailsBySandboxID(flows, sandboxID)
388+
ofport, podIP, podMAC, note, err := oc.getPodDetailsBySandboxID(sandboxID)
389389
if err != nil {
390390
return err
391391
}
@@ -398,11 +398,7 @@ func (oc *ovsController) UpdatePod(sandboxID string, vnid uint32) error {
398398

399399
func (oc *ovsController) TearDownPod(hostVeth, podIP, sandboxID string) error {
400400
if podIP == "" {
401-
flows, err := oc.ovs.DumpFlows()
402-
if err != nil {
403-
return err
404-
}
405-
_, ip, _, _, err := getPodDetailsBySandboxID(flows, sandboxID)
401+
_, ip, _, _, err := oc.getPodDetailsBySandboxID(sandboxID)
406402
if err != nil {
407403
// OVS flows related to sandboxID not found
408404
// Nothing needs to be done in that case

‎pkg/network/node/ovscontroller_test.go‎

Lines changed: 9 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -321,8 +321,6 @@ func TestOVSPod(t *testing.T) {
321321
func TestGetPodDetails(t *testing.T) {
322322
type testcase struct {
323323
sandboxID string
324-
flows []string
325-
ofport int
326324
ip string
327325
mac string
328326
note string
@@ -332,71 +330,20 @@ func TestGetPodDetails(t *testing.T) {
332330
testcases := []testcase{
333331
{
334332
sandboxID: sandboxID,
335-
flows: []string{
336-
"cookie=0x0, duration=12.243s, table=0, n_packets=0, n_bytes=0, priority=250,ip,in_port=2,nw_dst=224.0.0.0/4 actions=drop",
337-
"cookie=0x0, duration=12.258s, table=0, n_packets=0, n_bytes=0, priority=200,arp,in_port=1,arp_spa=10.128.0.0/14,arp_tpa=10.130.0.0/23 actions=move:NXM_NX_TUN_ID[0..31]->NXM_NX_REG0[],goto_table:10",
338-
"cookie=0x0, duration=12.255s, table=0, n_packets=0, n_bytes=0, priority=200,ip,in_port=1,nw_src=10.128.0.0/14,nw_dst=10.130.0.0/23 actions=move:NXM_NX_TUN_ID[0..31]->NXM_NX_REG0[],goto_table:10",
339-
"cookie=0x0, duration=12.252s, table=0, n_packets=0, n_bytes=0, priority=200,ip,in_port=1,nw_src=10.128.0.0/14,nw_dst=224.0.0.0/4 actions=move:NXM_NX_TUN_ID[0..31]->NXM_NX_REG0[],goto_table:10",
340-
"cookie=0x0, duration=12.237s, table=0, n_packets=0, n_bytes=0, priority=200,arp,in_port=2,arp_spa=10.130.0.1,arp_tpa=10.128.0.0/14 actions=goto_table:30",
341-
"cookie=0x0, duration=12.234s, table=0, n_packets=0, n_bytes=0, priority=200,ip,in_port=2 actions=goto_table:30",
342-
"cookie=0x0, duration=12.248s, table=0, n_packets=0, n_bytes=0, priority=150,in_port=1 actions=drop",
343-
"cookie=0x0, duration=12.230s, table=0, n_packets=7, n_bytes=586, priority=150,in_port=2 actions=drop",
344-
"cookie=0x0, duration=12.222s, table=0, n_packets=0, n_bytes=0, priority=100,arp actions=goto_table:20",
345-
"cookie=0x0, duration=12.218s, table=0, n_packets=0, n_bytes=0, priority=100,ip actions=goto_table:20",
346-
"cookie=0x0, duration=12.215s, table=0, n_packets=5, n_bytes=426, priority=0 actions=drop",
347-
"cookie=0x0, duration=12.063s, table=10, n_packets=0, n_bytes=0, priority=100,tun_src=172.17.0.3 actions=goto_table:30",
348-
"cookie=0x0, duration=12.041s, table=10, n_packets=0, n_bytes=0, priority=100,tun_src=172.17.0.4 actions=goto_table:30",
349-
"cookie=0x0, duration=12.211s, table=10, n_packets=0, n_bytes=0, priority=0 actions=drop",
350-
"cookie=0x0, duration=3.202s, table=20, n_packets=0, n_bytes=0, priority=100,arp,in_port=3,arp_spa=10.130.0.2,arp_sha=4a:77:32:e4:ab:9d actions=load:0->NXM_NX_REG0[],note:bc.b5.d8.d2.87.fc.f9.74.58.c4.8a.d6.43.b1.01.07.9e.3b.c2.65.a9.4e.09.7e.74.07.44.07.16.11.2f.69.00.00.00.00.00.00,goto_table:21",
351-
"cookie=0x0, duration=3.197s, table=20, n_packets=0, n_bytes=0, priority=100,ip,in_port=3,nw_src=10.130.0.2 actions=load:0->NXM_NX_REG0[],goto_table:21",
352-
"cookie=0x0, duration=12.205s, table=20, n_packets=0, n_bytes=0, priority=0 actions=drop",
353-
"cookie=0x0, duration=12.201s, table=21, n_packets=0, n_bytes=0, priority=0 actions=goto_table:30",
354-
"cookie=0x0, duration=12.196s, table=30, n_packets=0, n_bytes=0, priority=300,arp,arp_tpa=10.130.0.1 actions=output:2",
355-
"cookie=0x0, duration=12.182s, table=30, n_packets=0, n_bytes=0, priority=300,ip,nw_dst=10.130.0.1 actions=output:2",
356-
"cookie=0x0, duration=12.190s, table=30, n_packets=0, n_bytes=0, priority=200,arp,arp_tpa=10.130.0.0/23 actions=goto_table:40",
357-
"cookie=0x0, duration=12.173s, table=30, n_packets=0, n_bytes=0, priority=200,ip,nw_dst=10.130.0.0/23 actions=goto_table:70",
358-
"cookie=0x0, duration=12.186s, table=30, n_packets=0, n_bytes=0, priority=100,arp,arp_tpa=10.128.0.0/14 actions=goto_table:50",
359-
"cookie=0x0, duration=12.169s, table=30, n_packets=0, n_bytes=0, priority=100,ip,nw_dst=10.128.0.0/14 actions=goto_table:90",
360-
"cookie=0x0, duration=12.178s, table=30, n_packets=0, n_bytes=0, priority=100,ip,nw_dst=172.30.0.0/16 actions=goto_table:60",
361-
"cookie=0x0, duration=12.165s, table=30, n_packets=0, n_bytes=0, priority=50,ip,in_port=1,nw_dst=224.0.0.0/4 actions=goto_table:120",
362-
"cookie=0x0, duration=12.160s, table=30, n_packets=0, n_bytes=0, priority=25,ip,nw_dst=224.0.0.0/4 actions=goto_table:110",
363-
"cookie=0x0, duration=12.154s, table=30, n_packets=0, n_bytes=0, priority=0,ip actions=goto_table:100",
364-
"cookie=0x0, duration=12.150s, table=30, n_packets=0, n_bytes=0, priority=0,arp actions=drop",
365-
"cookie=0x0, duration=3.190s, table=40, n_packets=0, n_bytes=0, priority=100,arp,arp_tpa=10.130.0.2 actions=output:3",
366-
"cookie=0x0, duration=12.147s, table=40, n_packets=0, n_bytes=0, priority=0 actions=drop",
367-
"cookie=0x0, duration=12.058s, table=50, n_packets=0, n_bytes=0, priority=100,arp,arp_tpa=10.128.0.0/23 actions=move:NXM_NX_REG0[]->NXM_NX_TUN_ID[0..31],set_field:172.17.0.3->tun_dst,output:1",
368-
"cookie=0x0, duration=12.037s, table=50, n_packets=0, n_bytes=0, priority=100,arp,arp_tpa=10.129.0.0/23 actions=move:NXM_NX_REG0[]->NXM_NX_TUN_ID[0..31],set_field:172.17.0.4->tun_dst,output:1",
369-
"cookie=0x0, duration=12.143s, table=50, n_packets=0, n_bytes=0, priority=0 actions=drop",
370-
"cookie=0x0, duration=12.139s, table=60, n_packets=0, n_bytes=0, priority=200,reg0=0 actions=output:2",
371-
"cookie=0x0, duration=11.938s, table=60, n_packets=0, n_bytes=0, priority=100,ip,nw_dst=172.30.0.1,nw_frag=later actions=load:0->NXM_NX_REG1[],load:0x2->NXM_NX_REG2[],goto_table:80",
372-
"cookie=0x0, duration=11.929s, table=60, n_packets=0, n_bytes=0, priority=100,tcp,nw_dst=172.30.0.1,tp_dst=443 actions=load:0->NXM_NX_REG1[],load:0x2->NXM_NX_REG2[],goto_table:80",
373-
"cookie=0x0, duration=11.926s, table=60, n_packets=0, n_bytes=0, priority=100,udp,nw_dst=172.30.0.1,tp_dst=53 actions=load:0->NXM_NX_REG1[],load:0x2->NXM_NX_REG2[],goto_table:80",
374-
"cookie=0x0, duration=11.922s, table=60, n_packets=0, n_bytes=0, priority=100,tcp,nw_dst=172.30.0.1,tp_dst=53 actions=load:0->NXM_NX_REG1[],load:0x2->NXM_NX_REG2[],goto_table:80",
375-
"cookie=0x0, duration=12.136s, table=60, n_packets=0, n_bytes=0, priority=0 actions=drop",
376-
"cookie=0x0, duration=3.179s, table=70, n_packets=0, n_bytes=0, priority=100,ip,nw_dst=10.130.0.2 actions=load:0->NXM_NX_REG1[],load:0x3->NXM_NX_REG2[],goto_table:80",
377-
"cookie=0x0, duration=12.133s, table=70, n_packets=0, n_bytes=0, priority=0 actions=drop",
378-
"cookie=0x0, duration=12.129s, table=80, n_packets=0, n_bytes=0, priority=300,ip,nw_src=10.130.0.1 actions=output:NXM_NX_REG2[]",
379-
"cookie=0x0, duration=12.062s, table=80, n_packets=0, n_bytes=0, priority=200,reg0=0 actions=output:NXM_NX_REG2[]",
380-
"cookie=0x0, duration=12.055s, table=80, n_packets=0, n_bytes=0, priority=200,reg1=0 actions=output:NXM_NX_REG2[]",
381-
"cookie=0x0, duration=12.124s, table=80, n_packets=0, n_bytes=0, priority=0 actions=drop",
382-
"cookie=0x0, duration=12.053s, table=90, n_packets=0, n_bytes=0, priority=100,ip,nw_dst=10.128.0.0/23 actions=move:NXM_NX_REG0[]->NXM_NX_TUN_ID[0..31],set_field:172.17.0.3->tun_dst,output:1",
383-
"cookie=0x0, duration=12.030s, table=90, n_packets=0, n_bytes=0, priority=100,ip,nw_dst=10.129.0.0/23 actions=move:NXM_NX_REG0[]->NXM_NX_TUN_ID[0..31],set_field:172.17.0.4->tun_dst,output:1",
384-
"cookie=0x0, duration=12.120s, table=90, n_packets=0, n_bytes=0, priority=0 actions=drop",
385-
"cookie=0x0, duration=12.116s, table=100, n_packets=0, n_bytes=0, priority=0 actions=output:2",
386-
"cookie=0x0, duration=12.112s, table=110, n_packets=0, n_bytes=0, priority=0 actions=drop",
387-
"cookie=0x0, duration=12.024s, table=111, n_packets=0, n_bytes=0, priority=100 actions=move:NXM_NX_REG0[]->NXM_NX_TUN_ID[0..31],set_field:172.17.0.3->tun_dst,output:1,set_field:172.17.0.4->tun_dst,output:1,goto_table:120",
388-
"cookie=0x0, duration=12.105s, table=120, n_packets=0, n_bytes=0, priority=0 actions=drop",
389-
"cookie=0x0, duration=12.101s, table=253, n_packets=0, n_bytes=0, actions=note:01.03.00.00.00.00",
390-
},
391-
ofport: 3,
392333
ip: "10.130.0.2",
393334
mac: "4a:77:32:e4:ab:9d",
394335
note: sandboxNote,
395336
},
396337
}
397338

398339
for _, tc := range testcases {
399-
ofport, ip, mac, note, err := getPodDetailsBySandboxID(tc.flows, tc.sandboxID)
340+
_, oc, _ := setupOVSController(t)
341+
tcOFPort, err := oc.SetUpPod("veth1", tc.ip, tc.mac, tc.sandboxID, 42)
342+
if err != nil {
343+
t.Fatalf("Unexpected error adding pod rules: %v", err)
344+
}
345+
346+
ofport, ip, mac, note, err := oc.getPodDetailsBySandboxID(tc.sandboxID)
400347
if err != nil {
401348
if tc.errStr != "" {
402349
if !strings.Contains(err.Error(), tc.errStr) {
@@ -408,8 +355,8 @@ func TestGetPodDetails(t *testing.T) {
408355
} else if tc.errStr != "" {
409356
t.Fatalf("expected error %q", tc.errStr)
410357
}
411-
if ofport != tc.ofport {
412-
t.Fatalf("unexpected ofport %d (expected %d)", ofport, tc.ofport)
358+
if ofport != tcOFPort {
359+
t.Fatalf("unexpected ofport %d (expected %d)", ofport, tcOFPort)
413360
}
414361
if ip != tc.ip {
415362
t.Fatalf("unexpected ip %q (expected %q)", ip, tc.ip)

0 commit comments

Comments
 (0)