fix stream bug when use ssl#2422
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Novelfor The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
The committers listed above are authorized under a signed CLA. |
|
Welcome @Novelfor! |
598c0fc to
764cc34
Compare
|
I think this could be improved by calling poll/select only when ssl_pending is 0. |
|
/assign |
| (self.sock.sock, ), (), (), timeout) | ||
|
|
||
| if r: | ||
| if r or ssl_pending > 0: |
There was a problem hiding this comment.
Current e2e test has stream exec test. but i think it's not easy to find ssl blocking by current test code.
- The blocking is not clearly observable in the program (if the data is sent continuously, once the next chunk is sent, it will also carry over any previously blocked data).
- Setting up SSL in minikube requires additional work.
I can think about designing a reliable testing approach later, it's not easy to write a work test (current e2e case https://github.com/kubernetes-client/python/blob/master/kubernetes/e2e_test/test_client.py#L130 contain stream exec test, but it hard to reflect blocking). But I hope we can merge this first so that I don't have to keep using my own forked version on my cluster.
|
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
|
The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle rotten |
|
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /close |
|
@k8s-triage-robot: Closed this PR. DetailsIn response to this:
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-sigs/prow repository. |
What type of PR is this?
/kind bug
What this PR does / why we need it:
Stream api will block data when use ssl socket.
Which issue(s) this PR fixes:
Fixes #2414
Special notes for your reviewer:
According to this document https://docs.python.org/3/library/ssl.html#ssl-nonblocking, select() may lose some data, it only happen when use ssl socket
I test the code in my self-host kubernetes server
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: