Repository: linkerd/linkerd2
Pull request #4724 (2020-07-07 05:29:43)

Remove/Relax prometheus related checks
Implements second part of https://github.com/linkerd/rfc/pull/16, https://github.com/linkerd/linkerd2/issues/4375

Now that prometheus is an add-on (i.e #4362 ), There can be cases where prometheus is
disabled at which the check should show a warning but not fail. This
decouples the tight depedency.

This changes the following checks:

- Removes serviceAccount and pod checks in the CLI.
- Relaxes `linkerd-api` checks to only check for prometheus access when
the URL is not empty. This should work seamlessly with external
prometheus as that URL will be passed and it performs the same
check.
- Updates `queryProm` function to perform a check, to not crash but
report a failure to the dashboard and cli

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/main/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/main/CONTRIBUTING.md#committing
-->


alpeb: How feasible would it be to move those checks to its own section under `healthcheck_addons.go` so prometheus gets own checks section in the `linkerd check output`?
Pothulapati: @alpeb Yep, That's the plan :)
Pothulapati: @alpeb Updated the controller instance to fail gracefully when there is no prometheus, works as expected both with CLI and as well as the dashboard.

Patch:
diff --git a/CHANGES.md b/CHANGES.md
index 1c7fb0e16..18a4549f7 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -1,5 +1,11 @@
 # Changes
 
+## edge-20.7.3
+
+* Add preliminary support for EndpointSlices which will be usable in future
+  releases (thanks @Matei207!)
+* Internal improvements to the CI process for testing Helm installations
+
 ## edge-20.7.2
 
 This edge release moves Linkerd's bundled Prometheus into an add-on. This makes
diff --git a/controller/api/public/grpc_server.go b/controller/api/public/grpc_server.go
index c34ba2ed7..c8b461c06 100644
--- a/controller/api/public/grpc_server.go
+++ b/controller/api/public/grpc_server.go
@@ -207,23 +207,27 @@ func (s *grpcServer) SelfCheck(ctx context.Context, in *healthcheckPb.SelfCheckR
 		k8sClientCheck.FriendlyMessageToUser = fmt.Sprintf("Error calling the Kubernetes API: %s", err)
 	}
 
-	promClientCheck := &healthcheckPb.CheckResult{
-		SubsystemName:    promClientSubsystemName,
-		CheckDescription: promClientCheckDescription,
-		Status:           healthcheckPb.CheckStatus_OK,
-	}
-	_, err = s.queryProm(ctx, fmt.Sprintf(podQuery, ""))
-	if err != nil {
-		promClientCheck.Status = healthcheckPb.CheckStatus_ERROR
-		promClientCheck.FriendlyMessageToUser = fmt.Sprintf("Error calling Prometheus from the control plane: %s", err)
-	}
-
 	response := &healthcheckPb.SelfCheckResponse{
 		Results: []*healthcheckPb.CheckResult{
 			k8sClientCheck,
-			promClientCheck,
 		},
 	}
+
+	if s.prometheusAPI != nil {
+		promClientCheck := &healthcheckPb.CheckResult{
+			SubsystemName:    promClientSubsystemName,
+			CheckDescription: promClientCheckDescription,
+			Status:           healthcheckPb.CheckStatus_OK,
+		}
+		_, err = s.queryProm(ctx, fmt.Sprintf(podQuery, ""))
+		if err != nil {
+			promClientCheck.Status = healthcheckPb.CheckStatus_ERROR
+			promClientCheck.FriendlyMessageToUser = fmt.Sprintf("Error calling Prometheus from the control plane: %s", err)
+		}
+
+		response.Results = append(response.Results, promClientCheck)
+	}
+
 	return response, nil
 }
 
