From 4cfca59c5c6241b3f994a59fe4a345887535a5f8 Mon Sep 17 00:00:00 2001 From: Matt Rickard Date: Thu, 13 Oct 2016 13:53:38 -0700 Subject: [PATCH] Add RetryableError for the Retry util This way we can selectively retry the errors that are caused by some temporary or ephemeral condition such as the pods not being up yet. --- cmd/minikube/cmd/service.go | 6 ++-- pkg/minikube/cluster/cluster.go | 2 +- pkg/minikube/cluster/localkube_caching.go | 2 +- pkg/util/utils.go | 9 ++++++ pkg/util/utils_test.go | 34 +++++++++++++++++++---- test/integration/addons_test.go | 6 ++-- test/integration/cluster_dns_test.go | 2 +- test/integration/cluster_env_test.go | 5 +++- test/integration/cluster_status_test.go | 2 +- test/integration/persistence_test.go | 6 ++-- test/integration/util/util.go | 2 +- 11 files changed, 55 insertions(+), 21 deletions(-) diff --git a/cmd/minikube/cmd/service.go b/cmd/minikube/cmd/service.go index 929a84ac6..1a6cd3138 100644 --- a/cmd/minikube/cmd/service.go +++ b/cmd/minikube/cmd/service.go @@ -95,7 +95,7 @@ func CheckService(namespace string, service string) error { } endpoint, err := endpoints.Get(service) if err != nil { - return err + return &util.RetriableError{Err: err} } return CheckEndpointReady(endpoint) } @@ -105,12 +105,12 @@ const notReadyMsg = "Waiting, endpoint for service is not ready yet...\n" func CheckEndpointReady(endpoint *kubeApi.Endpoints) error { if len(endpoint.Subsets) == 0 { fmt.Fprintf(os.Stderr, notReadyMsg) - return errors.New("Endpoint for service is not ready yet") + return &util.RetriableError{Err: errors.New("Endpoint for service is not ready yet")} } for _, subset := range endpoint.Subsets { if len(subset.Addresses) == 0 { fmt.Fprintf(os.Stderr, notReadyMsg) - return errors.New("No endpoints for service are ready yet") + return &util.RetriableError{Err: errors.New("No endpoints for service are ready yet")} } } return nil diff --git a/pkg/minikube/cluster/cluster.go b/pkg/minikube/cluster/cluster.go index 5e3212d73..325cdc397 100644 --- a/pkg/minikube/cluster/cluster.go +++ b/pkg/minikube/cluster/cluster.go @@ -94,7 +94,7 @@ func StartHost(api libmachine.API, config MachineConfig) (*host.Host, error) { } if err := h.ConfigureAuth(); err != nil { - return nil, errors.Wrap(err, "Error configuring auth on host: %s") + return nil, errors.Wrap(&util.RetriableError{Err: err}, "Error configuring auth on host: %s") } return h, nil } diff --git a/pkg/minikube/cluster/localkube_caching.go b/pkg/minikube/cluster/localkube_caching.go index 6f3e5b096..e072d1c47 100644 --- a/pkg/minikube/cluster/localkube_caching.go +++ b/pkg/minikube/cluster/localkube_caching.go @@ -88,7 +88,7 @@ func (l *localkubeCacher) downloadAndCacheLocalkube() error { downloader := func() (err error) { resp, err = http.Get(url) if err != nil { - return errors.Wrap(err, "Error downloading localkube via http") + return &util.RetriableError{Err: errors.Wrap(err, "Error downloading localkube via http")} } if resp.StatusCode != http.StatusOK { return errors.New("Remote server error in downloading localkube via http") diff --git a/pkg/util/utils.go b/pkg/util/utils.go index c4a4817ac..5a7fa87e6 100644 --- a/pkg/util/utils.go +++ b/pkg/util/utils.go @@ -33,6 +33,12 @@ import ( "k8s.io/minikube/pkg/version" ) +type RetriableError struct { + Err error +} + +func (r RetriableError) Error() string { return "Temporary Error: " + r.Err.Error() } + // Until endlessly loops the provided function until a message is received on the done channel. // The function will wait the duration provided in sleep between function calls. Errors will be sent on provider Writer. func Until(fn func() error, w io.Writer, name string, sleep time.Duration, done <-chan struct{}) { @@ -84,6 +90,9 @@ func RetryAfter(attempts int, callback func() error, d time.Duration) (err error return nil } m.Collect(err) + if _, ok := err.(*RetriableError); !ok { + return m.ToError() + } time.Sleep(d) } return m.ToError() diff --git a/pkg/util/utils_test.go b/pkg/util/utils_test.go index b4f50f0ec..64b821211 100644 --- a/pkg/util/utils_test.go +++ b/pkg/util/utils_test.go @@ -24,20 +24,27 @@ import ( ) // Returns a function that will return n errors, then return successfully forever. -func errorGenerator(n int) func() error { +func errorGenerator(n int, retryable bool) func() error { errorCount := 0 return func() (err error) { if errorCount < n { errorCount += 1 - return errors.New("Error!") + e := errors.New("Error!") + if retryable { + return &RetriableError{Err: e} + } else { + return e + } + } + return nil } } func TestErrorGenerator(t *testing.T) { errors := 3 - f := errorGenerator(errors) + f := errorGenerator(errors, false) for i := 0; i < errors-1; i++ { if err := f(); err == nil { t.Fatalf("Error should have been thrown at iteration %v", i) @@ -49,16 +56,31 @@ func TestErrorGenerator(t *testing.T) { } func TestRetry(t *testing.T) { - - f := errorGenerator(4) + f := errorGenerator(4, true) if err := Retry(5, f); err != nil { t.Fatalf("Error should not have been raised by retry.") } - f = errorGenerator(5) + f = errorGenerator(5, true) + if err := Retry(4, f); err == nil { + t.Fatalf("Error should have been raised by retry.") + } +} + +func TestRetryNotRetriableError(t *testing.T) { + f := errorGenerator(4, false) + if err := Retry(5, f); err == nil { + t.Fatalf("Error should have been raised by retry.") + } + + f = errorGenerator(5, false) if err := Retry(4, f); err == nil { t.Fatalf("Error should have been raised by retry.") } + f = errorGenerator(0, false) + if err := Retry(5, f); err != nil { + t.Fatalf("Error should not have been raised by retry.") + } } type getLocalkubeArgs struct { diff --git a/test/integration/addons_test.go b/test/integration/addons_test.go index 4efc63875..66377594f 100644 --- a/test/integration/addons_test.go +++ b/test/integration/addons_test.go @@ -56,11 +56,11 @@ func TestAddons(t *testing.T) { if p.Status.Phase == "Running" { return nil } - return fmt.Errorf("Pod is not Running. Status: %s", p.Status.Phase) + return &commonutil.RetriableError{Err: fmt.Errorf("Pod is not Running. Status: %s", p.Status.Phase)} } } - return fmt.Errorf("Addon manager not found. Found pods: %s", pods) + return &commonutil.RetriableError{Err: fmt.Errorf("Addon manager not found. Found pods: %s", pods)} } if err := commonutil.RetryAfter(20, checkAddon, 5*time.Second); err != nil { @@ -89,7 +89,7 @@ func TestDashboard(t *testing.T) { } if rc.Status.Replicas != rc.Status.FullyLabeledReplicas { - return fmt.Errorf("Not enough pods running. Expected %s, got %s.", rc.Status.Replicas, rc.Status.FullyLabeledReplicas) + return &commonutil.RetriableError{Err: fmt.Errorf("Not enough pods running. Expected %s, got %s.", rc.Status.Replicas, rc.Status.FullyLabeledReplicas)} } if svc.Spec.Ports[0].NodePort != 30000 { diff --git a/test/integration/cluster_dns_test.go b/test/integration/cluster_dns_test.go index ffc55e1ca..afd821c94 100644 --- a/test/integration/cluster_dns_test.go +++ b/test/integration/cluster_dns_test.go @@ -59,7 +59,7 @@ func TestClusterDNS(t *testing.T) { "nslookup", "kubernetes.default"}) dnsOutput := string(dnsByteArr) if err != nil { - return err + return &commonutil.RetriableError{Err: err} } if !strings.Contains(dnsOutput, "10.0.0.1") || !strings.Contains(dnsOutput, "10.0.0.10") { diff --git a/test/integration/cluster_env_test.go b/test/integration/cluster_env_test.go index 64d5a6919..92cf1293f 100644 --- a/test/integration/cluster_env_test.go +++ b/test/integration/cluster_env_test.go @@ -44,7 +44,10 @@ func TestClusterEnv(t *testing.T) { dockerPs := func() error { cmd := exec.Command(path, "ps") output, err = cmd.CombinedOutput() - return err + if err != nil { + return &commonutil.RetriableError{Err: err} + } + return nil } if err := commonutil.RetryAfter(5, dockerPs, 3*time.Second); err != nil { t.Fatalf("Error running command: %s. Error: %s Output: %s", "docker ps", err, output) diff --git a/test/integration/cluster_status_test.go b/test/integration/cluster_status_test.go index 4fa3b50a2..b314ec445 100644 --- a/test/integration/cluster_status_test.go +++ b/test/integration/cluster_status_test.go @@ -52,7 +52,7 @@ func TestClusterStatus(t *testing.T) { status = c.Status } if status != api.ConditionTrue { - return fmt.Errorf("Component %s is not Healthy! Status: %s", i.GetName(), status) + return &commonutil.RetriableError{Err: fmt.Errorf("Component %s is not Healthy! Status: %s", i.GetName(), status)} } } return nil diff --git a/test/integration/persistence_test.go b/test/integration/persistence_test.go index ca63736cf..df3b50ab8 100644 --- a/test/integration/persistence_test.go +++ b/test/integration/persistence_test.go @@ -51,7 +51,7 @@ func TestPersistence(t *testing.T) { if util.IsPodReady(p) { return nil } - return fmt.Errorf("Pod %s is not ready yet.", podName) + return &commonutil.RetriableError{Err: fmt.Errorf("Pod %s is not ready yet.", podName)} } if err := commonutil.RetryAfter(20, checkPod, 6*time.Second); err != nil { @@ -65,13 +65,13 @@ func TestPersistence(t *testing.T) { return err } if len(pods.Items) < 1 { - return fmt.Errorf("No pods found matching query: %v", cmd) + return &commonutil.RetriableError{Err: fmt.Errorf("No pods found matching query: %v", cmd)} } db := pods.Items[0] if util.IsPodReady(&db) { return nil } - return fmt.Errorf("Dashboard pod is not ready yet.") + return &commonutil.RetriableError{Err: fmt.Errorf("Dashboard pod is not ready yet.")} } // Make sure the dashboard is running before we stop the VM. diff --git a/test/integration/util/util.go b/test/integration/util/util.go index 7a3c045d0..4242a2527 100644 --- a/test/integration/util/util.go +++ b/test/integration/util/util.go @@ -142,7 +142,7 @@ func (k *KubectlRunner) RunCommand(args []string) (stdout []byte, err error) { stdout, err = cmd.CombinedOutput() if err != nil { log.Errorf("Error %s running command %s. Return code: %s", stdout, args, err) - return fmt.Errorf("Error running command. Error %s. Output: %s", err, stdout) + return &commonutil.RetriableError{Err: fmt.Errorf("Error running command. Error %s. Output: %s", err, stdout)} } return nil } -- GitLab