kic: Use SSHRunner by default (20% faster startup) #7591
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tstromberg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
|
/ok-to-test |
k8s-ci-robot
added
the
ok-to-test
tstromberg
changed the title
tstromberg
changed the title
Codecov Report
@@ Coverage Diff @@
## master #7591 +/- ##
==========================================
+ Coverage 36.50% 36.54% +0.04%
==========================================
Files 147 147
Lines 9120 9126 +6
==========================================
+ Hits 3329 3335 +6
Misses 5403 5403
Partials 388 388
|
|
kvm2 Driver Times for Minikube (PR 7591): [64.078939564 63.330269377 69.17715550500002] Averages Time Per Log docker Driver Times for Minikube (PR 7591): [25.984234593 26.549697043 25.702005188] Averages Time Per Log |
|
the none test failures all seem related to deleting minikube: |
|
Interesting. I'm going to re-run the tests, but this seems very suspicious.
|
|
/ok-to-test |
Travis tests have failedHey @tstromberg, TravisBuddy Request Identifier: c01e8f10-7b5b-11ea-a96b-a3ab63a79735 |
|
kvm2 Driver Times for Minikube (PR 7591): [64.31815089000001 67.85329587000001 63.427240332000004] Averages Time Per Log docker Driver Times for Minikube (PR 7591): [26.180753661000004 25.242443026000004 25.257489346999996] Averages Time Per Log |
There was a problem hiding this comment.
great numbers !!! looks very promising.
I wonder if we could impleent this unimplemented func for kic
// RunSSHCommandFromDriver implements direct ssh control to the driver
func (d *Driver) RunSSHCommandFromDriver() error {
return fmt.Errorf("driver does not support RunSSHCommandFromDriver commands")
}
|
Oof. I forgot to close the ssh session, so some integration tests ran out of open connections. |
|
kvm2 Driver Times for Minikube (PR 7591): [62.516410323 66.417703285 64.83825979] Averages Time Per Log docker Driver Times for Minikube (PR 7591): [26.940987497999995 26.892967931000005 24.506006893] Averages Time Per Log |
|
I wonder if this PR will break the hac/preload geneator script |
|
kvm2 Driver Times for Minikube (PR 7591): [64.37491644500001 64.02427695300001 64.44591507100002] Averages Time Per Log docker Driver Times for Minikube (PR 7591): [26.266589997999997 25.205298537 25.785967129000003] Averages Time Per Log |
It does not appear to. |
|
what happened ! the failures bumped up daramticly ! |
|
so I think your latest update on the PR broke kic I think the kic overlay is pasted into the std input |
|
kvm2 Driver Times for Minikube (PR 7591): [61.751772166 65.881311273 65.129223454] Averages Time Per Log docker Driver Times for Minikube (PR 7591): [26.450581168 24.974887237999997 27.052936074999998] Averages Time Per Log |
my test summary (in progress)1. https://storage.googleapis.com/minikube-builds/logs/7591/6ddb934/Docker_Linux.html#fail_TestErrorSpam2. https://storage.googleapis.com/minikube-builds/logs/7591/6ddb934/Docker_Linux.html#fail_TestStartStop%2fgroup%2fcontainerdyou might need to refactor apply KicOvelay to not past the file in stdin and just copy the file 3. https://storage.googleapis.com/minikube-builds/logs/7591/6ddb934/Docker_Linux.html#fail_TestAddons%2fparallel%2fRegistry |
|
kvm2 Driver Times for Minikube (PR 7591): [63.66858147999999 64.144064191 61.580021673999994] Averages Time Per Log docker Driver Times for Minikube (PR 7591): [25.188715816000002 24.536731481 26.173140618] Averages Time Per Log |
The TL;DR is that ssh is faster than docker for command-execution and file copies, as it benefits from connection reuse.
Fixes #6891 and #7573
commandRunnerfunctionThis reduces start latency with Docker for macOS by ~20% (9 seconds)