Repository: linkerd/linkerd2
Pull request #4407 (2020-05-15 10:57:48)

Add HealthChecks for Tracing Add-On
This PR adds the following healthchecks which are transient i.e ran only when the relevant tracing add-on configuration is present 
- checks for service accounts of collector and Jaeger
- check for collector config map
- checks for collector and jaeger pods to be running

Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com>

<!--  Thanks for sending a pull request!

If you already have a well-structured git commit message, chances are GitHub
set the title and description of this PR to the git commit message subject and
body, respectively. If so, you may delete these instructions and submit your PR.

If this is your first time, please read our contributor guide:
https://github.com/linkerd/linkerd2/blob/master/CONTRIBUTING.md

The title and description of your Pull Request should match the git commit
subject and body, respectively. Git commit messages are structured as follows:

```
Subject

Problem

Solution

Validation

Fixes #[GitHub issue ID]

DCO Sign off
```

Example git commit message:

```
Introduce Pull Request Template

GitHub's community guidelines recommend a pull request template, the repo was
lacking one.

Introduce a `PULL_REQUEST_TEMPLATE.md` file.

Once merged, the
[Community profile checklist](https://github.com/linkerd/linkerd2/community)
should indicate the repo now provides a pull request template.

Fixes #3321

Signed-off-by: Jane Smith <jane.smith@example.com>
```

Note the git commit message subject becomes the pull request title.

For more details around git commits, see the section on Committing in our
contributor guide:
https://github.com/linkerd/linkerd2/blob/master/CONTRIBUTING.md#committing
-->


Pothulapati: @alpeb Updated the code to use only safe casts :) 
Pothulapati: @alpeb @zaharidichev  Updated this PR to make multicluster checks run after the add-on tests. 
```yaml
linkerd-addons
--------------
√ 'linkerd-config-addons' config map exists

linkerd-grafana
---------------
√ grafana add-on service account exists
√ grafana add-on config map exists
√ grafana pod is running

linkerd-multicluster
--------------------
√ service mirror controller is running
√ service mirror controller ClusterRoles exist
√ service mirror controller ClusterRoleBindings exist
√ service mirror controller Roles exist
√ service mirror controller RoleBindings exist
√ service mirror controller ServiceAccounts exist
√ service mirror controller has required permissions
√ service mirror controller can access remote clusters
√ clusters share trust anchors
```

As all the add-on tests are `warning=true`, even if something fails it should still run tests that come after!

Patch:
diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
index 696fef893..d8c0fd0dc 100644
--- a/.github/workflows/release.yml
+++ b/.github/workflows/release.yml
@@ -227,7 +227,7 @@ jobs:
         GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
       with:
         draft: false
-        prelease: false
+        prerelease: false
         body_path: NOTES.md
         files: |
           ./target/release/linkerd2-cli-*-darwin
diff --git a/pkg/charts/linkerd2/tracing.go b/pkg/charts/linkerd2/tracing.go
index c41ea42fb..ce63132e6 100644
--- a/pkg/charts/linkerd2/tracing.go
+++ b/pkg/charts/linkerd2/tracing.go
@@ -6,7 +6,8 @@ import (
 )
 
 var (
-	tracingAddOn = "tracing"
+	// TracingAddOn represents the name of the tracing add-on
+	TracingAddOn = "tracing"
 )
 
 // Tracing is an add-on that installs the distributed tracing
@@ -15,7 +16,7 @@ type Tracing map[string]interface{}
 
 // Name returns the name of the Tracing add-on
 func (t Tracing) Name() string {
-	return tracingAddOn
+	return TracingAddOn
 }
 
 // Values returns the configuration values that were assigned for this add-on
diff --git a/pkg/healthcheck/healthcheck_addons.go b/pkg/healthcheck/healthcheck_addons.go
index 8ea88db18..397a18394 100644
--- a/pkg/healthcheck/healthcheck_addons.go
+++ b/pkg/healthcheck/healthcheck_addons.go
@@ -2,6 +2,7 @@ package healthcheck
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
 	l5dcharts "github.com/linkerd/linkerd2/pkg/charts/linkerd2"
@@ -14,13 +15,16 @@ const (
 	// LinkerdAddOnChecks adds checks to validate the add-on components
 	LinkerdAddOnChecks CategoryID = "linkerd-addons"
 
-	// LinkerdGrafanaAddOnChecks adds checks to validate the add-on components
+	// LinkerdGrafanaAddOnChecks adds checks related to grafana add-on components
 	LinkerdGrafanaAddOnChecks CategoryID = "linkerd-grafana"
+
+	// LinkerdTracingAddOnChecks adds checks related to tracing add-on components
+	LinkerdTracingAddOnChecks CategoryID = "linkerd-tracing"
 )
 
 var (
 	// AddOnCategories is the list of add-on category checks
-	AddOnCategories = []CategoryID{LinkerdAddOnChecks, LinkerdGrafanaAddOnChecks}
+	AddOnCategories = []CategoryID{LinkerdAddOnChecks, LinkerdGrafanaAddOnChecks, LinkerdTracingAddOnChecks}
 )
 
 // addOnCategories contain all the checks w.r.t add-ons. It is strongly advised to
