From 84f2c86a500bef1080e66c73d26e96cab78f8664 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Tue, 6 Apr 2021 17:07:43 -0700 Subject: [PATCH 1/9] minikube-pr-bot enhancements --- .../installers/check_install_golang.sh | 2 +- hack/jenkins/prbot.sh | 47 +++++++++++++++++++ pkg/minikube/perf/result_manager.go | 2 - pkg/minikube/perf/start.go | 40 ++++++++++------ 4 files changed, 74 insertions(+), 17 deletions(-) create mode 100644 hack/jenkins/prbot.sh diff --git a/hack/jenkins/installers/check_install_golang.sh b/hack/jenkins/installers/check_install_golang.sh index b01ef8595d..3009d7aea5 100755 --- a/hack/jenkins/installers/check_install_golang.sh +++ b/hack/jenkins/installers/check_install_golang.sh @@ -62,7 +62,7 @@ function install_golang() { # using sudo because previously installed versions might have been installed by a different user. # as it was the case on jenkins VM. sudo curl -qL -O "https://storage.googleapis.com/golang/go${1}.${INSTALLOS}-${ARCH}.tar.gz" && - sudo tar -xf go${1}.${INSTALLOS}-amd64.tar.gz && + sudo tar -xzf go${1}.${INSTALLOS}-amd64.tar.gz && sudo rm -rf "${2}/go" && sudo mv go "${2}/" && sudo chown -R $(whoami): ${2}/go popd >/dev/null diff --git a/hack/jenkins/prbot.sh b/hack/jenkins/prbot.sh new file mode 100644 index 0000000000..2849f1c5a4 --- /dev/null +++ b/hack/jenkins/prbot.sh @@ -0,0 +1,47 @@ +#!/bin/bash + +# Copyright 2021 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. + +set -x -o pipefail +# Only run this on PRs +if [[ "${MINIKUBE_LOCATION}" == "master" ]]; then + exit 0 +fi + +# Make sure docker is installed and configured +./hack/jenkins/installers/check_install_docker.sh + +# Make sure gh is installed and configured +./hack/jenkins/installers/check_install_gh.sh + +# Make sure go is installed and configured +./hack/jenkins/installers/check_install_golang.sh + +# Grab latest code +git clone https://github.com/kubernetes/minikube.git +cd minikube + +# Build minikube binary and mkcmp binary +make out/minikube out/mkcmp + +# Run mkcmp +out/mkcmp out/minikube pr://${MINIKUBE_LOCATION} | mkcmp.log +if [ $? -gt 0 ]; then + # Comment that mkcmp failed + gh pr comment ${MINIKUBE_LOCATION} --body "timing minikube failed, please try again" + exit 1 +fi +output=$(cat mkcmp.log) +gh pr comment ${MINIKUBE_LOCATION} --body ${output} diff --git a/pkg/minikube/perf/result_manager.go b/pkg/minikube/perf/result_manager.go index 61f857d4c1..7cce174cc0 100644 --- a/pkg/minikube/perf/result_manager.go +++ b/pkg/minikube/perf/result_manager.go @@ -66,8 +66,6 @@ func (rm *resultManager) averageTime(binary *Binary) float64 { func (rm *resultManager) summarizeResults(binaries []*Binary, driver string) { // print total and average times - fmt.Printf("**%s Driver**\n", driver) - for _, b := range binaries { fmt.Printf("Times for %s: ", b.Name()) for _, tt := range rm.totalTimes(b) { diff --git a/pkg/minikube/perf/start.go b/pkg/minikube/perf/start.go index 3a9c62bf9b..a8d85371dd 100644 --- a/pkg/minikube/perf/start.go +++ b/pkg/minikube/perf/start.go @@ -23,18 +23,22 @@ import ( "log" "os" "os/exec" + "runtime" "github.com/pkg/errors" ) const ( // runs is the number of times each binary will be timed for 'minikube start' - runs = 3 + runs = 5 ) // CompareMinikubeStart compares the time to run `minikube start` between two minikube binaries func CompareMinikubeStart(ctx context.Context, out io.Writer, binaries []*Binary) error { drivers := []string{"kvm2", "docker"} + if runtime.GOOS == "darwin" { + drivers = []string{"hyperkit", "docker"} + } for _, d := range drivers { fmt.Printf("**%s Driver**\n", d) if err := downloadArtifacts(ctx, binaries, d); err != nil { @@ -62,11 +66,17 @@ func collectResults(ctx context.Context, binaries []*Binary, driver string) (*re return nil, errors.Wrapf(err, "timing run %d with %s", run, binary.Name()) } rm.addResult(binary, r) - r, err = timeEnableIngress(ctx, binary) - if err != nil { - return nil, errors.Wrapf(err, "timing run %d with %s", run, binary.Name()) + if runtime.GOOS != "darwin" { + r, err = timeEnableIngress(ctx, binary) + if err != nil { + return nil, errors.Wrapf(err, "timing run %d with %s", run, binary.Name()) + } + rm.addResult(binary, r) + } + deleteCmd := exec.CommandContext(ctx, binary.path, "delete") + if err := deleteCmd.Run(); err != nil { + log.Printf("error deleting minikube: %v", err) } - rm.addResult(binary, r) } } return rm, nil @@ -74,10 +84,19 @@ func collectResults(ctx context.Context, binaries []*Binary, driver string) (*re func average(nums []float64) float64 { total := float64(0) + max := float64(0) + min := float64(0) for _, a := range nums { + if a > max { + max = a + } + if min > a { + min = a + } total += a } - return total / float64(len(nums)) + total = total - min - max + return total / float64(len(nums)-2) } func downloadArtifacts(ctx context.Context, binaries []*Binary, driver string) error { @@ -108,16 +127,9 @@ func timeMinikubeStart(ctx context.Context, binary *Binary, driver string) (*res // timeEnableIngress returns the time it takes to execute `minikube addons enable ingress` // It deletes the VM after `minikube addons enable ingress`. func timeEnableIngress(ctx context.Context, binary *Binary) (*result, error) { - enableCmd := exec.CommandContext(ctx, binary.path, "addons enable ingress") + enableCmd := exec.CommandContext(ctx, binary.path, "addons", "enable", "ingress") enableCmd.Stderr = os.Stderr - deleteCmd := exec.CommandContext(ctx, binary.path, "delete") - defer func() { - if err := deleteCmd.Run(); err != nil { - log.Printf("error deleting minikube: %v", err) - } - }() - log.Printf("Running: %v...", enableCmd.Args) r, err := timeCommandLogs(enableCmd) if err != nil { From 782bdae4ed4c7003b10b877be25b4c80176a0c36 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Tue, 6 Apr 2021 17:28:22 -0700 Subject: [PATCH 2/9] lint --- hack/jenkins/prbot.sh | 26 +++++++++++++------------- pkg/minikube/perf/result_manager.go | 2 +- pkg/minikube/perf/start.go | 2 +- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/hack/jenkins/prbot.sh b/hack/jenkins/prbot.sh index 2849f1c5a4..684ab7dc7e 100644 --- a/hack/jenkins/prbot.sh +++ b/hack/jenkins/prbot.sh @@ -1,18 +1,18 @@ #!/bin/bash -# Copyright 2021 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. +# Copyright 2021 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. set -x -o pipefail # Only run this on PRs diff --git a/pkg/minikube/perf/result_manager.go b/pkg/minikube/perf/result_manager.go index 7cce174cc0..06f3a71ba3 100644 --- a/pkg/minikube/perf/result_manager.go +++ b/pkg/minikube/perf/result_manager.go @@ -64,7 +64,7 @@ func (rm *resultManager) averageTime(binary *Binary) float64 { return average(times) } -func (rm *resultManager) summarizeResults(binaries []*Binary, driver string) { +func (rm *resultManager) summarizeResults(binaries []*Binary) { // print total and average times for _, b := range binaries { fmt.Printf("Times for %s: ", b.Name()) diff --git a/pkg/minikube/perf/start.go b/pkg/minikube/perf/start.go index a8d85371dd..8afe8aa8af 100644 --- a/pkg/minikube/perf/start.go +++ b/pkg/minikube/perf/start.go @@ -50,7 +50,7 @@ func CompareMinikubeStart(ctx context.Context, out io.Writer, binaries []*Binary fmt.Printf("error collecting results for %s driver: %v\n", d, err) continue } - rm.summarizeResults(binaries, d) + rm.summarizeResults(binaries) fmt.Println() } return nil From a944e7547c1b953c341ae2d2ac5b65cc7db4e6c8 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Tue, 6 Apr 2021 17:39:49 -0700 Subject: [PATCH 3/9] revert average back to normal --- pkg/minikube/perf/start.go | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/pkg/minikube/perf/start.go b/pkg/minikube/perf/start.go index 8afe8aa8af..a694b98ee4 100644 --- a/pkg/minikube/perf/start.go +++ b/pkg/minikube/perf/start.go @@ -84,19 +84,10 @@ func collectResults(ctx context.Context, binaries []*Binary, driver string) (*re func average(nums []float64) float64 { total := float64(0) - max := float64(0) - min := float64(0) for _, a := range nums { - if a > max { - max = a - } - if min > a { - min = a - } total += a } - total = total - min - max - return total / float64(len(nums)-2) + return total / float64(len(nums)) } func downloadArtifacts(ctx context.Context, binaries []*Binary, driver string) error { From 57eb831883cf674ecf63e870d7b0f254b10686b9 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Tue, 6 Apr 2021 18:20:41 -0700 Subject: [PATCH 4/9] fix pathing --- hack/jenkins/prbot.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hack/jenkins/prbot.sh b/hack/jenkins/prbot.sh index 684ab7dc7e..49bbb51634 100644 --- a/hack/jenkins/prbot.sh +++ b/hack/jenkins/prbot.sh @@ -21,13 +21,13 @@ if [[ "${MINIKUBE_LOCATION}" == "master" ]]; then fi # Make sure docker is installed and configured -./hack/jenkins/installers/check_install_docker.sh +./installers/check_install_docker.sh # Make sure gh is installed and configured -./hack/jenkins/installers/check_install_gh.sh +./installers/check_install_gh.sh # Make sure go is installed and configured -./hack/jenkins/installers/check_install_golang.sh +./installers/check_install_golang.sh # Grab latest code git clone https://github.com/kubernetes/minikube.git From 493f7f8edcc1e4ac8b42041a5fedad48c4010e65 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Tue, 6 Apr 2021 18:59:04 -0700 Subject: [PATCH 5/9] fixes --- hack/jenkins/prbot.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hack/jenkins/prbot.sh b/hack/jenkins/prbot.sh index 49bbb51634..87c48c144b 100644 --- a/hack/jenkins/prbot.sh +++ b/hack/jenkins/prbot.sh @@ -24,7 +24,7 @@ fi ./installers/check_install_docker.sh # Make sure gh is installed and configured -./installers/check_install_gh.sh +./installers/check_install_gh.sh v1.16.3 /usr/local # Make sure go is installed and configured ./installers/check_install_golang.sh @@ -37,7 +37,7 @@ cd minikube make out/minikube out/mkcmp # Run mkcmp -out/mkcmp out/minikube pr://${MINIKUBE_LOCATION} | mkcmp.log +out/mkcmp out/minikube pr://${MINIKUBE_LOCATION} | tee mkcmp.log if [ $? -gt 0 ]; then # Comment that mkcmp failed gh pr comment ${MINIKUBE_LOCATION} --body "timing minikube failed, please try again" From 68b903cbb6c2e1b0e18dcd3289e59e01226ebbb7 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Wed, 7 Apr 2021 09:04:47 -0700 Subject: [PATCH 6/9] escapism --- hack/jenkins/prbot.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hack/jenkins/prbot.sh b/hack/jenkins/prbot.sh index 87c48c144b..b15698b19b 100644 --- a/hack/jenkins/prbot.sh +++ b/hack/jenkins/prbot.sh @@ -44,4 +44,4 @@ if [ $? -gt 0 ]; then exit 1 fi output=$(cat mkcmp.log) -gh pr comment ${MINIKUBE_LOCATION} --body ${output} +gh pr comment ${MINIKUBE_LOCATION} --body "${output}" From 400fcc1b9dbe8022efd77be4dc1b428e024c6249 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Wed, 7 Apr 2021 09:54:39 -0700 Subject: [PATCH 7/9] use params on correct script --- hack/jenkins/prbot.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hack/jenkins/prbot.sh b/hack/jenkins/prbot.sh index b15698b19b..4211940e6d 100644 --- a/hack/jenkins/prbot.sh +++ b/hack/jenkins/prbot.sh @@ -24,10 +24,10 @@ fi ./installers/check_install_docker.sh # Make sure gh is installed and configured -./installers/check_install_gh.sh v1.16.3 /usr/local +./installers/check_install_gh.sh # Make sure go is installed and configured -./installers/check_install_golang.sh +./installers/check_install_golang.sh v1.16.3 /usr/local # Grab latest code git clone https://github.com/kubernetes/minikube.git From dad89e8223682d426630312cb8a4d43f5725802b Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Wed, 7 Apr 2021 14:12:59 -0700 Subject: [PATCH 8/9] skip docker for now --- hack/jenkins/prbot.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hack/jenkins/prbot.sh b/hack/jenkins/prbot.sh index 4211940e6d..b0b019a340 100644 --- a/hack/jenkins/prbot.sh +++ b/hack/jenkins/prbot.sh @@ -21,7 +21,7 @@ if [[ "${MINIKUBE_LOCATION}" == "master" ]]; then fi # Make sure docker is installed and configured -./installers/check_install_docker.sh +#./installers/check_install_docker.sh # Make sure gh is installed and configured ./installers/check_install_gh.sh From 2ce5ac04670973ff46b65a6237b9ed97ed0463a8 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Wed, 7 Apr 2021 15:28:12 -0700 Subject: [PATCH 9/9] run minikube delete before testing --- hack/jenkins/prbot.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/hack/jenkins/prbot.sh b/hack/jenkins/prbot.sh index b0b019a340..0f68772e9b 100644 --- a/hack/jenkins/prbot.sh +++ b/hack/jenkins/prbot.sh @@ -27,7 +27,7 @@ fi ./installers/check_install_gh.sh # Make sure go is installed and configured -./installers/check_install_golang.sh v1.16.3 /usr/local +./installers/check_install_golang.sh "1.16" "/usr/local" || true # Grab latest code git clone https://github.com/kubernetes/minikube.git @@ -36,6 +36,9 @@ cd minikube # Build minikube binary and mkcmp binary make out/minikube out/mkcmp +# Make sure there aren't any old minikube clusters laying around +out/minikube delete --all + # Run mkcmp out/mkcmp out/minikube pr://${MINIKUBE_LOCATION} | tee mkcmp.log if [ $? -gt 0 ]; then