Enhance testing for prometheus metrics at agent #916

srikartati · 2020-07-06T23:37:00Z

Added testing for following metrics in existing integration tests:
antrea_agent_ovs_total_flow_count
antrea_agent_ovs_flow_count
antrea_agent_local_pod_count

Fixes #799

antrea-bot · 2020-07-06T23:37:14Z

Thanks for your PR.
Unit tests and code linters are run automatically every time the PR is updated.
E2e, conformance and network policy tests can only be triggered by a member of the vmware-tanzu organization. Regular contributors to the project should join the org.

The following commands are available:

/test-e2e: to trigger e2e tests.
/skip-e2e: to skip e2e tests.
/test-conformance: to trigger conformance tests.
/skip-conformance: to skip conformance tests.
/test-whole-conformance: to trigger all conformance tests on linux.
/skip-whole-conformance: to skip all conformance tests on linux.
/test-networkpolicy: to trigger networkpolicy tests.
/skip-networkpolicy: to skip networkpolicy tests.
/test-windows-conformance: to trigger windows conformance tests.
/skip-windows-conformance: to skip windows conformance tests.
/test-all: to trigger all tests (except whole conformance).
/skip-all: to skip all tests (except whole conformance).

These commands can only be run by members of the vmware-tanzu organization.

srikartati · 2020-07-07T00:35:12Z

/test-all

ksamoray · 2020-07-07T15:44:53Z

Hi @srikartati,
Do we still the Prometheus tests in e2e?

srikartati · 2020-07-07T20:18:02Z

Hi @srikartati,
Do we still the Prometheus tests in e2e?

These tests do not cover all metrics yet. In addition, e2e test seems to be testing the workflow involving the Prometheus server. Maybe if the above tests are available for all agent and controller metrics, we could remove TestPrometheusMetricsOnController and TestPrometheusMetricsOnAgent, but still keep Prometheus server-based tests.
Since you added the e2e test, you may have more insights on this issue.

tnqn

LGTM, could you add a space between "Fixes" and "#NUM"? otherwise it doesn't link to the issue and cannot auto close it.

srikartati · 2020-07-09T17:27:33Z

/test-all

srikartati · 2020-07-09T18:17:18Z

/test-conformance
/test-networkpolicy
/test-windows-networkpolicy

srikartati · 2020-07-09T19:23:38Z

/test-conformance

srikartati · 2020-07-09T20:52:32Z

/test-networkpolicy

pkg/agent/metrics/prometheus.go

Makefile

antoninbas · 2020-07-09T21:06:56Z

test/integration/agent/cniserver_test.go

@@ -777,6 +783,14 @@ func TestCNIServerChaining(t *testing.T) {
 		testRequire.Nil(err)
 		testRequire.Equal(tc.networkConfig, string(cniResp.CniResult))

+		// Check pod count metric


I am definitely not in favor of piggybacking a metrics test in a highly "specialized" test like TestCNIServerChaining. There should be a dedicated test case for metrics.

Thanks for the comment. I will try an alternative way.

antoninbas · 2020-07-09T21:13:50Z

test/integration/agent/openflow_test.go

@@ -510,6 +523,60 @@ func checkConjunctionFlows(t *testing.T, ruleTable uint8, dropTable uint8, allow
 	}
 }

