From af61bf790c1c3374907fce1ad6d86dd7694ef31a Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Fri, 28 Sep 2018 11:57:48 -0700 Subject: [PATCH 1/3] Remove lint issues from integration tests, mostly by adding error handlers. --- test/integration/addons_test.go | 9 +++++++-- test/integration/cluster_dns_test.go | 14 ++++++++++---- test/integration/cluster_status_test.go | 2 +- test/integration/docker_test.go | 3 ++- test/integration/flags.go | 2 +- test/integration/functional_test.go | 2 +- test/integration/mount_test.go | 13 +++++++++++-- test/integration/pv_test.go | 13 +++++++++---- 8 files changed, 42 insertions(+), 16 deletions(-) diff --git a/test/integration/addons_test.go b/test/integration/addons_test.go index 29f0c608c8..02b4605b3d 100644 --- a/test/integration/addons_test.go +++ b/test/integration/addons_test.go @@ -122,8 +122,13 @@ func testIngressController(t *testing.T) { t.Fatalf(err.Error()) } - defer kubectlRunner.RunCommand([]string{"delete", "-f", podPath}) - defer kubectlRunner.RunCommand([]string{"delete", "-f", ingressPath}) + defer func() { + for _, p := range []string{podPath, ingressPath} { + if out, err := kubectlRunner.RunCommand([]string{"delete", "-f", p}); err != nil { + t.Logf("delete -f %s failed: %v\noutput: %s\n", p, err, out) + } + } + }() minikubeRunner.RunCommand("addons disable ingress", true) } diff --git a/test/integration/cluster_dns_test.go b/test/integration/cluster_dns_test.go index 542018ea72..6a41eaaebd 100644 --- a/test/integration/cluster_dns_test.go +++ b/test/integration/cluster_dns_test.go @@ -49,15 +49,21 @@ func testClusterDNS(t *testing.T) { } listOpts := metav1.ListOptions{LabelSelector: "integration-test=busybox"} pods, err := client.CoreV1().Pods("default").List(listOpts) + if err != nil { + t.Fatalf("Unable to list default pods: %v", err) + } if len(pods.Items) == 0 { t.Fatal("Expected a busybox pod to be running") } - podName := pods.Items[0].Name - defer kubectlRunner.RunCommand([]string{"delete", "po", podName}) + bbox := pods.Items[0].Name + defer func() { + if out, err := kubectlRunner.RunCommand([]string{"delete", "po", bbox}); err != nil { + t.Logf("delete po %s failed: %v\noutput: %s\n", bbox, err, out) + } + }() - dnsByteArr, err := kubectlRunner.RunCommand([]string{"exec", podName, - "nslookup", "kubernetes"}) + dnsByteArr, err := kubectlRunner.RunCommand([]string{"exec", bbox, "nslookup", "kubernetes"}) if err != nil { t.Fatalf("running nslookup in pod:%s", err) } diff --git a/test/integration/cluster_status_test.go b/test/integration/cluster_status_test.go index e16332ed8c..bfefcbab95 100644 --- a/test/integration/cluster_status_test.go +++ b/test/integration/cluster_status_test.go @@ -23,7 +23,7 @@ import ( "testing" "time" - api "k8s.io/api/core/v1" + api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/minikube/test/integration/util" ) diff --git a/test/integration/docker_test.go b/test/integration/docker_test.go index 8ae5cceb6a..dd722ec570 100644 --- a/test/integration/docker_test.go +++ b/test/integration/docker_test.go @@ -32,7 +32,8 @@ func TestDocker(t *testing.T) { } minikubeRunner.RunCommand("delete", false) - startCmd := fmt.Sprintf("start %s %s %s", minikubeRunner.StartArgs, minikubeRunner.Args, "--docker-env=FOO=BAR --docker-env=BAZ=BAT --docker-opt=debug --docker-opt=icc=true") + startCmd := fmt.Sprintf("start %s %s %s", minikubeRunner.StartArgs, minikubeRunner.Args, + "--docker-env=FOO=BAR --docker-env=BAZ=BAT --docker-opt=debug --docker-opt=icc=true") minikubeRunner.RunCommand(startCmd, true) minikubeRunner.EnsureRunning() diff --git a/test/integration/flags.go b/test/integration/flags.go index 89db1fda0b..8d5b2a24a9 100644 --- a/test/integration/flags.go +++ b/test/integration/flags.go @@ -32,7 +32,7 @@ func TestMain(m *testing.M) { var binaryPath = flag.String("binary", "../../out/minikube", "path to minikube binary") var args = flag.String("minikube-args", "", "Arguments to pass to minikube") var startArgs = flag.String("minikube-start-args", "", "Arguments to pass to minikube start") -var testdataDir = flag.String("testdata-dir", "testdata", "the directory relative to test/integration where the testdata lives") +var testdataDir = flag.String("testdata-dir", "testdata", "source of testdata relative to test/integration") func NewMinikubeRunner(t *testing.T) util.MinikubeRunner { return util.MinikubeRunner{ diff --git a/test/integration/functional_test.go b/test/integration/functional_test.go index c70c30683b..0fe7c29fe2 100644 --- a/test/integration/functional_test.go +++ b/test/integration/functional_test.go @@ -41,6 +41,6 @@ func TestFunctional(t *testing.T) { t.Run("EnvVars", testClusterEnv) t.Run("SSH", testClusterSSH) t.Run("IngressController", testIngressController) - // t.Run("Mounting", testMounting) + t.Run("Mounting", testMounting) } } diff --git a/test/integration/mount_test.go b/test/integration/mount_test.go index 476a9e3c94..83e0acf171 100644 --- a/test/integration/mount_test.go +++ b/test/integration/mount_test.go @@ -47,7 +47,12 @@ func testMounting(t *testing.T) { mountCmd := fmt.Sprintf("mount %s:/mount-9p", tempDir) cmd := minikubeRunner.RunDaemon(mountCmd) - defer cmd.Process.Kill() + defer func() { + err := cmd.Process.Kill() + if err != nil { + t.Logf("Failed to kill mount command: %v", err) + } + }() kubectlRunner := util.NewKubectlRunner(t) podName := "busybox-mount" @@ -71,7 +76,11 @@ func testMounting(t *testing.T) { } return nil } - defer kubectlRunner.RunCommand([]string{"delete", "-f", podPath}) + defer func() { + if out, err := kubectlRunner.RunCommand([]string{"delete", "-f", podPath}); err != nil { + t.Logf("delete -f %s failed: %v\noutput: %s\n", podPath, err, out) + } + }() if err := util.Retry(t, setupTest, 5*time.Second, 40); err != nil { t.Fatal("mountTest failed with error:", err) diff --git a/test/integration/pv_test.go b/test/integration/pv_test.go index 9667f3c53d..8f3c870acd 100644 --- a/test/integration/pv_test.go +++ b/test/integration/pv_test.go @@ -43,7 +43,9 @@ func testProvisioning(t *testing.T) { kubectlRunner := util.NewKubectlRunner(t) defer func() { - kubectlRunner.RunCommand([]string{"delete", "pvc", pvcName}) + if out, err := kubectlRunner.RunCommand([]string{"delete", "pvc", pvcName}); err != nil { + t.Logf("delete pvc %s failed: %v\noutput: %s\n", pvcName, err, out) + } }() // We have to make sure the addon-manager has created the StorageClass before creating @@ -51,15 +53,18 @@ func testProvisioning(t *testing.T) { checkStorageClass := func() error { scl := storage.StorageClassList{} - kubectlRunner.RunCommandParseOutput([]string{"get", "storageclass"}, &scl) + if err := kubectlRunner.RunCommandParseOutput([]string{"get", "storageclass"}, &scl); err != nil { + return fmt.Errorf("get storageclass: %v", err) + } + if len(scl.Items) > 0 { return nil } - return fmt.Errorf("No default StorageClass yet.") + return fmt.Errorf("no StorageClass yet") } if err := util.Retry(t, checkStorageClass, 5*time.Second, 20); err != nil { - t.Fatalf("No default storage class: %s", err) + t.Fatalf("no default storage class after retry: %s", err) } // Check that the storage provisioner pod is running From ea62af5a37f6f6c2ac7744f7fdff385f7eeeb45e Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Fri, 28 Sep 2018 16:17:44 -0700 Subject: [PATCH 2/3] Fix incorrect format for an integer by using the unparseable string. --- pkg/minikube/bootstrapper/exec_runner.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/minikube/bootstrapper/exec_runner.go b/pkg/minikube/bootstrapper/exec_runner.go index df83e6f0ea..bdc3959948 100644 --- a/pkg/minikube/bootstrapper/exec_runner.go +++ b/pkg/minikube/bootstrapper/exec_runner.go @@ -89,7 +89,7 @@ func (*ExecRunner) Copy(f assets.CopyableFile) error { } perms, err := strconv.Atoi(f.GetPermissions()) if err != nil { - return errors.Wrapf(err, "error converting permissions %s to integer", perms) + return errors.Wrapf(err, "error converting permissions %s to integer", f.GetPermissions()) } if err := os.Chmod(targetPath, os.FileMode(perms)); err != nil { return errors.Wrapf(err, "error changing file permissions for %s", targetPath) From 8540e3a038519fe2abb7ea163490074f5ce1bd26 Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Fri, 28 Sep 2018 16:18:22 -0700 Subject: [PATCH 3/3] Consistently use %v for formatting error messages. --- test/integration/addons_test.go | 14 +++++++------- test/integration/cluster_dns_test.go | 8 ++++---- test/integration/cluster_env_test.go | 2 +- test/integration/cluster_status_test.go | 4 ++-- test/integration/mount_test.go | 6 +++--- test/integration/persistence_test.go | 8 ++++---- test/integration/pv_test.go | 2 +- test/integration/start_stop_delete_test.go | 2 +- test/integration/util/util.go | 4 ++-- 9 files changed, 25 insertions(+), 25 deletions(-) diff --git a/test/integration/addons_test.go b/test/integration/addons_test.go index 02b4605b3d..6c7ac759e8 100644 --- a/test/integration/addons_test.go +++ b/test/integration/addons_test.go @@ -37,7 +37,7 @@ func testAddons(t *testing.T) { t.Parallel() client, err := pkgutil.GetClient() if err != nil { - t.Fatalf("Could not get kubernetes client: %s", err) + t.Fatalf("Could not get kubernetes client: %v", err) } selector := labels.SelectorFromSet(labels.Set(map[string]string{"component": "kube-addon-manager"})) if err := pkgutil.WaitForPodsWithLabelRunning(client, "kube-system", selector); err != nil { @@ -65,7 +65,7 @@ func testDashboard(t *testing.T) { } if err := util.Retry(t, checkDashboard, 2*time.Second, 60); err != nil { - t.Fatalf("error checking dashboard URL: %s", err) + t.Fatalf("error checking dashboard URL: %v", err) } if u.Scheme != "http" { @@ -87,25 +87,25 @@ func testIngressController(t *testing.T) { minikubeRunner.RunCommand("addons enable ingress", true) if err := util.WaitForIngressControllerRunning(t); err != nil { - t.Fatalf("waiting for ingress-controller to be up: %s", err) + t.Fatalf("waiting for ingress-controller to be up: %v", err) } if err := util.WaitForIngressDefaultBackendRunning(t); err != nil { - t.Fatalf("waiting for default-http-backend to be up: %s", err) + t.Fatalf("waiting for default-http-backend to be up: %v", err) } ingressPath, _ := filepath.Abs("testdata/nginx-ing.yaml") if _, err := kubectlRunner.RunCommand([]string{"create", "-f", ingressPath}); err != nil { - t.Fatalf("creating nginx ingress resource: %s", err) + t.Fatalf("creating nginx ingress resource: %v", err) } podPath, _ := filepath.Abs("testdata/nginx-pod-svc.yaml") if _, err := kubectlRunner.RunCommand([]string{"create", "-f", podPath}); err != nil { - t.Fatalf("creating nginx ingress resource: %s", err) + t.Fatalf("creating nginx ingress resource: %v", err) } if err := util.WaitForNginxRunning(t); err != nil { - t.Fatalf("waiting for nginx to be up: %s", err) + t.Fatalf("waiting for nginx to be up: %v", err) } checkIngress := func() error { diff --git a/test/integration/cluster_dns_test.go b/test/integration/cluster_dns_test.go index 6a41eaaebd..d59766a6a9 100644 --- a/test/integration/cluster_dns_test.go +++ b/test/integration/cluster_dns_test.go @@ -37,15 +37,15 @@ func testClusterDNS(t *testing.T) { client, err := pkgutil.GetClient() if err != nil { - t.Fatalf("Error getting kubernetes client %s", err) + t.Fatalf("Error getting kubernetes client %v", err) } if _, err := kubectlRunner.RunCommand([]string{"create", "-f", podPath}); err != nil { - t.Fatalf("creating busybox pod: %s", err) + t.Fatalf("creating busybox pod: %v", err) } if err := util.WaitForBusyboxRunning(t, "default"); err != nil { - t.Fatalf("Waiting for busybox pod to be up: %s", err) + t.Fatalf("Waiting for busybox pod to be up: %v", err) } listOpts := metav1.ListOptions{LabelSelector: "integration-test=busybox"} pods, err := client.CoreV1().Pods("default").List(listOpts) @@ -65,7 +65,7 @@ func testClusterDNS(t *testing.T) { dnsByteArr, err := kubectlRunner.RunCommand([]string{"exec", bbox, "nslookup", "kubernetes"}) if err != nil { - t.Fatalf("running nslookup in pod:%s", err) + t.Fatalf("running nslookup in pod:%v", err) } dnsOutput := string(dnsByteArr) if !strings.Contains(dnsOutput, "10.96.0.1") || !strings.Contains(dnsOutput, "10.96.0.10") { diff --git a/test/integration/cluster_env_test.go b/test/integration/cluster_env_test.go index 04ba05ce09..137581d634 100644 --- a/test/integration/cluster_env_test.go +++ b/test/integration/cluster_env_test.go @@ -33,7 +33,7 @@ func testClusterEnv(t *testing.T) { dockerEnvVars := minikubeRunner.RunCommand("docker-env", true) if err := minikubeRunner.SetEnvFromEnvCmdOutput(dockerEnvVars); err != nil { - t.Fatalf("Error parsing output: %s", err) + t.Fatalf("Error parsing output: %v", err) } path, err := exec.LookPath("docker") diff --git a/test/integration/cluster_status_test.go b/test/integration/cluster_status_test.go index bfefcbab95..b5cef118d5 100644 --- a/test/integration/cluster_status_test.go +++ b/test/integration/cluster_status_test.go @@ -47,7 +47,7 @@ func testClusterStatus(t *testing.T) { } if status != api.ConditionTrue { err := fmt.Errorf("Component %s is not Healthy! Status: %s", i.GetName(), status) - t.Logf("Retrying, %s", err) + t.Logf("Retrying, %v", err) return err } } @@ -55,6 +55,6 @@ func testClusterStatus(t *testing.T) { } if err := util.Retry(t, healthy, 1*time.Second, 5); err != nil { - t.Fatalf("Cluster is not healthy: %s", err) + t.Fatalf("Cluster is not healthy: %v", err) } } diff --git a/test/integration/mount_test.go b/test/integration/mount_test.go index 83e0acf171..9975d205ae 100644 --- a/test/integration/mount_test.go +++ b/test/integration/mount_test.go @@ -41,7 +41,7 @@ func testMounting(t *testing.T) { tempDir, err := ioutil.TempDir("", "mounttest") if err != nil { - t.Fatalf("Unexpected error while creating tempDir: %s", err) + t.Fatalf("Unexpected error while creating tempDir: %v", err) } defer os.RemoveAll(tempDir) @@ -88,11 +88,11 @@ func testMounting(t *testing.T) { client, err := pkgutil.GetClient() if err != nil { - t.Fatalf("getting kubernetes client: %s", err) + t.Fatalf("getting kubernetes client: %v", err) } selector := labels.SelectorFromSet(labels.Set(map[string]string{"integration-test": "busybox-mount"})) if err := pkgutil.WaitForPodsWithLabelRunning(client, "default", selector); err != nil { - t.Fatalf("Error waiting for busybox mount pod to be up: %s", err) + t.Fatalf("Error waiting for busybox mount pod to be up: %v", err) } mountTest := func() error { diff --git a/test/integration/persistence_test.go b/test/integration/persistence_test.go index b63538f7ca..5f048c8b2f 100644 --- a/test/integration/persistence_test.go +++ b/test/integration/persistence_test.go @@ -40,16 +40,16 @@ func TestPersistence(t *testing.T) { // Create a pod and wait for it to be running. if _, err := kubectlRunner.RunCommand([]string{"create", "-f", podPath}); err != nil { - t.Fatalf("Error creating test pod: %s", err) + t.Fatalf("Error creating test pod: %v", err) } verify := func(t *testing.T) { if err := util.WaitForDashboardRunning(t); err != nil { - t.Fatalf("waiting for dashboard to be up: %s", err) + t.Fatalf("waiting for dashboard to be up: %v", err) } if err := util.WaitForBusyboxRunning(t, "default"); err != nil { - t.Fatalf("waiting for busybox to be up: %s", err) + t.Fatalf("waiting for busybox to be up: %v", err) } } @@ -66,7 +66,7 @@ func TestPersistence(t *testing.T) { } if err := util.Retry(t, checkStop, 5*time.Second, 6); err != nil { - t.Fatalf("timed out while checking stopped status: %s", err) + t.Fatalf("timed out while checking stopped status: %v", err) } minikubeRunner.Start() diff --git a/test/integration/pv_test.go b/test/integration/pv_test.go index 8f3c870acd..3d9bfa6f62 100644 --- a/test/integration/pv_test.go +++ b/test/integration/pv_test.go @@ -64,7 +64,7 @@ func testProvisioning(t *testing.T) { } if err := util.Retry(t, checkStorageClass, 5*time.Second, 20); err != nil { - t.Fatalf("no default storage class after retry: %s", err) + t.Fatalf("no default storage class after retry: %v", err) } // Check that the storage provisioner pod is running diff --git a/test/integration/start_stop_delete_test.go b/test/integration/start_stop_delete_test.go index f94e72d184..9efd5cf35c 100644 --- a/test/integration/start_stop_delete_test.go +++ b/test/integration/start_stop_delete_test.go @@ -50,7 +50,7 @@ func TestStartStop(t *testing.T) { } if err := util.Retry(t, checkStop, 5*time.Second, 6); err != nil { - t.Fatalf("timed out while checking stopped status: %s", err) + t.Fatalf("timed out while checking stopped status: %v", err) } runner.Start() diff --git a/test/integration/util/util.go b/test/integration/util/util.go index 96bdad2928..7945ac4e18 100644 --- a/test/integration/util/util.go +++ b/test/integration/util/util.go @@ -183,7 +183,7 @@ func (k *KubectlRunner) RunCommand(args []string) (stdout []byte, err error) { stdout, err = cmd.CombinedOutput() if err != nil { k.T.Logf("Error %s running command %s. Return code: %s", stdout, args, err) - return &commonutil.RetriableError{Err: fmt.Errorf("Error running command. Error %s. Output: %s", err, stdout)} + return &commonutil.RetriableError{Err: fmt.Errorf("Error running command: %v. Output: %s", err, stdout)} } return nil } @@ -196,7 +196,7 @@ func (k *KubectlRunner) CreateRandomNamespace() string { const strLen = 20 name := genRandString(strLen) if _, err := k.RunCommand([]string{"create", "namespace", name}); err != nil { - k.T.Fatalf("Error creating namespace: %s", err) + k.T.Fatalf("Error creating namespace: %v", err) } return name }