Merge pull request #5678 from priyawadhwa/compare

[mkcmp] add code to time `minikube start` for each binary

Makefile

@@ -73,6 +73,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")
@@ -627,6 +628,15 @@ out/mkcmp:
 out/performance-monitor:
 	GOOS=$(GOOS) GOARCH=$(GOARCH) go build -o $@ cmd/performance/monitor/monitor.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
+	
 
 .PHONY: help
 help:

cmd/performance/mkcmp/cmd/mkcmp.go

@@ -17,20 +17,26 @@ limitations under the License.
 package cmd
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"os"
 
 	"github.com/spf13/cobra"
+	"k8s.io/minikube/pkg/minikube/perf"
 )
 
 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 {
+		return perf.CompareMinikubeStart(context.Background(), os.Stdout, args)
+	},
 }
 
 func validateArgs(args []string) error {
@@ -43,7 +49,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)
 	}
 }

pkg/minikube/perf/start.go

@@ -0,0 +1,102 @@
+/*
+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 perf
+
+import (
+	"context"
+	"fmt"
+	"io"
+	"log"
+	"os"
+	"os/exec"
+	"time"
+
+	"github.com/pkg/errors"
+)
+
+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 []string) 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 []string) ([][]float64, 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 := collectTimeMinikubeStart(ctx, binary)
+			if err != nil {
+				return nil, errors.Wrapf(err, "timing run %d with %s", r, binary)
+			}
+			durations[index][r] = duration
+		}
+	}
+
+	return durations, nil
+}
+
+func average(nums []float64) float64 {
+	total := float64(0)
+	for _, a := range nums {
+		total += a
+	}
+	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 string) (float64, error) {
+	startCmd := exec.CommandContext(ctx, binary, "start")
+	startCmd.Stdout = os.Stdout
+	startCmd.Stderr = os.Stderr
+
+	deleteCmd := exec.CommandContext(ctx, binary, "delete")
+	defer func() {
+		if err := deleteCmd.Run(); err != nil {
+			log.Printf("error deleting minikube: %v", err)
+		}
+	}()
+
+	log.Printf("Running: %v...", startCmd.Args)
+	start := time.Now()
+	if err := startCmd.Run(); err != nil {
+		return 0, errors.Wrap(err, "starting minikube")
+	}
+
+	s := time.Since(start).Seconds()
+	return s, nil
+}

pkg/minikube/perf/start_test.go

@@ -0,0 +1,127 @@
+/*
+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 perf
+
+import (
+	"bytes"
+	"context"
+	"reflect"
+	"testing"
+)
+
+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 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, []string{"", ""})
+			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
+		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
+		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)
+			}
+		})
+	}
+}