From 5dd0f26cd724537e220fdb83eb50febb20538c2d Mon Sep 17 00:00:00 2001 From: Steven Powell Date: Fri, 31 Mar 2023 11:15:34 -0700 Subject: [PATCH] cleanup: Check for scanner errors --- cmd/minikube/cmd/config/addons_list_test.go | 3 +++ cmd/minikube/cmd/status.go | 2 +- .../process_last_90/process_last_90.go | 6 ++--- pkg/drivers/kic/oci/network_create.go | 2 +- pkg/drivers/kic/oci/oci.go | 16 +++++++++----- pkg/drivers/kic/oci/volumes.go | 6 +++++ pkg/minikube/bootstrapper/kubeadm/kubeadm.go | 3 +++ pkg/minikube/command/command_runner.go | 2 +- pkg/minikube/logs/logs.go | 7 ++++++ .../registry/drvs/hyperv/powershell.go | 3 +++ pkg/minikube/sshutil/sshutil.go | 3 +++ pkg/minikube/tunnel/kic/ssh_conn.go | 3 +++ test/integration/aaa_download_only_test.go | 22 ++++++++----------- test/integration/util_test.go | 2 +- 14 files changed, 55 insertions(+), 25 deletions(-) diff --git a/cmd/minikube/cmd/config/addons_list_test.go b/cmd/minikube/cmd/config/addons_list_test.go index 5941d94853..716fb2bdfa 100644 --- a/cmd/minikube/cmd/config/addons_list_test.go +++ b/cmd/minikube/cmd/config/addons_list_test.go @@ -60,6 +60,9 @@ func TestAddonsList(t *testing.T) { pipeCount += strings.Count(buf.Text(), "|") got += buf.Text() } + if err := buf.Err(); err != nil { + t.Errorf("failed to read stdout: %v", err) + } // The lines we pull should look something like // |------------|------------|(------|) // | ADDON NAME | MAINTAINER |( DOCS |) diff --git a/cmd/minikube/cmd/status.go b/cmd/minikube/cmd/status.go index 7dbabf3410..aaee94cbb7 100644 --- a/cmd/minikube/cmd/status.go +++ b/cmd/minikube/cmd/status.go @@ -501,7 +501,7 @@ func readEventLog(name string) ([]cloudevents.Event, time.Time, error) { events = append(events, ev) } - return events, st.ModTime(), nil + return events, st.ModTime(), scanner.Err() } // clusterState converts Status structs into a ClusterState struct diff --git a/hack/jenkins/test-flake-chart/process_last_90/process_last_90.go b/hack/jenkins/test-flake-chart/process_last_90/process_last_90.go index a7eb60b620..50d0ab2d0b 100644 --- a/hack/jenkins/test-flake-chart/process_last_90/process_last_90.go +++ b/hack/jenkins/test-flake-chart/process_last_90/process_last_90.go @@ -87,13 +87,13 @@ func main() { validDate = true write(dw, line) } + if err := s.Err(); err != nil { + log.Fatalf("failed to read file: %v", err) + } if err := dw.Flush(); err != nil { log.Fatalf("failed to flush data writer: %v", err) } - if err := s.Err(); err != nil { - log.Fatalf("scanner had an error: %v", err) - } if err := data.Close(); err != nil { log.Fatalf("failed to close source file: %v", err) } diff --git a/pkg/drivers/kic/oci/network_create.go b/pkg/drivers/kic/oci/network_create.go index bb8ace0aae..777db5fbe2 100644 --- a/pkg/drivers/kic/oci/network_create.go +++ b/pkg/drivers/kic/oci/network_create.go @@ -326,7 +326,7 @@ func networkNamesByLabel(ociBin string, label string) ([]string, error) { lines = append(lines, strings.TrimSpace(scanner.Text())) } - return lines, nil + return lines, scanner.Err() } // DeleteAllKICKNetworksByLabel deletes all networks that have a specific label diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index daec06e412..29dc26293d 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -407,6 +407,12 @@ func inspect(ociBin string, containerNameOrID, format string) ([]string, error) for scanner.Scan() { lines = append(lines, scanner.Text()) } + if scanErr := scanner.Err(); scanErr != nil { + klog.Warningf("failed to read output: %v", scanErr) + if err == nil { + err = scanErr + } + } return lines, err } @@ -473,6 +479,9 @@ func isUsernsRemapEnabled(ociBin string) bool { for scanner.Scan() { lines = append(lines, scanner.Text()) } + if err := scanner.Err(); err != nil { + klog.Warningf("failed to read output: %v", err) + } if len(lines) > 0 { if strings.Contains(lines[0], "name=userns") { @@ -533,7 +542,7 @@ func ListContainersByLabel(ctx context.Context, ociBin string, label string, war names = append(names, n) } } - return names, err + return names, s.Err() } // ListImagesRepository returns all the images names @@ -554,10 +563,7 @@ func ListImagesRepository(ctx context.Context, ociBin string) ([]string, error) names = append(names, n) } } - if err := s.Err(); err != nil { - return nil, err - } - return names, nil + return names, s.Err() } // PointToHostDockerDaemon will unset env variables that point to docker inside minikube diff --git a/pkg/drivers/kic/oci/volumes.go b/pkg/drivers/kic/oci/volumes.go index bf32155511..30a451f622 100644 --- a/pkg/drivers/kic/oci/volumes.go +++ b/pkg/drivers/kic/oci/volumes.go @@ -119,6 +119,12 @@ func allVolumesByLabel(ociBin string, label string) ([]string, error) { vols = append(vols, v) } } + if scanErr := s.Err(); scanErr != nil { + klog.Warningf("failed to read output: %v", scanErr) + if err == nil { + err = scanErr + } + } return vols, err } diff --git a/pkg/minikube/bootstrapper/kubeadm/kubeadm.go b/pkg/minikube/bootstrapper/kubeadm/kubeadm.go index 7a85fe387a..5e84f23cb6 100644 --- a/pkg/minikube/bootstrapper/kubeadm/kubeadm.go +++ b/pkg/minikube/bootstrapper/kubeadm/kubeadm.go @@ -341,6 +341,9 @@ func outputKubeadmInitSteps(logs io.Reader, wg *sync.WaitGroup) { nextStepIndex++ } + if err := scanner.Err(); err != nil { + klog.Warningf("failed to read logs: %v", err) + } wg.Done() } diff --git a/pkg/minikube/command/command_runner.go b/pkg/minikube/command/command_runner.go index ad13548f5a..88b6d5ad3b 100644 --- a/pkg/minikube/command/command_runner.go +++ b/pkg/minikube/command/command_runner.go @@ -138,7 +138,7 @@ func teePrefix(prefix string, r io.Reader, w io.Writer, logger func(format strin if line.Len() > 0 { logger("%s%s", prefix, line.String()) } - return nil + return scanner.Err() } // fileExists checks that the same file exists on the other end diff --git a/pkg/minikube/logs/logs.go b/pkg/minikube/logs/logs.go index 2c9aab2801..d29a860a14 100644 --- a/pkg/minikube/logs/logs.go +++ b/pkg/minikube/logs/logs.go @@ -139,6 +139,9 @@ func FindProblems(r cruntime.Manager, bs bootstrapper.Bootstrapper, cfg config.C problems = append(problems, l) } } + if err := scanner.Err(); err != nil { + klog.Warningf("failed to read output: %v", err) + } if len(problems) > 0 { pMap[name] = problems } @@ -198,6 +201,10 @@ func Output(r cruntime.Manager, bs bootstrapper.Bootstrapper, cfg config.Cluster for scanner.Scan() { l += scanner.Text() + "\n" } + if err := scanner.Err(); err != nil { + klog.Errorf("failed to read output: %v", err) + failed = append(failed, name) + } out.Styled(style.Empty, l) } diff --git a/pkg/minikube/registry/drvs/hyperv/powershell.go b/pkg/minikube/registry/drvs/hyperv/powershell.go index 6e3f6d14dd..d9730f32a1 100644 --- a/pkg/minikube/registry/drvs/hyperv/powershell.go +++ b/pkg/minikube/registry/drvs/hyperv/powershell.go @@ -62,6 +62,9 @@ func parseLines(stdout string) []string { for s.Scan() { resp = append(resp, s.Text()) } + if err := s.Err(); err != nil { + klog.Warningf("failed to read stdout: %v", err) + } return resp } diff --git a/pkg/minikube/sshutil/sshutil.go b/pkg/minikube/sshutil/sshutil.go index 5638de47b4..2ef0454a10 100644 --- a/pkg/minikube/sshutil/sshutil.go +++ b/pkg/minikube/sshutil/sshutil.go @@ -120,6 +120,9 @@ func KnownHost(host string, knownHosts string) bool { } } } + if err := scanner.Err(); err != nil { + klog.Warningf("failed to read file: %v", err) + } return false } diff --git a/pkg/minikube/tunnel/kic/ssh_conn.go b/pkg/minikube/tunnel/kic/ssh_conn.go index 0d8cc4e7e9..ead2ec5768 100644 --- a/pkg/minikube/tunnel/kic/ssh_conn.go +++ b/pkg/minikube/tunnel/kic/ssh_conn.go @@ -186,6 +186,9 @@ func logOutput(r io.Reader, service string) { for s.Scan() { klog.Infof("%s tunnel: %s", service, s.Text()) } + if err := s.Err(); err != nil { + klog.Warningf("failed to read: %v", err) + } } func (c *sshConn) stop() error { diff --git a/test/integration/aaa_download_only_test.go b/test/integration/aaa_download_only_test.go index b59dcea1d2..a080b5673d 100644 --- a/test/integration/aaa_download_only_test.go +++ b/test/integration/aaa_download_only_test.go @@ -43,8 +43,6 @@ import ( // TestDownloadOnly makes sure the --download-only parameter in minikube start caches the appropriate images and tarballs. func TestDownloadOnly(t *testing.T) { - // Stores the startup run result for later error messages - var rrr *RunResult profile := UniqueProfileName("download-only") ctx, cancel := context.WithTimeout(context.Background(), Minutes(30)) defer Cleanup(t, profile, cancel) @@ -69,10 +67,6 @@ func TestDownloadOnly(t *testing.T) { // --force to avoid uid check args := append([]string{"start", "-o=json", "--download-only", "-p", profile, "--force", "--alsologtostderr", fmt.Sprintf("--kubernetes-version=%s", v), fmt.Sprintf("--container-runtime=%s", containerRuntime)}, StartArgs()...) rt, err := Run(t, exec.CommandContext(ctx, Target(), args...)) - if rrr == nil { - // Preserve the initial run-result for debugging - rrr = rt - } if err != nil { t.Errorf("failed to download only. args: %q %v", args, err) } @@ -91,6 +85,9 @@ func TestDownloadOnly(t *testing.T) { } } } + if err := s.Err(); err != nil { + t.Errorf("failed to read output: %v", err) + } }) preloadExists := false @@ -101,15 +98,14 @@ func TestDownloadOnly(t *testing.T) { } // Driver does not matter here, since the only exception is none driver, // which cannot occur here. - if download.PreloadExists(v, containerRuntime, "docker", true) { - // Just make sure the tarball path exists - if _, err := os.Stat(download.TarballPath(v, containerRuntime)); err != nil { - t.Errorf("failed to verify preloaded tarball file exists: %v", err) - } - preloadExists = true - } else { + if !download.PreloadExists(v, containerRuntime, "docker", true) { t.Skip("No preload image") } + // Just make sure the tarball path exists + if _, err := os.Stat(download.TarballPath(v, containerRuntime)); err != nil { + t.Errorf("failed to verify preloaded tarball file exists: %v", err) + } + preloadExists = true }) t.Run("cached-images", func(t *testing.T) { diff --git a/test/integration/util_test.go b/test/integration/util_test.go index b65f8b58cf..6b872ef50c 100644 --- a/test/integration/util_test.go +++ b/test/integration/util_test.go @@ -52,5 +52,5 @@ func auditContains(substr string) (bool, error) { return true, nil } } - return false, nil + return false, s.Err() }