From a00d632930deb3d1da068654dd8cbe1bc71c9d78 Mon Sep 17 00:00:00 2001 From: Daehyeok Mun Date: Fri, 8 Jan 2021 19:21:16 -0800 Subject: [PATCH 01/14] Check profile, node name to prevent duplication Check profile and node name before add to prevent conflict with machine name on multinode cluster. --- cmd/minikube/cmd/start.go | 21 ++++++++++++++++ pkg/minikube/config/profile.go | 22 +++++++++++++++-- pkg/minikube/machine/cluster_test.go | 4 +-- pkg/minikube/node/node.go | 18 ++++++++++++++ test/integration/multinode_test.go | 37 ++++++++++++++++++++++++++++ 5 files changed, 97 insertions(+), 5 deletions(-) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 24cda29dfe..78b201677c 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -176,6 +176,8 @@ func runStart(cmd *cobra.Command, args []string) { if existing != nil { upgradeExistingConfig(existing) + } else { + validateProfileName() } validateSpecifiedDriver(existing) @@ -638,6 +640,25 @@ func hostDriver(existing *config.ClusterConfig) string { return h.Driver.DriverName() } +// validateProfileName makes sure that new profile name not duplicated with any of machine names in existing multi-node clusters. +func validateProfileName() { + profiles, err := config.ListValidProfiles() + if err != nil { + exit.Message(reason.InternalListConfig, "Unable to list profiles: {{.error}}", out.V{"error": err}) + } + for _, p := range profiles { + for _, n := range p.Config.Nodes { + machineName := config.MachineName(*p.Config, n) + if ClusterFlagValue() == machineName { + out.WarningT("Profile name '{{.name}}' is duplicated with machine name '{{.machine}}' in profile '{{.profile}}'", out.V{"name": ClusterFlagValue(), + "machine": machineName, + "profile": p.Name}) + exit.Message(reason.Usage, "Profile name should be unique") + } + } + } +} + // validateSpecifiedDriver makes sure that if a user has passed in a driver // it matches the existing cluster if there is one func validateSpecifiedDriver(existing *config.ClusterConfig) { diff --git a/pkg/minikube/config/profile.go b/pkg/minikube/config/profile.go index a632f04859..d1b58947c2 100644 --- a/pkg/minikube/config/profile.go +++ b/pkg/minikube/config/profile.go @@ -223,7 +223,25 @@ func ListProfiles(miniHome ...string) (validPs []*Profile, inValidPs []*Profile, return validPs, inValidPs, nil } -// removeDupes removes duplicates +// ListValidProfiles returns profiles in minikube home dir +// Unlike `ListProfiles` this function doens't try to get profile from container +func ListValidProfiles(miniHome ...string) (ps []*Profile, err error) { + // try to get profiles list based on left over evidences such as directory + pDirs, err := profileDirs(miniHome...) + if err != nil { + return nil, err + } + + for _, n := range pDirs { + p, err := LoadProfile(n, miniHome...) + if err == nil && p.IsValid() { + ps = append(ps, p) + } + } + return ps, nil +} + +// removeDupes removes duplipcates func removeDupes(profiles []string) []string { // Use map to record duplicates as we find them. seen := map[string]bool{} @@ -291,7 +309,7 @@ func ProfileFolderPath(profile string, miniHome ...string) string { // MachineName returns the name of the machine, as seen by the hypervisor given the cluster and node names func MachineName(cc ClusterConfig, n Node) string { // For single node cluster, default to back to old naming - if len(cc.Nodes) == 1 || n.ControlPlane { + if (len(cc.Nodes) == 1 && cc.Nodes[0].Name == n.Name) || n.ControlPlane { return cc.Name } return fmt.Sprintf("%s-%s", cc.Name, n.Name) diff --git a/pkg/minikube/machine/cluster_test.go b/pkg/minikube/machine/cluster_test.go index ea32cc36a7..b33f8e65ff 100644 --- a/pkg/minikube/machine/cluster_test.go +++ b/pkg/minikube/machine/cluster_test.go @@ -146,10 +146,8 @@ func TestStartHostExists(t *testing.T) { mc := defaultClusterConfig mc.Name = ih.Name - n := config.Node{Name: ih.Name} - // This should pass without calling Create because the host exists already. - h, _, err := StartHost(api, &mc, &n) + h, _, err := StartHost(api, &mc, &(mc.Nodes[0])) if err != nil { t.Fatalf("Error starting host: %v", err) } diff --git a/pkg/minikube/node/node.go b/pkg/minikube/node/node.go index 672e52276d..2f41f49a5f 100644 --- a/pkg/minikube/node/node.go +++ b/pkg/minikube/node/node.go @@ -38,6 +38,24 @@ const ( // Add adds a new node config to an existing cluster. func Add(cc *config.ClusterConfig, n config.Node, delOnFail bool) error { + profiles, err := config.ListValidProfiles() + if err != nil { + return err + } + + machineName := config.MachineName(*cc, n) + for _, p := range profiles { + if p.Config.Name == cc.Name { + continue + } + + for _, existNode := range p.Config.Nodes { + if machineName == config.MachineName(*p.Config, existNode) { + return errors.Errorf("Node %s already exists in %s profile", machineName, p.Name) + } + } + } + if err := config.SaveNode(cc, &n); err != nil { return errors.Wrap(err, "save node") } diff --git a/test/integration/multinode_test.go b/test/integration/multinode_test.go index bad0bf7817..268bbf25a9 100644 --- a/test/integration/multinode_test.go +++ b/test/integration/multinode_test.go @@ -48,6 +48,7 @@ func TestMultiNode(t *testing.T) { {"DeleteNode", validateDeleteNodeFromMultiNode}, {"StopMultiNode", validateStopMultiNodeCluster}, {"RestartMultiNode", validateRestartMultiNodeCluster}, + {"ValidateNameConflict", validatNameConflict}, } for _, tc := range tests { tc := tc @@ -308,3 +309,39 @@ func validateDeleteNodeFromMultiNode(ctx context.Context, t *testing.T, profile t.Errorf("expected 2 nodes Ready status to be True, got %v", rr.Output()) } } + +func validatNameConflict(ctx context.Context, t *testing.T, profile string) { + rr, err := Run(t, exec.CommandContext(ctx, Target(), "node", "list", "-p", profile)) + if err != nil { + t.Errorf("failed to run node list. args %q : %v", rr.Command(), err) + } + curNodeNum := strings.Count(rr.Stdout.String(), profile) + + // Start new profile. It's expected failture + profileName := fmt.Sprintf("%s-m0%d", profile, curNodeNum) + startArgs := append([]string{"start", "-p", profileName}, StartArgs()...) + rr, err = Run(t, exec.CommandContext(ctx, Target(), startArgs...)) + if err == nil { + t.Errorf("expected start profile command to fail. args %q", rr.Command()) + } + + // Start new profile temporary profile to conflict node name. + profileName = fmt.Sprintf("%s-m0%d", profile, curNodeNum+1) + startArgs = append([]string{"start", "-p", profileName}, StartArgs()...) + rr, err = Run(t, exec.CommandContext(ctx, Target(), startArgs...)) + if err != nil { + t.Errorf("failed to start profile. args %q : %v", rr.Command(), err) + } + + // Add a node to the current cluster. It's expected failture + addArgs := []string{"node", "add", "-p", profile} + rr, err = Run(t, exec.CommandContext(ctx, Target(), addArgs...)) + if err == nil { + t.Errorf("expected add node command to fail. args %q : %v", rr.Command(), err) + } + + rr, err = Run(t, exec.CommandContext(ctx, Target(), "delete", "-p", profileName)) + if err != nil { + t.Logf("failed to clean temporary profile. args %q : %v", rr.Command(), err) + } +} From 5ddaf5f54512a87d39fad68c745fd202bf3acf79 Mon Sep 17 00:00:00 2001 From: Daehyeok Mun Date: Thu, 17 Dec 2020 13:34:21 -0800 Subject: [PATCH 02/14] Remove cluster's subnode from ListProfiles result. Fix prolbem which ListProfiles return subnodes in Mult-Node clusters as a part of inValidPs. --- pkg/minikube/config/profile.go | 23 ++++++++++++++-- test/integration/multinode_test.go | 42 ++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/pkg/minikube/config/profile.go b/pkg/minikube/config/profile.go index a632f04859..c7edf047e6 100644 --- a/pkg/minikube/config/profile.go +++ b/pkg/minikube/config/profile.go @@ -207,8 +207,9 @@ func ListProfiles(miniHome ...string) (validPs []*Profile, inValidPs []*Profile, if err == nil { pDirs = append(pDirs, cs...) } - pDirs = removeDupes(pDirs) - for _, n := range pDirs { + + nodeNames := map[string]bool{} + for _, n := range removeDupes(pDirs) { p, err := LoadProfile(n, miniHome...) if err != nil { inValidPs = append(inValidPs, p) @@ -219,7 +220,13 @@ func ListProfiles(miniHome ...string) (validPs []*Profile, inValidPs []*Profile, continue } validPs = append(validPs, p) + + for _, child := range p.Config.Nodes { + nodeNames[MachineName(*p.Config, child)] = true + } } + + inValidPs = removeChildNodes(inValidPs, nodeNames) return validPs, inValidPs, nil } @@ -243,6 +250,18 @@ func removeDupes(profiles []string) []string { return result } +// removeChildNodes remove invalid profiles which have a same name with any sub-node's machine name +// it will return nil if invalid profiles are not exists. +func removeChildNodes(inValidPs []*Profile, nodeNames map[string]bool) (ps []*Profile) { + for _, p := range inValidPs { + if _, ok := nodeNames[p.Name]; !ok { + ps = append(ps, p) + } + } + + return ps +} + // LoadProfile loads type Profile based on its name func LoadProfile(name string, miniHome ...string) (*Profile, error) { cfg, err := DefaultLoader.LoadConfigFromFile(name, miniHome...) diff --git a/test/integration/multinode_test.go b/test/integration/multinode_test.go index bad0bf7817..285f79e577 100644 --- a/test/integration/multinode_test.go +++ b/test/integration/multinode_test.go @@ -20,10 +20,13 @@ package integration import ( "context" + "encoding/json" "fmt" "os/exec" "strings" "testing" + + "k8s.io/minikube/pkg/minikube/config" ) func TestMultiNode(t *testing.T) { @@ -43,6 +46,7 @@ func TestMultiNode(t *testing.T) { }{ {"FreshStart2Nodes", validateMultiNodeStart}, {"AddNode", validateAddNodeToMultiNode}, + {"ProfileList", validateProfileListWithMultiNode}, {"StopNode", validateStopRunningNode}, {"StartAfterStop", validateStartNodeAfterStop}, {"DeleteNode", validateDeleteNodeFromMultiNode}, @@ -109,6 +113,44 @@ func validateAddNodeToMultiNode(ctx context.Context, t *testing.T, profile strin } } +func validateProfileListWithMultiNode(ctx context.Context, t *testing.T, profile string) { + rr, err := Run(t, exec.CommandContext(ctx, Target(), "profile", "list", "--output", "json")) + if err != nil { + t.Errorf("failed to list profiles with json format. args %q: %v", rr.Command(), err) + } + + var jsonObject map[string][]config.Profile + err = json.Unmarshal(rr.Stdout.Bytes(), &jsonObject) + if err != nil { + t.Errorf("failed to decode json from profile list: args %q: %v", rr.Command(), err) + } + + validProfiles := jsonObject["valid"] + var profileObject *config.Profile + for _, obj := range validProfiles { + if obj.Name == profile { + profileObject = &obj + break + } + } + + if profileObject == nil { + t.Errorf("expected the json of 'profile list' to include %q but got *%q*. args: %q", profile, rr.Stdout.String(), rr.Command()) + } else if expected, numNodes := 3, len(profileObject.Config.Nodes); expected != numNodes { + t.Errorf("expected profile %q in json of 'profile list' include %d nodes but have %d nodes. got *%q*. args: %q", profile, expected, numNodes, rr.Stdout.String(), rr.Command()) + } + + if invalidPs, ok := jsonObject["invalid"]; ok { + for _, ps := range invalidPs { + if strings.Contains(ps.Name, profile) { + t.Errorf("expected the json of 'profile list' to not include profile or node in invalid profile but got *%q*. args: %q", rr.Stdout.String(), rr.Command()) + } + } + + } + +} + func validateStopRunningNode(ctx context.Context, t *testing.T, profile string) { // Run minikube node stop on that node rr, err := Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "node", "stop", ThirdNodeName)) From f94cb815cfb1d1e8d4175f5891be22682dbd4058 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Fri, 29 Jan 2021 17:12:23 +0100 Subject: [PATCH 03/14] Make sure that ssh driver gets an ip address --- cmd/minikube/cmd/start.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 24cda29dfe..26bcbf218a 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -1097,6 +1097,13 @@ func validateFlags(cmd *cobra.Command, drvName string) { } } + if driver.IsSSH(drvName) { + sshIPAddress := viper.GetString(sshIPAddress) + if sshIPAddress == "" { + exit.Message(reason.Usage, "No IP address provided. Try specifying --ssh-ip-address, or see https://minikube.sigs.k8s.io/docs/drivers/ssh/") + } + } + // validate kubeadm extra args if invalidOpts := bsutil.FindInvalidExtraConfigFlags(config.ExtraOptions); len(invalidOpts) > 0 { out.WarningT( From bbc2bd59c5f5a5982b1e856fce337e1530a7cde2 Mon Sep 17 00:00:00 2001 From: Ilya Zuyev Date: Mon, 1 Feb 2021 16:50:04 -0800 Subject: [PATCH 04/14] site: update installation instructions for darwin/arm64 --- site/content/en/docs/start/_index.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/site/content/en/docs/start/_index.md b/site/content/en/docs/start/_index.md index 85f2497263..cc33d27ab7 100644 --- a/site/content/en/docs/start/_index.md +++ b/site/content/en/docs/start/_index.md @@ -90,11 +90,20 @@ brew link minikube Otherwise, download minikube directly: +### x86 + ```shell curl -LO https://storage.googleapis.com/minikube/releases/latest/minikube-darwin-amd64 sudo install minikube-darwin-amd64 /usr/local/bin/minikube ``` +### ARM + +```shell +curl -LO https://storage.googleapis.com/minikube/releases/latest/minikube-darwin-arm64 +sudo install minikube-darwin-arm64 /usr/local/bin/minikube +``` + {{% /mactab %}} {{% windowstab %}} From 4ca56afb03fcf7a613bc1fbb16b6e7c8f3ed499d Mon Sep 17 00:00:00 2001 From: Ilya Zuyev Date: Mon, 1 Feb 2021 16:56:50 -0800 Subject: [PATCH 05/14] site: update building instructions and known issues related to arm64 --- site/content/en/docs/contrib/building/binaries.md | 2 ++ site/content/en/docs/drivers/docker.md | 2 -- site/content/en/docs/drivers/includes/docker_usage.inc | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/site/content/en/docs/contrib/building/binaries.md b/site/content/en/docs/contrib/building/binaries.md index f13a5472b9..ade5f54f99 100644 --- a/site/content/en/docs/contrib/building/binaries.md +++ b/site/content/en/docs/contrib/building/binaries.md @@ -64,7 +64,9 @@ See [Building the minikube ISO](../iso) We publish CI builds of minikube, built at every Pull Request. Builds are available at (substitute in the relevant PR number): - +- - +- - We also publish CI builds of minikube-iso, built at every Pull Request that touches deploy/iso/minikube-iso. Builds are available at: diff --git a/site/content/en/docs/drivers/docker.md b/site/content/en/docs/drivers/docker.md index 041763be68..1300cd0867 100644 --- a/site/content/en/docs/drivers/docker.md +++ b/site/content/en/docs/drivers/docker.md @@ -23,8 +23,6 @@ The Docker driver allows you to install Kubernetes into an existing Docker insta - [userns-remap](https://docs.docker.com/engine/security/userns-remap/) - [rootless](https://docs.docker.com/engine/security/rootless/) -- Docker driver is not supported on non-amd64 architectures such as arm yet. For non-amd64 archs please use [other drivers]({{< ref "/docs/drivers/_index.md" >}}) - - On macOS, containers might get hung and require a restart of Docker for Desktop. See [docker/for-mac#1835](https://github.com/docker/for-mac/issues/1835) - The `ingress`, and `ingress-dns` addons are currently only supported on Linux. See [#7332](https://github.com/kubernetes/minikube/issues/7332) diff --git a/site/content/en/docs/drivers/includes/docker_usage.inc b/site/content/en/docs/drivers/includes/docker_usage.inc index 8b1ad0533a..d98a449d01 100644 --- a/site/content/en/docs/drivers/includes/docker_usage.inc +++ b/site/content/en/docs/drivers/includes/docker_usage.inc @@ -1,7 +1,7 @@ ## Requirements - [Install Docker](https://hub.docker.com/search?q=&type=edition&offering=community&sort=updated_at&order=desc) -- amd64 system. +- amd64 or arm64 system. ## Usage From abbd148fd167b1faa73aeb8061760e3803c4ff4f Mon Sep 17 00:00:00 2001 From: Ilya Zuyev Date: Mon, 1 Feb 2021 18:36:05 -0800 Subject: [PATCH 06/14] trigger build --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 9d08c8953a..a90e354fc9 100644 --- a/README.md +++ b/README.md @@ -64,3 +64,4 @@ minikube is a Kubernetes [#sig-cluster-lifecycle](https://github.com/kubernetes/ Join our meetings: * [Bi-weekly office hours, Mondays @ 11am PST](https://tinyurl.com/minikube-oh) * [Triage Party](https://minikube.sigs.k8s.io/docs/contrib/triage/) + From 72e26a29529cece2813a0e88bb01afbdc852a07d Mon Sep 17 00:00:00 2001 From: Ilya Zuyev Date: Mon, 1 Feb 2021 18:39:49 -0800 Subject: [PATCH 07/14] Revert "trigger build" This reverts commit abbd148fd167b1faa73aeb8061760e3803c4ff4f. --- README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/README.md b/README.md index a90e354fc9..9d08c8953a 100644 --- a/README.md +++ b/README.md @@ -64,4 +64,3 @@ minikube is a Kubernetes [#sig-cluster-lifecycle](https://github.com/kubernetes/ Join our meetings: * [Bi-weekly office hours, Mondays @ 11am PST](https://tinyurl.com/minikube-oh) * [Triage Party](https://minikube.sigs.k8s.io/docs/contrib/triage/) - From 21206402a955803d839a0e62a5afdc90550b814a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Tue, 2 Feb 2021 07:54:10 +0100 Subject: [PATCH 08/14] Check that the ssh-ip-address is a valid IP --- cmd/minikube/cmd/start.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 26bcbf218a..11eb03fdce 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -1102,6 +1102,13 @@ func validateFlags(cmd *cobra.Command, drvName string) { if sshIPAddress == "" { exit.Message(reason.Usage, "No IP address provided. Try specifying --ssh-ip-address, or see https://minikube.sigs.k8s.io/docs/drivers/ssh/") } + + if net.ParseIP(sshIPAddress) == nil { + _, err := net.LookupIP(sshIPAddress) + if err != nil { + exit.Error(reason.Usage, "Could not resolve IP address", err) + } + } } // validate kubeadm extra args From 397b7b22863a76ab6f7bc65c1312fbb381db4b35 Mon Sep 17 00:00:00 2001 From: Yanshu Zhao Date: Tue, 2 Feb 2021 16:28:04 +0000 Subject: [PATCH 09/14] Use global flag and only print one warning for the same cmd per binaray run --- pkg/drivers/kic/oci/cli_runner.go | 17 +++++++--- pkg/drivers/kic/oci/cli_runner_test.go | 44 ++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 4 deletions(-) create mode 100644 pkg/drivers/kic/oci/cli_runner_test.go diff --git a/pkg/drivers/kic/oci/cli_runner.go b/pkg/drivers/kic/oci/cli_runner.go index b141305f7d..53021e3522 100644 --- a/pkg/drivers/kic/oci/cli_runner.go +++ b/pkg/drivers/kic/oci/cli_runner.go @@ -24,6 +24,7 @@ import ( "os/exec" "runtime" "strings" + "sync" "time" "k8s.io/klog/v2" @@ -32,6 +33,9 @@ import ( "k8s.io/minikube/pkg/minikube/style" ) +var Lock sync.Mutex +var WarningPrintedSet = make(map[string]bool) + // RunResult holds the results of a Runner type RunResult struct { Stdout bytes.Buffer @@ -133,10 +137,15 @@ func runCmd(cmd *exec.Cmd, warnSlow ...bool) (*RunResult, error) { elapsed := time.Since(start) if warn { if elapsed > warnTime { - out.WarningT(`Executing "{{.command}}" took an unusually long time: {{.duration}}`, out.V{"command": rr.Command(), "duration": elapsed}) - // Don't show any restarting hint, when running podman locally (on linux, with sudo). Only when having a service. - if cmd.Args[0] != "sudo" { - out.ErrT(style.Tip, `Restarting the {{.name}} service may improve performance.`, out.V{"name": cmd.Args[0]}) + Lock.Lock() + defer Lock.Unlock() + if _, ok := WarningPrintedSet[rr.Command()]; !ok { + out.WarningT(`Executing "{{.command}}" took an unusually long time: {{.duration}}`, out.V{"command": rr.Command(), "duration": elapsed}) + // Don't show any restarting hint, when running podman locally (on linux, with sudo). Only when having a service. + if cmd.Args[0] != "sudo" { + out.ErrT(style.Tip, `Restarting the {{.name}} service may improve performance.`, out.V{"name": cmd.Args[0]}) + } + WarningPrintedSet[rr.Command()] = true } } diff --git a/pkg/drivers/kic/oci/cli_runner_test.go b/pkg/drivers/kic/oci/cli_runner_test.go new file mode 100644 index 0000000000..ef3157e53a --- /dev/null +++ b/pkg/drivers/kic/oci/cli_runner_test.go @@ -0,0 +1,44 @@ +package oci + +import ( + "os/exec" + "runtime" + "strings" + "testing" + + "k8s.io/minikube/pkg/minikube/out" + "k8s.io/minikube/pkg/minikube/tests" +) + +func TestCliRunnerOnlyPrintOnce(t *testing.T) { + if runtime.GOOS != "linux" { + return + } + f1 := tests.NewFakeFile() + out.SetErrFile(f1) + + cmd := exec.Command("sleep", "3") + _, err := runCmd(cmd, true) + + if err != nil { + t.Errorf("runCmd has error: %v", err) + } + + if !strings.Contains(f1.String(), "Executing \"sleep 3\" took an unusually long time") { + t.Errorf("runCmd does not print the correct log, instead print :%v", f1.String()) + } + + f2 := tests.NewFakeFile() + out.SetErrFile(f2) + + cmd = exec.Command("sleep", "3") + _, err = runCmd(cmd, true) + + if err != nil { + t.Errorf("runCmd has error: %v", err) + } + + if strings.Contains(f2.String(), "Executing \"sleep 3\" took an unusually long time") { + t.Errorf("runCmd does not print the correct log, instead print :%v", f2.String()) + } +} From 4665993109f64be39034723b063a848e2f3f164b Mon Sep 17 00:00:00 2001 From: Yanshu Zhao Date: Tue, 2 Feb 2021 16:57:47 +0000 Subject: [PATCH 10/14] Add header file for the test --- pkg/drivers/kic/oci/cli_runner_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/pkg/drivers/kic/oci/cli_runner_test.go b/pkg/drivers/kic/oci/cli_runner_test.go index ef3157e53a..d3fc1b3163 100644 --- a/pkg/drivers/kic/oci/cli_runner_test.go +++ b/pkg/drivers/kic/oci/cli_runner_test.go @@ -1,3 +1,19 @@ +/* +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 oci import ( From 8009a5b3a8a8c15656b34ce5bf550b6628fa1c9d Mon Sep 17 00:00:00 2001 From: Yanshu Zhao Date: Tue, 2 Feb 2021 20:59:29 +0000 Subject: [PATCH 11/14] change naming --- pkg/drivers/kic/oci/cli_runner.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/drivers/kic/oci/cli_runner.go b/pkg/drivers/kic/oci/cli_runner.go index 53021e3522..af1fdfffd6 100644 --- a/pkg/drivers/kic/oci/cli_runner.go +++ b/pkg/drivers/kic/oci/cli_runner.go @@ -33,8 +33,8 @@ import ( "k8s.io/minikube/pkg/minikube/style" ) -var Lock sync.Mutex -var WarningPrintedSet = make(map[string]bool) +var warnLock sync.Mutex +var alreadyWarnedCmds = make(map[string]bool) // RunResult holds the results of a Runner type RunResult struct { @@ -137,15 +137,15 @@ func runCmd(cmd *exec.Cmd, warnSlow ...bool) (*RunResult, error) { elapsed := time.Since(start) if warn { if elapsed > warnTime { - Lock.Lock() - defer Lock.Unlock() - if _, ok := WarningPrintedSet[rr.Command()]; !ok { + warnLock.Lock() + defer warnLock.Unlock() + if _, ok := alreadyWarnedCmds[rr.Command()]; !ok { out.WarningT(`Executing "{{.command}}" took an unusually long time: {{.duration}}`, out.V{"command": rr.Command(), "duration": elapsed}) // Don't show any restarting hint, when running podman locally (on linux, with sudo). Only when having a service. if cmd.Args[0] != "sudo" { out.ErrT(style.Tip, `Restarting the {{.name}} service may improve performance.`, out.V{"name": cmd.Args[0]}) } - WarningPrintedSet[rr.Command()] = true + alreadyWarnedCmds[rr.Command()] = true } } From 84e7c47e8ad0cfcd4d92e3687d03e958ec76c783 Mon Sep 17 00:00:00 2001 From: Yanshu Zhao Date: Tue, 2 Feb 2021 21:20:31 +0000 Subject: [PATCH 12/14] reduce lock scope --- pkg/drivers/kic/oci/cli_runner.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/pkg/drivers/kic/oci/cli_runner.go b/pkg/drivers/kic/oci/cli_runner.go index af1fdfffd6..27857d51a4 100644 --- a/pkg/drivers/kic/oci/cli_runner.go +++ b/pkg/drivers/kic/oci/cli_runner.go @@ -138,14 +138,18 @@ func runCmd(cmd *exec.Cmd, warnSlow ...bool) (*RunResult, error) { if warn { if elapsed > warnTime { warnLock.Lock() - defer warnLock.Unlock() - if _, ok := alreadyWarnedCmds[rr.Command()]; !ok { + _, ok := alreadyWarnedCmds[rr.Command()] + if !ok { + alreadyWarnedCmds[rr.Command()] = true + } + warnLock.Unlock() + + if !ok { out.WarningT(`Executing "{{.command}}" took an unusually long time: {{.duration}}`, out.V{"command": rr.Command(), "duration": elapsed}) // Don't show any restarting hint, when running podman locally (on linux, with sudo). Only when having a service. if cmd.Args[0] != "sudo" { out.ErrT(style.Tip, `Restarting the {{.name}} service may improve performance.`, out.V{"name": cmd.Args[0]}) } - alreadyWarnedCmds[rr.Command()] = true } } From 67458418ca72c4af58cf37d4a6dd63dd4dc50a8b Mon Sep 17 00:00:00 2001 From: Yanshu Zhao Date: Tue, 2 Feb 2021 21:39:34 +0000 Subject: [PATCH 13/14] Change test naming --- pkg/drivers/kic/oci/cli_runner_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/drivers/kic/oci/cli_runner_test.go b/pkg/drivers/kic/oci/cli_runner_test.go index d3fc1b3163..0ad18894ed 100644 --- a/pkg/drivers/kic/oci/cli_runner_test.go +++ b/pkg/drivers/kic/oci/cli_runner_test.go @@ -26,7 +26,7 @@ import ( "k8s.io/minikube/pkg/minikube/tests" ) -func TestCliRunnerOnlyPrintOnce(t *testing.T) { +func TestRunCmdWarnSlowOnce(t *testing.T) { if runtime.GOOS != "linux" { return } From 2bb38c657c4b91c875dd9920fc5cfcdb498da584 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Wed, 3 Feb 2021 11:18:36 -0800 Subject: [PATCH 14/14] add ilya-zuyev as approver --- OWNERS | 1 + 1 file changed, 1 insertion(+) diff --git a/OWNERS b/OWNERS index 7585b438f2..0c6ce866e2 100644 --- a/OWNERS +++ b/OWNERS @@ -18,6 +18,7 @@ approvers: - medyagh - josedonizetti - priyawadhwa + - ilya-zuyev emeritus_approvers: - dlorenc - luxas