diff --git a/internal/controllers/core/cluster/monitor.go b/internal/controllers/core/cluster/monitor.go index eb5f684dc0..056a71575f 100644 --- a/internal/controllers/core/cluster/monitor.go +++ b/internal/controllers/core/cluster/monitor.go @@ -2,7 +2,8 @@ package cluster import ( "context" - "errors" + "fmt" + "strings" "sync" "github.com/jonboulle/clockwork" @@ -113,20 +114,45 @@ func (c *clusterHealthMonitor) run(ctx context.Context, clusterNN types.Namespac } func doKubernetesHealthCheck(ctx context.Context, client k8s.Client) error { - // TODO(milas): use verbose=true and propagate the info to the Tilt API - // cluster obj to show in the web UI - health, err := client.ClusterHealth(ctx, false) + health, err := client.ClusterHealth(ctx, true) if err != nil { return err } if !health.Live { - return errors.New("cluster did not pass liveness check") + return healthCheckError("cluster did not pass liveness check", health.LiveOutput) } if !health.Ready { - return errors.New("cluster not ready") + return healthCheckError("cluster not ready", health.ReadyOutput) } return nil } + +func healthCheckError(msg, output string) error { + output = summarizeHealthCheckOutput(output) + if output != "" { + return fmt.Errorf("%s: %s", msg, output) + } + return fmt.Errorf("%s", msg) +} + +func summarizeHealthCheckOutput(output string) string { + output = strings.TrimSpace(output) + if output == "" { + return "" + } + + var failures []string + for _, line := range strings.Split(output, "\n") { + line = strings.TrimSpace(line) + if strings.HasPrefix(line, "[-]") || strings.Contains(line, " check failed") { + failures = append(failures, line) + } + } + if len(failures) == 0 { + return output + } + return strings.Join(failures, "; ") +} diff --git a/internal/controllers/core/cluster/reconciler_test.go b/internal/controllers/core/cluster/reconciler_test.go index 6823839c20..6ab63e687b 100644 --- a/internal/controllers/core/cluster/reconciler_test.go +++ b/internal/controllers/core/cluster/reconciler_test.go @@ -306,6 +306,37 @@ func TestKubernetesMonitor(t *testing.T) { f.MustGet(nn, cluster) assert.Equal(t, "fake cluster health error", cluster.Status.Error) timecmp.RequireTimeEqual(t, connectedAt, cluster.Status.ConnectedAt) + assert.True(t, f.k8sClient.ClusterHealthVerbose) +} + +func TestKubernetesMonitorIncludesHealthCheckOutput(t *testing.T) { + f := newFixture(t) + cluster := &v1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{Name: "default"}, + Spec: v1alpha1.ClusterSpec{ + Connection: &v1alpha1.ClusterConnection{ + Kubernetes: &v1alpha1.KubernetesClusterConnection{}, + }, + }, + } + nn := apis.Key(cluster) + + f.Create(cluster) + f.MustGet(nn, cluster) + connectedAt := *cluster.Status.ConnectedAt + f.assertSteadyState(cluster) + + f.k8sClient.ClusterHealthStatus = &k8s.ClusterHealth{ + Live: false, + LiveOutput: "[+]ping ok\n[-]etcd failed: reason withheld\nlivez check failed", + Ready: true, + } + f.clock.Advance(time.Minute) + <-f.requeues + + f.MustGet(nn, cluster) + assert.Equal(t, "cluster did not pass liveness check: [-]etcd failed: reason withheld; livez check failed", cluster.Status.Error) + timecmp.RequireTimeEqual(t, connectedAt, cluster.Status.ConnectedAt) } func TestDockerError(t *testing.T) { diff --git a/internal/k8s/client.go b/internal/k8s/client.go index a00138206a..be02425b78 100644 --- a/internal/k8s/client.go +++ b/internal/k8s/client.go @@ -8,6 +8,7 @@ import ( "net/http" "net/url" "regexp" + "strconv" "strings" "sync" "time" @@ -891,13 +892,26 @@ func (k *K8sClient) apiServerHealthCheck(ctx context.Context, route string, verb if err != nil { var statusErr *apierrors.StatusError if errors.As(err, &statusErr) { - return false, statusErr.ErrStatus.Message, nil + return false, cleanHealthCheckOutput(statusErr.ErrStatus.Message), nil } return false, "", err } return true, string(body), nil } +func cleanHealthCheckOutput(output string) string { + output = strings.TrimSpace(output) + const prefix = "an error on the server (" + const suffix = ") has prevented the request from succeeding" + if strings.HasPrefix(output, prefix) && strings.HasSuffix(output, suffix) { + quoted := strings.TrimSuffix(strings.TrimPrefix(output, prefix), suffix) + if unquoted, err := strconv.Unquote(quoted); err == nil { + return strings.TrimSpace(unquoted) + } + } + return output +} + // Tests whether a string is a valid version for a k8s resource type. // from https://kubernetes.io/docs/tasks/access-kubernetes-api/custom-resources/custom-resource-definition-versioning/#version-priority // Versions start with a v followed by a number, an optional beta or alpha designation, and optional additional numeric diff --git a/internal/k8s/client_test.go b/internal/k8s/client_test.go index b36206f9df..0a3a20a7b7 100644 --- a/internal/k8s/client_test.go +++ b/internal/k8s/client_test.go @@ -423,9 +423,6 @@ func TestServerHealth(t *testing.T) { require.NoError(t, err) } - // verbose output is only checked on success - some of the standard - // error handling in the K8s helpers massages non-200 requests, so - // it's too brittle to check against isLive := tc.liveStatusCode == http.StatusOK if assert.Equal(t, isLive, health.Live, "livez") && isLive { assert.Equal(t, "fake livez response", health.LiveOutput) @@ -439,6 +436,12 @@ func TestServerHealth(t *testing.T) { } +func TestCleanHealthCheckOutput(t *testing.T) { + assert.Equal(t, + "[-]etcd failed: reason withheld\nlivez check failed", + cleanHealthCheckOutput(`an error on the server ("[-]etcd failed: reason withheld\nlivez check failed") has prevented the request from succeeding`)) +} + type fakeResourceClient struct { updates kube.ResourceList creates kube.ResourceList diff --git a/internal/k8s/fake_client.go b/internal/k8s/fake_client.go index a6979efd90..d472f37bd2 100644 --- a/internal/k8s/fake_client.go +++ b/internal/k8s/fake_client.go @@ -95,12 +95,13 @@ type FakeK8sClient struct { listCallCount int listReturnsEmpty bool - ExecCalls []ExecCall - ExecOutputs []io.Reader - ExecErrors []error - ClusterHealthStatus *ClusterHealth - ClusterHealthError error - FakeAPIConfig *api.Config + ExecCalls []ExecCall + ExecOutputs []io.Reader + ExecErrors []error + ClusterHealthStatus *ClusterHealth + ClusterHealthError error + ClusterHealthVerbose bool + FakeAPIConfig *api.Config } var _ Client = &FakeK8sClient{} @@ -538,10 +539,12 @@ func (c *FakeK8sClient) APIConfig() *api.Config { return c.FakeAPIConfig } -func (c *FakeK8sClient) ClusterHealth(_ context.Context, _ bool) (ClusterHealth, error) { +func (c *FakeK8sClient) ClusterHealth(_ context.Context, verbose bool) (ClusterHealth, error) { c.mu.Lock() defer c.mu.Unlock() + c.ClusterHealthVerbose = verbose + if c.ClusterHealthStatus != nil { return *c.ClusterHealthStatus, nil }