add portforwarding support to node/api #1102
Conversation
There was a problem hiding this comment.
Overall LGTM but:
- It would be great to have some form of end-to-end testing proving the feature;
- I would add a notice that SPDY is deprecated and the community has been looking - for a long while - for supporting HTTP/2 kubernetes/kubernetes#7452 (comment)
- Copy the portforward package from kubernetes repository - Add helpers to support PortForward http requests from API Server Co-authored-by: aka-somix <somix.land@null.net> Co-authored-by: Pablo Borrelli <pablo.borrelli0@gmail.com>
e81c737 to
7e4e1a9
Compare
For the first point we will try working on that, but since there is no reference of e2e test regarding the Exec helper (or at least I couldn't find any) it could take a while to figure out how to add one. For the latter, would be ok with you if we just add a TODO comment where SPDY is used that says to replace it asap since SPDY is deprecated ? |
|
@aka-somix, on the first bullet, please, take a look and report back for our own knowledge. Maybe even add a comment in the code detailing your findings. On the second bullet, yes, the TODO/notice is enough for me. |
|
@aka-somix So, based on my actual usage scenario, I made some modifications, and it is work in my environment. I make a pr to your repo. Please review my PR if it is correct or wrong. |
|
@windyear, thanks for pointing this out. Could you please open a PR in this repo so we can review/approve it? |
When I was using this virtual-kubelet#1102 about portforward, I found that the function definition of portforward had not been added to the provider's interface definition. This resulted in the portforward feature not being implemented when using virtual-kubelet as a node. please reivew this fix with the pr virtual-kubelet#1102
|
@pires , @helayoty in order to implement the e2e test for port forwarding we also require the piece of code developed by @windyear so to be able to proceed we see two possible paths:
What do you think could be the best option? |
|
The latter. |
optimize: add PortForward to the Provider interface and add the PortForward from the provider to the PodHandlerConfig.
Summary
This PR adds support for PortForwarding implementation in virtual kubelet.
Added
/internal/kubernetes/portforward)Issues involved
This should close #170 , LIQO provider has already an implementation in place for this feature, you can look at it here