+func checkOVSFlowMetrics(t *testing.T, installPolicyRules bool) {


this feels like a clunky test to me, and a bit hard to maintain

would the following be at all possible: simply count the flows in all the OVS tables and check that the metrics match?

I will check if that can be done.

srikartati · 2020-07-13T21:18:25Z

/test-all

antoninbas · 2020-07-14T23:05:53Z

test/integration/agent/cniserver_test.go

 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
+			if strings.Contains(tc.name, "Prometheus") {


why don't we add a new validateMetrics boolean field to the testCase struct?

Just went with slightly hacky approach, since it is only one testcase. Added a boolean in testcase.

test/integration/agent/cniserver_test.go

antoninbas · 2020-07-14T23:06:57Z

test/integration/agent/openflow_test.go

@@ -84,6 +88,9 @@ type testConfig struct {
 }

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


s/prometheus/Prometheus

same below

antoninbas · 2020-07-14T23:07:28Z

test/integration/agent/openflow_test.go

@@ -84,6 +88,9 @@ type testConfig struct {
 }

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


I see that we call metrics.InitializeOVSMetrics() in multiple tests. I assume there is no issue with calling the function multiple times?

Yes, I made sure by calling this multiple times in same test. If the metric is already registered, the registry ignores subsequent calls.

antoninbas · 2020-07-14T23:09:54Z

test/integration/agent/openflow_test.go

+	totalFlowCount := 0
+	for _, table := range tableStatus {
+		expectedFlowCount = expectedFlowCount +
+			`antrea_agent_ovs_flow_count{table_id="` + strconv.Itoa(int(table.ID)) + `"} ` + strconv.Itoa(int(table.FlowCount)) + "\n"


would maybe be more readable with fmt.Sprintf? what do you think?

That is a good suggestion. Done.

Added testing for following metrics in integration tests: antrea_agent_ovs_total_flow_count antrea_agent_ovs_flow_count antrea_agent_local_pod_count Fixes#799

When running integration test on MacOS, hit an error because of an old docker image for antrea/openvswitch:2.13.0. Pulling that docker image explicitly which is used as base image for test container image.

antrea-bot · 2020-07-15T18:49:08Z

Thanks for your PR.
Unit tests and code linters are run automatically every time the PR is updated.
E2e, conformance and network policy tests can only be triggered by a member of the vmware-tanzu organization. Regular contributors to the project should join the org.

The following commands are available:

/test-e2e: to trigger e2e tests.
/skip-e2e: to skip e2e tests.
/test-conformance: to trigger conformance tests.
/skip-conformance: to skip conformance tests.
/test-whole-conformance: to trigger all conformance tests on linux.
/skip-whole-conformance: to skip all conformance tests on linux.
/test-networkpolicy: to trigger networkpolicy tests.
/skip-networkpolicy: to skip networkpolicy tests.
/test-windows-conformance: to trigger windows conformance tests.
/skip-windows-conformance: to skip windows conformance tests.
/test-windows-networkpolicy: to trigger windows networkpolicy tests.
/skip-windows-networkpolicy: to skip windows networkpolicy tests.
/test-all: to trigger all tests (except whole conformance).
/skip-all: to skip all tests (except whole conformance).

These commands can only be run by members of the vmware-tanzu organization.

srikartati · 2020-07-15T19:22:33Z

/test-whole-conformance
/test-all

srikartati · 2020-07-15T22:43:33Z

/test-conformance
/test-windows-conformance

srikartati · 2020-07-15T22:44:12Z

/test-networkpolicy
/test-windows-networkpolicy

antoninbas

one more nit, otherwise LGTM

antoninbas · 2020-07-15T23:55:07Z

test/integration/agent/openflow_test.go

+	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)))


you can use %d and then you no longer need to the casts to int and the calls to strconv.Itoa

Done. Missed this. Thanks.

srikartati · 2020-07-16T01:57:26Z

/test-all

antoninbas

LGTM, thanks for making the changes

* Enhance testing for prometheus metrics at agent Added testing for following metrics in integration tests: antrea_agent_ovs_total_flow_count antrea_agent_ovs_flow_count antrea_agent_local_pod_count Fixes#799 * Changes to Makefile to run integration test on MacOS When running integration test on MacOS, hit an error because of an old docker image for antrea/openvswitch:2.13.0. Pulling that docker image explicitly which is used as base image for test container image. * Addressed latest comments

vmwclabot added the cla-not-required label Jul 6, 2020

srikartati requested review from ksamoray and antoninbas July 6, 2020 23:45

srikartati force-pushed the prometheus_test branch from ab2abea to 8509700 Compare July 7, 2020 03:10

srikartati requested a review from tnqn July 8, 2020 23:34

