From 906af5bbc66608b40a22fd11f4e745777f81ac55 Mon Sep 17 00:00:00 2001 From: Jake Correnti Date: Tue, 1 Aug 2023 08:52:23 -0400 Subject: [PATCH 1/7] Move `getDevNullFiles` into a common file Moves `getDevNullFiles` into a new common file, `pkg/machine/machine_common.go`, preventing the re-implementation of the function across the different hypervisor implementations. Signed-off-by: Jake Correnti --- pkg/machine/hyperv/machine.go | 20 +------------------- pkg/machine/machine_common.go | 26 ++++++++++++++++++++++++++ pkg/machine/qemu/machine.go | 22 ++-------------------- 3 files changed, 29 insertions(+), 39 deletions(-) create mode 100644 pkg/machine/machine_common.go diff --git a/pkg/machine/hyperv/machine.go b/pkg/machine/hyperv/machine.go index 850d205f9a..5f18bb8a17 100644 --- a/pkg/machine/hyperv/machine.go +++ b/pkg/machine/hyperv/machine.go @@ -616,24 +616,6 @@ func (m *HyperVMachine) loadHyperVMachineFromJSON(fqConfigPath string) error { return json.Unmarshal(b, m) } -// getDevNullFiles returns pointers to Read-only and Write-only DevNull files -func getDevNullFiles() (*os.File, *os.File, error) { - dnr, err := os.OpenFile(os.DevNull, os.O_RDONLY, 0755) - if err != nil { - return nil, nil, err - } - - dnw, err := os.OpenFile(os.DevNull, os.O_WRONLY, 0755) - if err != nil { - if e := dnr.Close(); e != nil { - err = e - } - return nil, nil, err - } - - return dnr, dnw, nil -} - func (m *HyperVMachine) startHostNetworking() (string, machine.APIForwardingState, error) { var ( forwardSock string @@ -645,7 +627,7 @@ func (m *HyperVMachine) startHostNetworking() (string, machine.APIForwardingStat } attr := new(os.ProcAttr) - dnr, dnw, err := getDevNullFiles() + dnr, dnw, err := machine.GetDevNullFiles() if err != nil { return "", machine.NoForwarding, err } diff --git a/pkg/machine/machine_common.go b/pkg/machine/machine_common.go new file mode 100644 index 0000000000..c781dd9a74 --- /dev/null +++ b/pkg/machine/machine_common.go @@ -0,0 +1,26 @@ +//go:build amd64 || arm64 +// +build amd64 arm64 + +package machine + +import ( + "os" +) + +// getDevNullFiles returns pointers to Read-only and Write-only DevNull files +func GetDevNullFiles() (*os.File, *os.File, error) { + dnr, err := os.OpenFile(os.DevNull, os.O_RDONLY, 0755) + if err != nil { + return nil, nil, err + } + + dnw, err := os.OpenFile(os.DevNull, os.O_WRONLY, 0755) + if err != nil { + if e := dnr.Close(); e != nil { + err = e + } + return nil, nil, err + } + + return dnr, dnw, nil +} diff --git a/pkg/machine/qemu/machine.go b/pkg/machine/qemu/machine.go index 4950131ec2..b04ccc7844 100644 --- a/pkg/machine/qemu/machine.go +++ b/pkg/machine/qemu/machine.go @@ -629,24 +629,6 @@ func (v *MachineVM) connectToPodmanSocket(maxBackoffs int, backoff time.Duration return } -// getDevNullFiles returns pointers to Read-only and Write-only DevNull files -func getDevNullFiles() (*os.File, *os.File, error) { - dnr, err := os.OpenFile(os.DevNull, os.O_RDONLY, 0755) - if err != nil { - return nil, nil, err - } - - dnw, err := os.OpenFile(os.DevNull, os.O_WRONLY, 0755) - if err != nil { - if e := dnr.Close(); e != nil { - err = e - } - return nil, nil, err - } - - return dnr, dnw, nil -} - // Start executes the qemu command line and forks it func (v *MachineVM) Start(name string, opts machine.StartOptions) error { var ( @@ -739,7 +721,7 @@ func (v *MachineVM) Start(name string, opts machine.StartOptions) error { } defer fd.Close() - dnr, dnw, err := getDevNullFiles() + dnr, dnw, err := machine.GetDevNullFiles() if err != nil { return err } @@ -1336,7 +1318,7 @@ func (v *MachineVM) startHostNetworking() (string, machine.APIForwardingState, e } attr := new(os.ProcAttr) - dnr, dnw, err := getDevNullFiles() + dnr, dnw, err := machine.GetDevNullFiles() if err != nil { return "", machine.NoForwarding, err } From 850482b3145db838228215f30ae1736bf07b14eb Mon Sep 17 00:00:00 2001 From: Jake Correnti Date: Tue, 1 Aug 2023 09:53:38 -0400 Subject: [PATCH 2/7] Move alternate image acquisition to separate function Moves acquisition of an alternate image provided by the user out of `acquireVMImage` in `pkg/machine//machine.go` and into `pkg/machine/pull.go` as its own function. Signed-off-by: Jake Correnti --- pkg/machine/applehv/machine.go | 9 +-------- pkg/machine/pull.go | 20 ++++++++++++++++++++ pkg/machine/qemu/machine.go | 11 +---------- 3 files changed, 22 insertions(+), 18 deletions(-) diff --git a/pkg/machine/applehv/machine.go b/pkg/machine/applehv/machine.go index 48c76df4fa..e018d71d78 100644 --- a/pkg/machine/applehv/machine.go +++ b/pkg/machine/applehv/machine.go @@ -100,18 +100,11 @@ func (m *MacMachine) acquireVMImage(opts machine.InitOptions, dataDir string) er // The user has provided an alternate image which can be a file path // or URL. m.ImageStream = "custom" - g, err := machine.NewGenericDownloader(vmtype, m.Name, opts.ImagePath) - if err != nil { - return err - } - imagePath, err := machine.NewMachineFile(g.Get().LocalUncompressedFile, nil) + imagePath, err := machine.AcquireAlternateImage(m.Name, vmtype, opts) if err != nil { return err } m.ImagePath = *imagePath - if err := machine.DownloadImage(g); err != nil { - return err - } } return nil } diff --git a/pkg/machine/pull.go b/pkg/machine/pull.go index 113cf5da97..16abcdf794 100644 --- a/pkg/machine/pull.go +++ b/pkg/machine/pull.go @@ -385,3 +385,23 @@ func RemoveImageAfterExpire(dir string, expire time.Duration) error { }) return err } + +// AcquireAlternateImage downloads the alternate image the user provided, which +// can be a file path or URL +func AcquireAlternateImage(name string, vmtype VMType, opts InitOptions) (*VMFile, error) { + g, err := NewGenericDownloader(vmtype, name, opts.ImagePath) + if err != nil { + return nil, err + } + + imagePath, err := NewMachineFile(g.Get().LocalUncompressedFile, nil) + if err != nil { + return nil, err + } + + if err := DownloadImage(g); err != nil { + return nil, err + } + + return imagePath, nil +} diff --git a/pkg/machine/qemu/machine.go b/pkg/machine/qemu/machine.go index b04ccc7844..699fccfca6 100644 --- a/pkg/machine/qemu/machine.go +++ b/pkg/machine/qemu/machine.go @@ -225,20 +225,11 @@ func (v *MachineVM) acquireVMImage(opts machine.InitOptions) error { // The user has provided an alternate image which can be a file path // or URL. v.ImageStream = "custom" - g, err := machine.NewGenericDownloader(vmtype, v.Name, opts.ImagePath) + imagePath, err := machine.AcquireAlternateImage(v.Name, vmtype, opts) if err != nil { return err } - - imagePath, err := machine.NewMachineFile(g.Get().LocalUncompressedFile, nil) - if err != nil { - return err - } - v.ImagePath = *imagePath - if err := machine.DownloadImage(g); err != nil { - return err - } } return nil } From 55c7b5cecae2bc445a9af7c7c9a2ee8c8f8f6840 Mon Sep 17 00:00:00 2001 From: Jake Correnti Date: Tue, 1 Aug 2023 10:52:46 -0400 Subject: [PATCH 3/7] Move `addSSHConnectionsToPodmanSocket` code to shared file Moves the implementation of `addSSHConnectionsToPodmanSocket` into the common file `pkg/machine/machine_common.go`. The implementation was shared between the hypervisors and does not need to be implemented multiple times. Signed-off-by: Jake Correnti --- pkg/machine/applehv/machine.go | 39 ++++++++------------------------- pkg/machine/hyperv/machine.go | 39 ++++++++------------------------- pkg/machine/machine_common.go | 30 +++++++++++++++++++++++++ pkg/machine/qemu/machine.go | 40 ++++++++-------------------------- 4 files changed, 57 insertions(+), 91 deletions(-) diff --git a/pkg/machine/applehv/machine.go b/pkg/machine/applehv/machine.go index e018d71d78..6677712c9f 100644 --- a/pkg/machine/applehv/machine.go +++ b/pkg/machine/applehv/machine.go @@ -10,7 +10,6 @@ import ( "fmt" "io/fs" "net" - "net/url" "os" "os/exec" "path/filepath" @@ -169,34 +168,6 @@ func (m *MacMachine) addMountsToVM(opts machine.InitOptions, virtiofsMnts *[]mac return nil } -func (m *MacMachine) addSSHConnectionsToPodmanSocket(opts machine.InitOptions) error { - if len(opts.IgnitionPath) < 1 { - // TODO localhost needs to be restored here - uri := machine.SSHRemoteConnection.MakeSSHURL("localhost", fmt.Sprintf("/run/user/%d/podman/podman.sock", m.UID), strconv.Itoa(m.Port), m.RemoteUsername) - uriRoot := machine.SSHRemoteConnection.MakeSSHURL("localhost", "/run/podman/podman.sock", strconv.Itoa(m.Port), "root") - identity := m.IdentityPath - - uris := []url.URL{uri, uriRoot} - names := []string{m.Name, m.Name + "-root"} - - // The first connection defined when connections is empty will become the default - // regardless of IsDefault, so order according to rootful - if opts.Rootful { - uris[0], names[0], uris[1], names[1] = uris[1], names[1], uris[0], names[0] - } - - for i := 0; i < 2; i++ { - if err := machine.AddConnection(&uris[i], names[i], identity, opts.IsDefault && i == 0); err != nil { - return err - } - } - } else { - fmt.Println("An ignition path was provided. No SSH connection was added to Podman") - } - - return nil -} - // writeIgnitionConfigFile generates the ignition config and writes it to the filesystem func (m *MacMachine) writeIgnitionConfigFile(opts machine.InitOptions, key string, virtiofsMnts *[]machine.VirtIoFs) error { // Write the ignition file @@ -300,7 +271,15 @@ func (m *MacMachine) Init(opts machine.InitOptions) (bool, error) { return false, err } - if err := m.addSSHConnectionsToPodmanSocket(opts); err != nil { + err = machine.AddSSHConnectionsToPodmanSocket( + m.UID, + m.Port, + m.IdentityPath, + m.Name, + m.RemoteUsername, + opts, + ) + if err != nil { return false, err } diff --git a/pkg/machine/hyperv/machine.go b/pkg/machine/hyperv/machine.go index 5f18bb8a17..353b3a3502 100644 --- a/pkg/machine/hyperv/machine.go +++ b/pkg/machine/hyperv/machine.go @@ -9,10 +9,8 @@ import ( "errors" "fmt" "io/fs" - "net/url" "os" "path/filepath" - "strconv" "time" "github.com/containers/common/pkg/config" @@ -83,33 +81,6 @@ func (m *HyperVMachine) addNetworkAndReadySocketsToRegistry() error { return nil } -// addSSHConnectionsToPodmanSocket adds SSH connections to the podman socket if -// no ignition path was provided -func (m *HyperVMachine) addSSHConnectionsToPodmanSocket(opts machine.InitOptions) error { - if len(opts.IgnitionPath) < 1 { - uri := machine.SSHRemoteConnection.MakeSSHURL(machine.LocalhostIP, fmt.Sprintf("/run/user/%d/podman/podman.sock", m.UID), strconv.Itoa(m.Port), m.RemoteUsername) - uriRoot := machine.SSHRemoteConnection.MakeSSHURL(machine.LocalhostIP, "/run/podman/podman.sock", strconv.Itoa(m.Port), "root") - - uris := []url.URL{uri, uriRoot} - names := []string{m.Name, m.Name + "-root"} - - // The first connection defined when connections is empty will become the default - // regardless of IsDefault, so order according to rootful - if opts.Rootful { - uris[0], names[0], uris[1], names[1] = uris[1], names[1], uris[0], names[0] - } - - for i := 0; i < 2; i++ { - if err := machine.AddConnection(&uris[i], names[i], m.IdentityPath, opts.IsDefault && i == 0); err != nil { - return err - } - } - } else { - fmt.Println("An ignition path was provided. No SSH connection was added to Podman") - } - return nil -} - // writeIgnitionConfigFile generates the ignition config and writes it to the // filesystem func (m *HyperVMachine) writeIgnitionConfigFile(opts machine.InitOptions, user, key string) error { @@ -246,7 +217,15 @@ func (m *HyperVMachine) Init(opts machine.InitOptions) (bool, error) { } m.Port = sshPort - if err := m.addSSHConnectionsToPodmanSocket(opts); err != nil { + err = machine.AddSSHConnectionsToPodmanSocket( + m.UID, + m.Port, + m.IdentityPath, + m.Name, + m.RemoteUsername, + opts, + ) + if err != nil { return false, err } diff --git a/pkg/machine/machine_common.go b/pkg/machine/machine_common.go index c781dd9a74..ae34d771b6 100644 --- a/pkg/machine/machine_common.go +++ b/pkg/machine/machine_common.go @@ -4,7 +4,10 @@ package machine import ( + "fmt" + "net/url" "os" + "strconv" ) // getDevNullFiles returns pointers to Read-only and Write-only DevNull files @@ -24,3 +27,30 @@ func GetDevNullFiles() (*os.File, *os.File, error) { return dnr, dnw, nil } + +// AddSSHConnectionsToPodmanSocket adds SSH connections to the podman socket if +// no ignition path is provided +func AddSSHConnectionsToPodmanSocket(uid, port int, identityPath, name, remoteUsername string, opts InitOptions) error { + if len(opts.IgnitionPath) < 1 { + uri := SSHRemoteConnection.MakeSSHURL(LocalhostIP, fmt.Sprintf("/run/user/%d/podman/podman.sock", uid), strconv.Itoa(port), remoteUsername) + uriRoot := SSHRemoteConnection.MakeSSHURL(LocalhostIP, "/run/podman/podman.sock", strconv.Itoa(port), "root") + + uris := []url.URL{uri, uriRoot} + names := []string{name, name + "-root"} + + // The first connection defined when connections is empty will become the default + // regardless of IsDefault, so order according to rootful + if opts.Rootful { + uris[0], names[0], uris[1], names[1] = uris[1], names[1], uris[0], names[0] + } + + for i := 0; i < 2; i++ { + if err := AddConnection(&uris[i], names[i], identityPath, opts.IsDefault && i == 0); err != nil { + return err + } + } + } else { + fmt.Println("An ignition path was provided. No SSH connection was added to Podman") + } + return nil +} diff --git a/pkg/machine/qemu/machine.go b/pkg/machine/qemu/machine.go index 699fccfca6..bc23298a3c 100644 --- a/pkg/machine/qemu/machine.go +++ b/pkg/machine/qemu/machine.go @@ -12,7 +12,6 @@ import ( "fmt" "io/fs" "net" - "net/url" "os" "os/exec" "os/signal" @@ -266,35 +265,6 @@ func (v *MachineVM) addMountsToVM(opts machine.InitOptions) error { return nil } -// addSSHConnectionsToPodmanSocket adds SSH connections to the podman socket if -// no ignition path is provided -func (v *MachineVM) addSSHConnectionsToPodmanSocket(opts machine.InitOptions) error { - // This kind of stinks but no other way around this r/n - if len(opts.IgnitionPath) > 0 { - fmt.Println("An ignition path was provided. No SSH connection was added to Podman") - return nil - } - - uri := machine.SSHRemoteConnection.MakeSSHURL(machine.LocalhostIP, fmt.Sprintf("/run/user/%d/podman/podman.sock", v.UID), strconv.Itoa(v.Port), v.RemoteUsername) - uriRoot := machine.SSHRemoteConnection.MakeSSHURL(machine.LocalhostIP, "/run/podman/podman.sock", strconv.Itoa(v.Port), "root") - - uris := []url.URL{uri, uriRoot} - names := []string{v.Name, v.Name + "-root"} - - // The first connection defined when connections is empty will become the default - // regardless of IsDefault, so order according to rootful - if opts.Rootful { - uris[0], names[0], uris[1], names[1] = uris[1], names[1], uris[0], names[0] - } - - for i := 0; i < 2; i++ { - if err := machine.AddConnection(&uris[i], names[i], v.IdentityPath, opts.IsDefault && i == 0); err != nil { - return err - } - } - return nil -} - // writeIgnitionConfigFile generates the ignition config and writes it to the // filesystem func (v *MachineVM) writeIgnitionConfigFile(opts machine.InitOptions, key string) error { @@ -364,7 +334,15 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) { // Add location of bootable image v.CmdLine = append(v.CmdLine, "-drive", "if=virtio,file="+v.getImageFile()) - if err := v.addSSHConnectionsToPodmanSocket(opts); err != nil { + err := machine.AddSSHConnectionsToPodmanSocket( + v.UID, + v.Port, + v.IdentityPath, + v.Name, + v.RemoteUsername, + opts, + ) + if err != nil { return false, err } From 75a8f13c4a1cbe33c931c690eda4923770e1f5f2 Mon Sep 17 00:00:00 2001 From: Jake Correnti Date: Tue, 1 Aug 2023 16:00:31 -0400 Subject: [PATCH 4/7] Move `waitAPIAndPrintInfo` to common file Moves `waitAPIAndPrintInfo` into the common file `pkg/machine/machine_common.go` allowing applehv and qemu to share the code. Signed-off-by: Jake Correnti --- pkg/machine/applehv/machine.go | 76 ++++-------------------------- pkg/machine/machine_common.go | 68 +++++++++++++++++++++++++++ pkg/machine/qemu/machine.go | 86 +++++++--------------------------- 3 files changed, 95 insertions(+), 135 deletions(-) diff --git a/pkg/machine/applehv/machine.go b/pkg/machine/applehv/machine.go index 6677712c9f..68b93ae76f 100644 --- a/pkg/machine/applehv/machine.go +++ b/pkg/machine/applehv/machine.go @@ -688,7 +688,15 @@ func (m *MacMachine) Start(name string, opts machine.StartOptions) error { } logrus.Debug("ready notification received") - m.waitAPIAndPrintInfo(forwardState, forwardSock, opts.NoInfo) + machine.WaitAPIAndPrintInfo( + forwardState, + m.Name, + findClaimHelper(), + forwardSock, + opts.NoInfo, + m.isIncompatible(), + m.Rootful, + ) return nil } @@ -1074,72 +1082,6 @@ func (m *MacMachine) userGlobalSocketLink() (string, error) { return filepath.Join(filepath.Dir(path), "podman.sock"), err } -func (m *MacMachine) waitAPIAndPrintInfo(forwardState machine.APIForwardingState, forwardSock string, noInfo bool) { - suffix := "" - if m.Name != machine.DefaultMachineName { - suffix = " " + m.Name - } - - if m.isIncompatible() { - fmt.Fprintf(os.Stderr, "\n!!! ACTION REQUIRED: INCOMPATIBLE MACHINE !!!\n") - - fmt.Fprintf(os.Stderr, "\nThis machine was created by an older Podman release that is incompatible\n") - fmt.Fprintf(os.Stderr, "with this release of Podman. It has been started in a limited operational\n") - fmt.Fprintf(os.Stderr, "mode to allow you to copy any necessary files before recreating it. This\n") - fmt.Fprintf(os.Stderr, "can be accomplished with the following commands:\n\n") - fmt.Fprintf(os.Stderr, "\t# Login and copy desired files (Optional)\n") - fmt.Fprintf(os.Stderr, "\t# Podman machine ssh%s tar cvPf - /path/to/files > backup.tar\n\n", suffix) - fmt.Fprintf(os.Stderr, "\t# Recreate machine (DESTRUCTIVE!) \n") - fmt.Fprintf(os.Stderr, "\tpodman machine stop%s\n", suffix) - fmt.Fprintf(os.Stderr, "\tpodman machine rm -f%s\n", suffix) - fmt.Fprintf(os.Stderr, "\tpodman machine init --now%s\n\n", suffix) - fmt.Fprintf(os.Stderr, "\t# Copy back files (Optional)\n") - fmt.Fprintf(os.Stderr, "\t# cat backup.tar | podman machine ssh%s tar xvPf - \n\n", suffix) - } - - if forwardState == machine.NoForwarding { - return - } - - machine.WaitAndPingAPI(forwardSock) - - if !noInfo { - if !m.Rootful { - fmt.Printf("\nThis machine is currently configured in rootless mode. If your containers\n") - fmt.Printf("require root permissions (e.g. ports < 1024), or if you run into compatibility\n") - fmt.Printf("issues with non-Podman clients, you can switch using the following command: \n") - fmt.Printf("\n\tpodman machine set --rootful%s\n\n", suffix) - } - - fmt.Printf("API forwarding listening on: %s\n", forwardSock) - if forwardState == machine.DockerGlobal { - fmt.Printf("Docker API clients default to this address. You do not need to set DOCKER_HOST.\n\n") - } else { - stillString := "still " - switch forwardState { - case machine.NotInstalled: - fmt.Printf("\nThe system helper service is not installed; the default Docker API socket\n") - fmt.Printf("address can't be used by Podman. ") - if helper := findClaimHelper(); len(helper) > 0 { - fmt.Printf("If you would like to install it run the\nfollowing commands:\n") - fmt.Printf("\n\tsudo %s install\n", helper) - fmt.Printf("\tpodman machine stop%s; podman machine start%s\n\n", suffix, suffix) - } - case machine.MachineLocal: - fmt.Printf("\nAnother process was listening on the default Docker API socket address.\n") - case machine.ClaimUnsupported: - fallthrough - default: - stillString = "" - } - - fmt.Printf("You can %sconnect Docker API clients by setting DOCKER_HOST using the\n", stillString) - fmt.Printf("following command in your terminal session:\n") - fmt.Printf("\n\texport DOCKER_HOST='unix://%s'\n\n", forwardSock) - } - } -} - func (m *MacMachine) isIncompatible() bool { return m.UID == -1 } diff --git a/pkg/machine/machine_common.go b/pkg/machine/machine_common.go index ae34d771b6..10f76fe58d 100644 --- a/pkg/machine/machine_common.go +++ b/pkg/machine/machine_common.go @@ -54,3 +54,71 @@ func AddSSHConnectionsToPodmanSocket(uid, port int, identityPath, name, remoteUs } return nil } + +// WaitAPIAndPrintInfo prints info about the machine and does a ping test on the +// API socket +func WaitAPIAndPrintInfo(forwardState APIForwardingState, name, helper, forwardSock string, noInfo, isIncompatible, rootful bool) { + suffix := "" + if name != DefaultMachineName { + suffix = " " + name + } + + if isIncompatible { + fmt.Fprintf(os.Stderr, "\n!!! ACTION REQUIRED: INCOMPATIBLE MACHINE !!!\n") + + fmt.Fprintf(os.Stderr, "\nThis machine was created by an older podman release that is incompatible\n") + fmt.Fprintf(os.Stderr, "with this release of podman. It has been started in a limited operational\n") + fmt.Fprintf(os.Stderr, "mode to allow you to copy any necessary files before recreating it. This\n") + fmt.Fprintf(os.Stderr, "can be accomplished with the following commands:\n\n") + fmt.Fprintf(os.Stderr, "\t# Login and copy desired files (Optional)\n") + fmt.Fprintf(os.Stderr, "\t# podman machine ssh%s tar cvPf - /path/to/files > backup.tar\n\n", suffix) + fmt.Fprintf(os.Stderr, "\t# Recreate machine (DESTRUCTIVE!) \n") + fmt.Fprintf(os.Stderr, "\tpodman machine stop%s\n", suffix) + fmt.Fprintf(os.Stderr, "\tpodman machine rm -f%s\n", suffix) + fmt.Fprintf(os.Stderr, "\tpodman machine init --now%s\n\n", suffix) + fmt.Fprintf(os.Stderr, "\t# Copy back files (Optional)\n") + fmt.Fprintf(os.Stderr, "\t# cat backup.tar | podman machine ssh%s tar xvPf - \n\n", suffix) + } + + if forwardState == NoForwarding { + return + } + + WaitAndPingAPI(forwardSock) + + if !noInfo { + if !rootful { + fmt.Printf("\nThis machine is currently configured in rootless mode. If your containers\n") + fmt.Printf("require root permissions (e.g. ports < 1024), or if you run into compatibility\n") + fmt.Printf("issues with non-podman clients, you can switch using the following command: \n") + fmt.Printf("\n\tpodman machine set --rootful%s\n\n", suffix) + } + + fmt.Printf("API forwarding listening on: %s\n", forwardSock) + if forwardState == DockerGlobal { + fmt.Printf("Docker API clients default to this address. You do not need to set DOCKER_HOST.\n\n") + } else { + stillString := "still " + switch forwardState { + case NotInstalled: + fmt.Printf("\nThe system helper service is not installed; the default Docker API socket\n") + fmt.Printf("address can't be used by podman. ") + if len(helper) > 0 { + fmt.Printf("If you would like to install it run the\nfollowing commands:\n") + fmt.Printf("\n\tsudo %s install\n", helper) + fmt.Printf("\tpodman machine stop%s; podman machine start%s\n\n", suffix, suffix) + } + case MachineLocal: + fmt.Printf("\nAnother process was listening on the default Docker API socket address.\n") + case ClaimUnsupported: + fallthrough + default: + stillString = "" + } + + fmt.Printf("You can %sconnect Docker API clients by setting DOCKER_HOST using the\n", stillString) + fmt.Printf("following command in your terminal session:\n") + fmt.Printf("\n\texport DOCKER_HOST='unix://%s'\n\n", forwardSock) + } + } +} diff --git a/pkg/machine/qemu/machine.go b/pkg/machine/qemu/machine.go index bc23298a3c..953b4ce302 100644 --- a/pkg/machine/qemu/machine.go +++ b/pkg/machine/qemu/machine.go @@ -755,7 +755,15 @@ func (v *MachineVM) Start(name string, opts machine.StartOptions) error { } if len(v.Mounts) == 0 { - v.waitAPIAndPrintInfo(forwardState, forwardSock, opts.NoInfo) + machine.WaitAPIAndPrintInfo( + forwardState, + v.Name, + findClaimHelper(), + forwardSock, + opts.NoInfo, + v.isIncompatible(), + v.Rootful, + ) return nil } @@ -777,7 +785,15 @@ func (v *MachineVM) Start(name string, opts machine.StartOptions) error { return err } - v.waitAPIAndPrintInfo(forwardState, forwardSock, opts.NoInfo) + machine.WaitAPIAndPrintInfo( + forwardState, + v.Name, + findClaimHelper(), + forwardSock, + opts.NoInfo, + v.isIncompatible(), + v.Rootful, + ) return nil } @@ -1487,72 +1503,6 @@ func alreadyLinked(target string, link string) bool { return err == nil && read == target } -func (v *MachineVM) waitAPIAndPrintInfo(forwardState machine.APIForwardingState, forwardSock string, noInfo bool) { - suffix := "" - if v.Name != machine.DefaultMachineName { - suffix = " " + v.Name - } - - if v.isIncompatible() { - fmt.Fprintf(os.Stderr, "\n!!! ACTION REQUIRED: INCOMPATIBLE MACHINE !!!\n") - - fmt.Fprintf(os.Stderr, "\nThis machine was created by an older podman release that is incompatible\n") - fmt.Fprintf(os.Stderr, "with this release of podman. It has been started in a limited operational\n") - fmt.Fprintf(os.Stderr, "mode to allow you to copy any necessary files before recreating it. This\n") - fmt.Fprintf(os.Stderr, "can be accomplished with the following commands:\n\n") - fmt.Fprintf(os.Stderr, "\t# Login and copy desired files (Optional)\n") - fmt.Fprintf(os.Stderr, "\t# podman machine ssh%s tar cvPf - /path/to/files > backup.tar\n\n", suffix) - fmt.Fprintf(os.Stderr, "\t# Recreate machine (DESTRUCTIVE!) \n") - fmt.Fprintf(os.Stderr, "\tpodman machine stop%s\n", suffix) - fmt.Fprintf(os.Stderr, "\tpodman machine rm -f%s\n", suffix) - fmt.Fprintf(os.Stderr, "\tpodman machine init --now%s\n\n", suffix) - fmt.Fprintf(os.Stderr, "\t# Copy back files (Optional)\n") - fmt.Fprintf(os.Stderr, "\t# cat backup.tar | podman machine ssh%s tar xvPf - \n\n", suffix) - } - - if forwardState == machine.NoForwarding { - return - } - - machine.WaitAndPingAPI(forwardSock) - - if !noInfo { - if !v.Rootful { - fmt.Printf("\nThis machine is currently configured in rootless mode. If your containers\n") - fmt.Printf("require root permissions (e.g. ports < 1024), or if you run into compatibility\n") - fmt.Printf("issues with non-podman clients, you can switch using the following command: \n") - fmt.Printf("\n\tpodman machine set --rootful%s\n\n", suffix) - } - - fmt.Printf("API forwarding listening on: %s\n", forwardSock) - if forwardState == machine.DockerGlobal { - fmt.Printf("Docker API clients default to this address. You do not need to set DOCKER_HOST.\n\n") - } else { - stillString := "still " - switch forwardState { - case machine.NotInstalled: - fmt.Printf("\nThe system helper service is not installed; the default Docker API socket\n") - fmt.Printf("address can't be used by podman. ") - if helper := findClaimHelper(); len(helper) > 0 { - fmt.Printf("If you would like to install it run the\nfollowing commands:\n") - fmt.Printf("\n\tsudo %s install\n", helper) - fmt.Printf("\tpodman machine stop%s; podman machine start%s\n\n", suffix, suffix) - } - case machine.MachineLocal: - fmt.Printf("\nAnother process was listening on the default Docker API socket address.\n") - case machine.ClaimUnsupported: - fallthrough - default: - stillString = "" - } - - fmt.Printf("You can %sconnect Docker API clients by setting DOCKER_HOST using the\n", stillString) - fmt.Printf("following command in your terminal session:\n") - fmt.Printf("\n\texport DOCKER_HOST='unix://%s'\n\n", forwardSock) - } - } -} - // update returns the content of the VM's // configuration file in json func (v *MachineVM) update() error { From 98cf8462ad43eea1014ed0dbc59e457258f0a810 Mon Sep 17 00:00:00 2001 From: Jake Correnti Date: Tue, 1 Aug 2023 18:37:42 -0400 Subject: [PATCH 5/7] move `removeFilesAndConnections` to a common file Moves `removeFilesAndConnections` to the common file `pkg/machine/connections.go` to be reused by multiple hypervisors. Signed-off-by: Jake Correnti --- pkg/machine/applehv/machine.go | 18 +----------------- pkg/machine/connection.go | 14 ++++++++++++++ pkg/machine/hyperv/machine.go | 15 +-------------- pkg/machine/qemu/machine.go | 15 +-------------- 4 files changed, 17 insertions(+), 45 deletions(-) diff --git a/pkg/machine/applehv/machine.go b/pkg/machine/applehv/machine.go index 68b93ae76f..78bf01f4fc 100644 --- a/pkg/machine/applehv/machine.go +++ b/pkg/machine/applehv/machine.go @@ -360,22 +360,6 @@ func (m *MacMachine) collectFilesToDestroy(opts machine.RemoveOptions) []string return files } -// removeFilesAndConnections removes any files and connections associated with -// the machine during `Remove` -func (m *MacMachine) removeFilesAndConnections(files []string) { - for _, f := range files { - if err := os.Remove(f); err != nil && !errors.Is(err, os.ErrNotExist) { - logrus.Error(err) - } - } - if err := machine.RemoveConnections(m.Name); err != nil { - logrus.Error(err) - } - if err := machine.RemoveConnections(m.Name + "-root"); err != nil { - logrus.Error(err) - } -} - func (m *MacMachine) Remove(name string, opts machine.RemoveOptions) (string, func() error, error) { var ( files []string @@ -404,7 +388,7 @@ func (m *MacMachine) Remove(name string, opts machine.RemoveOptions) (string, fu confirmationMessage += "\n" return confirmationMessage, func() error { - m.removeFilesAndConnections(files) + machine.RemoveFilesAndConnections(files, m.Name, m.Name+"-root") // TODO We will need something like this for applehv too i think /* // Remove the HVSOCK for networking diff --git a/pkg/machine/connection.go b/pkg/machine/connection.go index 74db4ff50f..5633cd5cce 100644 --- a/pkg/machine/connection.go +++ b/pkg/machine/connection.go @@ -6,8 +6,10 @@ package machine import ( "errors" "fmt" + "os" "github.com/containers/common/pkg/config" + "github.com/sirupsen/logrus" ) const LocalhostIP = "127.0.0.1" @@ -89,3 +91,15 @@ func RemoveConnections(names ...string) error { } return cfg.Write() } + +// removeFilesAndConnections removes any files and connections with the given names +func RemoveFilesAndConnections(files []string, names ...string) { + for _, f := range files { + if err := os.Remove(f); err != nil && !errors.Is(err, os.ErrNotExist) { + logrus.Error(err) + } + } + if err := RemoveConnections(names...); err != nil { + logrus.Error(err) + } +} diff --git a/pkg/machine/hyperv/machine.go b/pkg/machine/hyperv/machine.go index 353b3a3502..185efd6561 100644 --- a/pkg/machine/hyperv/machine.go +++ b/pkg/machine/hyperv/machine.go @@ -328,19 +328,6 @@ func (m *HyperVMachine) collectFilesToDestroy(opts machine.RemoveOptions, diskPa return files } -// removeFilesAndConnections removes any files and connections associated with -// the machine during `Remove` -func (m *HyperVMachine) removeFilesAndConnections(files []string) { - for _, f := range files { - if err := os.Remove(f); err != nil && !errors.Is(err, os.ErrNotExist) { - logrus.Error(err) - } - } - if err := machine.RemoveConnections(m.Name, m.Name+"-root"); err != nil { - logrus.Error(err) - } -} - // removeNetworkAndReadySocketsFromRegistry removes the Network and Ready sockets // from the Windows Registry func (m *HyperVMachine) removeNetworkAndReadySocketsFromRegistry() { @@ -385,7 +372,7 @@ func (m *HyperVMachine) Remove(_ string, opts machine.RemoveOptions) (string, fu confirmationMessage += "\n" return confirmationMessage, func() error { - m.removeFilesAndConnections(files) + machine.RemoveFilesAndConnections(files, m.Name, m.Name+"-root") m.removeNetworkAndReadySocketsFromRegistry() return vm.Remove(diskPath) }, nil diff --git a/pkg/machine/qemu/machine.go b/pkg/machine/qemu/machine.go index 953b4ce302..9a3f2cf68f 100644 --- a/pkg/machine/qemu/machine.go +++ b/pkg/machine/qemu/machine.go @@ -1127,19 +1127,6 @@ func (v *MachineVM) removeQMPMonitorSocketAndVMPidFile() { } } -// removeFilesAndConnections removes any files and connections associated with -// the machine during `Remove` -func (v *MachineVM) removeFilesAndConnections(files []string) { - for _, f := range files { - if err := os.Remove(f); err != nil && !errors.Is(err, os.ErrNotExist) { - logrus.Error(err) - } - } - if err := machine.RemoveConnections(v.Name, v.Name+"-root"); err != nil { - logrus.Error(err) - } -} - // Remove deletes all the files associated with a machine including ssh keys, the image itself func (v *MachineVM) Remove(_ string, opts machine.RemoveOptions) (string, func() error, error) { var ( @@ -1181,7 +1168,7 @@ func (v *MachineVM) Remove(_ string, opts machine.RemoveOptions) (string, func() confirmationMessage += "\n" return confirmationMessage, func() error { - v.removeFilesAndConnections(files) + machine.RemoveFilesAndConnections(files, v.Name, v.Name+"-root") return nil }, nil } From 597ccff0bca2f4bd53655f6874656be7756a328e Mon Sep 17 00:00:00 2001 From: Jake Correnti Date: Tue, 1 Aug 2023 19:49:26 -0400 Subject: [PATCH 6/7] Move some logic of `setRootful` to a common file Moves most of the logic of `setRootful` to the common file `pkg/machine/machine_common.go`. Signed-off-by: Jake Correnti --- pkg/machine/hyperv/machine.go | 14 +------------- pkg/machine/machine_common.go | 21 +++++++++++++++++++++ pkg/machine/qemu/machine.go | 14 +------------- pkg/machine/wsl/machine.go | 14 +------------- 4 files changed, 24 insertions(+), 39 deletions(-) diff --git a/pkg/machine/hyperv/machine.go b/pkg/machine/hyperv/machine.go index 185efd6561..677cb16b8b 100644 --- a/pkg/machine/hyperv/machine.go +++ b/pkg/machine/hyperv/machine.go @@ -692,22 +692,10 @@ func (m *HyperVMachine) writeConfig() error { } func (m *HyperVMachine) setRootful(rootful bool) error { - changeCon, err := machine.AnyConnectionDefault(m.Name, m.Name+"-root") - if err != nil { + if err := machine.SetRootful(rootful, m.Name, m.Name+"-root"); err != nil { return err } - if changeCon { - newDefault := m.Name - if rootful { - newDefault += "-root" - } - err := machine.ChangeDefault(newDefault) - if err != nil { - return err - } - } - m.HostUser.Modified = true return nil } diff --git a/pkg/machine/machine_common.go b/pkg/machine/machine_common.go index 10f76fe58d..a447de8f12 100644 --- a/pkg/machine/machine_common.go +++ b/pkg/machine/machine_common.go @@ -122,3 +122,24 @@ func WaitAPIAndPrintInfo(forwardState APIForwardingState, name, helper, forwardS } } } + +// SetRootful modifies the machine's default connection to be either rootful or +// rootless +func SetRootful(rootful bool, name, rootfulName string) error { + changeCon, err := AnyConnectionDefault(name, rootfulName) + if err != nil { + return err + } + + if changeCon { + newDefault := name + if rootful { + newDefault += "-root" + } + err := ChangeDefault(newDefault) + if err != nil { + return err + } + } + return nil +} diff --git a/pkg/machine/qemu/machine.go b/pkg/machine/qemu/machine.go index 9a3f2cf68f..67aac835ea 100644 --- a/pkg/machine/qemu/machine.go +++ b/pkg/machine/qemu/machine.go @@ -1607,22 +1607,10 @@ func (v *MachineVM) resizeDisk(diskSize uint64, oldSize uint64) error { } func (v *MachineVM) setRootful(rootful bool) error { - changeCon, err := machine.AnyConnectionDefault(v.Name, v.Name+"-root") - if err != nil { + if err := machine.SetRootful(rootful, v.Name, v.Name+"-root"); err != nil { return err } - if changeCon { - newDefault := v.Name - if rootful { - newDefault += "-root" - } - err := machine.ChangeDefault(newDefault) - if err != nil { - return err - } - } - v.HostUser.Modified = true return nil } diff --git a/pkg/machine/wsl/machine.go b/pkg/machine/wsl/machine.go index 1de1d2df95..2ff886ed2e 100644 --- a/pkg/machine/wsl/machine.go +++ b/pkg/machine/wsl/machine.go @@ -1525,22 +1525,10 @@ func getMem(vm *MachineVM) (uint64, error) { } func (v *MachineVM) setRootful(rootful bool) error { - changeCon, err := machine.AnyConnectionDefault(v.Name, v.Name+"-root") - if err != nil { + if err := machine.SetRootful(rootful, v.Name, v.Name+"-root"); err != nil { return err } - if changeCon { - newDefault := v.Name - if rootful { - newDefault += "-root" - } - err := machine.ChangeDefault(newDefault) - if err != nil { - return err - } - } - dist := toDist(v.Name) return v.setupPodmanDockerSock(dist, rootful) } From 21ebe0e90aab95e164804add51c8fe81b64d31b3 Mon Sep 17 00:00:00 2001 From: Jake Correnti Date: Tue, 1 Aug 2023 20:46:13 -0400 Subject: [PATCH 7/7] Move `writeConfig` logic to shared function Moves the shared logic from `writeConfig` into a shared function in `pkg/machine/machine_common.go` [NO NEW TESTS NEEDED] Signed-off-by: Jake Correnti --- pkg/machine/hyperv/machine.go | 18 +----------------- pkg/machine/machine_common.go | 23 +++++++++++++++++++++++ pkg/machine/qemu/machine.go | 18 +----------------- pkg/machine/wsl/machine.go | 24 +----------------------- 4 files changed, 26 insertions(+), 57 deletions(-) diff --git a/pkg/machine/hyperv/machine.go b/pkg/machine/hyperv/machine.go index 677cb16b8b..371daa57a1 100644 --- a/pkg/machine/hyperv/machine.go +++ b/pkg/machine/hyperv/machine.go @@ -18,7 +18,6 @@ import ( "github.com/containers/podman/v4/pkg/machine" "github.com/containers/podman/v4/pkg/util" "github.com/containers/podman/v4/utils" - "github.com/containers/storage/pkg/ioutils" "github.com/docker/go-units" "github.com/sirupsen/logrus" ) @@ -673,22 +672,7 @@ func (m *HyperVMachine) forwardSocketPath() (*machine.VMFile, error) { func (m *HyperVMachine) writeConfig() error { // Write the JSON file - opts := &ioutils.AtomicFileWriterOptions{ExplicitCommit: true} - w, err := ioutils.NewAtomicFileWriterWithOpts(m.ConfigPath.GetPath(), 0644, opts) - if err != nil { - return err - } - defer w.Close() - - enc := json.NewEncoder(w) - enc.SetIndent("", " ") - - if err := enc.Encode(m); err != nil { - return err - } - - // Commit the changes to disk if no errors - return w.Commit() + return machine.WriteConfig(m.ConfigPath.Path, m) } func (m *HyperVMachine) setRootful(rootful bool) error { diff --git a/pkg/machine/machine_common.go b/pkg/machine/machine_common.go index a447de8f12..27eabb5035 100644 --- a/pkg/machine/machine_common.go +++ b/pkg/machine/machine_common.go @@ -4,10 +4,13 @@ package machine import ( + "encoding/json" "fmt" "net/url" "os" "strconv" + + "github.com/containers/storage/pkg/ioutils" ) // getDevNullFiles returns pointers to Read-only and Write-only DevNull files @@ -143,3 +146,23 @@ func SetRootful(rootful bool, name, rootfulName string) error { } return nil } + +// WriteConfig writes the machine's JSON config file +func WriteConfig(configPath string, v VM) error { + opts := &ioutils.AtomicFileWriterOptions{ExplicitCommit: true} + w, err := ioutils.NewAtomicFileWriterWithOpts(configPath, 0644, opts) + if err != nil { + return err + } + defer w.Close() + + enc := json.NewEncoder(w) + enc.SetIndent("", " ") + + if err := enc.Encode(v); err != nil { + return err + } + + // Commit the changes to disk if no errors + return w.Commit() +} diff --git a/pkg/machine/qemu/machine.go b/pkg/machine/qemu/machine.go index 67aac835ea..000a6b3bff 100644 --- a/pkg/machine/qemu/machine.go +++ b/pkg/machine/qemu/machine.go @@ -25,7 +25,6 @@ import ( "github.com/containers/podman/v4/pkg/machine" "github.com/containers/podman/v4/pkg/rootless" "github.com/containers/podman/v4/pkg/util" - "github.com/containers/storage/pkg/ioutils" "github.com/containers/storage/pkg/lockfile" "github.com/digitalocean/go-qemu/qmp" "github.com/sirupsen/logrus" @@ -1523,22 +1522,7 @@ func (v *MachineVM) writeConfig() error { return err } // Write the JSON file - opts := &ioutils.AtomicFileWriterOptions{ExplicitCommit: true} - w, err := ioutils.NewAtomicFileWriterWithOpts(v.ConfigPath.GetPath(), 0644, opts) - if err != nil { - return err - } - defer w.Close() - - enc := json.NewEncoder(w) - enc.SetIndent("", " ") - - if err := enc.Encode(v); err != nil { - return err - } - - // Commit the changes to disk if no errors - return w.Commit() + return machine.WriteConfig(v.ConfigPath.Path, v) } // getImageFile wrapper returns the path to the image used diff --git a/pkg/machine/wsl/machine.go b/pkg/machine/wsl/machine.go index 2ff886ed2e..65cbcd3629 100644 --- a/pkg/machine/wsl/machine.go +++ b/pkg/machine/wsl/machine.go @@ -22,7 +22,6 @@ import ( "github.com/containers/podman/v4/pkg/machine/wsl/wutil" "github.com/containers/podman/v4/pkg/util" "github.com/containers/storage/pkg/homedir" - "github.com/containers/storage/pkg/ioutils" "github.com/sirupsen/logrus" "golang.org/x/text/encoding/unicode" "golang.org/x/text/transform" @@ -405,28 +404,7 @@ func downloadDistro(v *MachineVM, opts machine.InitOptions) error { } func (v *MachineVM) writeConfig() error { - const format = "could not write machine json config: %w" - jsonFile := v.ConfigPath - - opts := &ioutils.AtomicFileWriterOptions{ExplicitCommit: true} - w, err := ioutils.NewAtomicFileWriterWithOpts(jsonFile, 0644, opts) - if err != nil { - return fmt.Errorf(format, err) - } - defer w.Close() - - enc := json.NewEncoder(w) - enc.SetIndent("", " ") - if err := enc.Encode(v); err != nil { - return fmt.Errorf(format, err) - } - - // Commit the changes to disk if no error has occurred - if err := w.Commit(); err != nil { - return fmt.Errorf(format, err) - } - - return nil + return machine.WriteConfig(v.ConfigPath, v) } func setupConnections(v *MachineVM, opts machine.InitOptions) error {