fix: error propagation in http-connect mode

[ipochi]

If there is an error encountered by the proxy-server, such as DNS lookup, this is currently not propagated currently back to the client in http-connect mode.

This PR adds a http response, conveying the error message to the client before closing the http connection.

I'm unsure about the Status Code, hence it is merely as a placeholder for now whether the status code should be in the 400 or 500 range.

fixes: #458

[ipochi]

/cc @cheftako @jkh52

[k8s-ci-robot]

Hi @ipochi. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[ipochi]

/ok-to-test

[k8s-ci-robot]

@ipochi: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/ok-to-test

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[jkh52]

/ok-to-test

[jkh52]

Thanks for sending this PR!

Is it feasible to add a unit test around this propagation?

[ipochi]

/ok-to-test

[jkh52]

/ok-to-test

[jkh52]

/retest

[ipochi]

/retest

[ipochi]

@cheftako Need some help in understanding why the test fails ?

 go test -race -run TestFailedDial_HTTPCONN
I0330 20:35:07.750773   22278 client.go:467] "error dialing backend" error="dial tcp 127.0.0.1:59864: connect: connection refused" dialID=8674665223082153551 connectionID=1 dialAddress="127.0.0.1:59864"
E0330 20:35:07.755274   22278 server.go:937] "DIAL_RSP contains failure" err="dial tcp 127.0.0.1:59864: connect: connection refused" dialID=8674665223082153551 agentID="1ef4d2b3-2f36-4084-bb01-55d048cb588d"
E0330 20:35:07.759688   22278 tunnel.go:150] "Received failure on connection" err="read tcp [::1]:59868->[::1]:59870: use of closed network connection"
E0330 20:35:07.761144   22278 server.go:861] "Receive stream from agent read failure" err="rpc error: code = Canceled desc = context canceled"
http connect server error:  accept tcp [::]:59868: use of closed network connection
E0330 20:35:07.862339   22278 server.go:861] "Receive stream from agent read failure" err="rpc error: code = Canceled desc = context canceled"
E0330 20:35:07.862569   22278 client.go:388] "could not read stream" err="rpc error: code = Canceled desc = grpc: the client connection is closing" serverID="b300f878-cbfb-42e7-8f76-2a8c021d5ab3" agentID="1ef4d2b3-2f36-4084-bb01-55d048cb588d"
--- FAIL: TestFailedDial_HTTPCONN (0.60s)
    proxy_test.go:735: <nil>
    proxy_test.go:1035: found unexpected goroutines:
        [Goroutine 84 in state select, with net/http.(*persistConn).readLoop on top of the stack:
        goroutine 84 [select]:
        net/http.(*persistConn).readLoop(0xc000313d40)
        	/usr/local/go/src/net/http/transport.go:2213 +0x14ef
        created by net/http.(*Transport).dialConn
        	/usr/local/go/src/net/http/transport.go:1751 +0x2586

         Goroutine 85 in state select, with net/http.(*persistConn).writeLoop on top of the stack:
        goroutine 85 [select]:
        net/http.(*persistConn).writeLoop(0xc000313d40)
        	/usr/local/go/src/net/http/transport.go:2392 +0x1a9
        created by net/http.(*Transport).dialConn
        	/usr/local/go/src/net/http/transport.go:1752 +0x261a
        ]
FAIL
exit status 1
FAIL	sigs.k8s.io/apiserver-network-proxy/tests

[ipochi]

@cheftako Need some help in understanding why the test fails ?

 go test -race -run TestFailedDial_HTTPCONN
I0330 20:35:07.750773   22278 client.go:467] "error dialing backend" error="dial tcp 127.0.0.1:59864: connect: connection refused" dialID=8674665223082153551 connectionID=1 dialAddress="127.0.0.1:59864"
E0330 20:35:07.755274   22278 server.go:937] "DIAL_RSP contains failure" err="dial tcp 127.0.0.1:59864: connect: connection refused" dialID=8674665223082153551 agentID="1ef4d2b3-2f36-4084-bb01-55d048cb588d"
E0330 20:35:07.759688   22278 tunnel.go:150] "Received failure on connection" err="read tcp [::1]:59868->[::1]:59870: use of closed network connection"
E0330 20:35:07.761144   22278 server.go:861] "Receive stream from agent read failure" err="rpc error: code = Canceled desc = context canceled"
http connect server error:  accept tcp [::]:59868: use of closed network connection
E0330 20:35:07.862339   22278 server.go:861] "Receive stream from agent read failure" err="rpc error: code = Canceled desc = context canceled"
E0330 20:35:07.862569   22278 client.go:388] "could not read stream" err="rpc error: code = Canceled desc = grpc: the client connection is closing" serverID="b300f878-cbfb-42e7-8f76-2a8c021d5ab3" agentID="1ef4d2b3-2f36-4084-bb01-55d048cb588d"
--- FAIL: TestFailedDial_HTTPCONN (0.60s)
    proxy_test.go:735: <nil>
    proxy_test.go:1035: found unexpected goroutines:
        [Goroutine 84 in state select, with net/http.(*persistConn).readLoop on top of the stack:
        goroutine 84 [select]:
        net/http.(*persistConn).readLoop(0xc000313d40)
        	/usr/local/go/src/net/http/transport.go:2213 +0x14ef
        created by net/http.(*Transport).dialConn
        	/usr/local/go/src/net/http/transport.go:1751 +0x2586

         Goroutine 85 in state select, with net/http.(*persistConn).writeLoop on top of the stack:
        goroutine 85 [select]:
        net/http.(*persistConn).writeLoop(0xc000313d40)
        	/usr/local/go/src/net/http/transport.go:2392 +0x1a9
        created by net/http.(*Transport).dialConn
        	/usr/local/go/src/net/http/transport.go:1752 +0x261a
        ]
FAIL
exit status 1
FAIL	sigs.k8s.io/apiserver-network-proxy/tests

never mind, figured it out. Added a unit test as well.

/cc @jkh52 @cheftako

[cheftako]

Seems like a good change. In answer to your earlier question, I think a 5xx series error makes more sense.

[ipochi]

Seems like a good change. In answer to your earlier question, I think a 5xx series error makes more sense.

@cheftako Thank you for the review. 400 was merely a placeholder. I agree with a 5XX range as the status code.

[ipochi]

/test pull-apiserver-network-proxy-test

[cheftako]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cheftako, ipochi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [cheftako]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment