Improve the linkerd check --proxy experience if data plane pods do not have tap configured

[scottcarol]

Thanks for your work on Linkerd! 💪

When I ran linkerd check --proxy on edge-21.2.2 I got the following output for in the linkerd-viz-data-plane section:

linkerd-viz-data-plane
----------------------
√ data plane namespace exists
√ data plane proxy metrics are present in Prometheus
E0218 18:01:41.009155   67200 portforward.go:385] error copying from local connection to remote stream: read tcp6 [::1]:58271->[::1]:58273: read: connection reset by peer
‼ data-plane pods have tap enabled
    Some data plane pods do not have tap configured and cannot be tapped:
	* linkerd-web-7cd9784d4-7rnxb
	* tap-injector-779c586c89-9gjnw
    see https://linkerd.io/checks/#l5d-viz-data-plane-tap for hints

Status check results are √

The in-the-moment experience of running the check was somewhat jarring. I typed linkerd check --proxy and when the check got to the linkerd-viz-data-plane section, the check hung for ~5 seconds and I saw:

E0218 18:07:59.040139   67306 portforward.go:385] error copying from local connection to remote stream: read tcp6 [::1]:58356->[::1]:58358: read: connection reset by peer

before the check printed the explanation. Initially, I was also unclear whether the warning was concerning or not (@adleong filled me in on the context).

As a user, it would be great to see something other than the error copying... message if this is not an error within my configuration, OR documentation about the right way to set things up to avoid this message.

[kleimkuhler]

@scottcarol Are you consistently seeing error copying from local connection to remote stream: read tcp6 [::1]:58271->[::1]:58273: read: connection reset by peer?

This should not be printing as part of that check, so I think this may be related to a cluster issue rather than a check issue. For example on a fresh install I don't see this:

linkerd-viz-data-plane
----------------------
√ data plane namespace exists
√ data plane proxy metrics are present in Prometheus
‼ data-plane pods have tap enabled
    Some data plane pods do not have tap configured and cannot be tapped:
        * linkerd-identity-fd8d448f9-w7fg7
        * linkerd-proxy-injector-56d7b98d4c-vl9rk
...

To warn users that they will not be able to tap these pods we are only looking at each of the pod's annotations; we are not actually trying to establish a tap connection or anything.

[scottcarol]

Thanks for the quick response @kleimkuhler -- you're right, that error and hanging was an issue with my cluster. I think the remaining issue is that it's a little confusing to read a warning:

    Some data plane pods do not have tap configured and cannot be tapped:

instead of a clean bill of health when I'm running linkerd check --proxy since I'm expecting that if I follow the install/upgrade instructions for Linkerd that installation should pass all of the checks by default.

[kleimkuhler]

Would a message that describes the warning better be more helpful? For example, if it was something along the lines of: The following pods were running before the Viz extension was installed; in order for tap to work they should be restarted so that the tap-injector can configure them correctly.

The intention here is that when running check --proxy, the user is already looking at the more detailed configuration about their cluster. In doing so, we want to give them a heads up that tap will not work on pods that were not injected by the tap-injector. Since it's just a warning—not an error—and not part of the check output, we thought it may be helpful to include.

Similarly, after installing the Jaeger extension and running check --proxy, it will give similar warnings that pods running being the jaeger-injector was running are not configured for tracing.

Another option we can consider is to make it only a runtime error; when a user tries to actually tap a pod an error will be returned saying it's not tap-able. There is not really an equivalent like this for tracing though.

[adleong]

Addressed this in: #5799