diff --git a/cmd/performance/mkcmp/cmd/mkcmp.go b/cmd/performance/mkcmp/cmd/mkcmp.go index 57c5715f4a..2a40dd9d5f 100644 --- a/cmd/performance/mkcmp/cmd/mkcmp.go +++ b/cmd/performance/mkcmp/cmd/mkcmp.go @@ -35,7 +35,11 @@ var rootCmd = &cobra.Command{ return validateArgs(args) }, RunE: func(cmd *cobra.Command, args []string) error { - return perf.CompareMinikubeStart(context.Background(), os.Stdout, args) + binaries, err := retrieveBinaries(args) + if err != nil { + return err + } + return perf.CompareMinikubeStart(context.Background(), os.Stdout, binaries) }, } @@ -46,6 +50,18 @@ func validateArgs(args []string) error { return nil } +func retrieveBinaries(args []string) ([]*perf.Binary, error) { + binaries := []*perf.Binary{} + for _, a := range args { + binary, err := perf.NewBinary(a) + if err != nil { + return nil, err + } + binaries = append(binaries, binary) + } + return binaries, nil +} + // Execute runs the mkcmp command func Execute() { if err := rootCmd.Execute(); err != nil { diff --git a/pkg/minikube/perf/binary.go b/pkg/minikube/perf/binary.go new file mode 100644 index 0000000000..5fe6d7f6b9 --- /dev/null +++ b/pkg/minikube/perf/binary.go @@ -0,0 +1,108 @@ +/* +Copyright 2020 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 perf + +import ( + "fmt" + "io" + "net/http" + "os" + "path/filepath" + "strconv" + "strings" + + "github.com/pkg/errors" + "k8s.io/minikube/pkg/minikube/constants" +) + +type Binary struct { + path string + pr int +} + +const ( + prPrefix = "pr://" +) + +// NewBinary returns a new binary type +func NewBinary(b string) (*Binary, error) { + // If it doesn't have the prefix, assume a path + if !strings.HasPrefix(b, prPrefix) { + return &Binary{ + path: b, + }, nil + } + return newBinaryFromPR(b) +} + +// Name returns the name of the binary +func (b *Binary) Name() string { + if b.pr != 0 { + return fmt.Sprintf("Minikube (PR %d)", b.pr) + } + return filepath.Base(b.path) +} + +// newBinaryFromPR downloads the minikube binary built for the pr by Jenkins from GCS +func newBinaryFromPR(pr string) (*Binary, error) { + pr = strings.TrimPrefix(pr, prPrefix) + // try to convert to int + i, err := strconv.Atoi(pr) + if err != nil { + return nil, errors.Wrapf(err, "converting %s to an integer", pr) + } + + b := &Binary{ + path: localMinikubePath(i), + pr: i, + } + + if err := downloadBinary(remoteMinikubeURL(i), b.path); err != nil { + return nil, errors.Wrapf(err, "downloading minikube") + } + + return b, nil +} + +func remoteMinikubeURL(pr int) string { + return fmt.Sprintf("https://storage.googleapis.com/minikube-builds/%d/minikube-linux-amd64", pr) +} + +func localMinikubePath(pr int) string { + return fmt.Sprintf("%s/minikube-binaries/%d/minikube", constants.DefaultMinipath, pr) +} + +func downloadBinary(url, path string) error { + resp, err := http.Get(url) + if err != nil { + return err + } + defer resp.Body.Close() + + if err := os.MkdirAll(filepath.Dir(path), 0777); err != nil { + return err + } + + f, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, 0777) + if err != nil { + return err + } + defer f.Close() + + _, err = io.Copy(f, resp.Body) + return err +} diff --git a/pkg/minikube/perf/start.go b/pkg/minikube/perf/start.go index a942863f8c..7b7bd28bf3 100644 --- a/pkg/minikube/perf/start.go +++ b/pkg/minikube/perf/start.go @@ -35,21 +35,26 @@ const ( var ( // For testing - collectTimeMinikubeStart = timeMinikubeStart + collectTimeMinikubeStart = collectTimes ) // CompareMinikubeStart compares the time to run `minikube start` between two minikube binaries -func CompareMinikubeStart(ctx context.Context, out io.Writer, binaries []string) error { - durations, err := collectTimes(ctx, binaries) +func CompareMinikubeStart(ctx context.Context, out io.Writer, binaries []*Binary) error { + durations, err := collectTimeMinikubeStart(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])) + for i, d := range durations { + fmt.Fprintf(out, "Results for %s:\n", binaries[i].Name()) + fmt.Fprintf(out, "Times: %v\n", d) + fmt.Fprintf(out, "Average Time: %f\n\n", average(d)) + } + return nil } -func collectTimes(ctx context.Context, binaries []string) ([][]float64, error) { +func collectTimes(ctx context.Context, binaries []*Binary) ([][]float64, error) { durations := make([][]float64, len(binaries)) for i := range durations { durations[i] = make([]float64, runs) @@ -58,9 +63,9 @@ func collectTimes(ctx context.Context, binaries []string) ([][]float64, error) { for r := 0; r < runs; r++ { log.Printf("Executing run %d...", r) for index, binary := range binaries { - duration, err := collectTimeMinikubeStart(ctx, binary) + duration, err := timeMinikubeStart(ctx, binary) if err != nil { - return nil, errors.Wrapf(err, "timing run %d with %s", r, binary) + return nil, errors.Wrapf(err, "timing run %d with %s", r, binary.Name()) } durations[index][r] = duration } @@ -79,12 +84,12 @@ func average(nums []float64) float64 { // timeMinikubeStart returns the time it takes to execute `minikube start` // It deletes the VM after `minikube start`. -func timeMinikubeStart(ctx context.Context, binary string) (float64, error) { - startCmd := exec.CommandContext(ctx, binary, "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, "delete") + deleteCmd := exec.CommandContext(ctx, binary.path, "delete") defer func() { if err := deleteCmd.Run(); err != nil { log.Printf("error deleting minikube: %v", err) diff --git a/pkg/minikube/perf/start_test.go b/pkg/minikube/perf/start_test.go index 539d57500d..2d802f7d9c 100644 --- a/pkg/minikube/perf/start_test.go +++ b/pkg/minikube/perf/start_test.go @@ -19,86 +19,64 @@ package perf import ( "bytes" "context" - "reflect" "testing" + + "github.com/google/go-cmp/cmp" ) -func mockCollectTimeMinikubeStart(durations []float64) func(ctx context.Context, binary string) (float64, error) { - index := 0 - return func(context.Context, string) (float64, error) { - duration := durations[index] - index++ - return duration, nil +func mockCollectTimes(times [][]float64) func(ctx context.Context, binaries []*Binary) ([][]float64, error) { + return func(ctx context.Context, binaries []*Binary) ([][]float64, error) { + return times, nil } } func TestCompareMinikubeStartOutput(t *testing.T) { + binaries := []*Binary{ + { + path: "minikube1", + }, { + path: "minikube2", + }, + } tests := []struct { description string - durations []float64 + times [][]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", + times: [][]float64{{4.5, 6}, {1, 2}}, + expected: `Results for minikube1: +Times: [4.5 6] +Average Time: 5.250000 + +Results for minikube2: +Times: [1 2] +Average Time: 1.500000 + +`, }, } for _, test := range tests { t.Run(test.description, func(t *testing.T) { - originalCollectTimes := collectTimeMinikubeStart - collectTimeMinikubeStart = mockCollectTimeMinikubeStart(test.durations) + originalCollectTimes := collectTimes + collectTimeMinikubeStart = mockCollectTimes(test.times) defer func() { collectTimeMinikubeStart = originalCollectTimes }() buf := bytes.NewBuffer([]byte{}) - err := CompareMinikubeStart(context.Background(), buf, []string{"", ""}) + err := CompareMinikubeStart(context.Background(), buf, binaries) 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) + if diff := cmp.Diff(test.expected, actual); diff != "" { + t.Errorf("machines mismatch (-want +got):\n%s", diff) } }) } } - -func TestCollectTimes(t *testing.T) { - tests := []struct { - description string - durations []float64 - expected [][]float64 - }{ - { - description: "test collect time", - durations: []float64{1, 2}, - expected: [][]float64{ - {1}, - {2}, - }, - }, - } - - for _, test := range tests { - t.Run(test.description, func(t *testing.T) { - originalCollectTimes := collectTimeMinikubeStart - collectTimeMinikubeStart = mockCollectTimeMinikubeStart(test.durations) - defer func() { collectTimeMinikubeStart = originalCollectTimes }() - - actual, err := collectTimes(context.Background(), []string{"", ""}) - 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