Addressed review comments

pull/5678/head
Priya Wadhwa 2019-12-19 11:27:53 -08:00
parent 3574879fde
commit 1e5aeb6aa9
5 changed files with 20 additions and 81 deletions

View File

@ -23,7 +23,7 @@ import (
"os"
"github.com/spf13/cobra"
"k8s.io/minikube/pkg/minikube/performance"
"k8s.io/minikube/pkg/minikube/perf"
)
var rootCmd = &cobra.Command{
@ -35,11 +35,7 @@ var rootCmd = &cobra.Command{
return validateArgs(args)
},
RunE: func(cmd *cobra.Command, args []string) error {
binaries, err := performance.Binaries(args)
if err != nil {
return err
}
return performance.CompareMinikubeStart(context.Background(), os.Stdout, binaries)
return perf.CompareMinikubeStart(context.Background(), os.Stdout, args)
},
}

3
go.mod
View File

@ -21,14 +21,11 @@ require (
github.com/docker/machine v0.7.1-0.20190718054102-a555e4f7a8f5 // version is 0.7.1 to pin to a555e4f7a8f5
github.com/elazarl/goproxy v0.0.0-20190421051319-9d40249d3c2f
github.com/elazarl/goproxy/ext v0.0.0-20190421051319-9d40249d3c2f // indirect
github.com/ghodss/yaml v1.0.0 // indirect
github.com/go-ole/go-ole v1.2.4 // indirect
github.com/golang-collections/collections v0.0.0-20130729185459-604e922904d3
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b
github.com/google/btree v1.0.0 // indirect
github.com/google/go-cmp v0.3.0
github.com/gorilla/mux v1.7.1 // indirect
github.com/grpc-ecosystem/grpc-gateway v1.5.0 // indirect
github.com/hashicorp/errwrap v0.0.0-20141028054710-7554cd9344ce // indirect
github.com/hashicorp/go-getter v1.4.0
github.com/hashicorp/go-multierror v0.0.0-20160811015721-8c5f0ad93604 // indirect

View File

@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
package performance
package perf
import (
"context"
@ -28,14 +28,18 @@ import (
"github.com/pkg/errors"
)
var (
const (
// runs is the number of times each binary will be timed for 'minikube start'
runs = 1
)
var (
// For testing
collectTimeMinikubeStart = timeMinikubeStart
)
// CompareMinikubeStart compares the time to run `minikube start` between two minikube binaries
func CompareMinikubeStart(ctx context.Context, out io.Writer, binaries []*Binary) error {
func CompareMinikubeStart(ctx context.Context, out io.Writer, binaries []string) error {
durations, err := collectTimes(ctx, binaries)
if err != nil {
return err
@ -45,7 +49,7 @@ func CompareMinikubeStart(ctx context.Context, out io.Writer, binaries []*Binary
return nil
}
func collectTimes(ctx context.Context, binaries []*Binary) ([][]float64, error) {
func collectTimes(ctx context.Context, binaries []string) ([][]float64, error) {
durations := make([][]float64, len(binaries))
for i := range durations {
durations[i] = make([]float64, runs)
@ -56,7 +60,7 @@ func collectTimes(ctx context.Context, binaries []*Binary) ([][]float64, error)
for index, binary := range binaries {
duration, err := collectTimeMinikubeStart(ctx, binary)
if err != nil {
return nil, errors.Wrapf(err, "timing run %d with %s", r, binary.path)
return nil, errors.Wrapf(err, "timing run %d with %s", r, binary)
}
durations[index][r] = duration
}
@ -65,22 +69,22 @@ func collectTimes(ctx context.Context, binaries []*Binary) ([][]float64, error)
return durations, nil
}
func average(array []float64) float64 {
func average(nums []float64) float64 {
total := float64(0)
for _, a := range array {
for _, a := range nums {
total += a
}
return total / float64(len(array))
return total / float64(len(nums))
}
// timeMinikubeStart returns the time it takes to execute `minikube start`
// It deletes the VM after `minikube start`.
func timeMinikubeStart(ctx context.Context, binary *Binary) (float64, error) {
startCmd := exec.CommandContext(ctx, binary.path, "start")
func timeMinikubeStart(ctx context.Context, binary string) (float64, error) {
startCmd := exec.CommandContext(ctx, binary, "start")
startCmd.Stdout = os.Stdout
startCmd.Stderr = os.Stderr
deleteCmd := exec.CommandContext(ctx, binary.path, "delete")
deleteCmd := exec.CommandContext(ctx, binary, "delete")
defer func() {
if err := deleteCmd.Run(); err != nil {
log.Printf("error deleting minikube: %v", err)
@ -93,6 +97,6 @@ func timeMinikubeStart(ctx context.Context, binary *Binary) (float64, error) {
return 0, errors.Wrap(err, "starting minikube")
}
startDuration := time.Since(start).Seconds()
return startDuration, nil
s := time.Since(start).Seconds()
return s, nil
}

View File

@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
package performance
package perf
import (
"bytes"

View File

@ -1,58 +0,0 @@
/*
Copyright 2019 The Kubernetes Authors All rights reserved.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package performance
import (
"os"
"path/filepath"
)
// Binary holds all necessary information
// to call a minikube binary
type Binary struct {
path string
}
// Binaries returns the type *Binary for each provided path
func Binaries(paths []string) ([]*Binary, error) {
var binaries []*Binary
for _, path := range paths {
b, err := newBinary(path)
if err != nil {
return nil, err
}
binaries = append(binaries, b)
}
return binaries, nil
}
func newBinary(path string) (*Binary, error) {
var err error
if !filepath.IsAbs(path) {
path, err = filepath.Abs(path)
if err != nil {
return nil, err
}
}
// make sure binary exists
if _, err := os.Stat(path); err != nil {
return nil, err
}
return &Binary{
path: path,
}, nil
}