add debugging transport for client #10032

deads2k · 2015-06-18T17:03:28Z

This dumps detailed information about REST request based on your logging level. Different levels produce different information. For instance

[deads@deads-dev-01 kubernetes]$ $kubectl get pods --v=6
I0618 13:33:42.614135   23411 debugging_client.go:120] GET https://localhost:8443/api 200 OK in 22 milliseconds
I0618 13:33:42.629394   23411 debugging_client.go:120] GET https://localhost:8443/api/v1/namespaces/foo/pods 200 OK in 14 milliseconds
NAME      READY     REASON    RESTARTS   AGE


[deads@deads-dev-01 kubernetes]$ $kubectl get pods --v=7
I0618 13:33:45.439789   23422 debugging_client.go:98] GET https://localhost:8443/api
I0618 13:33:45.439846   23422 debugging_client.go:105] Request Headers:
I0618 13:33:45.439856   23422 debugging_client.go:108]     User-Agent: kubectl/v0.18.2 (linux/amd64) kubernetes/5c41bc2
I0618 13:33:45.439866   23422 debugging_client.go:108]     Authorization: Bearer sbug3JSvS-PA3GGYvRwuQKz_n7mtxPfLRaTcO4ajFOY
I0618 13:33:45.455468   23422 debugging_client.go:123] Response Status: 200 OK in 15 milliseconds
I0618 13:33:45.455837   23422 debugging_client.go:98] GET https://localhost:8443/api/v1/namespaces/foo/pods
I0618 13:33:45.455852   23422 debugging_client.go:105] Request Headers:
I0618 13:33:45.455862   23422 debugging_client.go:108]     User-Agent: kubectl/v0.18.2 (linux/amd64) kubernetes/5c41bc2
I0618 13:33:45.455872   23422 debugging_client.go:108]     Authorization: Bearer sbug3JSvS-PA3GGYvRwuQKz_n7mtxPfLRaTcO4ajFOY
I0618 13:33:45.470806   23422 debugging_client.go:123] Response Status: 200 OK in 14 milliseconds
NAME      READY     REASON    RESTARTS   AGE


[deads@deads-dev-01 kubernetes]$ $kubectl get pods --v=8
I0618 13:33:48.018687   23434 debugging_client.go:98] GET https://localhost:8443/api
I0618 13:33:48.018751   23434 debugging_client.go:105] Request Headers:
I0618 13:33:48.018762   23434 debugging_client.go:108]     User-Agent: kubectl/v0.18.2 (linux/amd64) kubernetes/5c41bc2
I0618 13:33:48.018771   23434 debugging_client.go:108]     Authorization: Bearer sbug3JSvS-PA3GGYvRwuQKz_n7mtxPfLRaTcO4ajFOY
I0618 13:33:48.034233   23434 debugging_client.go:123] Response Status: 200 OK in 15 milliseconds
I0618 13:33:48.034265   23434 debugging_client.go:126] Response Headers:
I0618 13:33:48.034275   23434 debugging_client.go:129]     Cache-Control: no-store
I0618 13:33:48.034284   23434 debugging_client.go:129]     Content-Type: application/json
I0618 13:33:48.034293   23434 debugging_client.go:129]     Date: Thu, 18 Jun 2015 17:33:48 GMT
I0618 13:33:48.034302   23434 debugging_client.go:129]     Content-Length: 47
I0618 13:33:48.034353   23434 request.go:762] Response Body: {
  "versions": [
    "v1beta3",
    "v1"
  ]
}
I0618 13:33:48.034645   23434 debugging_client.go:98] GET https://localhost:8443/api/v1/namespaces/foo/pods
I0618 13:33:48.034659   23434 debugging_client.go:105] Request Headers:
I0618 13:33:48.034668   23434 debugging_client.go:108]     User-Agent: kubectl/v0.18.2 (linux/amd64) kubernetes/5c41bc2
I0618 13:33:48.034677   23434 debugging_client.go:108]     Authorization: Bearer sbug3JSvS-PA3GGYvRwuQKz_n7mtxPfLRaTcO4ajFOY
I0618 13:33:48.049494   23434 debugging_client.go:123] Response Status: 200 OK in 14 milliseconds
I0618 13:33:48.049516   23434 debugging_client.go:126] Response Headers:
I0618 13:33:48.049521   23434 debugging_client.go:129]     Date: Thu, 18 Jun 2015 17:33:48 GMT
I0618 13:33:48.049525   23434 debugging_client.go:129]     Content-Length: 125
I0618 13:33:48.049529   23434 debugging_client.go:129]     Cache-Control: no-store
I0618 13:33:48.049532   23434 debugging_client.go:129]     Content-Type: application/json
I0618 13:33:48.049561   23434 request.go:762] Response Body: {"kind":"PodList","apiVersion":"v1","metadata":{"selfLink":"/api/v1/namespaces/foo/pods","resourceVersion":"429"},"items":[]}
NAME      READY     REASON    RESTARTS   AGE


