Skip to content

Commit

Permalink
Improve unit test coverage
Browse files Browse the repository at this point in the history
  • Loading branch information
@srikartati
srikartati committed Aug 11, 2020
1 parent c3b6a7f commit a22a245
Show file tree
Hide file tree
Showing 5 changed files with 225 additions and 78 deletions.
127 changes: 124 additions & 3 deletions pkg/agent/flowexporter/connections/connections_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,13 +117,13 @@ func TestConnectionStore_addAndUpdateConn(t *testing.T) {
testFlow1Tuple := flowexporter.NewConnectionKey(&testFlow1)
connStore.connections[testFlow1Tuple] = oldTestFlow1

updateConnTests := []struct {
addOrUpdateConnTests := []struct {
flow flowexporter.Connection
}{
{testFlow1}, // To test update part of function
{testFlow2}, // To test add part of function
}
for i, test := range updateConnTests {
for i, test := range addOrUpdateConnTests {
flowTuple := flowexporter.NewConnectionKey(&test.flow)
var expConn flowexporter.Connection
if i == 0 {
Expand All @@ -138,7 +138,128 @@ func TestConnectionStore_addAndUpdateConn(t *testing.T) {
iStore.EXPECT().GetInterfaceByIP(test.flow.TupleReply.SourceAddress.String()).Return(interfaceFlow2, true)
}
connStore.addOrUpdateConn(&test.flow)
actualConn, _ := connStore.GetConnByKey(flowTuple)
actualConn, ok := connStore.GetConnByKey(flowTuple)
assert.Equal(t, ok, true, "connection should be there in connection store")
assert.Equal(t, expConn, *actualConn, "Connections should be equal")
}
}

func TestConnectionStore_ForAllConnectionsDo(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
// Create two flows; one is already in connectionStore and other one is new
testFlows := make([]*flowexporter.Connection, 2)
testFlowKeys := make([]*flowexporter.ConnectionKey, 2)
refTime := time.Now()
// Flow-1, which is already in connectionStore
tuple1, revTuple1 := makeTuple(&net.IP{1, 2, 3, 4}, &net.IP{4, 3, 2, 1}, 6, 65280, 255)
testFlows[0] = &flowexporter.Connection{
StartTime: refTime.Add(-(time.Second * 50)),
StopTime: refTime,
OriginalPackets: 0xffff,
OriginalBytes: 0xbaaaaa0000000000,
ReversePackets: 0xff,
ReverseBytes: 0xbaaa,
TupleOrig: *tuple1,
TupleReply: *revTuple1,
IsActive: true,
}
// Flow-2, which is not in connectionStore
tuple2, revTuple2 := makeTuple(&net.IP{5, 6, 7, 8}, &net.IP{8, 7, 6, 5}, 6, 60001, 200)
testFlows[1] = &flowexporter.Connection{
StartTime: refTime.Add(-(time.Second * 20)),
StopTime: refTime,
OriginalPackets: 0xbb,
OriginalBytes: 0xcbbb,
ReversePackets: 0xbbbb,
ReverseBytes: 0xcbbbb0000000000,
TupleOrig: *tuple2,
TupleReply: *revTuple2,
IsActive: true,
}
for i, flow := range testFlows {
connKey := flowexporter.NewConnectionKey(flow)
testFlowKeys[i] = &connKey
}
// Create connectionStore
connStore := &connectionStore{
connections: make(map[flowexporter.ConnectionKey]flowexporter.Connection),
connDumper: nil,
ifaceStore: nil,
}
// Add flows to the Connection store
for i, flow := range testFlows {
connStore.connections[*testFlowKeys[i]] = *flow
}

resetTwoFields := func(key flowexporter.ConnectionKey, conn flowexporter.Connection) error {
conn.IsActive = false
conn.OriginalPackets = 0
connStore.connections[key] = conn
return nil
}
connStore.ForAllConnectionsDo(resetTwoFields)
// Check isActive and OriginalPackets, if they are reset or not.
for i := 0; i < len(testFlows); i++ {
conn, ok := connStore.GetConnByKey(*testFlowKeys[i])
assert.Equal(t, ok, true, "connection should be there in connection store")
assert.Equal(t, conn.IsActive, false, "isActive flag should be reset")
assert.Equal(t, conn.OriginalPackets, uint64(0), "OriginalPackets should be reset")
}
}

func TestConnectionStore_DeleteConnectionByKey(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
// Create two flows; one is already in connectionStore and other one is new
testFlows := make([]*flowexporter.Connection, 2)
testFlowKeys := make([]*flowexporter.ConnectionKey, 2)
refTime := time.Now()
// Flow-1, which is already in connectionStore
tuple1, revTuple1 := makeTuple(&net.IP{1, 2, 3, 4}, &net.IP{4, 3, 2, 1}, 6, 65280, 255)
testFlows[0] = &flowexporter.Connection{
StartTime: refTime.Add(-(time.Second * 50)),
StopTime: refTime,
OriginalPackets: 0xffff,
OriginalBytes: 0xbaaaaa0000000000,
ReversePackets: 0xff,
ReverseBytes: 0xbaaa,
TupleOrig: *tuple1,
TupleReply: *revTuple1,
IsActive: true,
}
// Flow-2, which is not in connectionStore
tuple2, revTuple2 := makeTuple(&net.IP{5, 6, 7, 8}, &net.IP{8, 7, 6, 5}, 6, 60001, 200)
testFlows[1] = &flowexporter.Connection{
StartTime: refTime.Add(-(time.Second * 20)),
StopTime: refTime,
OriginalPackets: 0xbb,
OriginalBytes: 0xcbbb,
ReversePackets: 0xbbbb,
ReverseBytes: 0xcbbbb0000000000,
TupleOrig: *tuple2,
TupleReply: *revTuple2,
IsActive: true,
}
for i, flow := range testFlows {
connKey := flowexporter.NewConnectionKey(flow)
testFlowKeys[i] = &connKey
}
// Create connectionStore
connStore := &connectionStore{
connections: make(map[flowexporter.ConnectionKey]flowexporter.Connection),
connDumper: nil,
ifaceStore: nil,
}
// Add flows to the connection store.
for i, flow := range testFlows {
connStore.connections[*testFlowKeys[i]] = *flow
}
// Delete the connections in connection store.
for i := 0; i < len(testFlows); i++ {
err := connStore.DeleteConnectionByKey(*testFlowKeys[i])
assert.Nil(t, err, "DeleteConnectionByKey should return nil")
_, exists := connStore.GetConnByKey(*testFlowKeys[i])
assert.Equal(t, exists, false, "connection should be deleted in connection store")
}
}
5 changes: 3 additions & 2 deletions pkg/agent/flowexporter/connections/conntrack_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ func (ctnd *connTrackNetdev) DumpFilter(filter interface{}) ([]*flowexporter.Con
conn := flowexporter.Connection{}
flowSlice := strings.Split(flow, ",")
isReply := false
inZone := true
inZone := false
for _, fs := range flowSlice {
// Indicator to populate reply or reverse fields
if strings.Contains(fs, "reply") {
Expand Down Expand Up @@ -277,9 +277,9 @@ func (ctnd *connTrackNetdev) DumpFilter(filter interface{}) ([]*flowexporter.Con
continue
}
if zoneFilter != uint16(val) {
inZone = false
break
} else {
inZone = true
conn.Zone = uint16(val)
}
} else if strings.Contains(fs, "timeout") {
Expand All @@ -301,6 +301,7 @@ func (ctnd *connTrackNetdev) DumpFilter(filter interface{}) ([]*flowexporter.Con
}
}
if inZone {
conn.IsActive = true
antreaConns = append(antreaConns, &conn)
}
}
Expand Down
146 changes: 88 additions & 58 deletions pkg/agent/flowexporter/connections/conntrack_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package connections
import (
"net"
"testing"
"time"

"github.com/golang/mock/gomock"
"github.com/stretchr/testify/assert"
Expand All @@ -32,76 +33,33 @@ import (
ovsctltest "github.com/vmware-tanzu/antrea/pkg/ovs/ovsctl/testing"
)

var (
tuple3 = flowexporter.Tuple{
SourceAddress: net.IP{1, 2, 3, 4},
DestinationAddress: net.IP{4, 3, 2, 1},
Protocol: 6,
SourcePort: 65280,
DestinationPort: 255,
}
revTuple3 = flowexporter.Tuple{
SourceAddress: net.IP{4, 3, 2, 1},
DestinationAddress: net.IP{1, 2, 3, 4},
Protocol: 6,
SourcePort: 255,
DestinationPort: 65280,
}
tuple4 = flowexporter.Tuple{
SourceAddress: net.IP{5, 6, 7, 8},
DestinationAddress: net.IP{8, 7, 6, 5},
Protocol: 6,
SourcePort: 60001,
DestinationPort: 200,
}
revTuple4 = flowexporter.Tuple{
SourceAddress: net.IP{8, 7, 6, 5},
DestinationAddress: net.IP{5, 6, 7, 8},
Protocol: 6,
SourcePort: 200,
DestinationPort: 60001,
}
tuple5 = flowexporter.Tuple{
SourceAddress: net.IP{1, 2, 3, 4},
DestinationAddress: net.IP{100, 50, 25, 5},
Protocol: 6,
SourcePort: 60001,
DestinationPort: 200,
}
revTuple5 = flowexporter.Tuple{
SourceAddress: net.IP{100, 50, 25, 5},
DestinationAddress: net.IP{1, 2, 3, 4},
Protocol: 6,
SourcePort: 200,
DestinationPort: 60001,
}
)

func TestConnTrack_DumpFlows(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
// Create flows to test
// Create flows for test
tuple, revTuple := makeTuple(&net.IP{1, 2, 3, 4}, &net.IP{4, 3, 2, 1}, 6, 65280, 255)
antreaFlow := &flowexporter.Connection{
TupleOrig: tuple3,
TupleReply: revTuple3,
TupleOrig: *tuple,
TupleReply: *revTuple,
Zone: openflow.CtZone,
}
tuple, revTuple = makeTuple(&net.IP{1, 2, 3, 4}, &net.IP{100, 50, 25, 5}, 6, 60001, 200)
antreaServiceFlow := &flowexporter.Connection{
TupleOrig: tuple5,
TupleReply: revTuple5,
TupleOrig: *tuple,
TupleReply: *revTuple,
Zone: openflow.CtZone,
}
tuple, revTuple = makeTuple(&net.IP{5, 6, 7, 8}, &net.IP{8, 7, 6, 5}, 6, 60001, 200)
antreaGWFlow := &flowexporter.Connection{
TupleOrig: tuple4,
TupleReply: revTuple4,
TupleOrig: *tuple,
TupleReply: *revTuple,
Zone: openflow.CtZone,
}
nonAntreaFlow := &flowexporter.Connection{
TupleOrig: tuple4,
TupleReply: revTuple4,
TupleOrig: *tuple,
TupleReply: *revTuple,
Zone: 100,
}

testFlows := []*flowexporter.Connection{antreaFlow, antreaServiceFlow, antreaGWFlow, nonAntreaFlow}

// Create mock interfaces
Expand All @@ -120,14 +78,86 @@ func TestConnTrack_DumpFlows(t *testing.T) {
IP: net.IP{100, 50, 25, 0},
Mask: net.IPMask{255, 255, 255, 0},
}
// set expects for mocks

// Test DumpFlows implementation of connTrackSystem
connDumperDPSystem := NewConnTrackDumper(mockCTInterfacer, nodeConfig, serviceCIDR, ovsconfig.OVSDatapathSystem, mockOVSCtlClient)
// Set expects for mocks
mockCTInterfacer.EXPECT().GetConnTrack(nil).Return(nil)
mockCTInterfacer.EXPECT().DumpFilter(conntrack.Filter{}).Return(testFlows, nil)

connDumper := NewConnTrackDumper(mockCTInterfacer, nodeConfig, serviceCIDR, ovsconfig.OVSDatapathSystem, mockOVSCtlClient)
conns, err := connDumper.DumpFlows(openflow.CtZone)
conns, err := connDumperDPSystem.DumpFlows(openflow.CtZone)
if err != nil {
t.Errorf("Dump flows function returned error: %v", err)
}
assert.Equal(t, 1, len(conns), "number of filtered connections should be equal")

// Test DumpFlows implementation of connTrackNetdev
connDumperDPNetdev := NewConnTrackDumper(mockCTInterfacer, nodeConfig, serviceCIDR, ovsconfig.OVSDatapathNetdev, mockOVSCtlClient)
// Re-initialize testFlows
testFlows = []*flowexporter.Connection{antreaFlow, antreaServiceFlow, antreaGWFlow, nonAntreaFlow}
// Set expects for mocks
mockCTInterfacer.EXPECT().GetConnTrack(mockOVSCtlClient).Return(nil)
mockCTInterfacer.EXPECT().DumpFilter(uint16(openflow.CtZone)).Return(testFlows, nil)

conns, err = connDumperDPNetdev.DumpFlows(openflow.CtZone)
if err != nil {
t.Errorf("Dump flows function returned error: %v", err)
}
assert.Equal(t, 1, len(conns), "number of filtered connections should be equal")
}

func TestConnTackNetdev_DumpFilter(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

// Create mock interfaces
mockOVSCtlClient := ovsctltest.NewMockOVSCtlClient(ctrl)
conntrackNetdev := NewConnTrackNetdev()
err := conntrackNetdev.GetConnTrack(mockOVSCtlClient)
assert.Nil(t, err, "GetConnTrack call should be successful")

// Set expect call for mock ovsCtlClient
ovsctlCmdOutput := []byte("tcp,orig=(src=127.0.0.1,dst=127.0.0.1,sport=45218,dport=2379,packets=320108,bytes=24615344),reply=(src=127.0.0.1,dst=127.0.0.1,sport=2379,dport=45218,packets=239595,bytes=24347883),start=2020-07-24T05:07:03.998,id=3750535678,status=SEEN_REPLY|ASSURED|CONFIRMED|SRC_NAT_DONE|DST_NAT_DONE,timeout=86399,protoinfo=(state_orig=ESTABLISHED,state_reply=ESTABLISHED,wscale_orig=7,wscale_reply=7,flags_orig=WINDOW_SCALE|SACK_PERM|MAXACK_SET,flags_reply=WINDOW_SCALE|SACK_PERM|MAXACK_SET)\n" +
"tcp,orig=(src=127.0.0.1,dst=127.0.0.1,sport=45170,dport=2379,packets=80743,bytes=5416239),reply=(src=127.0.0.1,dst=127.0.0.1,sport=2379,dport=45170,packets=63361,bytes=4811261),start=2020-07-24T05:07:01.591,id=462801621,status=SEEN_REPLY|ASSURED|CONFIRMED|SRC_NAT_DONE|DST_NAT_DONE,timeout=86397,protoinfo=(state_orig=ESTABLISHED,state_reply=ESTABLISHED,wscale_orig=7,wscale_reply=7,flags_orig=WINDOW_SCALE|SACK_PERM|MAXACK_SET,flags_reply=WINDOW_SCALE|SACK_PERM|MAXACK_SET)\n" +
"tcp,orig=(src=100.10.0.105,dst=10.96.0.1,sport=41284,dport=443,packets=343260,bytes=19340621),reply=(src=192.168.86.82,dst=100.10.0.105,sport=6443,dport=41284,packets=381035,bytes=181176472),start=2020-07-25T08:40:08.959,id=982464968,zone=65520,status=SEEN_REPLY|ASSURED|CONFIRMED|DST_NAT|DST_NAT_DONE,timeout=86399,mark=33,protoinfo=(state_orig=ESTABLISHED,state_reply=ESTABLISHED,wscale_orig=7,wscale_reply=7,flags_orig=WINDOW_SCALE|SACK_PERM|MAXACK_SET,flags_reply=WINDOW_SCALE|SACK_PERM|MAXACK_SET)")
expConn := &flowexporter.Connection{
ID: 982464968,
Timeout: 86399,
StartTime: time.Time{},
StopTime: time.Time{},
IsActive: true,
Zone: 65520,
StatusFlag: 0,
TupleOrig: flowexporter.Tuple{
net.ParseIP("100.10.0.105"),
net.ParseIP("10.96.0.1"),
6,
uint16(41284),
uint16(443),
},
TupleReply: flowexporter.Tuple{
net.ParseIP("192.168.86.82"),
net.ParseIP("100.10.0.105"),
6,
6443,
41284,
},
OriginalPackets: 0,
OriginalBytes: 0,
ReversePackets: 0,
ReverseBytes: 0,
SourcePodNamespace: "",
SourcePodName: "",
DestinationPodNamespace: "",
DestinationPodName: "",
}
mockOVSCtlClient.EXPECT().RunAppctlCmd("dpctl/dump-conntrack", false, "-m", "-s").Return(ovsctlCmdOutput, nil)

conns, err := conntrackNetdev.DumpFilter(uint16(openflow.CtZone))
if err != nil {
t.Errorf("conntrackNetdev.DumpFilter function returned error: %v", err)
}
assert.Equal(t, len(conns), 1)
assert.Equal(t, conns[0], expConn, "filtered connection and expected connection should be same")

}
24 changes: 9 additions & 15 deletions pkg/agent/flowexporter/exporter/exporter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,25 +153,19 @@ func TestFlowExporter_sendDataRecord(t *testing.T) {
// Expect calls required
var dataRecord ipfixentities.Record
tempBytes := uint16(0)
for i, ie := range flowExp.elementsList {
// Could not come up with a way to exclude if else conditions as different IEs have different data types.
if i == 0 || i == 1 {
// For time elements
mockDataRec.EXPECT().AddInfoElement(ie, record1.Conn.StartTime.Unix()).Return(tempBytes, nil)
} else if i == 2 || i == 3 {
// For IP addresses
for _, ie := range flowExp.elementsList {
switch ieName := ie.Name; ieName {
case "flowStartSeconds", "flowEndSeconds":
mockDataRec.EXPECT().AddInfoElement(ie, time.Time{}.Unix()).Return(tempBytes, nil)
case "sourceIPv4Address", "destinationIPv4Address":
mockDataRec.EXPECT().AddInfoElement(ie, nil).Return(tempBytes, nil)
} else if i == 4 || i == 5 {
// For transport ports
case "sourceTransportPort", "destinationTransportPort":
mockDataRec.EXPECT().AddInfoElement(ie, uint16(0)).Return(tempBytes, nil)
} else if i == 6 {
// For proto identifier
case "protocolIdentifier":
mockDataRec.EXPECT().AddInfoElement(ie, uint8(0)).Return(tempBytes, nil)
} else if i >= 7 && i < 15 {
// For packets and octets
case "packetTotalCount", "octetTotalCount", "packetDeltaCount", "octetDeltaCount", "reverse_PacketTotalCount", "reverse_OctetTotalCount", "reverse_PacketDeltaCount", "reverse_OctetDeltaCount":
mockDataRec.EXPECT().AddInfoElement(ie, uint64(0)).Return(tempBytes, nil)
} else {
// For string elements
case "sourcePodName", "sourcePodNamespace", "sourceNodeName", "destinationPodName", "destinationPodNamespace", "destinationNodeName":
mockDataRec.EXPECT().AddInfoElement(ie, "").Return(tempBytes, nil)
}
}
Expand Down
Loading

0 comments on commit a22a245

Please sign in to comment.