Repository: linkerd/linkerd2
Pull request #4760 (2020-07-15 20:35:53)

Uncomment EndpointSliceAccess function
### What
---

* Small PR that uncomments the `EndpointSliceAcess` method and cleans up left over todos in the destination service.
* Based on the past three PRs related to `EndpointSlices` (#4663 #4696 #4740); they should now be functional (albeit prone to bugs) and ready to use.

### Tests
---

* Although I haven't extensively tested all edge cases in the context of this PR, I did test that everything comes together nicely. To this, may approach has been:
  - Test how the flag behaves when we have (and don't have) support for slices in the cluster;
  - Test how the destination service behaves in combination with the flag and feature gate.

I tested a total of three broad scenarios:

1. When minikube has no `EndpointSlices`:
```sh
# install should fail when flag is enabled
$ target/cli/darwin/linkerd install --enable-endpoint-slices
# output
Error: --enableEndpointSlice=true not supported: no EndpointSlice resources exist in the cluster

# install should succeed when flag disabled
# everything should work as normal
# dest service should watch endpoints
$ k logs <dest-service-pod> -f destination
#output
ime="2020-07-15T19:40:22Z" level=debug msg="Watching Endpoints resources" addr=":8086" component=endpoints-watcher
time="2020-07-15T19:40:22Z" level=info msg="waiting for caches to sync"
I0715 19:40:22.143369       1 reflector.go:150] Starting reflector *v1.Service (10m0s) from pkg/mod/k8s.io/client-go@v0.17.4/tools/cache/reflector.go:105
```
* When the flag is disabled, works as expected, when flag is enabled, works as expected.
* No hanging on syncing cache, not watching wrong resource.

2. When minikube has `EndpointSlice` support, but not Linkerd installed with `EndpointSlice` support:
```sh
# Install should succeed
$ target/cli/darwin/linkerd install
# Output:
<installed successfully>

# Destination service should watch Endpoints
# shouldn't crash, shouldn't hang on informer syncing:
$ k logs <dest-service-pod> -f destination
# Output:
time="2020-07-15T20:09:44Z" level=debug msg="Watching Endpoints resources" addr=":8086" component=endpoints-watcher
time="2020-07-15T20:09:44Z" level=info msg="waiting for caches to sync"
<starting reflector> # forgot to copy line
# more logs in between
time="2020-07-15T20:34:05Z" level=debug msg="Sending destination add: add:{addrs:{addr:{ip:{ipv4:2886795270}  port:9996}  weight:10000}}" addr=":8086" component=endpoint-translator remote="127.0.0.1:49108" service="172.17.0.6:9996"
```
* When installed in a cluster with `EndpointSlice` support but without support in Linkerd, the right resource is watched
* No errors, no hanging on cache syncing

3. When minikube has `EndpointSlice` support, with Linkerd installed with `EndpointSlice` support:
```
# Install should succeed (resource active in cluster)
$ target/cli/darwin/linkerd install --enable-endpoint-slices
# Output:
<installed successfully>

# Destination service should watch slices
# shouldn't error out, should transmit updates to other control plane
# components (i.e finding endpoints in slices)
$ k logs <dest-service-pod> -f destination
# Output:
time="2020-07-15T20:12:09Z" level=debug msg="Watching EndpointSlice resources" addr=":8086" component=endpoints-watcher
time="2020-07-15T20:12:09Z" level=info msg="waiting for caches to sync"
I0715 20:12:09.463803       1 reflector.go:150] Starting reflector *v1.Pod (10m0s) from pkg/mod/k8s.io/client-go@v0.17.4/tools/cache/reflector.go:105

time="2020-07-15T20:12:09Z" level=debug msg="Adding EndpointSlice for linkerd/linkerd-dst" addr=":8086" component=service-publisher ns=linkerd svc=linkerd-dst
time="2020-07-15T20:12:09Z" level=debug msg="Adding EndpointSlice for linkerd/linkerd-sp-validator" addr=":8086" component=service-publisher ns=linkerd svc=linkerd-sp-validator
# ... extra logs in between
time="2020-07-15T20:12:20Z" level=debug msg="Sending destination add: add:{addrs:{addr:{ip:{ipv4:2886795273}  port:4191}  weight:10000  metric_labels:{key}
```
* Everything works as expected, no nasty errors, destination messages are being sent to the proxy

My conclusion is that at first glance everything seems to be working well enough to have them enabled for experimental use. The same messages are being sent as for the Endpoints resource, the flag does a well enough job of preventing installation when the resource does not exist and a well enough job of allowing the installation to proceed when the access criteria is met.

Signed-off-by: Matei David <matei.david.35@gmail.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
-->


Matei207: @adleong sorry about that, I think I made a typo when I copied the logs from my notes. I'll add some pictures with timestamps to show you the output of the commands just to be sure:
![Screen Shot 2020-07-16 at 1 08 38 AM](https://user-images.githubusercontent.com/18265670/87611891-120da400-c701-11ea-8be2-2a242f8295ca.png)
![Screen Shot 2020-07-16 at 1 09 08 AM](https://user-images.githubusercontent.com/18265670/87611907-16d25800-c701-11ea-99f4-3a4243ca7bc8.png)

Will edit the body of the PR so it's not misleading.





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/destination/watcher/endpoints_watcher.go b/controller/api/destination/watcher/endpoints_watcher.go
index 5d2f92fd6..68df50243 100644
--- a/controller/api/destination/watcher/endpoints_watcher.go
+++ b/controller/api/destination/watcher/endpoints_watcher.go
@@ -129,7 +129,6 @@ var endpointsVecs = newEndpointsMetricsVecs()
 // NewEndpointsWatcher creates an EndpointsWatcher and begins watching the
 // k8sAPI for pod, service, and endpoint changes. An EndpointsWatcher will
 // watch on Endpoints or EndpointSlice resources, depending on cluster configuration.
-//TODO: Allow EndpointSlice resources to be used once opt-in functionality is supported.
 func NewEndpointsWatcher(k8sAPI *k8s.API, log *logging.Entry, enableEndpointSlices bool) *EndpointsWatcher {
 	ew := &EndpointsWatcher{
 		publishers:           make(map[ServiceID]*servicePublisher),
@@ -161,7 +160,6 @@ func NewEndpointsWatcher(k8sAPI *k8s.API, log *logging.Entry, enableEndpointSlic
 			UpdateFunc: ew.updateEndpointSlice,
 		})
 	} else {
-		// ew.log.Debugf("Cluster does not have EndpointSlice access:%v", err)
 		ew.log.Debugf("Watching Endpoints resources")
 		k8sAPI.Endpoint().Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
 			AddFunc:    ew.addEndpoints,
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/pkg/k8s/authz.go b/pkg/k8s/authz.go
index 2f1434494..f8f7cf4d7 100644
--- a/pkg/k8s/authz.go
+++ b/pkg/k8s/authz.go
@@ -5,6 +5,8 @@ import (
 	"fmt"
 
 	authV1 "k8s.io/api/authorization/v1"
+	discovery "k8s.io/api/discovery/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/kubernetes"
 )
@@ -107,40 +109,36 @@ func ServiceProfilesAccess(k8sClient kubernetes.Interface) error {
 
 // EndpointSliceAccess verifies whether the K8s cluster has
 // access to EndpointSlice resources.
-//TODO: Uncomment function and change return type once EndpointSlices
-// are supported and made opt-in through install flag
 func EndpointSliceAccess(k8sClient kubernetes.Interface) error {
-	// gv := discovery.SchemeGroupVersion.String()
-	// res, err := k8sClient.Discovery().ServerResourcesForGroupVersion(gv)
-	// if err != nil {
-	// 	return err
-	// }
-
-	// if res.GroupVersion == gv {
-	// 	for _, apiRes := range res.APIResources {
-	// 		if apiRes.Kind == "EndpointSlice" {
-	// 			return checkEndpointSlicesExist(k8sClient)
-	// 		}
-	// 	}
-	// }
-
-	// return errors.New("EndpointSlice resource not found")
-	return errors.New("EndpointSlice not supported")
+	gv := discovery.SchemeGroupVersion.String()
+	res, err := k8sClient.Discovery().ServerResourcesForGroupVersion(gv)
+	if err != nil {
+		return err
+	}
+
+	if res.GroupVersion == gv {
+		for _, apiRes := range res.APIResources {
+			if apiRes.Kind == "EndpointSlice" {
+				return checkEndpointSlicesExist(k8sClient)
+			}
+		}
+	}
+
+	return errors.New("EndpointSlice resource not found")
 }
 
-//TODO: Uncomment function once EndpointSlices are supported and opt-in
-//func checkEndpointSlicesExist(k8sclient kubernetes.Interface) error {
-//	sliceList, err := k8sclient.DiscoveryV1beta1().EndpointSlices("").List(metav1.ListOptions{})
-//	if err != nil {
-//		return err
-//	}
-//
-//	if len(sliceList.Items) > 0 {
-//		return nil
-//	}
-//
-//	return errors.New("no EndpointSlice resources exist in the cluster")
-//}
+func checkEndpointSlicesExist(k8sClient kubernetes.Interface) error {
+	sliceList, err := k8sClient.DiscoveryV1beta1().EndpointSlices("").List(metav1.ListOptions{})
+	if err != nil {
+		return err
+	}
+
+	if len(sliceList.Items) > 0 {
+		return nil
+	}
+
+	return errors.New("no EndpointSlice resources exist in the cluster")
+}
 
 // ClusterAccess verifies whether k8sClient is authorized to access all pods in
 // all namespaces in the cluster.
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())
 		}
 	})