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

[wip] Implement pod exec functionality #192

Closed
wants to merge 2 commits into from
Closed

Conversation

jacksgt
Copy link

@jacksgt jacksgt commented Oct 20, 2023

This is an attempt to solve #169 , unfortunately it got stuck due to limitations in the httpx library.

httpx does not support the "Connection: Upgrade" mechanism to switch the transport to SPDY or Websockets:

Here's an example of what kubectl does under the hood:

kubectl exec -n my-namespace my-pod -c my-container -v=8 -- cat /etc/passwd

I1020 08:02:52.688543    3744 round_trippers.go:463] POST https://1.2.3.4:6443/api/v1/namespaces/my-namespace/pods/my-pod/exec?command=cat+%2Fetc%2Fpasswd&container=my-container&stderr=true&stdout=true
I1020 08:02:52.688556    3744 round_trippers.go:469] Request Headers:
I1020 08:02:52.688563    3744 round_trippers.go:473]     X-Stream-Protocol-Version: v4.channel.k8s.io
I1020 08:02:52.688569    3744 round_trippers.go:473]     X-Stream-Protocol-Version: v3.channel.k8s.io
I1020 08:02:52.688574    3744 round_trippers.go:473]     X-Stream-Protocol-Version: v2.channel.k8s.io
I1020 08:02:52.688579    3744 round_trippers.go:473]     X-Stream-Protocol-Version: channel.k8s.io
I1020 08:02:52.688585    3744 round_trippers.go:473]     User-Agent: kubectl/v1.28.2 (linux/amd64) kubernetes/89a4ea3
I1020 08:02:59.791415    3744 round_trippers.go:574] Response Status: 101 Switching Protocols in 7102 milliseconds
I1020 08:02:59.791454    3744 round_trippers.go:577] Response Headers:
I1020 08:02:59.791468    3744 round_trippers.go:580]     Connection: Upgrade
I1020 08:02:59.791478    3744 round_trippers.go:580]     Upgrade: SPDY/3.1
I1020 08:02:59.791488    3744 round_trippers.go:580]     X-Stream-Protocol-Version: v4.channel.k8s.io

root:x:0:0:root:/root:/bin/bash
daemon:x:1:1:daemon:/usr/sbin:/usr/sbin/nologin
bin:x:2:2:bin:/bin:/usr/sbin/nologin
sys:x:3:3:sys:/dev:/usr/sbin/nologin
sync:x:4:65534:sync:/bin:/bin/sync
...

For more details about the communication protocol, I recommend having a look at this article: https://cloud.redhat.com/blog/executing-commands-in-pods-using-k8s-api

@github-actions github-actions bot added the kr8s label Oct 20, 2023
@codecov
Copy link

codecov bot commented Oct 20, 2023

Codecov Report

Merging #192 (8a475af) into main (ac350dc) will decrease coverage by 0.48%.
The diff coverage is 7.69%.

@@            Coverage Diff             @@
##             main     #192      +/-   ##
==========================================
- Coverage   94.91%   94.43%   -0.48%     
==========================================
  Files          25       25              
  Lines        2378     2391      +13     
==========================================
+ Hits         2257     2258       +1     
- Misses        121      133      +12     
Files Coverage Δ
kr8s/_objects.py 90.26% <7.69%> (-1.62%) ⬇️

@jacobtomlinson jacobtomlinson marked this pull request as draft October 20, 2023 09:33
@jacobtomlinson
Copy link
Member

Thanks so much for raising this @jacksgt! I've also been working on an implementation on the side but have been struggling to reverse engineer the protocol, so that blog post you linked is very interesting.

For port forwarding we use the websocket functionality in aiohttp. This handles upgrading the connection for us. In #164 I've been experimenting with replacing aiohttp's websockets with httpx-ws which is quite young, but should allow us to support trio (which aiohttp does not).

As you say kubectl uses SPDY today, but generally the Kubernetes community is looking to migrate to HTTP/2 instead so I expect the use of SPDY will go away. There are a few SPDY libraries for Python but none of them seem particularly functional, maintained or complete, so I expect we will need to stick with websockets for now.

I've converted this to a draft for now, but excited to see any progress you make.

params["command"] = command

if container:
params["container"] = container

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about defaulting to the first container right here?

params["container"] = container or self.spec.containers[0].name

@jacobtomlinson
Copy link
Member

Thanks so much @jacksgt for taking the time to work on this PR. However, I've merged #211 which supersedes this PR.

@tomchristie
Copy link

httpx does not support the "Connection: Upgrade" mechanism to switch the transport to SPDY or Websockets

Just noticed this from a backlink, and pinging in to note that httpx does support connection upgrades it's just not very well documented yet.

There's a "network_stream" extension available on the response, that allows you to deal with this use case, currently only documented in the underlying httpcore library, but also available through httpx...

client = httpx.Client()

with client.stream("GET", url, headers=headers) as response:
    if response.status != 101:
        raise Exception("Failed to upgrade", response)

    # Get the raw network stream.
    network_steam = response.extensions["network_stream"]

    # Read and write operations now available on the upgraded connection.
    ...

(On the off-chance that this might be useful for ya)

@jacobtomlinson
Copy link
Member

Thanks @tomchristie thats useful to know. We are currently using aiohttp for websocket connections (used in exec and portforward) but are looking to migrate to httpx to allow supporting trio. Our initial plan is to use httpx-ws, you can see the WIP PR #164.

@tomchristie
Copy link

to allow supporting trio

Great choice there. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants