From 1e5aeb6aa9bf0d7a58e473ac161227628d9df95e Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Thu, 19 Dec 2019 11:27:53 -0800 Subject: [PATCH] Addressed review comments --- cmd/performance/cmd/mkcmp.go | 8 +-- go.mod | 3 - pkg/minikube/{performance => perf}/start.go | 30 +++++----- .../{performance => perf}/start_test.go | 2 +- pkg/minikube/performance/binary.go | 58 ------------------- 5 files changed, 20 insertions(+), 81 deletions(-) rename pkg/minikube/{performance => perf}/start.go (79%) rename pkg/minikube/{performance => perf}/start_test.go (99%) delete mode 100644 pkg/minikube/performance/binary.go diff --git a/cmd/performance/cmd/mkcmp.go b/cmd/performance/cmd/mkcmp.go index d146bb9bed..57c5715f4a 100644 --- a/cmd/performance/cmd/mkcmp.go +++ b/cmd/performance/cmd/mkcmp.go @@ -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) }, } diff --git a/go.mod b/go.mod index 46cf856a68..8976fb1bec 100644 --- a/go.mod +++ b/go.mod @@ -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 diff --git a/pkg/minikube/performance/start.go b/pkg/minikube/perf/start.go similarity index 79% rename from pkg/minikube/performance/start.go rename to pkg/minikube/perf/start.go index c50828ea10..ed4ec1d737 100644 --- a/pkg/minikube/performance/start.go +++ b/pkg/minikube/perf/start.go @@ -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 } diff --git a/pkg/minikube/performance/start_test.go b/pkg/minikube/perf/start_test.go similarity index 99% rename from pkg/minikube/performance/start_test.go rename to pkg/minikube/perf/start_test.go index ecabbd9617..1c73ba9af5 100644 --- a/pkg/minikube/performance/start_test.go +++ b/pkg/minikube/perf/start_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package performance +package perf import ( "bytes" diff --git a/pkg/minikube/performance/binary.go b/pkg/minikube/performance/binary.go deleted file mode 100644 index 91e2a82d90..0000000000 --- a/pkg/minikube/performance/binary.go +++ /dev/null @@ -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 -}