diff --git a/controller/api/public/http_server.go b/controller/api/public/http_server.go
index 66f69ec98..6c7548336 100644
--- a/controller/api/public/http_server.go
+++ b/controller/api/public/http_server.go
@@ -331,9 +331,15 @@ func NewServer(
 	clusterDomain string,
 	ignoredNamespaces []string,
 ) *http.Server {
+
+	var promAPI promv1.API
+	if prometheusClient != nil {
+		promAPI = promv1.NewAPI(prometheusClient)
+	}
+
 	baseHandler := &handler{
 		grpcServer: newGrpcServer(
-			promv1.NewAPI(prometheusClient),
+			promAPI,
 			destinationClient,
 			k8sAPI,
 			controllerNamespace,
diff --git a/controller/api/public/prometheus.go b/controller/api/public/prometheus.go
index a03aba69f..dffc55a9b 100644
--- a/controller/api/public/prometheus.go
+++ b/controller/api/public/prometheus.go
@@ -2,6 +2,7 @@ package public
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"math"
 	"sort"
@@ -56,6 +57,10 @@ func (s *grpcServer) queryProm(ctx context.Context, query string) (model.Vector,
 	defer span.End()
 	span.AddAttributes(trace.StringAttribute("queryString", query))
 
+	if s.prometheusAPI == nil {
+		return nil, errors.New("No prometheus instance to connect")
+	}
+
 	// single data point (aka summary) query
 	res, warn, err := s.prometheusAPI.Query(ctx, query, time.Time{})
 	if err != nil {
diff --git a/controller/cmd/public-api/main.go b/controller/cmd/public-api/main.go
index 006313a5a..97b4f1f27 100644
--- a/controller/cmd/public-api/main.go
+++ b/controller/cmd/public-api/main.go
@@ -26,7 +26,7 @@ func Main(args []string) {
 
 	addr := cmd.String("addr", ":8085", "address to serve on")
 	kubeConfigPath := cmd.String("kubeconfig", "", "path to kube config")
-	prometheusURL := cmd.String("prometheus-url", "http://127.0.0.1:9090", "prometheus url")
+	prometheusURL := cmd.String("prometheus-url", "", "prometheus url")
 	metricsAddr := cmd.String("metrics-addr", ":9995", "address to serve scrapable metrics on")
 	destinationAPIAddr := cmd.String("destination-addr", "127.0.0.1:8086", "address of destination service")
 	controllerNamespace := cmd.String("controller-namespace", "linkerd", "namespace in which Linkerd is installed")
@@ -53,9 +53,12 @@ func Main(args []string) {
 		log.Fatalf("Failed to initialize K8s API: %s", err)
 	}
 
-	prometheusClient, err := promApi.NewClient(promApi.Config{Address: *prometheusURL})
-	if err != nil {
-		log.Fatal(err.Error())
+	var prometheusClient promApi.Client
+	if *prometheusURL != "" {
+		prometheusClient, err = promApi.NewClient(promApi.Config{Address: *prometheusURL})
+		if err != nil {
+			log.Fatal(err.Error())
+		}
 	}
 
 	globalConfig, err := config.Global(pkgK8s.MountPathGlobalConfig)
diff --git a/pkg/healthcheck/healthcheck.go b/pkg/healthcheck/healthcheck.go
index 6165de7e2..876db1c0f 100644
--- a/pkg/healthcheck/healthcheck.go
+++ b/pkg/healthcheck/healthcheck.go
@@ -1508,7 +1508,6 @@ func (hc *HealthChecker) expectedRBACNames() []string {
 	return []string{
 		fmt.Sprintf("linkerd-%s-controller", hc.ControlPlaneNamespace),
 		fmt.Sprintf("linkerd-%s-identity", hc.ControlPlaneNamespace),
-		fmt.Sprintf("linkerd-%s-prometheus", hc.ControlPlaneNamespace),
 		fmt.Sprintf("linkerd-%s-proxy-injector", hc.ControlPlaneNamespace),
 		fmt.Sprintf("linkerd-%s-sp-validator", hc.ControlPlaneNamespace),
 		fmt.Sprintf("linkerd-%s-tap", hc.ControlPlaneNamespace),
@@ -2067,7 +2066,7 @@ const running = "Running"
 func validateControlPlanePods(pods []corev1.Pod) error {
 	statuses := getPodStatuses(pods)
 
-	names := []string{"controller", "identity", "prometheus", "sp-validator", "web", "tap"}
+	names := []string{"controller", "identity", "sp-validator", "web", "tap"}
 	// TODO: deprecate this when we drop support for checking pre-default proxy-injector control-planes
 	if _, found := statuses["proxy-injector"]; found {
 		names = append(names, "proxy-injector")
diff --git a/web/app/package.json b/web/app/package.json
index 4988880bf..8e9d4c37d 100644
--- a/web/app/package.json
+++ b/web/app/package.json
@@ -18,7 +18,7 @@
     "d3-format": "1.4.4",
     "d3-selection": "1.4.1",
     "date-fns": "2.12.0",
-    "lodash": "4.17.15",
+    "lodash": "4.17.19",
     "path": "0.12.7",
     "prop-types": "15.7.2",
     "query-string": "^6.12.1",
diff --git a/web/app/yarn.lock b/web/app/yarn.lock
index c0423fb65..a0f1a005b 100644
--- a/web/app/yarn.lock
+++ b/web/app/yarn.lock
@@ -7062,10 +7062,10 @@ lodash.zip@^4.2.0:
   resolved "https://registry.yarnpkg.com/lodash.zip/-/lodash.zip-4.2.0.tgz#ec6662e4896408ed4ab6c542a3990b72cc080020"
   integrity sha1-7GZi5IlkCO1KtsVCo5kLcswIACA=
 
-lodash@4.17.15, lodash@^4.15.0, lodash@^4.17.11, lodash@^4.17.13, lodash@^4.17.14, lodash@^4.17.15, lodash@^4.17.4, lodash@~4.17.12:
-  version "4.17.15"
-  resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.15.tgz#b447f6670a0455bbfeedd11392eff330ea097548"
-  integrity sha512-8xOcRHvCjnocdS5cpwXQXVzmmh5e5+saE2QGoeQmbKmRS6J3VQppPOIt0MnmE+4xlZoumy0GPG0D0MVIQbNA1A==
+lodash@4.17.19, lodash@^4.15.0, lodash@^4.17.11, lodash@^4.17.13, lodash@^4.17.14, lodash@^4.17.15, lodash@^4.17.4, lodash@~4.17.12:
+  version "4.17.19"
+  resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.19.tgz#e48ddedbe30b3321783c5b4301fbd353bc1e4a4b"
+  integrity sha512-JNvd8XER9GQX0v2qJgsaN/mzFCNA5BRe/j8JN9d+tWyGLSodKQHKFicdwNYzWwI3wjRnaKPsGj1XkBjx/F96DQ==
 
 log-symbols@3.0.0:
   version "3.0.0"


Test patch:
diff --git a/pkg/healthcheck/healthcheck_test.go b/pkg/healthcheck/healthcheck_test.go
index 9607610f7..a96f8cb3d 100644
--- a/pkg/healthcheck/healthcheck_test.go
+++ b/pkg/healthcheck/healthcheck_test.go
@@ -570,7 +570,7 @@ metadata:
 			},
 			[]string{
 				"linkerd-config control plane Namespace exists",
-				"linkerd-config control plane ClusterRoles exist: missing ClusterRoles: linkerd-test-ns-controller, linkerd-test-ns-identity, linkerd-test-ns-prometheus, linkerd-test-ns-proxy-injector, linkerd-test-ns-sp-validator, linkerd-test-ns-tap",
+				"linkerd-config control plane ClusterRoles exist: missing ClusterRoles: linkerd-test-ns-controller, linkerd-test-ns-identity, linkerd-test-ns-proxy-injector, linkerd-test-ns-sp-validator, linkerd-test-ns-tap",
 			},
 		},
 		{
@@ -632,7 +632,7 @@ metadata:
 			[]string{
 				"linkerd-config control plane Namespace exists",
 				"linkerd-config control plane ClusterRoles exist",
-				"linkerd-config control plane ClusterRoleBindings exist: missing ClusterRoleBindings: linkerd-test-ns-controller, linkerd-test-ns-identity, linkerd-test-ns-prometheus, linkerd-test-ns-proxy-injector, linkerd-test-ns-sp-validator, linkerd-test-ns-tap",
+				"linkerd-config control plane ClusterRoleBindings exist: missing ClusterRoleBindings: linkerd-test-ns-controller, linkerd-test-ns-identity, linkerd-test-ns-proxy-injector, linkerd-test-ns-sp-validator, linkerd-test-ns-tap",
 			},
 		},
 		{
@@ -1995,8 +1995,8 @@ func TestValidateControlPlanePods(t *testing.T) {
 			pod("linkerd-controller-6f78cbd47-bc557", corev1.PodRunning, true),
 			pod("linkerd-grafana-5b7d796646-hh46d", corev1.PodRunning, true),
 			pod("linkerd-identity-6849948664-27982", corev1.PodRunning, true),
-			pod("linkerd-prometheus-74d6879cd6-bbdk6", corev1.PodFailed, false),
-			pod("linkerd-tap-6c878df6c8-2hmtd", corev1.PodRunning, true),
+			pod("linkerd-prometheus-74d6879cd6-bbdk6", corev1.PodRunning, true),
+			pod("linkerd-tap-6c878df6c8-2hmtd", corev1.PodFailed, false),
 			pod("linkerd-sp-validator-24d2879ce6-cddk9", corev1.PodRunning, true),
 			pod("linkerd-web-98c9ddbcd-7b5lh", corev1.PodRunning, true),
 		}
@@ -2005,7 +2005,7 @@ func TestValidateControlPlanePods(t *testing.T) {
 		if err == nil {
 			t.Fatal("Expected error, got nothing")
 		}
-		if err.Error() != "No running pods for \"linkerd-prometheus\"" {
+		if err.Error() != "No running pods for \"linkerd-tap\"" {
 			t.Fatalf("Unexpected error message: %s", err.Error())
 		}
 	})