Enable verbose Kubernetes health checks

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, "; ")
+}

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) {

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

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

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
 	}