diff --git a/cli/cmd/check_test.go b/cli/cmd/check_test.go index d89c487e8c8fd..ecf2c33bd4165 100644 --- a/cli/cmd/check_test.go +++ b/cli/cmd/check_test.go @@ -16,12 +16,19 @@ func TestCheckStatus(t *testing.T) { []healthcheck.CategoryID{}, &healthcheck.Options{}, ) - hc.Add("category", "check1", "", func(context.Context) error { - return nil - }) - hc.Add("category", "check2", "hint-anchor", func(context.Context) error { - return fmt.Errorf("This should contain instructions for fail") - }) + hc.AppendCategories(*healthcheck.NewCategory("category", []healthcheck.Checker{ + *healthcheck.NewChecker("check1"). + WithCheck(func(context.Context) error { + return nil + }), + *healthcheck.NewChecker("check2"). + WithHintAnchor("hint-anchor"). + WithCheck(func(context.Context) error { + return fmt.Errorf("This should contain instructions for fail") + }), + }, + true, + )) output := bytes.NewBufferString("") healthcheck.RunChecks(output, stderr, hc, tableOutput) @@ -43,12 +50,19 @@ func TestCheckStatus(t *testing.T) { []healthcheck.CategoryID{}, &healthcheck.Options{}, ) - hc.Add("category", "check1", "", func(context.Context) error { - return nil - }) - hc.Add("category", "check2", "hint-anchor", func(context.Context) error { - return fmt.Errorf("This should contain instructions for fail") - }) + hc.AppendCategories(*healthcheck.NewCategory("category", []healthcheck.Checker{ + *healthcheck.NewChecker("check1"). + WithCheck(func(context.Context) error { + return nil + }), + *healthcheck.NewChecker("check2"). + WithHintAnchor("hint-anchor"). + WithCheck(func(context.Context) error { + return fmt.Errorf("This should contain instructions for fail") + }), + }, + true, + )) output := bytes.NewBufferString("") healthcheck.RunChecks(output, stderr, hc, jsonOutput) diff --git a/jaeger/cmd/check.go b/jaeger/cmd/check.go index e3a4f0425ab3d..6f93ddde2a6cb 100644 --- a/jaeger/cmd/check.go +++ b/jaeger/cmd/check.go @@ -41,14 +41,19 @@ func jaegerCategory(hc *healthcheck.HealthChecker) (*healthcheck.Category, error checkers := []healthcheck.Checker{} checkers = append(checkers, - *healthcheck.NewChecker("collector and jaeger service account exists", "l5d-jaeger-sc-exists", true, true, time.Time{}, false). + *healthcheck.NewChecker("collector and jaeger service account exists"). + WithHintAnchor("l5d-jaeger-sc-exists"). + Fatal(). + Warning(). WithCheck(func(ctx context.Context) error { // Check for Collector Service Account return healthcheck.CheckServiceAccounts(ctx, kubeAPI, []string{"collector", "jaeger"}, jaegerNamespace, "") })) checkers = append(checkers, - *healthcheck.NewChecker("collector config map exists", "l5d-jaeger-oc-cm-exists", false, true, time.Time{}, false). + *healthcheck.NewChecker("collector config map exists"). + WithHintAnchor("l5d-jaeger-oc-cm-exists"). + Warning(). WithCheck(func(ctx context.Context) error { // Check for Jaeger Service Account _, err = kubeAPI.CoreV1().ConfigMaps(jaegerNamespace).Get(ctx, "collector-config", metav1.GetOptions{}) @@ -59,7 +64,11 @@ func jaegerCategory(hc *healthcheck.HealthChecker) (*healthcheck.Category, error })) checkers = append(checkers, - *healthcheck.NewChecker("collector pod is running", "l5d-jaeger-collector-running", false, true, hc.RetryDeadline, true). + *healthcheck.NewChecker("collector pod is running"). + WithHintAnchor("l5d-jaeger-collector-running"). + Warning(). + WithRetryDeadline(hc.RetryDeadline). + SurfaceErrorOnRetry(). WithCheck(func(ctx context.Context) error { // Check for Collector pod podList, err := kubeAPI.CoreV1().Pods(namespace).List(ctx, metav1.ListOptions{LabelSelector: "component=collector"}) @@ -70,7 +79,11 @@ func jaegerCategory(hc *healthcheck.HealthChecker) (*healthcheck.Category, error })) checkers = append(checkers, - *healthcheck.NewChecker("jaeger pod is running", "l5d-jaeger-jaeger-running", false, true, hc.RetryDeadline, true). + *healthcheck.NewChecker("jaeger pod is running"). + WithHintAnchor("l5d-jaeger-jaeger-running"). + Warning(). + WithRetryDeadline(hc.RetryDeadline). + SurfaceErrorOnRetry(). WithCheck(func(ctx context.Context) error { // Check for Jaeger pod podList, err := kubeAPI.CoreV1().Pods(namespace).List(ctx, metav1.ListOptions{LabelSelector: "component=jaeger"}) @@ -81,7 +94,9 @@ func jaegerCategory(hc *healthcheck.HealthChecker) (*healthcheck.Category, error })) checkers = append(checkers, - *healthcheck.NewChecker("jaeger extension pods are injected", "l5d-jaeger-pods-injection", false, true, time.Time{}, false). + *healthcheck.NewChecker("jaeger extension pods are injected"). + WithHintAnchor("l5d-jaeger-pods-injection"). + Warning(). WithCheck(func(ctx context.Context) error { // Check for Jaeger pod pods, err := kubeAPI.GetPodsByNamespace(ctx, jaegerNamespace) diff --git a/multicluster/cmd/check.go b/multicluster/cmd/check.go index e9af10bf5f83e..ba5a923ebc1be 100644 --- a/multicluster/cmd/check.go +++ b/multicluster/cmd/check.go @@ -131,16 +131,22 @@ func configureAndRunChecks(wout io.Writer, werr io.Writer, options *checkOptions func multiclusterCategory(hc *healthChecker) *healthcheck.Category { checkers := []healthcheck.Checker{} checkers = append(checkers, - *healthcheck.NewChecker("Link CRD exists", "l5d-multicluster-link-crd-exists", true, false, time.Time{}, false). + *healthcheck.NewChecker("Link CRD exists"). + WithHintAnchor("l5d-multicluster-link-crd-exists"). + Fatal(). WithCheck(func(ctx context.Context) error { return hc.checkLinkCRD(ctx) })) checkers = append(checkers, - *healthcheck.NewChecker("Link resources are valid", "l5d-multicluster-links-are-valid", true, false, time.Time{}, false). + *healthcheck.NewChecker("Link resources are valid"). + WithHintAnchor("l5d-multicluster-links-are-valid"). + Fatal(). WithCheck(func(ctx context.Context) error { return hc.checkLinks(ctx) })) checkers = append(checkers, - *healthcheck.NewChecker("remote cluster access credentials are valid", "l5d-smc-target-clusters-access", false, false, time.Time{}, false). + *healthcheck.NewChecker("remote cluster access credentials are valid"). + WithHintAnchor("l5d-smc-target-clusters-access"). WithCheck(func(ctx context.Context) error { return hc.checkRemoteClusterConnectivity(ctx) })) checkers = append(checkers, - *healthcheck.NewChecker("clusters share trust anchors", "l5d-multicluster-clusters-share-anchors", false, false, time.Time{}, false). + *healthcheck.NewChecker("clusters share trust anchors"). + WithHintAnchor("l5d-multicluster-clusters-share-anchors"). WithCheck(func(ctx context.Context) error { localAnchors, err := tls.DecodePEMCertificates(hc.linkerdHC.LinkerdConfigGlobal().IdentityTrustAnchorsPEM) if err != nil { @@ -149,27 +155,35 @@ func multiclusterCategory(hc *healthChecker) *healthcheck.Category { return hc.checkRemoteClusterAnchors(ctx, localAnchors) })) checkers = append(checkers, - *healthcheck.NewChecker("service mirror controller has required permissions", "l5d-multicluster-source-rbac-correct", false, false, time.Time{}, false). + *healthcheck.NewChecker("service mirror controller has required permissions"). + WithHintAnchor("l5d-multicluster-source-rbac-correct"). WithCheck(func(ctx context.Context) error { return hc.checkServiceMirrorLocalRBAC(ctx) })) checkers = append(checkers, - *healthcheck.NewChecker("service mirror controllers are running", "l5d-multicluster-service-mirror-running", false, false, hc.linkerdHC.RetryDeadline, true). + *healthcheck.NewChecker("service mirror controllers are running"). + WithHintAnchor("l5d-multicluster-service-mirror-running"). + WithRetryDeadline(hc.linkerdHC.RetryDeadline). + SurfaceErrorOnRetry(). WithCheck(func(ctx context.Context) error { return hc.checkServiceMirrorController(ctx) })) checkers = append(checkers, - *healthcheck.NewChecker("all gateway mirrors are healthy", "l5d-multicluster-gateways-endpoints", false, false, time.Time{}, false). + *healthcheck.NewChecker("all gateway mirrors are healthy"). + WithHintAnchor("l5d-multicluster-gateways-endpoints"). WithCheck(func(ctx context.Context) error { return hc.checkIfGatewayMirrorsHaveEndpoints(ctx) })) checkers = append(checkers, - *healthcheck.NewChecker("all mirror services have endpoints", "l5d-multicluster-services-endpoints", false, false, time.Time{}, false). + *healthcheck.NewChecker("all mirror services have endpoints"). + WithHintAnchor("l5d-multicluster-services-endpoints"). WithCheck(func(ctx context.Context) error { return hc.checkIfMirrorServicesHaveEndpoints(ctx) })) checkers = append(checkers, - *healthcheck.NewChecker("all mirror services are part of a Link", "l5d-multicluster-orphaned-services", false, true, time.Time{}, false). + *healthcheck.NewChecker("all mirror services are part of a Link"). + WithHintAnchor("l5d-multicluster-orphaned-services"). + Warning(). WithCheck(func(ctx context.Context) error { return hc.checkForOrphanedServices(ctx) })) diff --git a/pkg/healthcheck/healthcheck.go b/pkg/healthcheck/healthcheck.go index 2b97ecb393ee1..605f19689d444 100644 --- a/pkg/healthcheck/healthcheck.go +++ b/pkg/healthcheck/healthcheck.go @@ -312,24 +312,50 @@ type Checker struct { } // NewChecker returns a new instance of checker type -func NewChecker(description, hintAnchor string, fatal, warning bool, retryDeadline time.Time, surfaceErrorOnRetry bool) *Checker { +func NewChecker(description string) *Checker { return &Checker{ - description: description, - hintAnchor: hintAnchor, - fatal: fatal, - warning: warning, - retryDeadline: retryDeadline, - surfaceErrorOnRetry: surfaceErrorOnRetry, + description: description, + retryDeadline: time.Time{}, } } -// WithCheck Returns a checker with the provided check func +// WithHintAnchor returns a checker with the given hint anchor +func (c *Checker) WithHintAnchor(hint string) *Checker { + c.hintAnchor = hint + return c +} + +// Fatal returns a checker with the fatal field set +func (c *Checker) Fatal() *Checker { + c.fatal = true + return c +} + +// Warning returns a checker with the warning field set +func (c *Checker) Warning() *Checker { + c.warning = true + return c +} + +// WithRetryDeadline returns a checker with the provided retry timeout +func (c *Checker) WithRetryDeadline(retryDeadLine time.Time) *Checker { + c.retryDeadline = retryDeadLine + return c +} + +// SurfaceErrorOnRetry returns a checker with the surfaceErrorOnRetry set +func (c *Checker) SurfaceErrorOnRetry() *Checker { + c.surfaceErrorOnRetry = true + return c +} + +// WithCheck returns a checker with the provided check func func (c *Checker) WithCheck(check func(context.Context) error) *Checker { c.check = check return c } -// WithCheckRPC Returns a checker with the provided checkRPC func +// WithCheckRPC returns a checker with the provided checkRPC func func (c *Checker) WithCheckRPC(checkRPC func(context.Context) (*healthcheckPb.SelfCheckResponse, error)) *Checker { c.checkRPC = checkRPC return c @@ -1404,30 +1430,6 @@ func (hc *HealthChecker) checkMinReplicasAvailable(ctx context.Context) error { return nil } -// Add adds an arbitrary checker. This should only be used for testing. For -// production code, pass in the desired set of checks when calling -// NewHealthChecker. -func (hc *HealthChecker) Add(categoryID CategoryID, description string, hintAnchor string, check func(context.Context) error) { - hc.addCategory( - Category{ - id: categoryID, - checkers: []Checker{ - { - description: description, - check: check, - hintAnchor: hintAnchor, - }, - }, - }, - ) -} - -// addCategory is also for testing -func (hc *HealthChecker) addCategory(c Category) { - c.enabled = true - hc.categories = append(hc.categories, c) -} - // RunChecks runs all configured checkers, and passes the results of each // check to the observer. If a check fails and is marked as fatal, then all // remaining checks are skipped. If at least one check fails, RunChecks returns diff --git a/pkg/healthcheck/healthcheck_test.go b/pkg/healthcheck/healthcheck_test.go index bb49082a61769..02e9eb2a9ba41 100644 --- a/pkg/healthcheck/healthcheck_test.go +++ b/pkg/healthcheck/healthcheck_test.go @@ -61,13 +61,14 @@ func (hc *HealthChecker) addCheckAsCategory( for _, ch := range cat.checkers { if ch.description == desc { testCategory.checkers = append(testCategory.checkers, ch) + testCategory.enabled = true break } } break } } - hc.addCategory(testCategory) + hc.AppendCategories(testCategory) } func TestHealthChecker(t *testing.T) { @@ -84,6 +85,7 @@ func TestHealthChecker(t *testing.T) { retryDeadline: time.Time{}, }, }, + enabled: true, } passingCheck2 := Category{ @@ -97,6 +99,7 @@ func TestHealthChecker(t *testing.T) { retryDeadline: time.Time{}, }, }, + enabled: true, } failingCheck := Category{ @@ -110,6 +113,7 @@ func TestHealthChecker(t *testing.T) { retryDeadline: time.Time{}, }, }, + enabled: true, } passingRPCClient := public.MockAPIClient{ @@ -136,6 +140,7 @@ func TestHealthChecker(t *testing.T) { retryDeadline: time.Time{}, }, }, + enabled: true, } failingRPCClient := public.MockAPIClient{ @@ -163,6 +168,7 @@ func TestHealthChecker(t *testing.T) { retryDeadline: time.Time{}, }, }, + enabled: true, } fatalCheck := Category{ @@ -177,6 +183,7 @@ func TestHealthChecker(t *testing.T) { retryDeadline: time.Time{}, }, }, + enabled: true, } skippingCheck := Category{ @@ -190,6 +197,7 @@ func TestHealthChecker(t *testing.T) { retryDeadline: time.Time{}, }, }, + enabled: true, } skippingRPCCheck := Category{ @@ -203,6 +211,7 @@ func TestHealthChecker(t *testing.T) { retryDeadline: time.Time{}, }, }, + enabled: true, } t.Run("Notifies observer of all results", func(t *testing.T) { @@ -210,11 +219,12 @@ func TestHealthChecker(t *testing.T) { []CategoryID{}, &Options{}, ) - hc.addCategory(passingCheck1) - hc.addCategory(passingCheck2) - hc.addCategory(failingCheck) - hc.addCategory(passingRPCCheck) - hc.addCategory(failingRPCCheck) + + hc.AppendCategories(passingCheck1) + hc.AppendCategories(passingCheck2) + hc.AppendCategories(failingCheck) + hc.AppendCategories(passingRPCCheck) + hc.AppendCategories(failingRPCCheck) expectedResults := []string{ "cat1 desc1", @@ -239,9 +249,9 @@ func TestHealthChecker(t *testing.T) { []CategoryID{}, &Options{}, ) - hc.addCategory(passingCheck1) - hc.addCategory(passingCheck2) - hc.addCategory(passingRPCCheck) + hc.AppendCategories(passingCheck1) + hc.AppendCategories(passingCheck2) + hc.AppendCategories(passingRPCCheck) success := hc.RunChecks(nullObserver) @@ -255,9 +265,9 @@ func TestHealthChecker(t *testing.T) { []CategoryID{}, &Options{}, ) - hc.addCategory(passingCheck1) - hc.addCategory(failingCheck) - hc.addCategory(passingCheck2) + hc.AppendCategories(passingCheck1) + hc.AppendCategories(failingCheck) + hc.AppendCategories(passingCheck2) success := hc.RunChecks(nullObserver) @@ -271,9 +281,9 @@ func TestHealthChecker(t *testing.T) { []CategoryID{}, &Options{}, ) - hc.addCategory(passingCheck1) - hc.addCategory(failingRPCCheck) - hc.addCategory(passingCheck2) + hc.AppendCategories(passingCheck1) + hc.AppendCategories(failingRPCCheck) + hc.AppendCategories(passingCheck2) success := hc.RunChecks(nullObserver) @@ -287,9 +297,9 @@ func TestHealthChecker(t *testing.T) { []CategoryID{}, &Options{}, ) - hc.addCategory(passingCheck1) - hc.addCategory(fatalCheck) - hc.addCategory(passingCheck2) + hc.AppendCategories(passingCheck1) + hc.AppendCategories(fatalCheck) + hc.AppendCategories(passingCheck2) expectedResults := []string{ "cat1 desc1", @@ -323,14 +333,15 @@ func TestHealthChecker(t *testing.T) { }, }, }, + enabled: true, } hc := NewHealthChecker( []CategoryID{}, &Options{}, ) - hc.addCategory(passingCheck1) - hc.addCategory(retryCheck) + hc.AppendCategories(passingCheck1) + hc.AppendCategories(retryCheck) observedResults := make([]string, 0) observer := func(result *CheckResult) { @@ -359,9 +370,9 @@ func TestHealthChecker(t *testing.T) { []CategoryID{}, &Options{}, ) - hc.addCategory(passingCheck1) - hc.addCategory(skippingCheck) - hc.addCategory(skippingRPCCheck) + hc.AppendCategories(passingCheck1) + hc.AppendCategories(skippingCheck) + hc.AppendCategories(skippingRPCCheck) expectedResults := []string{ "cat1 desc1",