Repository: linkerd/linkerd2
Pull request #6023 (2021-04-12 20:19:08)

Fix hint anchors for core health checks
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.



Patch:
diff --git a/GOVERNANCE.md b/GOVERNANCE.md
index 973bb1177..5aedecb0f 100644
--- a/GOVERNANCE.md
+++ b/GOVERNANCE.md
@@ -2,6 +2,14 @@
 
 This document defines project governance for Linkerd.
 
+> The Linkerd maintainers are 100% committed to open governance and to being
+> hosted by a neutral foundation. We believe that a diverse and active set of
+> maintainers is fundamental to the long-term health of an open source project.
+> And we want YOU to join us.
+
+&mdash; [Linkerd's Commitment to Open
+Governance](https://linkerd.io/2019/10/03/linkerds-commitment-to-open-governance/)
+
 ## Contributors
 
 Linkerd is for everyone. Anyone can become a Linkerd contributor simply by
diff --git a/jaeger/cmd/check.go b/jaeger/cmd/check.go
index 261e4cb26..c9ca20685 100644
--- 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)
 
diff --git a/jaeger/cmd/root.go b/jaeger/cmd/root.go
index 8849c07cf..d22cd8b29 100644
--- 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)
 }
diff --git a/multicluster/cmd/check.go b/multicluster/cmd/check.go
index cd12633a9..de28023bd 100644
--- 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)
diff --git a/pkg/healthcheck/healthcheck.go b/pkg/healthcheck/healthcheck.go
index 0c0f6f435..46af8a73a 100644
--- 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()
diff --git a/viz/pkg/healthcheck/healthcheck.go b/viz/pkg/healthcheck/healthcheck.go
index 6bcd7a2e0..ced394591 100644
--- 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().


Test patch:
diff --git a/cli/cmd/check_test.go b/cli/cmd/check_test.go
index ecf2c33bd..2e22fa251 100644
--- 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
diff --git a/pkg/healthcheck/healthcheck_test.go b/pkg/healthcheck/healthcheck_test.go
index 539b0564e..0171890ed 100644
--- 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{},