Skip to content

Commit

Permalink
Complete WSL implementation, refactor a few areas
Browse files Browse the repository at this point in the history
Also addresses a number of issues:
- StopHostNetworking isn't plumbed, win-sshproxy leaks on hyperv
- Wait api and print output doesn't work properly on Windows
- API forwarding doesn't work on WSL
- Terminal corruption with after start/stop on Windows
- Gvproxy is forcefully killed vs gracefully quit
- Switching rootful/rootless does not update /var/run/docker.sock on the guest
- File already closed error on init
- HyperV backend is publishing Unix sockets when it should be named pipes
- User-mode networking doesn't always work
- Stop state outside of lock boundaries
- WSL blocks parallel machined (should be supported)

[NO NEW TESTS NEEDED]

Signed-off-by: Jason T. Greene <[email protected]>
  • Loading branch information
n1hility committed Feb 11, 2024
1 parent d7cb664 commit 487219d
Show file tree
Hide file tree
Showing 44 changed files with 1,710 additions and 276 deletions.
1 change: 0 additions & 1 deletion cmd/podman/machine/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,6 @@ func setMachine(cmd *cobra.Command, args []string) error {
setOpts.DiskSize = &newDiskSizeGB
}
if cmd.Flags().Changed("user-mode-networking") {
// TODO This needs help
setOpts.UserModeNetworking = &setFlags.UserModeNetworking
}
if cmd.Flags().Changed("usb") {
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ require (
github.com/containers/ocicrypt v1.1.9
github.com/containers/psgo v1.9.0
github.com/containers/storage v1.52.1-0.20240202181245-1419a5980565
github.com/containers/winquit v1.1.0
github.com/coreos/go-systemd/v22 v22.5.1-0.20231103132048-7d375ecc2b09
github.com/coreos/stream-metadata-go v0.4.4
github.com/crc-org/crc/v2 v2.32.0
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ github.com/containers/psgo v1.9.0 h1:eJ74jzSaCHnWt26OlKZROSyUyRcGDf+gYBdXnxrMW4g
github.com/containers/psgo v1.9.0/go.mod h1:0YoluUm43Mz2UnBIh1P+6V6NWcbpTL5uRtXyOcH0B5A=
github.com/containers/storage v1.52.1-0.20240202181245-1419a5980565 h1:Gcirfx2DNoayB/+ypLgl5+ABzIPPDAoncs1qgZHHQHE=
github.com/containers/storage v1.52.1-0.20240202181245-1419a5980565/go.mod h1:2E/QBqWVcJXwumP7nVUrampwRNL4XKjHL/aQya7ZdhI=
github.com/containers/winquit v1.1.0 h1:jArun04BNDQvt2W0Y78kh9TazN2EIEMG5Im6/JY7+pE=
github.com/containers/winquit v1.1.0/go.mod h1:PsPeZlnbkmGGIToMPHF1zhWjBUkd8aHjMOr/vFcPxw8=
github.com/coreos/go-oidc/v3 v3.9.0 h1:0J/ogVOd4y8P0f0xUh8l9t07xRP/d8tccvjHl2dcsSo=
github.com/coreos/go-oidc/v3 v3.9.0/go.mod h1:rTKz2PYwftcrtoCzV5g5kvfJoWcm0Mk8AF8y1iAQro4=
github.com/coreos/go-systemd v0.0.0-20190719114852-fd7a80b32e1f h1:JOrtw2xFKzlg+cbHpyrpLDmnN1HqhBfnX7WDiW7eG2c=
Expand Down
10 changes: 9 additions & 1 deletion pkg/machine/applehv/stubber.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,14 @@ type AppleHVStubber struct {
}

func (a AppleHVStubber) UserModeNetworkEnabled(_ *vmconfigs.MachineConfig) bool {
return true
}

func (a AppleHVStubber) UseProviderNetworkSetup() bool {
return false
}

func (a AppleHVStubber) RequireExclusiveActive() bool {
return true
}

Expand Down Expand Up @@ -319,7 +327,7 @@ func (a AppleHVStubber) PrepareIgnition(_ *vmconfigs.MachineConfig, _ *ignition.
return nil, nil
}

func (a AppleHVStubber) PostStartNetworking(mc *vmconfigs.MachineConfig) error {
func (a AppleHVStubber) PostStartNetworking(mc *vmconfigs.MachineConfig, noInfo bool) error {
return nil
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/machine/compression/decompress.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ func Decompress(localPath *define.VMFile, uncompressedPath string) error {
return err
}
defer func() {
if err := uncompressedFileWriter.Close(); err != nil {
logrus.Errorf("unable to to close decompressed file %s: %q", uncompressedPath, err)
if err := uncompressedFileWriter.Close(); err != nil && !errors.Is(err, os.ErrClosed) {
logrus.Warnf("unable to close decompressed file %s: %q", uncompressedPath, err)
}
}()
sourceFile, err := localPath.Read()
Expand Down
46 changes: 45 additions & 1 deletion pkg/machine/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,11 +333,55 @@ func NewVirtualization(artifact define.Artifact, compression compression.ImageCo
}
}

func dialSocket(socket string, timeout time.Duration) (net.Conn, error) {
scheme := "unix"
if strings.Contains(socket, "://") {
url, err := url.Parse(socket)
if err != nil {
return nil, err
}
scheme = url.Scheme
socket = url.Path
}

ctx, cancel := context.WithTimeout(context.Background(), timeout)
defer cancel()
var dial func() (net.Conn, error)
switch scheme {
default:
fallthrough
case "unix":
dial = func() (net.Conn, error) {
var dialer net.Dialer
return dialer.DialContext(ctx, "unix", socket)
}
case "npipe":
dial = func() (net.Conn, error) {
return DialNamedPipe(ctx, socket)
}
}

backoff := 500 * time.Millisecond
for {
conn, err := dial()
if !errors.Is(err, os.ErrNotExist) {
return conn, err
}

select {
case <-time.After(backoff):
backoff *= 2
case <-ctx.Done():
return nil, ctx.Err()
}
}
}

func WaitAndPingAPI(sock string) {
client := http.Client{
Transport: &http.Transport{
DialContext: func(context.Context, string, string) (net.Conn, error) {
con, err := net.DialTimeout("unix", sock, apiUpTimeout)
con, err := dialSocket(sock, apiUpTimeout)
if err != nil {
return nil, err
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/machine/e2e/machine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package e2e_test

import (
"fmt"
"github.com/containers/podman/v4/pkg/machine/wsl"
"io"
url2 "net/url"
"os"
Expand All @@ -13,6 +12,8 @@ import (
"testing"
"time"

"github.com/containers/podman/v5/pkg/machine/wsl"

"github.com/containers/podman/v5/pkg/machine"
"github.com/containers/podman/v5/pkg/machine/compression"
"github.com/containers/podman/v5/pkg/machine/define"
Expand Down
47 changes: 0 additions & 47 deletions pkg/machine/gvproxy.go
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
package machine

import (
"errors"
"fmt"
"runtime"
"strconv"
"syscall"
"time"

"github.com/containers/podman/v5/pkg/machine/define"
psutil "github.com/shirou/gopsutil/v3/process"
"github.com/sirupsen/logrus"
)

const (
Expand Down Expand Up @@ -39,49 +35,6 @@ func backoffForProcess(p *psutil.Process) error {
return fmt.Errorf("process %d has not ended", p.Pid)
}

// waitOnProcess takes a pid and sends a sigterm to it. it then waits for the
// process to not exist. if the sigterm does not end the process after an interval,
// then sigkill is sent. it also waits for the process to exit after the sigkill too.
func waitOnProcess(processID int) error {
logrus.Infof("Going to stop gvproxy (PID %d)", processID)

p, err := psutil.NewProcess(int32(processID))
if err != nil {
return fmt.Errorf("looking up PID %d: %w", processID, err)
}

// Try to kill the pid with sigterm
if runtime.GOOS != "windows" { // FIXME: temporary work around because signals are lame in windows
if err := p.SendSignal(syscall.SIGTERM); err != nil {
if errors.Is(err, syscall.ESRCH) {
return nil
}
return fmt.Errorf("sending SIGTERM to grproxy: %w", err)
}

if err := backoffForProcess(p); err == nil {
return nil
}
}

running, err := p.IsRunning()
if err != nil {
return fmt.Errorf("checking if gvproxy is running: %w", err)
}
if !running {
return nil
}

if err := p.Kill(); err != nil {
if errors.Is(err, syscall.ESRCH) {
logrus.Debugf("Gvproxy already dead, exiting cleanly")
return nil
}
return err
}
return backoffForProcess(p)
}

// CleanupGVProxy reads the --pid-file for gvproxy attempts to stop it
func CleanupGVProxy(f define.VMFile) error {
gvPid, err := f.Read()
Expand Down
41 changes: 41 additions & 0 deletions pkg/machine/gvproxy_unix.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
//go:build darwin || dragonfly || freebsd || linux || netbsd || openbsd

package machine

import (
"errors"
"fmt"
"syscall"

psutil "github.com/shirou/gopsutil/v3/process"
"github.com/sirupsen/logrus"
)

// / waitOnProcess takes a pid and sends a sigterm to it. it then waits for the
// process to not exist. if the sigterm does not end the process after an interval,
// then sigkill is sent. it also waits for the process to exit after the sigkill too.
func waitOnProcess(processID int) error {
logrus.Infof("Going to stop gvproxy (PID %d)", processID)

p, err := psutil.NewProcess(int32(processID))
if err != nil {
return fmt.Errorf("looking up PID %d: %w", processID, err)
}

running, err := p.IsRunning()
if err != nil {
return fmt.Errorf("checking if gvproxy is running: %w", err)
}
if !running {
return nil
}

if err := p.Kill(); err != nil {
if errors.Is(err, syscall.ESRCH) {
logrus.Debugf("Gvproxy already dead, exiting cleanly")
return nil
}
return err
}
return backoffForProcess(p)
}
42 changes: 42 additions & 0 deletions pkg/machine/gvproxy_windows.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package machine

import (
"os"
"time"

"github.com/containers/winquit/pkg/winquit"
"github.com/sirupsen/logrus"
)

func waitOnProcess(processID int) error {
logrus.Infof("Going to stop gvproxy (PID %d)", processID)

p, err := os.FindProcess(processID)
if err != nil {
return nil
}

// Gracefully quit and force kill after 30 seconds
if err := winquit.QuitProcess(processID, 30*time.Second); err != nil {
return err
}

logrus.Debugf("completed grace quit || kill of gvproxy (PID %d)", processID)

// Make sure the process is gone
done := make(chan struct{})
go func() {
_, _ = p.Wait()
done <- struct{}{}
}()

select {
case <-done:
logrus.Debugf("verified gvproxy termination (PID %d)", processID)
case <-time.After(10 * time.Second):
// Very unlikely but track just in case
logrus.Errorf("was not able to kill gvproxy (PID %d)", processID)
}

return nil
}
29 changes: 9 additions & 20 deletions pkg/machine/hyperv/stubber.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,14 @@ func (h HyperVStubber) UserModeNetworkEnabled(mc *vmconfigs.MachineConfig) bool
return true
}

func (h HyperVStubber) UseProviderNetworkSetup() bool {
return false
}

func (h HyperVStubber) RequireExclusiveActive() bool {
return true
}

func (h HyperVStubber) CreateVM(opts define.CreateVMOpts, mc *vmconfigs.MachineConfig, builder *ignition.IgnitionBuilder) error {
var (
err error
Expand Down Expand Up @@ -374,7 +382,7 @@ func (h HyperVStubber) PrepareIgnition(mc *vmconfigs.MachineConfig, ignBuilder *
return &ignOpts, nil
}

func (h HyperVStubber) PostStartNetworking(mc *vmconfigs.MachineConfig) error {
func (h HyperVStubber) PostStartNetworking(mc *vmconfigs.MachineConfig, noInfo bool) error {
var (
err error
executable string
Expand All @@ -383,25 +391,6 @@ func (h HyperVStubber) PostStartNetworking(mc *vmconfigs.MachineConfig) error {
defer callbackFuncs.CleanIfErr(&err)
go callbackFuncs.CleanOnSignal()

winProxyOpts := machine.WinProxyOpts{
Name: mc.Name,
IdentityPath: mc.SSH.IdentityPath,
Port: mc.SSH.Port,
RemoteUsername: mc.SSH.RemoteUsername,
Rootful: mc.HostUser.Rootful,
VMType: h.VMType(),
}
// TODO Should this process be fatal on error; currenty, no error is
// returned but an error can occur in the func itself
// TODO we do not currently pass "noinfo" (quiet) into the StartVM
// func so this is hard set to false
machine.LaunchWinProxy(winProxyOpts, false)

winProxyCallbackFunc := func() error {
return machine.StopWinProxy(mc.Name, h.VMType())
}
callbackFuncs.Add(winProxyCallbackFunc)

if len(mc.Mounts) != 0 {
var (
dirs *define.MachineDirs
Expand Down
Loading

0 comments on commit 487219d

Please sign in to comment.