@@ -32,7 +36,7 @@ func (hc *HealthChecker) addOnCategories() []category {
 			id: LinkerdAddOnChecks,
 			checkers: []checker{
 				{
-					description: fmt.Sprintf("%s config map exists", k8s.AddOnsConfigMapName),
+					description: fmt.Sprintf("'%s' config map exists", k8s.AddOnsConfigMapName),
 					warning:     true,
 					check: func(context.Context) error {
 						return hc.checkIfAddOnsConfigMapExists()
@@ -48,7 +52,11 @@ func (hc *HealthChecker) addOnCategories() []category {
 					warning:     true,
 					check: func(context.Context) error {
 						if grafana, ok := hc.addOns[l5dcharts.GrafanaAddOn]; ok {
-							return hc.checkServiceAccounts([]string{grafana.(map[string]interface{})["name"].(string)}, hc.ControlPlaneNamespace, "")
+							name, err := getString(grafana, "name")
+							if err != nil {
+								return err
+							}
+							return hc.checkServiceAccounts([]string{name}, hc.ControlPlaneNamespace, "")
 						}
 						return &SkipError{Reason: "grafana add-on not enabled"}
 					},
@@ -58,7 +66,11 @@ func (hc *HealthChecker) addOnCategories() []category {
 					warning:     true,
 					check: func(context.Context) error {
 						if grafana, ok := hc.addOns[l5dcharts.GrafanaAddOn]; ok {
-							_, err := hc.kubeAPI.CoreV1().ConfigMaps(hc.ControlPlaneNamespace).Get(fmt.Sprintf("%s-config", grafana.(map[string]interface{})["name"].(string)), metav1.GetOptions{})
+							name, err := getString(grafana, "name")
+							if err != nil {
+								return err
+							}
+							_, err = hc.kubeAPI.CoreV1().ConfigMaps(hc.ControlPlaneNamespace).Get(fmt.Sprintf("%s-config", name), metav1.GetOptions{})
 							if err != nil {
 								return err
 							}
@@ -88,6 +100,113 @@ func (hc *HealthChecker) addOnCategories() []category {
 				},
 			},
 		},
+		{
+			id: LinkerdTracingAddOnChecks,
+			checkers: []checker{
+				{
+					description: "collector service account exists",
+					warning:     true,
+					check: func(context.Context) error {
+						if tracing, ok := hc.addOns[l5dcharts.TracingAddOn]; ok {
+
+							collector, err := getMap(tracing, "collector")
+							if err != nil {
+								return err
+							}
+
+							collectorName, err := getString(collector, "name")
+							if err != nil {
+								return err
+							}
+							return hc.checkServiceAccounts([]string{collectorName}, hc.ControlPlaneNamespace, "")
+						}
+						return &SkipError{Reason: "tracing add-on not enabled"}
+					},
+				},
+				{
+					description: "jaeger service account exists",
+					warning:     true,
+					check: func(context.Context) error {
+						if tracing, ok := hc.addOns[l5dcharts.TracingAddOn]; ok {
+							jaeger, err := getMap(tracing, "jaeger")
+							if err != nil {
+								return err
+							}
+
+							jaegerName, err := getString(jaeger, "name")
+							if err != nil {
+								return err
+							}
+
+							return hc.checkServiceAccounts([]string{jaegerName}, hc.ControlPlaneNamespace, "")
+						}
+						return &SkipError{Reason: "tracing add-on not enabled"}
+					},
+				},
+				{
+					description: "collector config map exists",
+					warning:     true,
+					check: func(context.Context) error {
+						if tracing, ok := hc.addOns[l5dcharts.TracingAddOn]; ok {
+							collector, err := getMap(tracing, "collector")
+							if err != nil {
+								return err
+							}
+
+							collectorName, err := getString(collector, "name")
+							if err != nil {
+								return err
+							}
+
+							_, err = hc.kubeAPI.CoreV1().ConfigMaps(hc.ControlPlaneNamespace).Get(fmt.Sprintf("%s-config", collectorName), metav1.GetOptions{})
+							if err != nil {
+								return err
+							}
+							return nil
+						}
+						return &SkipError{Reason: "tracing add-on not enabled"}
+					},
+				},
+				{
+					description:         "collector pod is running",
+					warning:             true,
+					retryDeadline:       hc.RetryDeadline,
+					surfaceErrorOnRetry: true,
+					check: func(context.Context) error {
+						if _, ok := hc.addOns[l5dcharts.TracingAddOn]; ok {
+							// populate controlPlanePods to get the latest status, during retries
+							var err error
+							hc.controlPlanePods, err = hc.kubeAPI.GetPodsByNamespace(hc.ControlPlaneNamespace)
+							if err != nil {
+								return err
+							}
+
+							return checkContainerRunning(hc.controlPlanePods, "collector")
+						}
+						return &SkipError{Reason: "tracing add-on not enabled"}
+					},
+				},
+				{
+					description:         "jaeger pod is running",
+					warning:             true,
+					retryDeadline:       hc.RetryDeadline,
+					surfaceErrorOnRetry: true,
+					check: func(context.Context) error {
+						if _, ok := hc.addOns[l5dcharts.TracingAddOn]; ok {
+							// populate controlPlanePods to get the latest status, during retries
+							var err error
+							hc.controlPlanePods, err = hc.kubeAPI.GetPodsByNamespace(hc.ControlPlaneNamespace)
+							if err != nil {
+								return err
+							}
+
+							return checkContainerRunning(hc.controlPlanePods, "jaeger")
+						}
+						return &SkipError{Reason: "tracing add-on not enabled"}
+					},
+				},
+			},
+		},
 	}
 }
 
@@ -140,3 +259,41 @@ func (hc *HealthChecker) checkForAddOnCM() (string, error) {
 
 	return values, nil
 }
+
+func getString(i interface{}, k string) (string, error) {
+	m, ok := i.(map[string]interface{})
+	if !ok {
+		return "", errors.New("config value is not a map")
+	}
+
+	v, ok := m[k]
+	if !ok {
+		return "", fmt.Errorf("key '%s' not found in config value", k)
+	}
+
+	res, ok := v.(string)
+	if !ok {
+		return "", fmt.Errorf("config value '%v' for key '%s' is not a string", v, k)
+	}
+
+	return res, nil
+}
+
+func getMap(i interface{}, k string) (map[string]interface{}, error) {
+	m, ok := i.(map[string]interface{})
+	if !ok {
+		return nil, errors.New("config value is not a map")
+	}
+
+	v, ok := m[k]
+	if !ok {
+		return nil, fmt.Errorf("key '%s' not found in config value", k)
+	}
+
+	res, ok := v.(map[string]interface{})
+	if !ok {
+		return nil, fmt.Errorf("config value '%v' for key '%s' is not a map", v, k)
+	}
+
+	return res, nil
+}


Test patch:
diff --git a/pkg/healthcheck/healthcheck_test.go b/pkg/healthcheck/healthcheck_test.go
index aa23dd235..684a7edcb 100644
--- a/pkg/healthcheck/healthcheck_test.go
+++ b/pkg/healthcheck/healthcheck_test.go
@@ -3250,6 +3250,68 @@ func TestMinReplicaCheck(t *testing.T) {
 	}
 }
 
+func TestGetString(t *testing.T) {
+	testCases := []struct {
+		i             interface{}
+		k             string
+		expected      string
+		expectedError error
+	}{
+		{
+			i: map[string]interface{}{
+				"key": "value",
+			},
+			k:             "key",
+			expected:      "value",
+			expectedError: nil,
+		},
+		{
+			i: map[string]interface{}{
+				"key": map[string]interface{}{
+					"key1": "value1",
+				},
+			},
+			k:             "key",
+			expected:      "",
+			expectedError: errors.New("config value 'map[key1:value1]' for key 'key' is not a string"),
+		},
+		{
+			i: map[string]interface{}{
+				"key": "value",
+			},
+			k:             "key1",
+			expected:      "",
+			expectedError: errors.New("key 'key1' not found in config value"),
+		},
+	}
+
+	for i, tc := range testCases {
+		tc := tc //pin
+		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
+			ans, err := getString(tc.i, tc.k)
+
+			if ans != tc.expected {
+				t.Logf("Expected value: %s\n", tc.expected)
+				t.Logf("Received value: %s\n", ans)
+				t.Fatal("test case failed")
+			}
+
+			if err == nil && tc.expectedError != nil {
+				t.Log("Expected error: nil")
+				t.Logf("Received error: %s\n", err)
+				t.Fatal("test case failed")
+			}
+			if err != nil {
+				if err.Error() != tc.expectedError.Error() {
+					t.Logf("Expected error: %s\n", tc.expectedError)
+					t.Logf("Received error: %s\n", err)
+					t.Fatal("test case failed")
+				}
+			}
+		})
+	}
+}
+
 type controlPlaneReplicaOptions struct {
 	controller    int
 	destination   int
diff --git a/test/testdata/check.golden b/test/testdata/check.golden
index 5b5d0345a..6dcfe52e8 100644
--- a/test/testdata/check.golden
+++ b/test/testdata/check.golden
@@ -60,7 +60,7 @@ control-plane-version
 
 linkerd-addons
 --------------
-√ linkerd-config-addons config map exists
+√ 'linkerd-config-addons' config map exists
 
 linkerd-grafana
 ---------------
diff --git a/test/testdata/check.proxy.golden b/test/testdata/check.proxy.golden
index 42d7cb0ba..e07c2836f 100644
--- a/test/testdata/check.proxy.golden
+++ b/test/testdata/check.proxy.golden
@@ -67,7 +67,7 @@ linkerd-data-plane
 
 linkerd-addons
 --------------
-√ linkerd-config-addons config map exists
+√ 'linkerd-config-addons' config map exists
 
 linkerd-grafana
 ---------------