[deads@deads-dev-01 kubernetes]$ $kubectl get pods --v=9
I0618 13:33:54.294448   23447 debugging_client.go:101] curl -k -v -XGET  -H "User-Agent: kubectl/v0.18.2 (linux/amd64) kubernetes/5c41bc2" -H "Authorization: Bearer sbug3JSvS-PA3GGYvRwuQKz_n7mtxPfLRaTcO4ajFOY" https://localhost:8443/api
I0618 13:33:54.310532   23447 debugging_client.go:120] GET https://localhost:8443/api 200 OK in 16 milliseconds
I0618 13:33:54.310555   23447 debugging_client.go:126] Response Headers:
I0618 13:33:54.310561   23447 debugging_client.go:129]     Content-Type: application/json
I0618 13:33:54.310567   23447 debugging_client.go:129]     Date: Thu, 18 Jun 2015 17:33:54 GMT
I0618 13:33:54.310573   23447 debugging_client.go:129]     Content-Length: 47
I0618 13:33:54.310578   23447 debugging_client.go:129]     Cache-Control: no-store
I0618 13:33:54.310640   23447 request.go:762] Response Body: {
  "versions": [
    "v1beta3",
    "v1"
  ]
}
I0618 13:33:54.310968   23447 debugging_client.go:101] curl -k -v -XGET  -H "Authorization: Bearer sbug3JSvS-PA3GGYvRwuQKz_n7mtxPfLRaTcO4ajFOY" -H "User-Agent: kubectl/v0.18.2 (linux/amd64) kubernetes/5c41bc2" https://localhost:8443/api/v1/namespaces/foo/pods
I0618 13:33:54.331508   23447 debugging_client.go:120] GET https://localhost:8443/api/v1/namespaces/foo/pods 200 OK in 20 milliseconds
I0618 13:33:54.331552   23447 debugging_client.go:126] Response Headers:
I0618 13:33:54.331562   23447 debugging_client.go:129]     Cache-Control: no-store
I0618 13:33:54.331573   23447 debugging_client.go:129]     Content-Type: application/json
I0618 13:33:54.331597   23447 debugging_client.go:129]     Date: Thu, 18 Jun 2015 17:33:54 GMT
I0618 13:33:54.331607   23447 debugging_client.go:129]     Content-Length: 125
I0618 13:33:54.331654   23447 request.go:762] Response Body: {"kind":"PodList","apiVersion":"v1","metadata":{"selfLink":"/api/v1/namespaces/foo/pods","resourceVersion":"429"},"items":[]}
NAME      READY     REASON    RESTARTS   AGE

deads2k · 2015-06-18T17:05:12Z

/cc @mikedanese @smarterclayton @philips

deads2k · 2015-06-18T17:05:58Z

pkg/client/debugging_client.go

+// IsClient is a global that indicates whether or not this process is a client or not.  This is used to determined
+// whether body inspection should be allowed to disallowed.  Only clients should set this value to true.  Servers
+// should not, because body inspection breaks streams
+var IsClient = false


I want to call this out in particular. It is a global that I put it here for safety to avoid intercepting streaming input and output. Now I'm going to go hide from @smarterclayton

Body inspection is probably too dangerous to do in the transport. Just move it up into pkg/client/request.go and call it a day.

----- Original Message -----

+import (

"bytes"

"fmt"

"io/ioutil"

"net/http"

"time"

"github.com/golang/glog"

"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
+)

+// IsClient is a global that indicates whether or not this process is a
client or not. This is used to determined
+// whether body inspection should be allowed to disallowed. Only clients
should set this value to true. Servers
+// should not, because body inspection breaks streams
+var IsClient = false

I want to call this out in particular. It is a global that I put it here for
safety to avoid intercepting streaming input and output. Now I'm going to
go hide from @smarterclayton

Reply to this email directly or view it on GitHub:
https://github.com/GoogleCloudPlatform/kubernetes/pull/10032/files#r32752937

Body inspection is probably too dangerous to do in the transport. Just move it up into pkg/client/request.go and call it a day.

I'm not sure that I think it's any safer to do somewhere else. Why does it matter where it's done?

We should never inspect the returned body. Since the vast majority of use is from pkg/client/request.go, and those methods do read the body and have a []byte, it's safe there to let the debugging logic print the response. It is not safe to use a global value like this, and it's really not safe to read bodies.

----- Original Message -----

+import (

"bytes"

"fmt"

"io/ioutil"

"net/http"

"time"

"github.com/golang/glog"

"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
+)

+// IsClient is a global that indicates whether or not this process is a
client or not. This is used to determined
+// whether body inspection should be allowed to disallowed. Only clients
should set this value to true. Servers
+// should not, because body inspection breaks streams
+var IsClient = false

Body inspection is probably too dangerous to do in the transport. Just
move it up into pkg/client/request.go and call it a day.

I'm not sure that I think it's any safer to do somewhere else. Why does it
matter where it's done?