tnqn previously approved these changes Jul 9, 2020

View reviewed changes

antoninbas reviewed Jul 9, 2020

View reviewed changes

srikartati dismissed tnqn’s stale review via 94e0ea3 July 13, 2020 15:00

srikartati force-pushed the prometheus_test branch 2 times, most recently from 94e0ea3 to 3f67ae0 Compare July 13, 2020 15:05

srikartati requested a review from antoninbas July 13, 2020 15:35

antoninbas reviewed Jul 14, 2020

View reviewed changes

srikartati added 2 commits July 14, 2020 17:50

Enhance testing for prometheus metrics at agent

7239160

Added testing for following metrics in integration tests: antrea_agent_ovs_total_flow_count antrea_agent_ovs_flow_count antrea_agent_local_pod_count Fixes#799

Changes to Makefile to run integration test on MacOS

b32a7c7

When running integration test on MacOS, hit an error because of an old docker image for antrea/openvswitch:2.13.0. Pulling that docker image explicitly which is used as base image for test container image.

srikartati force-pushed the prometheus_test branch from 3f67ae0 to 2818324 Compare July 15, 2020 18:48

srikartati requested a review from antoninbas July 15, 2020 22:43

antoninbas reviewed Jul 15, 2020

View reviewed changes

Addressed latest comments

504f64c

srikartati force-pushed the prometheus_test branch from 2818324 to 504f64c Compare July 16, 2020 01:51

srikartati requested a review from antoninbas July 16, 2020 05:19

antoninbas approved these changes Jul 16, 2020

View reviewed changes

srikartati merged commit 4aadebb into antrea-io:master Jul 16, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance testing for prometheus metrics at agent #916

Enhance testing for prometheus metrics at agent #916

srikartati commented Jul 6, 2020 •

edited

Loading

antrea-bot commented Jul 6, 2020

srikartati commented Jul 7, 2020

ksamoray commented Jul 7, 2020

srikartati commented Jul 7, 2020

tnqn left a comment

srikartati commented Jul 9, 2020

srikartati commented Jul 9, 2020

srikartati commented Jul 9, 2020

srikartati commented Jul 9, 2020

antoninbas Jul 9, 2020

srikartati Jul 10, 2020

antoninbas Jul 9, 2020

srikartati Jul 10, 2020

srikartati commented Jul 13, 2020

antoninbas Jul 14, 2020

srikartati Jul 15, 2020

antoninbas Jul 14, 2020

antoninbas Jul 14, 2020

srikartati Jul 15, 2020

antoninbas Jul 14, 2020

srikartati Jul 15, 2020

antrea-bot commented Jul 15, 2020

srikartati commented Jul 15, 2020 •

edited

Loading

srikartati commented Jul 15, 2020

srikartati commented Jul 15, 2020

antoninbas left a comment

antoninbas Jul 15, 2020

antoninbas Jul 15, 2020

srikartati Jul 16, 2020

srikartati commented Jul 16, 2020

antoninbas left a comment

Enhance testing for prometheus metrics at agent #916

Enhance testing for prometheus metrics at agent #916

Conversation

srikartati commented Jul 6, 2020 • edited Loading

antrea-bot commented Jul 6, 2020

srikartati commented Jul 7, 2020

ksamoray commented Jul 7, 2020

srikartati commented Jul 7, 2020

tnqn left a comment

Choose a reason for hiding this comment

srikartati commented Jul 9, 2020

srikartati commented Jul 9, 2020

srikartati commented Jul 9, 2020

srikartati commented Jul 9, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srikartati commented Jul 13, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antrea-bot commented Jul 15, 2020

srikartati commented Jul 15, 2020 • edited Loading

srikartati commented Jul 15, 2020

srikartati commented Jul 15, 2020

antoninbas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srikartati commented Jul 16, 2020

antoninbas left a comment

Choose a reason for hiding this comment

srikartati commented Jul 6, 2020 •

edited

Loading

srikartati commented Jul 15, 2020 •

edited

Loading