Implement gvproxy networking using cmdline wrapper #19723
Conversation
6c79466 to
84fee33
Compare
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: baude, jakecorrenti 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:
Approvers can indicate their approval by writing |
| cmd = append(cmd, []string{"-forward-dest", destSock}...) | ||
| cmd = append(cmd, []string{"-forward-user", forwardUser}...) | ||
| cmd = append(cmd, []string{"-forward-identity", m.IdentityPath}...) | ||
| cmd.AddForwardSock(socket.GetPath()) |
There was a problem hiding this comment.
i wonder ... should we follow a pattern here like:
cmd := gvproxy.Command().AddForwardSock(...).AddForwardDest(...)...
and then if we must afterwards:
cmd.foo = "bar"
There was a problem hiding this comment.
We can definitely do that if you don't mind that it blocks this PR
There was a problem hiding this comment.
I was giving this a go and, this is just me, but when we start chaining 2-3 functions the line gets really long. I think this is a little difficult to read when you have to go really far to the edge of the screen versus just reading straight down. Regardless, I can still implement it if it's what we want.
afaik go doesn't like it when we do something like this (which works in something like Rust):
gvproxy.NewGvproxyCommand().AddForwardSock()
.AddForwardDest()
.AddForwardIdentity()|
Do we want to ensure that we use tagged releases of gvisor in Podman releases, or is that not something we should be concerned about? |
|
containers/gvisor-tap-vsock#266 should fix the failing tests |
84fee33 to
86e40a8
Compare
86e40a8 to
f19c992
Compare
Converts the host networking code in `podman machine` to use the `GvproxyCommand` type introduced in containers/gvisor-tap-vsock#258 [NO NEW TESTS NEEDED] Signed-off-by: Jake Correnti <jakecorrenti+github@proton.me>
d2c0bf2 to
289e59e
Compare
|
LGTM |
|
/lgtm |
Removes the line in applehv and qemu `machine.go` file. These are remnants from containers#19723. This lines was written to add stdin, stdout, stderr as extra files, but that is not how `c.ExtraFiles` works (unlike `os.ProcAttr`). go source: https://cs.opensource.google/go/go/+/go1.21.1:src/os/exec/exec.go;l=147 [NO NEW TESTS NEEDED] Signed-off-by: Jake Correnti <jakecorrenti+github@proton.me>
Converts the host networking code in
podman machineto use theCommandtype introduced in containers/gvisor-tap-vsock#258[NO NEW TESTS NEEDED]
Does this PR introduce a user-facing change?