Skip to content

Commit

Permalink
Addressed latest comments
Browse files Browse the repository at this point in the history
  • Loading branch information
srikartati committed Jul 15, 2020
1 parent b32a7c7 commit 2818324
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 71 deletions.
4 changes: 2 additions & 2 deletions pkg/agent/metrics/prometheus.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func InitializePrometheusMetrics() {
gaugeHost.Set(1)

InitializePodMetrics()
initializeNetworkPolicyMetrics()
InitializeNetworkPolicyMetrics()
InitializeOVSMetrics()
}

Expand All @@ -101,7 +101,7 @@ func InitializePodMetrics() {
}
}

func initializeNetworkPolicyMetrics() {
func InitializeNetworkPolicyMetrics() {
if err := legacyregistry.Register(EgressNetworkPolicyRuleCount); err != nil {
klog.Error("Failed to register antrea_agent_egress_networkpolicy_rule_count with Prometheus")
}
Expand Down
56 changes: 39 additions & 17 deletions test/integration/agent/cniserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ type testCase struct {
expGatewayCIDRs []string // Expected Gateway addresses in CIDR form
addresses []string
networkConfig string
validateMetrics bool
}

func (tc testCase) netConfJSON(dataDir string) string {
Expand Down Expand Up @@ -501,6 +502,17 @@ func (tester *cmdAddDelTester) cmdCheckTest(tc testCase, conf *Net, dataDir stri

// Find the veth peer in the container namespace and the default route.
tester.checkContainerNetworking(tc)

// If validateMetrics flag is set, check pod count metrics.
if tc.validateMetrics {
expectedStr := `
# HELP antrea_agent_local_pod_count [STABLE] Number of pods on local node which are managed by the Antrea Agent.
# TYPE antrea_agent_local_pod_count gauge
antrea_agent_local_pod_count 1
`
assert.Equal(tc.t, nil, testutil.GatherAndCompare(legacyregistry.DefaultGatherer, strings.NewReader(expectedStr), "antrea_agent_local_pod_count"))
}

}

func (tester *cmdAddDelTester) cmdDelTest(tc testCase, dataDir string) {
Expand Down Expand Up @@ -536,6 +548,16 @@ func (tester *cmdAddDelTester) cmdDelTest(tc testCase, dataDir string) {
link, err = linkByName(tester.testNS, tester.vethName)
testRequire.NotNil(err)
testRequire.Nil(link)

// If validateMetrics flag is set, check Pod count metrics.
if tc.validateMetrics {
expectedStr := `
# HELP antrea_agent_local_pod_count [STABLE] Number of pods on local node which are managed by the Antrea Agent.
# TYPE antrea_agent_local_pod_count gauge
antrea_agent_local_pod_count 0
`
assert.Equal(tc.t, nil, testutil.GatherAndCompare(legacyregistry.DefaultGatherer, strings.NewReader(expectedStr), "antrea_agent_local_pod_count"))
}
}

func newTester() *cmdAddDelTester {
Expand Down Expand Up @@ -664,9 +686,25 @@ func TestAntreaServerFunc(t *testing.T) {
addresses: []string{"10.1.2.100/24,10.1.2.1,4"},
Routes: []string{"10.0.0.0/8,10.1.2.1", "0.0.0.0/0,10.1.2.1"},
},
{
name: "Prometheus metrics test with ADD/DEL/CHECK for 0.4.0 config",
CNIVersion: "0.4.0",
// IPv4 only
ranges: []rangeInfo{{
subnet: "10.1.2.0/24",
}},
expGatewayCIDRs: []string{"10.1.2.1/24"},
addresses: []string{"10.1.2.100/24,10.1.2.1,4"},
Routes: []string{"10.0.0.0/8,10.1.2.1", "0.0.0.0/0,10.1.2.1"},
validateMetrics: true,
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
if strings.Contains(tc.name, "Prometheus") {
// Initialize pod metrics
metrics.InitializePodMetrics()
}
setup()
defer teardown()
tc.t = t
Expand Down Expand Up @@ -715,8 +753,7 @@ func TestCNIServerChaining(t *testing.T) {
if _, inContainer := os.LookupEnv("INCONTAINER"); !inContainer {
t.Skipf("Skip test runs only in container")
}
// Initialize pod metrics
metrics.InitializePodMetrics()

testRequire := require.New(t)
controller := mock.NewController(t)
defer controller.Finish()
Expand Down Expand Up @@ -782,14 +819,6 @@ func TestCNIServerChaining(t *testing.T) {
testRequire.Nil(err)
testRequire.Equal(tc.networkConfig, string(cniResp.CniResult))

// Check pod count metric
expectedStr := `
# HELP antrea_agent_local_pod_count [STABLE] Number of pods on local node which are managed by the Antrea Agent.
# TYPE antrea_agent_local_pod_count gauge
antrea_agent_local_pod_count 1
`
assert.Equal(t, nil, testutil.GatherAndCompare(legacyregistry.DefaultGatherer, strings.NewReader(expectedStr), "antrea_agent_local_pod_count"))

// test cmdDel
containterHostRt := &net.IPNet{IP: podIP, Mask: net.CIDRMask(32, 32)}
orderedCalls = nil
Expand All @@ -807,13 +836,6 @@ func TestCNIServerChaining(t *testing.T) {
tmpLink, _ := netlink.LinkByName(hostVeth.Name)
_ = netlink.LinkDel(tmpLink)
newServer = false
// Check pod count metric
expectedStr = `
# HELP antrea_agent_local_pod_count [STABLE] Number of pods on local node which are managed by the Antrea Agent.
# TYPE antrea_agent_local_pod_count gauge
antrea_agent_local_pod_count 0
`
assert.Equal(t, nil, testutil.GatherAndCompare(legacyregistry.DefaultGatherer, strings.NewReader(expectedStr), "antrea_agent_local_pod_count"))
}
}

Expand Down
73 changes: 21 additions & 52 deletions test/integration/agent/openflow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package agent
import (
"fmt"
"net"
"strconv"
"strings"
"testing"

Expand Down Expand Up @@ -87,7 +88,7 @@ type testConfig struct {
}

func TestConnectivityFlows(t *testing.T) {
// Initialize ovs metrics (prometheus) to test them
// Initialize ovs metrics (Prometheus) to test them
metrics.InitializeOVSMetrics()

c = ofClient.NewClient(br, bridgeMgmtAddr, true)
Expand Down Expand Up @@ -226,7 +227,7 @@ func testInitialize(t *testing.T, config *testConfig) {
for _, tableFlow := range prepareDefaultFlows() {
ofTestUtils.CheckFlowExists(t, ovsCtlClient, tableFlow.tableID, true, tableFlow.flows)
}
checkOVSFlowMetrics(t, false)
checkOVSFlowMetrics(t, c)
}

func testInstallTunnelFlows(t *testing.T, config *testConfig) {
Expand Down Expand Up @@ -302,7 +303,7 @@ func testUninstallPodFlows(t *testing.T, config *testConfig) {
}

func TestNetworkPolicyFlows(t *testing.T) {
// Initialize ovs metrics (prometheus) to test them
// Initialize ovs metrics (Prometheus) to test them
metrics.InitializeOVSMetrics()

c = ofClient.NewClient(br, bridgeMgmtAddr, true)
Expand Down Expand Up @@ -386,7 +387,7 @@ func TestNetworkPolicyFlows(t *testing.T) {
t.Errorf("Failed to install conjunctive match flow")
}
require.True(t, ofTestUtils.OfctlFlowMatch(flowList, ingressRuleTable, flow3), "Failed to install service flow")
checkOVSFlowMetrics(t, true)
checkOVSFlowMetrics(t, c)

_, err = c.UninstallPolicyRuleFlows(ruleID2)
require.Nil(t, err, "Failed to InstallPolicyRuleFlows")
Expand All @@ -396,7 +397,7 @@ func TestNetworkPolicyFlows(t *testing.T) {
require.Nil(t, err, "Failed to DeletePolicyRuleService")
checkConjunctionFlows(t, ingressRuleTable, ingressDefaultTable, contrackCommitTable, priorityNormal, ruleID, rule, assert.False)
checkDefaultDropFlows(t, ingressDefaultTable, priorityNormal, types.DstAddress, toIPList, false)
checkOVSFlowMetrics(t, false)
checkOVSFlowMetrics(t, c)
}

func checkDefaultDropFlows(t *testing.T, table uint8, priority int, addrType types.AddressType, addresses []types.Address, add bool) {
Expand Down Expand Up @@ -523,57 +524,25 @@ func checkConjunctionFlows(t *testing.T, ruleTable uint8, dropTable uint8, allow
}
}

func checkOVSFlowMetrics(t *testing.T, installPolicyRules bool) {
expectedStr := `
func checkOVSFlowMetrics(t *testing.T, client ofClient.Client) {
expectedFlowCount := `
# HELP antrea_agent_ovs_flow_count [STABLE] Flow count for each OVS flow table. The TableID is used as a label.
# TYPE antrea_agent_ovs_flow_count gauge
`
tableStatus := client.GetFlowTableStatus()
totalFlowCount := 0
for _, table := range tableStatus {
expectedFlowCount = expectedFlowCount + fmt.Sprintf("antrea_agent_ovs_flow_count{table_id=\"%s\"} %s\n", strconv.Itoa(int(table.ID)), strconv.Itoa(int(table.FlowCount)))
totalFlowCount = totalFlowCount + int(table.FlowCount)
}
expectedTotalFlowCount := `
# HELP antrea_agent_ovs_total_flow_count [STABLE] Total flow count of all OVS flow tables.
# TYPE antrea_agent_ovs_total_flow_count gauge
antrea_agent_ovs_total_flow_count @@
`
if installPolicyRules {
// Change the number of flows if the test changes or the mechanism to add ovs flows for each network policy rule changes
expectedStr = strings.ReplaceAll(expectedStr, "@@", "44")
} else {
// Change the number of flows if Initialize method under Client interface changes the number of flows added.
expectedStr = strings.ReplaceAll(expectedStr, "@@", "31")
}
assert.Equal(t, nil, testutil.GatherAndCompare(legacyregistry.DefaultGatherer, strings.NewReader(expectedStr), "antrea_agent_ovs_total_flow_count"))
expectedTotalFlowCount = expectedTotalFlowCount + fmt.Sprintf("antrea_agent_ovs_total_flow_count %s\n", strconv.Itoa(totalFlowCount))

// Following is the break-up of flow count for individual flow tables.
expectedStr = `
# HELP antrea_agent_ovs_flow_count [STABLE] Flow count for each OVS flow table. The TableID is used as a label.
# TYPE antrea_agent_ovs_flow_count gauge
antrea_agent_ovs_flow_count{table_id="0"} 1
antrea_agent_ovs_flow_count{table_id="10"} 1
antrea_agent_ovs_flow_count{table_id="100"} $$
antrea_agent_ovs_flow_count{table_id="105"} 4
antrea_agent_ovs_flow_count{table_id="106"} 1
antrea_agent_ovs_flow_count{table_id="110"} 2
antrea_agent_ovs_flow_count{table_id="20"} 2
antrea_agent_ovs_flow_count{table_id="29"} 1
antrea_agent_ovs_flow_count{table_id="30"} 1
antrea_agent_ovs_flow_count{table_id="31"} 4
antrea_agent_ovs_flow_count{table_id="40"} 0
antrea_agent_ovs_flow_count{table_id="41"} 1
antrea_agent_ovs_flow_count{table_id="42"} 1
antrea_agent_ovs_flow_count{table_id="45"} 2
antrea_agent_ovs_flow_count{table_id="50"} 2
antrea_agent_ovs_flow_count{table_id="60"} 1
antrea_agent_ovs_flow_count{table_id="70"} 1
antrea_agent_ovs_flow_count{table_id="80"} 1
antrea_agent_ovs_flow_count{table_id="85"} 2
antrea_agent_ovs_flow_count{table_id="90"} @@
`
if installPolicyRules {
// Change the number of flows in ingressRuleTable if the test changes or the mechanism to add ovs flows for each network policy rule changes
expectedStr = strings.ReplaceAll(expectedStr, "@@", "12")
// Change the number of flows in ingressDefaultTable if the test changes or the mechanism to add ovs flows for each network policy rule changes
expectedStr = strings.ReplaceAll(expectedStr, "$$", "4")
} else {
// Change the number of flows if Initialize method under Client interface changes the number of flows added in ingressRuleTable and ingressDefaultTable.
expectedStr = strings.ReplaceAll(expectedStr, "@@", "2")
expectedStr = strings.ReplaceAll(expectedStr, "$$", "1")
}
assert.Equal(t, nil, testutil.GatherAndCompare(legacyregistry.DefaultGatherer, strings.NewReader(expectedStr), "antrea_agent_ovs_flow_count"))
assert.Equal(t, nil, testutil.GatherAndCompare(legacyregistry.DefaultGatherer, strings.NewReader(expectedTotalFlowCount), "antrea_agent_ovs_total_flow_count"))
assert.Equal(t, nil, testutil.GatherAndCompare(legacyregistry.DefaultGatherer, strings.NewReader(expectedFlowCount), "antrea_agent_ovs_flow_count"))

}

Expand Down

0 comments on commit 2818324

Please sign in to comment.