Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ClickHouse monitor e2e test #66

Merged
merged 1 commit into from
Jul 5, 2022
Merged

Conversation

yanjunz97
Copy link
Contributor

Signed-off-by: Yanjun Zhou zhouya@vmware.com

@yanjunz97
Copy link
Contributor Author

/theia-test-e2e

@yanjunz97 yanjunz97 marked this pull request as ready for review June 28, 2022 20:21
@yanjunz97 yanjunz97 force-pushed the monitor-e2e branch 2 times, most recently from 21dbd08 to b75f297 Compare June 29, 2022 17:27
@yanjunz97 yanjunz97 added this to the 0.2 milestone Jun 29, 2022
Copy link
Contributor

@dreamtalen dreamtalen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM

--ch-size <size> Deploy the ClickHouse with a specific storage size. Can be a
plain integer or as a fixed-point number using one of these quantity
suffixes: E, P, T, G, M, K. Or the power-of-two equivalents:
Ei, Pi, Ti, Gi, Mi, Ki. The default is 8Gi.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra space between Ki. and The default

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed space and updated the description a little bit. Thanks!

checkClickHouseMonitorLogs(t, data, false, 0)
var cmdStr string
if !isIPv6 {
cmdStr = fmt.Sprintf("iperf3 -u -c %s -P 128 -n 1", flow.dstIP)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to have a comment here describing the choice of 128 as the max value for parallel clients.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added, thanks!

@yuntanghsu
Copy link
Contributor

LGTM

Copy link
Contributor

@dreamtalen dreamtalen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Signed-off-by: Yanjun Zhou <zhouya@vmware.com>
@yanjunz97
Copy link
Contributor Author

Squashed commits

@yanjunz97 yanjunz97 merged commit f95e7ee into antrea-io:main Jul 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants