Fix hint anchors for core health checks (#6023)

Failing core checks weren't appropriately showing the hint anchor: ```console control-plane-version --------------------- ‼ control plane is up-to-date is running version 21.3.4 but the latest edge version is 21.4.3 see l5d-version-control for hints ``` This change replaces all the manually-created healthcheck categories and instead uses the `NewCategory()` constructor that correctly sets the hint anchor default. This constructor returns a reference instead of a value, so other places had to be updated accordingly.
2021-04-13 09:56:23 -05:00 · 2021-04-13 09:56:23 -05:00 · 02bac26019
parent bf2d7d0140
commit 02bac26019
7 changed files with 140 additions and 123 deletions
--- a/cli/cmd/check_test.go
+++ b/cli/cmd/check_test.go
@ -16,7 +16,7 @@ func TestCheckStatus(t *testing.T) {
 			[]healthcheck.CategoryID{},
 			&healthcheck.Options{},
 		)
-		hc.AppendCategories(*healthcheck.NewCategory("category", []healthcheck.Checker{
+		hc.AppendCategories(healthcheck.NewCategory("category", []healthcheck.Checker{
 			*healthcheck.NewChecker("check1").
 				WithCheck(func(context.Context) error {
 					return nil
@ -50,7 +50,7 @@ func TestCheckStatus(t *testing.T) {
 			[]healthcheck.CategoryID{},
 			&healthcheck.Options{},
 		)
-		hc.AppendCategories(*healthcheck.NewCategory("category", []healthcheck.Checker{
+		hc.AppendCategories(healthcheck.NewCategory("category", []healthcheck.Checker{
 			*healthcheck.NewChecker("check1").
 				WithCheck(func(context.Context) error {
 					return nil
--- a/jaeger/cmd/check.go
+++ b/jaeger/cmd/check.go
@ -175,7 +175,7 @@ func configureAndRunChecks(wout io.Writer, werr io.Writer, options *checkOptions
 		os.Exit(1)
 	}

-	hc.AppendCategories(*jaegerCategory(hc))
+	hc.AppendCategories(jaegerCategory(hc))

 	success := healthcheck.RunChecks(wout, werr, hc, options.output)

--- a/jaeger/cmd/root.go
+++ b/jaeger/cmd/root.go
@ -83,7 +83,7 @@ func checkForJaeger(hcOptions healthcheck.Options) {
 	}

 	hc := healthcheck.NewHealthChecker(checks, &hcOptions)
-	hc.AppendCategories(*jaegerCategory(hc))
+	hc.AppendCategories(jaegerCategory(hc))

 	hc.RunChecks(exitOnError)
 }
--- a/multicluster/cmd/check.go
+++ b/multicluster/cmd/check.go
@ -137,7 +137,7 @@ func configureAndRunChecks(wout io.Writer, werr io.Writer, options *checkOptions

 	hc := newHealthChecker(linkerdHC)
 	category := multiclusterCategory(hc)
-	hc.linkerdHC.AppendCategories(*category)
+	hc.linkerdHC.AppendCategories(category)
 	success := healthcheck.RunChecks(wout, werr, hc.linkerdHC, options.output)
 	if !success {
 		os.Exit(1)
--- a/pkg/healthcheck/healthcheck.go
+++ b/pkg/healthcheck/healthcheck.go
@ -410,7 +410,7 @@ type Options struct {
 // HealthChecker encapsulates all health check checkers, and clients required to
 // perform those checks.
 type HealthChecker struct {
-	categories []Category
+	categories []*Category
 	*Options

 	// these fields are set in the process of running checks
@ -485,13 +485,13 @@ func (hc *HealthChecker) InitializeLinkerdGlobalConfig(ctx context.Context) erro
 }

 // AppendCategories returns a HealthChecker instance appending the provided Categories
-func (hc *HealthChecker) AppendCategories(categories ...Category) *HealthChecker {
+func (hc *HealthChecker) AppendCategories(categories ...*Category) *HealthChecker {
 	hc.categories = append(hc.categories, categories...)
 	return hc
 }

 // GetCategories returns all the categories
-func (hc *HealthChecker) GetCategories() []Category {
+func (hc *HealthChecker) GetCategories() []*Category {
 	return hc.categories
 }

@ -506,11 +506,11 @@ func (hc *HealthChecker) GetCategories() []Category {
 // Note that all checks should include a `hintAnchor` with a corresponding section
 // in the linkerd check faq:
 // https://linkerd.io/checks/#
-func (hc *HealthChecker) allCategories() []Category {
-	return []Category{
-		{
-			ID: KubernetesAPIChecks,
-			checkers: []Checker{
+func (hc *HealthChecker) allCategories() []*Category {
+	return []*Category{
+		NewCategory(
+			KubernetesAPIChecks,
+			[]Checker{
 				{
 					description: "can initialize the client",
 					hintAnchor:  "k8s-api",
@ -530,10 +530,11 @@ func (hc *HealthChecker) allCategories() []Category {
 					},
 				},
 			},
-		},
-		{
-			ID: KubernetesVersionChecks,
-			checkers: []Checker{
+			false,
+		),
+		NewCategory(
+			KubernetesVersionChecks,
+			[]Checker{
 				{
 					description: "is running the minimum Kubernetes API version",
 					hintAnchor:  "k8s-version",
@ -549,10 +550,11 @@ func (hc *HealthChecker) allCategories() []Category {
 					},
 				},
 			},
-		},
-		{
-			ID: LinkerdPreInstallChecks,
-			checkers: []Checker{
+			false,
+		),
+		NewCategory(
+			LinkerdPreInstallChecks,
+			[]Checker{
 				{
 					description: "control plane namespace does not already exist",
 					hintAnchor:  "pre-ns",
@ -632,10 +634,11 @@ func (hc *HealthChecker) allCategories() []Category {
 					},
 				},
 			},
-		},
-		{
-			ID: LinkerdPreInstallCapabilityChecks,
-			checkers: []Checker{
+			false,
+		),
+		NewCategory(
+			LinkerdPreInstallCapabilityChecks,
+			[]Checker{
 				{
 					description: "has NET_ADMIN capability",
 					hintAnchor:  "pre-k8s-cluster-net-admin",
@ -653,10 +656,11 @@ func (hc *HealthChecker) allCategories() []Category {
 					},
 				},
 			},
-		},
-		{
-			ID: LinkerdPreInstallGlobalResourcesChecks,
-			checkers: []Checker{
+			false,
+		),
+		NewCategory(
+			LinkerdPreInstallGlobalResourcesChecks,
+			[]Checker{
 				{
 					description: "no ClusterRoles exist",
 					hintAnchor:  "pre-l5d-existence",
@ -700,10 +704,11 @@ func (hc *HealthChecker) allCategories() []Category {
 					},
 				},
 			},
-		},
-		{
-			ID: LinkerdControlPlaneExistenceChecks,
-			checkers: []Checker{
+			false,
+		),
+		NewCategory(
+			LinkerdControlPlaneExistenceChecks,
+			[]Checker{
 				{
 					description: "'linkerd-config' config map exists",
 					hintAnchor:  "l5d-existence-linkerd-config",
@ -772,10 +777,11 @@ func (hc *HealthChecker) allCategories() []Category {
 					},
 				},
 			},
-		},
-		{
-			ID: LinkerdConfigChecks,
-			checkers: []Checker{
+			false,
+		),
+		NewCategory(
+			LinkerdConfigChecks,
+			[]Checker{
 				{
 					description: "control plane Namespace exists",
 					hintAnchor:  "l5d-existence-ns",
@ -841,10 +847,11 @@ func (hc *HealthChecker) allCategories() []Category {
 					},
 				},
 			},
-		},
-		{
-			ID: LinkerdCNIPluginChecks,
-			checkers: []Checker{
+			false,
+		),
+		NewCategory(
+			LinkerdCNIPluginChecks,
+			[]Checker{
 				{
 					description: "cni plugin ConfigMap exists",
 					hintAnchor:  "cni-plugin-cm-exists",
@ -986,10 +993,11 @@ func (hc *HealthChecker) allCategories() []Category {
 					},
 				},
 			},
-		},
-		{
-			ID: LinkerdIdentity,
-			checkers: []Checker{
+			false,
+		),
+		NewCategory(
+			LinkerdIdentity,
+			[]Checker{
 				{
 					description: "certificate config is valid",
 					hintAnchor:  "l5d-identity-cert-config-valid",
@ -1092,10 +1100,11 @@ func (hc *HealthChecker) allCategories() []Category {
 					},
 				},
 			},
-		},
-		{
-			ID: LinkerdWebhooksAndAPISvcTLS,
-			checkers: []Checker{
+			false,
+		),
+		NewCategory(
+			LinkerdWebhooksAndAPISvcTLS,
+			[]Checker{
 				{
 					description: "proxy-injector webhook has valid cert",
 					hintAnchor:  "l5d-proxy-injector-webhook-cert-valid",
@ -1170,10 +1179,11 @@ func (hc *HealthChecker) allCategories() []Category {
 					},
 				},
 			},
-		},
-		{
-			ID: LinkerdIdentityDataPlane,
-			checkers: []Checker{
+			false,
+		),
+		NewCategory(
+			LinkerdIdentityDataPlane,
+			[]Checker{
 				{
 					description: "data plane proxies certificate match CA",
 					hintAnchor:  "l5d-identity-data-plane-proxies-certs-match-ca",
@ -1183,10 +1193,11 @@ func (hc *HealthChecker) allCategories() []Category {
 					},
 				},
 			},
-		},
-		{
-			ID: LinkerdAPIChecks,
-			checkers: []Checker{
+			false,
+		),
+		NewCategory(
+			LinkerdAPIChecks,
+			[]Checker{
 				{
 					description:         "control plane pods are ready",
 					hintAnchor:          "l5d-api-control-ready",
@ -1226,10 +1237,11 @@ func (hc *HealthChecker) allCategories() []Category {
 					},
 				},
 			},
-		},
-		{
-			ID: LinkerdVersionChecks,
-			checkers: []Checker{
+			false,
+		),
+		NewCategory(
+			LinkerdVersionChecks,
+			[]Checker{
 				{
 					description: "can determine the latest version",
 					hintAnchor:  "l5d-version-latest",
@ -1256,10 +1268,11 @@ func (hc *HealthChecker) allCategories() []Category {
 					},
 				},
 			},
-		},
-		{
-			ID: LinkerdControlPlaneVersionChecks,
-			checkers: []Checker{
+			false,
+		),
+		NewCategory(
+			LinkerdControlPlaneVersionChecks,
+			[]Checker{
 				{
 					description: "control plane is up-to-date",
 					hintAnchor:  "l5d-version-control",
@ -1280,10 +1293,11 @@ func (hc *HealthChecker) allCategories() []Category {
 					},
 				},
 			},
-		},
-		{
-			ID: LinkerdDataPlaneChecks,
-			checkers: []Checker{
+			false,
+		),
+		NewCategory(
+			LinkerdDataPlaneChecks,
+			[]Checker{
 				{
 					description: "data plane namespace exists",
 					hintAnchor:  "l5d-data-plane-exists",
@ -1354,10 +1368,11 @@ func (hc *HealthChecker) allCategories() []Category {
 					},
 				},
 			},
-		},
-		{
-			ID: LinkerdHAChecks,
-			checkers: []Checker{
+			false,
+		),
+		NewCategory(
+			LinkerdHAChecks,
+			[]Checker{
 				{
 					description: "pod injection disabled on kube-system",
 					hintAnchor:  "l5d-injection-disabled",
@ -1386,7 +1401,8 @@ func (hc *HealthChecker) allCategories() []Category {
 					},
 				},
 			},
-		},
+			false,
+		),
 	}
 }

@ -1515,7 +1531,7 @@ func (hc *HealthChecker) LinkerdConfig() *l5dcharts.Values {
 	return hc.linkerdConfig
 }

-func (hc *HealthChecker) runCheck(category Category, c *Checker, observer CheckObserver) bool {
+func (hc *HealthChecker) runCheck(category *Category, c *Checker, observer CheckObserver) bool {
 	for {
 		ctx, cancel := context.WithTimeout(context.Background(), RequestTimeout)
 		defer cancel()
--- a/pkg/healthcheck/healthcheck_test.go
+++ b/pkg/healthcheck/healthcheck_test.go
@ -60,10 +60,11 @@ func (hc *HealthChecker) addCheckAsCategory(
 	categoryID CategoryID,
 	desc string,
 ) {
-	testCategory := Category{
-		ID:       testCategoryID,
-		checkers: []Checker{},
-	}
+	testCategory := NewCategory(
+		testCategoryID,
+		[]Checker{},
+		false,
+	)

 	for _, cat := range hc.categories {
 		if cat.ID == categoryID {
@ -83,9 +84,9 @@ func (hc *HealthChecker) addCheckAsCategory(
 func TestHealthChecker(t *testing.T) {
 	nullObserver := func(*CheckResult) {}

-	passingCheck1 := Category{
-		ID: "cat1",
-		checkers: []Checker{
+	passingCheck1 := NewCategory(
+		"cat1",
+		[]Checker{
 			{
 				description: "desc1",
 				check: func(context.Context) error {
@ -94,12 +95,12 @@ func TestHealthChecker(t *testing.T) {
 				retryDeadline: time.Time{},
 			},
 		},
-		enabled: true,
-	}
+		true,
+	)

-	passingCheck2 := Category{
-		ID: "cat2",
-		checkers: []Checker{
+	passingCheck2 := NewCategory(
+		"cat2",
+		[]Checker{
 			{
 				description: "desc2",
 				check: func(context.Context) error {
@ -108,12 +109,12 @@ func TestHealthChecker(t *testing.T) {
 				retryDeadline: time.Time{},
 			},
 		},
-		enabled: true,
-	}
+		true,
+	)

-	failingCheck := Category{
-		ID: "cat3",
-		checkers: []Checker{
+	failingCheck := NewCategory(
+		"cat3",
+		[]Checker{
 			{
 				description: "desc3",
 				check: func(context.Context) error {
@ -122,12 +123,12 @@ func TestHealthChecker(t *testing.T) {
 				retryDeadline: time.Time{},
 			},
 		},
-		enabled: true,
-	}
+		true,
+	)

-	fatalCheck := Category{
-		ID: "cat6",
-		checkers: []Checker{
+	fatalCheck := NewCategory(
+		"cat6",
+		[]Checker{
 			{
 				description: "desc6",
 				fatal:       true,
@ -137,12 +138,12 @@ func TestHealthChecker(t *testing.T) {
 				retryDeadline: time.Time{},
 			},
 		},
-		enabled: true,
-	}
+		true,
+	)

-	skippingCheck := Category{
-		ID: "cat7",
-		checkers: []Checker{
+	skippingCheck := NewCategory(
+		"cat7",
+		[]Checker{
 			{
 				description: "skip",
 				check: func(context.Context) error {
@ -151,12 +152,12 @@ func TestHealthChecker(t *testing.T) {
 				retryDeadline: time.Time{},
 			},
 		},
-		enabled: true,
-	}
+		true,
+	)

-	skippingRPCCheck := Category{
-		ID: "cat8",
-		checkers: []Checker{
+	skippingRPCCheck := NewCategory(
+		"cat8",
+		[]Checker{
 			{
 				description: "skipRpc",
 				check: func(context.Context) error {
@ -165,12 +166,12 @@ func TestHealthChecker(t *testing.T) {
 				retryDeadline: time.Time{},
 			},
 		},
-		enabled: true,
-	}
+		true,
+	)

-	troubleshootingCheck := Category{
-		ID: "cat9",
-		checkers: []Checker{
+	troubleshootingCheck := NewCategory(
+		"cat9",
+		[]Checker{
 			{
 				description: "failCheck",
 				hintAnchor:  "cat9",
@ -179,8 +180,8 @@ func TestHealthChecker(t *testing.T) {
 				},
 			},
 		},
-		enabled: true,
-	}
+		true,
+	)

 	t.Run("Notifies observer of all results", func(t *testing.T) {
 		hc := NewHealthChecker(
@ -282,9 +283,9 @@ func TestHealthChecker(t *testing.T) {
 		retryWindow = 0
 		returnError := true

-		retryCheck := Category{
-			ID: "cat7",
-			checkers: []Checker{
+		retryCheck := NewCategory(
+			"cat7",
+			[]Checker{
 				{
 					description:   "desc7",
 					retryDeadline: time.Now().Add(100 * time.Second),
@ -297,8 +298,8 @@ func TestHealthChecker(t *testing.T) {
 					},
 				},
 			},
-			enabled: true,
-		}
+			true,
+		)

 		hc := NewHealthChecker(
 			[]CategoryID{},
--- a/viz/pkg/healthcheck/healthcheck.go
+++ b/viz/pkg/healthcheck/healthcheck.go
@ -67,9 +67,9 @@ func (hc *HealthChecker) RunChecks(observer healthcheck.CheckObserver) bool {

 // VizCategory returns a healthcheck.Category containing checkers
 // to verify the health of viz components
-func (hc *HealthChecker) VizCategory() healthcheck.Category {
+func (hc *HealthChecker) VizCategory() *healthcheck.Category {

-	return *healthcheck.NewCategory(LinkerdVizExtensionCheck, []healthcheck.Checker{
+	return healthcheck.NewCategory(LinkerdVizExtensionCheck, []healthcheck.Checker{
 		*healthcheck.NewChecker("linkerd-viz Namespace exists").
 			WithHintAnchor("l5d-viz-ns-exists").
 			Fatal().
@ -246,9 +246,9 @@ func (hc *HealthChecker) VizCategory() healthcheck.Category {

 // VizDataPlaneCategory returns a healthcheck.Category containing checkers
 // to verify the data-plane metrics in prometheus and the tap injection
-func (hc *HealthChecker) VizDataPlaneCategory() healthcheck.Category {
+func (hc *HealthChecker) VizDataPlaneCategory() *healthcheck.Category {

-	return *healthcheck.NewCategory(LinkerdVizExtensionDataPlaneCheck, []healthcheck.Checker{
+	return healthcheck.NewCategory(LinkerdVizExtensionDataPlaneCheck, []healthcheck.Checker{
 		*healthcheck.NewChecker("data plane namespace exists").
 			WithHintAnchor("l5d-data-plane-exists").
 			Fatal().