Reply to this email directly or view it on GitHub:
https://github.com/GoogleCloudPlatform/kubernetes/pull/10032/files#r32753587

deads2k · 2015-06-18T17:35:24Z

comments addressed, updated the example in the description.

mikedanese · 2015-06-18T18:45:31Z

@smarterclayton Can you give this a once over and sign off with the recent changes?

smarterclayton · 2015-06-18T18:48:28Z

Nit: pkg/client/debugging.go instead of repeating client

smarterclayton · 2015-06-18T18:48:58Z

The nit is optional, otherwise LGTM

deads2k · 2015-06-18T18:51:20Z

renamed.

mikedanese · 2015-06-18T18:51:45Z

pkg/client/helper.go

+		transport = NewDebuggingRoundTripper(transport, JustURL, RequestHeaders, ResponseStatus, ResponseHeaders)
+	case bool(glog.V(7)):
+		transport = NewDebuggingRoundTripper(transport, JustURL, RequestHeaders, ResponseStatus)
+	case bool(glog.V(6)):


I'm ignorant of glog. Will glog.V(6) be true if I pass --v=7 to the command? Can you explain how these transports aren't getting quadruple wrapped at --v=9 without breaks?

I used a switch/case, so only one case is executed.

Ahhh okay thanks. Did not realize that about go switches. I feel like that's different than other switches...

I feel like that's different than other switches...

The no explicit break is required. If you want to fallthrough, you have to ask for it.

mikedanese · 2015-06-18T19:08:27Z

LGTM

philips · 2015-06-18T19:28:50Z

lgtm

mikedanese · 2015-06-18T20:03:29Z

@k8s-bot ok to test

k8s-bot · 2015-06-18T20:35:40Z

GCE e2e build/test failed for commit bab0a61.

satnam6502 · 2015-06-18T21:13:49Z

@k8s-bot ok to test

add debugging transport for client

k8s-bot · 2015-06-18T21:32:50Z

GCE e2e build/test passed for commit bab0a61.

philips · 2015-06-19T15:39:32Z

Fantastic. Thanks for the fast movement on this one. Waaayyyyy easier to debug stuff.

googlebot added the cla: yes label Jun 18, 2015

deads2k mentioned this pull request Jun 18, 2015

implement HttpDebug and add --http-debug flag to kubectl flags #10000

Closed

deads2k reviewed Jun 18, 2015
View reviewed changes

deads2k force-pushed the client-debugging branch from 5c41bc2 to 9b20000 Compare June 18, 2015 17:34

satnam6502 assigned mikedanese Jun 18, 2015

bgrant0607 added this to the v1.0 milestone Jun 18, 2015

deads2k force-pushed the client-debugging branch from 9b20000 to 54e0280 Compare June 18, 2015 18:50

add debugging transport for client

bab0a61

deads2k force-pushed the client-debugging branch from 54e0280 to bab0a61 Compare June 18, 2015 18:51

mikedanese reviewed Jun 18, 2015
View reviewed changes

mikedanese added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 18, 2015

satnam6502 added a commit that referenced this pull request Jun 18, 2015

Merge pull request #10032 from deads2k/client-debugging

1d44d06

add debugging transport for client

satnam6502 merged commit 1d44d06 into kubernetes:master Jun 18, 2015

deads2k deleted the client-debugging branch June 19, 2015 11:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add debugging transport for client #10032

add debugging transport for client #10032

deads2k commented Jun 18, 2015

deads2k commented Jun 18, 2015

deads2k Jun 18, 2015

smarterclayton Jun 18, 2015

deads2k Jun 18, 2015

smarterclayton Jun 18, 2015

deads2k commented Jun 18, 2015

mikedanese commented Jun 18, 2015

smarterclayton commented Jun 18, 2015

smarterclayton commented Jun 18, 2015

deads2k commented Jun 18, 2015

mikedanese Jun 18, 2015

deads2k Jun 18, 2015

mikedanese Jun 18, 2015

deads2k Jun 18, 2015

mikedanese commented Jun 18, 2015

philips commented Jun 18, 2015

mikedanese commented Jun 18, 2015

k8s-bot commented Jun 18, 2015

satnam6502 commented Jun 18, 2015

k8s-bot commented Jun 18, 2015

philips commented Jun 19, 2015

add debugging transport for client #10032

add debugging transport for client #10032

Conversation

deads2k commented Jun 18, 2015

deads2k commented Jun 18, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deads2k commented Jun 18, 2015

mikedanese commented Jun 18, 2015

smarterclayton commented Jun 18, 2015

smarterclayton commented Jun 18, 2015

deads2k commented Jun 18, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikedanese commented Jun 18, 2015

philips commented Jun 18, 2015

mikedanese commented Jun 18, 2015

k8s-bot commented Jun 18, 2015

satnam6502 commented Jun 18, 2015

k8s-bot commented Jun 18, 2015

philips commented Jun 19, 2015