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

Implement gvproxy networking using cmdline wrapper #19723

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ require (
github.com/containers/buildah v1.32.0
github.com/containers/common v0.56.1-0.20230919073449-d1d9d38d8282
github.com/containers/conmon v2.0.20+incompatible
github.com/containers/gvisor-tap-vsock v0.7.1-0.20230907154503-507f56851e8b
github.com/containers/image/v5 v5.28.0
github.com/containers/libhvee v0.4.1-0.20230905135638-56fb23533417
github.com/containers/ocicrypt v1.1.8
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,8 @@ github.com/containers/conmon v2.0.20+incompatible h1:YbCVSFSCqFjjVwHTPINGdMX1F6J
github.com/containers/conmon v2.0.20+incompatible/go.mod h1:hgwZ2mtuDrppv78a/cOBNiCm6O0UMWGx1mu7P00nu5I=
github.com/containers/image/v5 v5.28.0 h1:H4cWbdI88UA/mDb6SxMo3IxpmS1BSs/Kifvhwt9g048=
github.com/containers/image/v5 v5.28.0/go.mod h1:9aPnNkwHNHgGl9VlQxXEshvmOJRbdRAc1rNDD6sP2eU=
github.com/containers/gvisor-tap-vsock v0.7.1-0.20230907154503-507f56851e8b h1:1nH+VNnclNbAXwcsK2MkLT3gFWfbxasQOPBkj1DUmCI=
github.com/containers/gvisor-tap-vsock v0.7.1-0.20230907154503-507f56851e8b/go.mod h1:n7xPNoTHhif+K4S9zZMUmBNUvt5tcLfkHlQHreTLomM=
github.com/containers/libhvee v0.4.1-0.20230905135638-56fb23533417 h1:fr+j21PD+IYR6Kvlf2Zrm1x9oAjV12T2Vz3oZIGTusw=
github.com/containers/libhvee v0.4.1-0.20230905135638-56fb23533417/go.mod h1:HiXu8GZyjzGjU834fROO00Ka/4B1IM8qxy/6q6x1f+4=
github.com/containers/libtrust v0.0.0-20230121012942-c1716e8a8d01 h1:Qzk5C6cYglewc+UyGf6lc8Mj2UaPTHy/iF2De0/77CA=
Expand Down
54 changes: 26 additions & 28 deletions pkg/machine/applehv/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"time"

"github.com/containers/common/pkg/config"
gvproxy "github.com/containers/gvisor-tap-vsock/pkg/types"
"github.com/containers/podman/v4/libpod/define"
"github.com/containers/podman/v4/pkg/machine"
"github.com/containers/podman/v4/pkg/util"
Expand Down Expand Up @@ -803,21 +804,19 @@ func getVMInfos() ([]*machine.ListResponse, error) {
// setupStartHostNetworkingCmd generates the cmd that will be used to start the
// host networking. Includes the ssh port, gvproxy pid file, gvproxy socket, and
// a debug flag depending on the logrus log level
func (m *MacMachine) setupStartHostNetworkingCmd(gvProxyBinary, forwardSock string, state machine.APIForwardingState) []string {
cmd := []string{gvProxyBinary}
// Add the ssh port
cmd = append(cmd, []string{"-ssh-port", fmt.Sprintf("%d", m.Port)}...)
// Add pid file
cmd = append(cmd, "-pid-file", m.GvProxyPid.GetPath())
// Add vfkit proxy listen
cmd = append(cmd, "-listen-vfkit", fmt.Sprintf("unixgram://%s", m.GvProxySock.GetPath()))
cmd, forwardSock, state = m.setupAPIForwarding(cmd)
if logrus.GetLevel() == logrus.DebugLevel {
cmd = append(cmd, "--debug")
fmt.Println(cmd)
}

return cmd
func (m *MacMachine) setupStartHostNetworkingCmd() (gvproxy.GvproxyCommand, string, machine.APIForwardingState) {
cmd := gvproxy.NewGvproxyCommand()
cmd.SSHPort = m.Port
cmd.PidFile = m.GvProxyPid.GetPath()
cmd.AddVfkitSocket(fmt.Sprintf("unixgram://%s", m.GvProxySock.GetPath()))
cmd.Debug = logrus.IsLevelEnabled(logrus.DebugLevel)

cmd, forwardSock, state := m.setupAPIForwarding(cmd)
if cmd.Debug {
logrus.Debug(cmd.ToCmdline())
}

return cmd, forwardSock, state
}

func (m *MacMachine) startHostNetworking(ioEater *os.File) (string, machine.APIForwardingState, error) {
Expand Down Expand Up @@ -855,23 +854,22 @@ func (m *MacMachine) startHostNetworking(ioEater *os.File) (string, machine.APIF
return "", machine.NoForwarding, err
}

attr := new(os.ProcAttr)
gvproxy, err := cfg.FindHelperBinary("gvproxy", false)
gvproxyBinary, err := cfg.FindHelperBinary("gvproxy", false)
if err != nil {
return "", 0, err
}

attr.Files = []*os.File{ioEater, ioEater, ioEater}
cmd := m.setupStartHostNetworkingCmd(gvproxy, forwardSock, state)

_, err = os.StartProcess(cmd[0], cmd, attr)
if err != nil {
return "", 0, fmt.Errorf("unable to execute: %q: %w", cmd, err)
cmd, forwardSock, state := m.setupStartHostNetworkingCmd()
c := cmd.Cmd(gvproxyBinary)
c.ExtraFiles = []*os.File{ioEater, ioEater, ioEater}
if err := c.Start(); err != nil {
return "", 0, fmt.Errorf("unable to execute: %q: %w", cmd.ToCmdline(), err)
}

return forwardSock, state, nil
}

func (m *MacMachine) setupAPIForwarding(cmd []string) ([]string, string, machine.APIForwardingState) {
func (m *MacMachine) setupAPIForwarding(cmd gvproxy.GvproxyCommand) (gvproxy.GvproxyCommand, string, machine.APIForwardingState) {
socket, err := m.forwardSocketPath()
if err != nil {
return cmd, "", machine.NoForwarding
Expand All @@ -885,10 +883,10 @@ func (m *MacMachine) setupAPIForwarding(cmd []string) ([]string, string, machine
forwardUser = "root"
}

cmd = append(cmd, []string{"-forward-sock", socket.GetPath()}...)
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())
Copy link
Member

Choose a reason for hiding this comment

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

i wonder ... should we follow a pattern here like:

cmd := gvproxy.Command().AddForwardSock(...).AddForwardDest(...)...

and then if we must afterwards:

cmd.foo = "bar"

Copy link
Member Author

Choose a reason for hiding this comment

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

We can definitely do that if you don't mind that it blocks this PR

Copy link
Member Author

@jakecorrenti jakecorrenti Sep 7, 2023

Choose a reason for hiding this comment

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

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()

cmd.AddForwardDest(destSock)
cmd.AddForwardUser(forwardUser)
cmd.AddForwardIdentity(m.IdentityPath)

link, err := m.userGlobalSocketLink()
if err != nil {
Expand Down
36 changes: 18 additions & 18 deletions pkg/machine/hyperv/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"time"

"github.com/containers/common/pkg/config"
gvproxy "github.com/containers/gvisor-tap-vsock/pkg/types"
"github.com/containers/libhvee/pkg/hypervctl"
"github.com/containers/podman/v4/pkg/machine"
"github.com/containers/podman/v4/pkg/util"
Expand Down Expand Up @@ -599,7 +600,6 @@ func (m *HyperVMachine) startHostNetworking() (string, machine.APIForwardingStat
return "", machine.NoForwarding, err
}

attr := new(os.ProcAttr)
dnr, dnw, err := machine.GetDevNullFiles()
if err != nil {
return "", machine.NoForwarding, err
Expand All @@ -616,31 +616,31 @@ func (m *HyperVMachine) startHostNetworking() (string, machine.APIForwardingStat
}
}()

gvproxy, err := cfg.FindHelperBinary("gvproxy.exe", false)
gvproxyBinary, err := cfg.FindHelperBinary("gvproxy.exe", false)
if err != nil {
return "", 0, err
}

attr.Files = []*os.File{dnr, dnw, dnw}
cmd := []string{gvproxy}
// Add the ssh port
cmd = append(cmd, []string{"-ssh-port", fmt.Sprintf("%d", m.Port)}...)
cmd = append(cmd, []string{"-listen", fmt.Sprintf("vsock://%s", m.NetworkHVSock.KeyName)}...)
cmd = append(cmd, "-pid-file", m.GvProxyPid.GetPath())
cmd := gvproxy.NewGvproxyCommand()
cmd.SSHPort = m.Port
cmd.AddEndpoint(fmt.Sprintf("vsock://%s", m.NetworkHVSock.KeyName))
cmd.PidFile = m.GvProxyPid.GetPath()

cmd, forwardSock, state = m.setupAPIForwarding(cmd)
if logrus.GetLevel() == logrus.DebugLevel {
cmd = append(cmd, "--debug")
fmt.Println(cmd)
if logrus.IsLevelEnabled(logrus.DebugLevel) {
cmd.Debug = true
logrus.Debug(cmd)
}
_, err = os.StartProcess(cmd[0], cmd, attr)
if err != nil {

c := cmd.Cmd(gvproxyBinary)
c.ExtraFiles = []*os.File{dnr, dnw, dnw}
if err := c.Start(); err != nil {
return "", 0, fmt.Errorf("unable to execute: %q: %w", cmd, err)
}
return forwardSock, state, nil
}

func (m *HyperVMachine) setupAPIForwarding(cmd []string) ([]string, string, machine.APIForwardingState) {
func (m *HyperVMachine) setupAPIForwarding(cmd gvproxy.GvproxyCommand) (gvproxy.GvproxyCommand, string, machine.APIForwardingState) {
socket, err := m.forwardSocketPath()
if err != nil {
return cmd, "", machine.NoForwarding
Expand All @@ -654,10 +654,10 @@ func (m *HyperVMachine) setupAPIForwarding(cmd []string) ([]string, string, mach
forwardUser = "root"
}

cmd = append(cmd, []string{"-forward-sock", socket.GetPath()}...)
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())
cmd.AddForwardDest(destSock)
cmd.AddForwardUser(forwardUser)
cmd.AddForwardIdentity(m.IdentityPath)

return cmd, "", machine.MachineLocal
}
Expand Down
35 changes: 18 additions & 17 deletions pkg/machine/qemu/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"time"

"github.com/containers/common/pkg/config"
gvproxy "github.com/containers/gvisor-tap-vsock/pkg/types"
"github.com/containers/podman/v4/pkg/machine"
"github.com/containers/podman/v4/pkg/rootless"
"github.com/containers/podman/v4/pkg/util"
Expand Down Expand Up @@ -1325,7 +1326,6 @@ func (v *MachineVM) startHostNetworking() (string, machine.APIForwardingState, e
return "", machine.NoForwarding, err
}

attr := new(os.ProcAttr)
dnr, dnw, err := machine.GetDevNullFiles()
if err != nil {
return "", machine.NoForwarding, err
Expand All @@ -1334,30 +1334,31 @@ func (v *MachineVM) startHostNetworking() (string, machine.APIForwardingState, e
defer dnr.Close()
defer dnw.Close()

attr.Files = []*os.File{dnr, dnw, dnw}
cmd := []string{binary}
cmd = append(cmd, []string{"-listen-qemu", fmt.Sprintf("unix://%s", v.QMPMonitor.Address.GetPath()), "-pid-file", v.PidFilePath.GetPath()}...)
// Add the ssh port
cmd = append(cmd, []string{"-ssh-port", fmt.Sprintf("%d", v.Port)}...)
cmd := gvproxy.NewGvproxyCommand()
cmd.AddQemuSocket(fmt.Sprintf("unix://%s", v.QMPMonitor.Address.GetPath()))
cmd.PidFile = v.PidFilePath.GetPath()
cmd.SSHPort = v.Port

var forwardSock string
var state machine.APIForwardingState
if !v.isIncompatible() {
cmd, forwardSock, state = v.setupAPIForwarding(cmd)
}

if logrus.GetLevel() == logrus.DebugLevel {
cmd = append(cmd, "--debug")
fmt.Println(cmd)
if logrus.IsLevelEnabled(logrus.DebugLevel) {
cmd.Debug = true
logrus.Debug(cmd)
}
_, err = os.StartProcess(cmd[0], cmd, attr)
if err != nil {
return "", 0, fmt.Errorf("unable to execute: %q: %w", cmd, err)

c := cmd.Cmd(binary)
c.ExtraFiles = []*os.File{dnr, dnw, dnw}
if err := c.Start(); err != nil {
return "", 0, fmt.Errorf("unable to execute: %q: %w", cmd.ToCmdline(), err)
}
return forwardSock, state, nil
}

func (v *MachineVM) setupAPIForwarding(cmd []string) ([]string, string, machine.APIForwardingState) {
func (v *MachineVM) setupAPIForwarding(cmd gvproxy.GvproxyCommand) (gvproxy.GvproxyCommand, string, machine.APIForwardingState) {
socket, err := v.forwardSocketPath()

if err != nil {
Expand All @@ -1372,10 +1373,10 @@ func (v *MachineVM) setupAPIForwarding(cmd []string) ([]string, string, machine.
forwardUser = "root"
}

cmd = append(cmd, []string{"-forward-sock", socket.GetPath()}...)
cmd = append(cmd, []string{"-forward-dest", destSock}...)
cmd = append(cmd, []string{"-forward-user", forwardUser}...)
cmd = append(cmd, []string{"-forward-identity", v.IdentityPath}...)
cmd.AddForwardSock(socket.GetPath())
cmd.AddForwardDest(destSock)
cmd.AddForwardUser(forwardUser)
cmd.AddForwardIdentity(v.IdentityPath)

// The linking pattern is /var/run/docker.sock -> user global sock (link) -> machine sock (socket)
// This allows the helper to only have to maintain one constant target to the user, which can be
Expand Down
Loading