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.

Sign up for GitHub

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

Jump to bottom

[Networking] [E2E] n-to-1 iperf client->server throughput benchmarking #22869

Merged
merged 1 commit into from Apr 29, 2016
Merged

[Networking] [E2E] n-to-1 iperf client->server throughput benchmarking #22869

merged 1 commit into from Apr 29, 2016

Conversation

jayunit100
Copy link
Member

No description provided.

@k8s-github-robot k8s-github-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 11, 2016
@spxtr spxtr added the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Mar 11, 2016
@jayunit100
Copy link
Member Author

@kubernetes/sig-testing a first pass ateempt at what we discussed tuesday, using iperf, which measures netowrk performance.
This is the first PR for #22707 REMAINING items

  • need to add some kind of assertion
  • need to have structured output sink for the data. maybe iperf can export json.
  • check iperf version (iperf2/3/...?) that is most appropriate for this.

@jayunit100 jayunit100 force-pushed the iperf-e2e branch 5 times, most recently from 30da5fe to e7d5fa7 Compare March 15, 2016 22:20
@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

@k8s-github-robot k8s-github-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 15, 2016
@googlebot
Copy link

CLAs look good, thanks!

@jayunit100
Copy link
Member Author

I've now run this on GCE several times and its stable.
Ready for a first look from anyone interested.
I'll remove the DO-NOT-MERGE after taking a fresh look at this in the morning.

A Test result for 3 nodes follows -- this is done on GCE. I've also tested this on OpenShift a quick first test it seemed to work similarly (and give reasonably numbers).

[begin] Node,Bandwith CSV
iperf-e2e-cli-pod-0-10.248.207.5-3--server=10.0.206.216 1.595592E+10
iperf-e2e-cli-pod-1-10.251.70.5-3--server=10.0.206.216 1.678759E+09
iperf-e2e-cli-pod-2-10.245.55.5-3--server=10.0.206.216 1.484329E+09
[end] Node,Bandwith CSV

@k8s-github-robot k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Mar 15, 2016
@jayunit100 jayunit100 force-pushed the iperf-e2e branch 2 times, most recently from d763e17 to 11319d1 Compare March 16, 2016 03:02
@jayunit100 jayunit100 changed the title [DO-NOT-MERGE] [WIP] [E2E] all-on-all iperf for throughput benchmarking [Networking] [E2E] all-on-all iperf for throughput benchmarking Mar 16, 2016
@jayunit100
Copy link
Member Author

Removed the do-not-merge label, and ran a few tests of this on GCE. It stable from what I can tell, but I labelled as flakey since it is a new test which we will want to bake into the CI over time.

@jayunit100 jayunit100 changed the title [Networking] [E2E] all-on-all iperf for throughput benchmarking [Networking] [E2E] n-to-1 iperf client->server throughput benchmarking Mar 16, 2016
@jayunit100 jayunit100 force-pushed the iperf-e2e branch 2 times, most recently from 9b545f9 to 89a607f Compare March 16, 2016 12:50
bprashanth
bprashanth reviewed Apr 26, 2016
View reviewed changes
test/e2e/framework/framework.go
@@ -527,6 +601,8 @@ ReturnPodsSoFar:
// Next: Pod must match at least one of the states that the user specified
for _, pod := range unfilteredPods {
if !(passesPhasesFilter(pod, p.ValidPhases) && passesPodNameFilter(pod, p.PodName)) {
// This can be used for debugging but otherwise is very verbose.
// Logf("phase or name not matched [[[ %v ]]] : %v %v ", pod, p.ValidPhases, p.PodName)
Copy link
Contributor

Choose a reason for hiding this comment

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

just delete it? or add a slimmer version

@bprashanth
Copy link
Contributor

This looks fine for a first cut as long as we move it to feature so it doesn't block anything, and the nits are addressed

@jayunit100
Copy link
Member Author

jayunit100 commented Apr 26, 2016 •
edited

@bprashanth comments addressed

  • GetReadyNodes seem to have snuck into utils at some point :) . so i just deleted List.
  • added 4 node cluster / clarification for smallClusterTimeout
  • also clarified the maxCount comment and the dynamic 1/10+seconds timeout that you had questions on.
  • I also added a feature tag just for now, but we should replace it after the strongly typed tags go on

re: iperf vs netperf vs ... they're all functionally equivalent for the use cases we care about i think.

thanks for the review !

@jayunit100
E2E Network performance run which uses iperf.
95e315e 
generic pod-per-node functionality for testing - 2 node test only
- update framework to decompose pod vs svc creation for composition.
- remove hard coded 2 and pointer to --scale
@k8s-bot
Copy link

k8s-bot commented Apr 26, 2016

GCE e2e build/test passed for commit 95e315e.

@jayunit100
Copy link
Member Author

Hi @bprashanth any other comments?

@bprashanth
Copy link
Contributor

re: iperf vs netperf vs ... they're all functionally equivalent for the use cases we care about i think.

In that case we already have a netperf tester?
https://github.com/kubernetes/contrib/tree/d9063241280cedfaac23a2283f6584e747aa2302/netperf-tester

LGTM

@bprashanth bprashanth added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 28, 2016
@k8s-github-robot
Copy link

Removing LGTM because the release note process has not been followed.
One of the following labels is required "release-note", "release-note-none", or "release-note-action-required"
Please see: https://github.com/kubernetes/kubernetes/blob/master/docs/devel/pull-requests.md#release-notes

@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 28, 2016
@jayunit100
Copy link
Member Author

@bprashant can we fix this label , thanks.!

@bprashanth bprashanth added lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Apr 29, 2016
@jayunit100
Copy link
Member Author

And yeah, netperf as a container can be swapped in here. But netperf-tester you linked appears to be measuring network performance for a simple one off scenario . This test attempts to scale with respect to cluster size and create replication controllers both for servers and clients...

@k8s-github-robot
Copy link

@k8s-bot test this

Tests are more than 48 hours old. Re-running tests.

@k8s-bot
Copy link

k8s-bot commented Apr 29, 2016

GCE e2e build/test passed for commit 95e315e.

@jayunit100
Copy link
Member Author

@k8s-bot test this please ISSUE: #24577

@k8s-bot
Copy link

k8s-bot commented Apr 29, 2016

GCE e2e build/test passed for commit 95e315e.

@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Apr 29, 2016

GCE e2e build/test passed for commit 95e315e.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit e6295f1 into kubernetes:master Apr 29, 2016
k8s-github-robot pushed a commit that referenced this pull request Sep 23, 2016
Merge pull request #24536 from jayunit100/LoggingSoak
5374e48 
Automatic merge from submit-queue

Logging soak

Implements #24427 

Needs 

- #24471 so that it doesnt clog test outputs for scale
- builds on the utils function added in support of #22869 

cc @timothysc @kubernetes/sig-testing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@bprashanth bprashanth

Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
10 participants
@jayunit100 @googlebot @spxtr @bprashanth @fejta @k8s-bot @k8s-github-robot @timothysc @ixdy @k8s-teamcity-mesosphere