Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes issue #3558 kubeconfig using server paths does not work with tap #3640

Open

Conversation

@pietervogelaar
Copy link

pietervogelaar commented Oct 25, 2019

Signed-off-by: Pieter Vogelaar pieter@pietervogelaar.nl

@grampelberg This pull request fixes issue #3558.

Signed-off-by: Pieter Vogelaar <pieter@pietervogelaar.nl>
@@ -37,7 +38,7 @@ func Reader(k8sAPI *k8s.KubernetesAPI, req *pb.TapByResourceRequest, timeout tim
if err != nil {
return nil, nil, err
}
url.Path = protohttp.TapReqToURL(req)
url.Path = path.Join(url.Path, protohttp.TapReqToURL(req))

This comment has been minimized.

Copy link
@adleong

adleong Oct 25, 2019

Member

This looks good but I wonder if we're going to run into this again for any other requests we make directly to the Kubernetes API server in the future. Do you think we could have a more general solution by adding a RoundTripper to the client in k8sAPI.NewClient() which prepends url path prefix to request paths?

This comment has been minimized.

Copy link
@grampelberg

grampelberg Oct 25, 2019

Member

That already works correctly. This specific instance doesn’t work because we’re building the URL specific to tap requests and replacing the already constructed one by NewClient() wholesale here.

This comment has been minimized.

Copy link
@adleong

adleong Oct 25, 2019

Member

ah, you're right! thanks!

Copy link
Member

adleong left a comment

Nice!

@ihcsim

This comment has been minimized.

Copy link
Member

ihcsim commented Oct 26, 2019

@pietervogelaar While testing this branch with my local Rancher v2.3.1, I am getting an error with tap using the following command: linkerd tap -n <ns> deploy. Notice that only the resource kind is provided. The error is coming from here. Can you see if it happens on your end too? I can also reproduce the same error on my kind cluster, with this branch.

Steps to repro:

 ⚡  bin/linkerd version                                                                             
Client version: git-a848bc04
Server version: git-a848bc04

⚡  bin/linkerd tap -n linkerd deploy/linkerd-controller                                          
req id=3:0 proxy=in  src=192.168.1.66:35676 dst=10.42.0.17:9996 tls=not_provided_by_remote :method=GET :authority=10.42.0.17:9996 :path=/ping
rsp id=3:0 proxy=in  src=192.168.1.66:35676 dst=10.42.0.17:9996 tls=not_provided_by_remote :status=200 latency=308µs
end id=3:0 proxy=in  src=192.168.1.66:35676 dst=10.42.0.17:9996 tls=not_provided_by_remote duration=20µs response-length=5B
req id=3:1 proxy=in  src=192.168.1.66:37542 dst=10.42.0.17:9995 tls=not_provided_by_remote :method=GET :authority=10.42.0.17:9995 :path=/ping
rsp id=3:1 proxy=in  src=192.168.1.66:37542 dst=10.42.0.17:9995 tls=not_provided_by_remote :status=200 latency=394µs
end id=3:1 proxy=in  src=192.168.1.66:37542 dst=10.42.0.17:9995 tls=not_provided_by_remote duration=21µs response-length=5B

⚡  bin/linkerd tap -n linkerd deploy                                                          
Error: HTTP error, status Code [404] (unexpected API response: {
  "paths": [
    "/apis",
    "/apis/tap.linkerd.io",
    "/apis/tap.linkerd.io/v1alpha1",
    "/healthz",
    "/healthz/log",
    "/healthz/ping",
    "/metrics",
    "/openapi/v2",
    "/version"
  ]
})

For reasons unknown to me, on Rancher, this works:

https://<rancher-server-ip>/k8s/clusters/c-b9hjr/apis/tap.linkerd.io/v1alpha1/watch/namespaces/linkerd/deployments/linkerd-controller/tap

But this doesn't:

https://<rancher-server-ip>/k8s/clusters/c-b9hjr/apis/tap.linkerd.io/v1alpha1/watch/namespaces/linkerd/deployments/tap

Notice linkerd-controller is missing in the 2nd URL.

For posterity, this is what I have to do to get Rancher to run locally as Docker containers 😅 :

  1. Make sure my local /etc/resolv.conf name server is set to a non-local one so that the CoreDNS pod doesn't crash (because Docker DNS)
  2. Set up the Rancher server URL as my-private-ip.nip.io. May not be necessary if the containers are using host network, in which case localhost might work.
  3. For kubectl proxy to work, we have to include the Rancher-generated k8s cluster ID in the URL. E.g. curl -v localhost:8001/k8s/clusters/c-b9hjr/apis/tap.linkerd.io/v1alpha1.

LMK if you see the same error.

@pietervogelaar

This comment has been minimized.

Copy link
Author

pietervogelaar commented Oct 29, 2019

@ihcsim Yes, I experience the very same. My Rancher version is v2.2.9.

If I do docker logs kube-apiserver on a Rancher managed Kubernetes node:

I1029 15:46:52.720678       1 controller.go:107] OpenAPI AggregationController: Processing item v1alpha1.tap.linkerd.io
E1029 15:46:53.396759       1 controller.go:114] loading OpenAPI spec for "v1alpha1.tap.linkerd.io" failed with: ERROR $root.paths./apis/tap.linkerd.io/v1alpha1.get is missing required property: responses
ERROR $root.paths./apis/tap.linkerd.io.get is missing required property: responses
I1029 15:46:53.397021       1 controller.go:127] OpenAPI AggregationController: action for item v1alpha1.tap.linkerd.io: Rate Limited Requeue.

It's not in the log directly when I execute the tap command, but it may be related.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.