From 8cb2a491d6e0fe7dac0cf6f9da3f366000d6a01d Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Mon, 21 Oct 2019 11:08:21 -0700 Subject: [PATCH 01/12] Add binary type to hold all necessary info about each minikube binary --- pkg/minikube/performance/binary.go | 58 ++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 pkg/minikube/performance/binary.go diff --git a/pkg/minikube/performance/binary.go b/pkg/minikube/performance/binary.go new file mode 100644 index 0000000000..3e0817b3e7 --- /dev/null +++ b/pkg/minikube/performance/binary.go @@ -0,0 +1,58 @@ +/* +Copyright 2017 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 +} From 4a57b8864da6b86b5e60ccd6ae251aea31f478ed Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Mon, 21 Oct 2019 11:08:31 -0700 Subject: [PATCH 02/12] Add code to time 'minikube start' --- pkg/minikube/performance/start.go | 83 +++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 pkg/minikube/performance/start.go diff --git a/pkg/minikube/performance/start.go b/pkg/minikube/performance/start.go new file mode 100644 index 0000000000..1fdb7cab37 --- /dev/null +++ b/pkg/minikube/performance/start.go @@ -0,0 +1,83 @@ +/* +Copyright 2017 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 ( + "context" + "fmt" + "log" + "os" + "os/exec" + "time" + + "github.com/pkg/errors" +) + +const ( + runs = 1 +) + +// CompareMinikubeStart compares the time to run `minikube start` between two minikube binaries +func CompareMinikubeStart(ctx context.Context, binaries []*Binary) error { + durations := make([][]float64, len(binaries)) + for i := range durations { + durations[i] = make([]float64, runs) + } + + for r := 0; r < runs; r++ { + log.Printf("Executing run %d...", r) + for index, binary := range binaries { + duration, err := timeMinikubeStart(ctx, binary) + if err != nil { + return errors.Wrapf(err, "timing run %d with %s", r, binary.path) + } + durations[index][r] = duration + } + } + + fmt.Printf("Old binary: %v\nNew binary: %v\nAverage Old: %f\nAverage New: %f\n", durations[0], durations[1], average(durations[0]), average(durations[1])) + + return nil +} + +func average(array []float64) float64 { + total := float64(0) + for _, a := range array { + total += a + } + return total / float64(len(array)) +} + +// 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") + startCmd.Stdout = os.Stdout + startCmd.Stderr = os.Stderr + + deleteCmd := exec.CommandContext(ctx, binary.path, "delete") + defer deleteCmd.Run() + + log.Printf("Running: %v...", startCmd.Args) + start := time.Now() + if err := startCmd.Run(); err != nil { + return 0, errors.Wrap(err, "starting minikube") + } + + startDuration := time.Since(start).Seconds() + return startDuration, nil +} From 5242dcb0b8353004c28c556b803c42285a79b85a Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Mon, 21 Oct 2019 11:10:49 -0700 Subject: [PATCH 03/12] hook up mkcmp command to code to time 'minikube start' --- cmd/performance/cmd/mkcmp.go | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/cmd/performance/cmd/mkcmp.go b/cmd/performance/cmd/mkcmp.go index f1a26fe953..91e887907c 100644 --- a/cmd/performance/cmd/mkcmp.go +++ b/cmd/performance/cmd/mkcmp.go @@ -17,20 +17,30 @@ limitations under the License. package cmd import ( + "context" "errors" "fmt" "os" "github.com/spf13/cobra" + "k8s.io/minikube/pkg/minikube/performance" ) var rootCmd = &cobra.Command{ - Use: "mkcmp [path to first binary] [path to second binary]", - Short: "mkcmp is used to compare performance of two minikube binaries", + Use: "mkcmp [path to first binary] [path to second binary]", + Short: "mkcmp is used to compare performance of two minikube binaries", + SilenceUsage: true, + SilenceErrors: true, PersistentPreRunE: func(cmd *cobra.Command, args []string) error { return validateArgs(args) }, - Run: func(cmd *cobra.Command, args []string) {}, + RunE: func(cmd *cobra.Command, args []string) error { + binaries, err := performance.Binaries(args) + if err != nil { + return err + } + return performance.CompareMinikubeStart(context.Background(), binaries) + }, } func validateArgs(args []string) error { @@ -43,7 +53,7 @@ func validateArgs(args []string) error { // Execute runs the mkcmp command func Execute() { if err := rootCmd.Execute(); err != nil { - fmt.Println(err) + fmt.Println("Error:", err) os.Exit(1) } } From 9856a04487a34b040b70e0def57402dec242d9fa Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Mon, 21 Oct 2019 11:11:26 -0700 Subject: [PATCH 04/12] Add compare Make rule to compare current branch with master branch --- Makefile | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Makefile b/Makefile index 0b159ac3ee..0e0ab93c19 100755 --- a/Makefile +++ b/Makefile @@ -70,6 +70,7 @@ GOARCH ?= $(shell go env GOARCH) GOPATH ?= $(shell go env GOPATH) BUILD_DIR ?= ./out $(shell mkdir -p $(BUILD_DIR)) +CURRENT_GIT_BRANCH ?= $(shell git branch | grep \* | cut -d ' ' -f2) # Use system python if it exists, otherwise use Docker. PYTHON := $(shell command -v python || echo "docker run --rm -it -v $(shell pwd):/minikube -w /minikube python python") @@ -571,3 +572,12 @@ site: site/themes/docsy/assets/vendor/bootstrap/package.js out/hugo/hugo .PHONY: out/mkcmp out/mkcmp: GOOS=$(GOOS) GOARCH=$(GOARCH) go build -o $@ cmd/performance/main.go + +.PHONY: compare +compare: out/mkcmp out/minikube + cp out/minikube out/$(CURRENT_GIT_BRANCH).minikube + git checkout master + make out/minikube + cp out/minikube out/master.minikube + git checkout $(CURRENT_GIT_BRANCH) + out/mkcmp out/master.minikube out/$(CURRENT_GIT_BRANCH).minikube From 80940538bd666d9190a2446a6eff42e152855c16 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Mon, 21 Oct 2019 11:19:13 -0700 Subject: [PATCH 05/12] remove 'make compare' rule as it won't work on windows --- Makefile | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/Makefile b/Makefile index 0e0ab93c19..0b159ac3ee 100755 --- a/Makefile +++ b/Makefile @@ -70,7 +70,6 @@ GOARCH ?= $(shell go env GOARCH) GOPATH ?= $(shell go env GOPATH) BUILD_DIR ?= ./out $(shell mkdir -p $(BUILD_DIR)) -CURRENT_GIT_BRANCH ?= $(shell git branch | grep \* | cut -d ' ' -f2) # Use system python if it exists, otherwise use Docker. PYTHON := $(shell command -v python || echo "docker run --rm -it -v $(shell pwd):/minikube -w /minikube python python") @@ -572,12 +571,3 @@ site: site/themes/docsy/assets/vendor/bootstrap/package.js out/hugo/hugo .PHONY: out/mkcmp out/mkcmp: GOOS=$(GOOS) GOARCH=$(GOARCH) go build -o $@ cmd/performance/main.go - -.PHONY: compare -compare: out/mkcmp out/minikube - cp out/minikube out/$(CURRENT_GIT_BRANCH).minikube - git checkout master - make out/minikube - cp out/minikube out/master.minikube - git checkout $(CURRENT_GIT_BRANCH) - out/mkcmp out/master.minikube out/$(CURRENT_GIT_BRANCH).minikube From f4e59d51bee7b2f522be361e8a937c9eeaae6d66 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Mon, 21 Oct 2019 11:55:43 -0700 Subject: [PATCH 06/12] Add unit tests --- cmd/performance/cmd/mkcmp.go | 2 +- pkg/minikube/performance/start.go | 25 +++-- pkg/minikube/performance/start_test.go | 130 +++++++++++++++++++++++++ 3 files changed, 149 insertions(+), 8 deletions(-) create mode 100644 pkg/minikube/performance/start_test.go diff --git a/cmd/performance/cmd/mkcmp.go b/cmd/performance/cmd/mkcmp.go index 91e887907c..d146bb9bed 100644 --- a/cmd/performance/cmd/mkcmp.go +++ b/cmd/performance/cmd/mkcmp.go @@ -39,7 +39,7 @@ var rootCmd = &cobra.Command{ if err != nil { return err } - return performance.CompareMinikubeStart(context.Background(), binaries) + return performance.CompareMinikubeStart(context.Background(), os.Stdout, binaries) }, } diff --git a/pkg/minikube/performance/start.go b/pkg/minikube/performance/start.go index 1fdb7cab37..7cd8ece6b5 100644 --- a/pkg/minikube/performance/start.go +++ b/pkg/minikube/performance/start.go @@ -19,6 +19,7 @@ package performance import ( "context" "fmt" + "io" "log" "os" "os/exec" @@ -27,12 +28,24 @@ import ( "github.com/pkg/errors" ) -const ( +var ( runs = 1 + // For testing + collectTimeMinikubeStart = timeMinikubeStart ) // CompareMinikubeStart compares the time to run `minikube start` between two minikube binaries -func CompareMinikubeStart(ctx context.Context, binaries []*Binary) error { +func CompareMinikubeStart(ctx context.Context, out io.Writer, binaries []*Binary) error { + durations, err := collectTimes(ctx, binaries) + if err != nil { + return err + } + + fmt.Fprintf(out, "Old binary: %v\nNew binary: %v\nAverage Old: %f\nAverage New: %f\n", durations[0], durations[1], average(durations[0]), average(durations[1])) + return nil +} + +func collectTimes(ctx context.Context, binaries []*Binary) ([][]float64, error) { durations := make([][]float64, len(binaries)) for i := range durations { durations[i] = make([]float64, runs) @@ -41,17 +54,15 @@ func CompareMinikubeStart(ctx context.Context, binaries []*Binary) error { for r := 0; r < runs; r++ { log.Printf("Executing run %d...", r) for index, binary := range binaries { - duration, err := timeMinikubeStart(ctx, binary) + duration, err := collectTimeMinikubeStart(ctx, binary) if err != nil { - return errors.Wrapf(err, "timing run %d with %s", r, binary.path) + return nil, errors.Wrapf(err, "timing run %d with %s", r, binary.path) } durations[index][r] = duration } } - fmt.Printf("Old binary: %v\nNew binary: %v\nAverage Old: %f\nAverage New: %f\n", durations[0], durations[1], average(durations[0]), average(durations[1])) - - return nil + return durations, nil } func average(array []float64) float64 { diff --git a/pkg/minikube/performance/start_test.go b/pkg/minikube/performance/start_test.go new file mode 100644 index 0000000000..1b36bb85d0 --- /dev/null +++ b/pkg/minikube/performance/start_test.go @@ -0,0 +1,130 @@ +/* +Copyright 2017 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 ( + "bytes" + "context" + "reflect" + "testing" +) + +func mockCollectTimeMinikubeStart(durations []float64) func(ctx context.Context, binary *Binary) (float64, error) { + index := 0 + return func(context.Context, *Binary) (float64, error) { + duration := durations[index] + index = index + 1 + return duration, nil + } +} + +func TestCompareMinikubeStartOutput(t *testing.T) { + tests := []struct { + description string + durations []float64 + expected string + }{ + { + description: "standard run", + durations: []float64{4.5, 6}, + expected: "Old binary: [4.5]\nNew binary: [6]\nAverage Old: 4.500000\nAverage New: 6.000000\n", + }, + } + + for _, test := range tests { + t.Run(test.description, func(t *testing.T) { + originalCollectTimes := collectTimeMinikubeStart + collectTimeMinikubeStart = mockCollectTimeMinikubeStart(test.durations) + defer func() { collectTimeMinikubeStart = originalCollectTimes }() + + buf := bytes.NewBuffer([]byte{}) + err := CompareMinikubeStart(context.Background(), buf, []*Binary{{}, {}}) + if err != nil { + t.Fatalf("error comparing minikube start: %v", err) + } + + actual := buf.String() + if test.expected != actual { + t.Fatalf("actual output does not match expected output\nActual: %v\nExpected: %v", actual, test.expected) + } + }) + } +} + +func TestCollectTimes(t *testing.T) { + tests := []struct { + description string + durations []float64 + runs int + expected [][]float64 + }{ + { + description: "two runs", + durations: []float64{1, 2, 3, 4}, + runs: 2, + expected: [][]float64{ + {1, 3}, + {2, 4}, + }, + }, + } + + for _, test := range tests { + t.Run(test.description, func(t *testing.T) { + originalCollectTimes := collectTimeMinikubeStart + collectTimeMinikubeStart = mockCollectTimeMinikubeStart(test.durations) + defer func() { collectTimeMinikubeStart = originalCollectTimes }() + + runs = test.runs + actual, err := collectTimes(context.Background(), []*Binary{{}, {}}) + if err != nil { + t.Fatalf("error collecting times: %v", err) + } + + if !reflect.DeepEqual(actual, test.expected) { + t.Fatalf("actual output does not match expected output\nActual: %v\nExpected: %v", actual, test.expected) + } + }) + } +} + +func TestAverage(t *testing.T) { + tests := []struct { + description string + nums []float64 + expected float64 + }{ + { + description: "one number", + nums: []float64{4}, + expected: 4, + }, { + description: "multiple numbers", + nums: []float64{1, 4}, + expected: 2.5, + }, + } + + for _, test := range tests { + t.Run(test.description, func(t *testing.T) { + actual := average(test.nums) + if actual != test.expected { + t.Fatalf("actual output does not match expected output\nActual: %v\nExpected: %v", actual, test.expected) + } + }) + } +} From 42304d8533357ff19ee64b8d4075fec7aefaeef4 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Mon, 21 Oct 2019 12:00:53 -0700 Subject: [PATCH 07/12] Fixed linting errors --- go.sum | 1 + pkg/minikube/performance/start.go | 6 +++++- pkg/minikube/performance/start_test.go | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/go.sum b/go.sum index 03c9eb6c9c..6ebce314fd 100644 --- a/go.sum +++ b/go.sum @@ -383,6 +383,7 @@ github.com/opencontainers/selinux v0.0.0-20170621221121-4a2974bf1ee9/go.mod h1:+ github.com/openzipkin/zipkin-go v0.1.1/go.mod h1:NtoC/o8u3JlF1lSlyPNswIbeQH9bJTmOf0Erfk+hxe8= github.com/otiai10/copy v1.0.2 h1:DDNipYy6RkIkjMwy+AWzgKiNTyj2RUI9yEMeETEpVyc= github.com/otiai10/copy v1.0.2/go.mod h1:c7RpqBkwMom4bYTSkLSym4VSJz/XtncWRAj/J4PEIMY= +github.com/otiai10/curr v0.0.0-20150429015615-9b4961190c95 h1:+OLn68pqasWca0z5ryit9KGfp3sUsW4Lqg32iRMJyzs= github.com/otiai10/curr v0.0.0-20150429015615-9b4961190c95/go.mod h1:9qAhocn7zKJG+0mI8eUu6xqkFDYS2kb2saOteoSB3cE= github.com/otiai10/mint v1.3.0 h1:Ady6MKVezQwHBkGzLFbrsywyp09Ah7rkmfjV3Bcr5uc= github.com/otiai10/mint v1.3.0/go.mod h1:F5AjcsTsWUqX+Na9fpHb52P8pcRX2CI6A3ctIT91xUo= diff --git a/pkg/minikube/performance/start.go b/pkg/minikube/performance/start.go index 7cd8ece6b5..c50828ea10 100644 --- a/pkg/minikube/performance/start.go +++ b/pkg/minikube/performance/start.go @@ -81,7 +81,11 @@ func timeMinikubeStart(ctx context.Context, binary *Binary) (float64, error) { startCmd.Stderr = os.Stderr deleteCmd := exec.CommandContext(ctx, binary.path, "delete") - defer deleteCmd.Run() + defer func() { + if err := deleteCmd.Run(); err != nil { + log.Printf("error deleting minikube: %v", err) + } + }() log.Printf("Running: %v...", startCmd.Args) start := time.Now() diff --git a/pkg/minikube/performance/start_test.go b/pkg/minikube/performance/start_test.go index 1b36bb85d0..ecabbd9617 100644 --- a/pkg/minikube/performance/start_test.go +++ b/pkg/minikube/performance/start_test.go @@ -27,7 +27,7 @@ func mockCollectTimeMinikubeStart(durations []float64) func(ctx context.Context, index := 0 return func(context.Context, *Binary) (float64, error) { duration := durations[index] - index = index + 1 + index++ return duration, nil } } From 76bfafb5377974df0fc05b5e68cefc66e68b65e0 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Mon, 21 Oct 2019 13:11:30 -0700 Subject: [PATCH 08/12] add 'make compare' Makefile rule --- Makefile | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Makefile b/Makefile index 0b159ac3ee..cbc9013971 100755 --- a/Makefile +++ b/Makefile @@ -70,6 +70,7 @@ GOARCH ?= $(shell go env GOARCH) GOPATH ?= $(shell go env GOPATH) BUILD_DIR ?= ./out $(shell mkdir -p $(BUILD_DIR)) +CURRENT_GIT_BRANCH ?= $(shell git branch | grep \* | cut -d ' ' -f2) # Use system python if it exists, otherwise use Docker. PYTHON := $(shell command -v python || echo "docker run --rm -it -v $(shell pwd):/minikube -w /minikube python python") @@ -571,3 +572,12 @@ site: site/themes/docsy/assets/vendor/bootstrap/package.js out/hugo/hugo .PHONY: out/mkcmp out/mkcmp: GOOS=$(GOOS) GOARCH=$(GOARCH) go build -o $@ cmd/performance/main.go + +.PHONY: compare +compare: out/mkcmp out/minikube + mv out/minikube out/$(CURRENT_GIT_BRANCH).minikube + git checkout master + make out/minikube + mv out/minikube out/master.minikube + git checkout $(CURRENT_GIT_BRANCH) + out/mkcmp out/master.minikube out/$(CURRENT_GIT_BRANCH).minikube From 0cb99d5fef4da01ff7ef672e29d6cb64c00411b9 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Tue, 26 Nov 2019 10:31:53 -0800 Subject: [PATCH 09/12] update boilerplate --- pkg/minikube/performance/binary.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/minikube/performance/binary.go b/pkg/minikube/performance/binary.go index 3e0817b3e7..91e2a82d90 100644 --- a/pkg/minikube/performance/binary.go +++ b/pkg/minikube/performance/binary.go @@ -1,5 +1,5 @@ /* -Copyright 2017 The Kubernetes Authors All rights reserved. +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. From 1e5aeb6aa9bf0d7a58e473ac161227628d9df95e Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Thu, 19 Dec 2019 11:27:53 -0800 Subject: [PATCH 10/12] 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 -} From 624d47b7812b2d0257a5b103aab084c3386c86fa Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Thu, 19 Dec 2019 12:47:20 -0800 Subject: [PATCH 11/12] Update unit tests --- pkg/minikube/perf/start_test.go | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/pkg/minikube/perf/start_test.go b/pkg/minikube/perf/start_test.go index 1c73ba9af5..539d57500d 100644 --- a/pkg/minikube/perf/start_test.go +++ b/pkg/minikube/perf/start_test.go @@ -23,9 +23,9 @@ import ( "testing" ) -func mockCollectTimeMinikubeStart(durations []float64) func(ctx context.Context, binary *Binary) (float64, error) { +func mockCollectTimeMinikubeStart(durations []float64) func(ctx context.Context, binary string) (float64, error) { index := 0 - return func(context.Context, *Binary) (float64, error) { + return func(context.Context, string) (float64, error) { duration := durations[index] index++ return duration, nil @@ -52,7 +52,7 @@ func TestCompareMinikubeStartOutput(t *testing.T) { defer func() { collectTimeMinikubeStart = originalCollectTimes }() buf := bytes.NewBuffer([]byte{}) - err := CompareMinikubeStart(context.Background(), buf, []*Binary{{}, {}}) + err := CompareMinikubeStart(context.Background(), buf, []string{"", ""}) if err != nil { t.Fatalf("error comparing minikube start: %v", err) } @@ -69,16 +69,14 @@ func TestCollectTimes(t *testing.T) { tests := []struct { description string durations []float64 - runs int expected [][]float64 }{ { - description: "two runs", - durations: []float64{1, 2, 3, 4}, - runs: 2, + description: "test collect time", + durations: []float64{1, 2}, expected: [][]float64{ - {1, 3}, - {2, 4}, + {1}, + {2}, }, }, } @@ -89,8 +87,7 @@ func TestCollectTimes(t *testing.T) { collectTimeMinikubeStart = mockCollectTimeMinikubeStart(test.durations) defer func() { collectTimeMinikubeStart = originalCollectTimes }() - runs = test.runs - actual, err := collectTimes(context.Background(), []*Binary{{}, {}}) + actual, err := collectTimes(context.Background(), []string{"", ""}) if err != nil { t.Fatalf("error collecting times: %v", err) } From 4ce8d7dc0cb26c6b870bd420f3d55305fb82f4d7 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Thu, 19 Dec 2019 13:58:13 -0800 Subject: [PATCH 12/12] fix boilerplate --- pkg/minikube/perf/start.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/minikube/perf/start.go b/pkg/minikube/perf/start.go index ed4ec1d737..a942863f8c 100644 --- a/pkg/minikube/perf/start.go +++ b/pkg/minikube/perf/start.go @@ -1,5 +1,5 @@ /* -Copyright 2017 The Kubernetes Authors All rights reserved. +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.