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

machine: Address some QEMU TODOs #21517

Merged
merged 3 commits into from
Feb 22, 2024
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
12 changes: 1 addition & 11 deletions cmd/podman/machine/inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,6 @@ func inspect(cmd *cobra.Command, args []string) error {
if err != nil {
return err
}
ignFile, err := mc.IgnitionFile()
if err != nil {
return err
}

podmanSocket, podmanPipe, err := mc.ConnectionInfo(provider.VMType())
if err != nil {
Expand All @@ -83,13 +79,7 @@ func inspect(cmd *cobra.Command, args []string) error {
PodmanSocket: podmanSocket,
PodmanPipe: podmanPipe,
},
Created: mc.Created,
// TODO This is no longer applicable; we dont care about the provenance
// of the image
Image: machine.ImageConfig{
IgnitionFile: *ignFile,
ImagePath: *mc.ImagePath,
},
Created: mc.Created,
LastUp: mc.LastUp,
Name: mc.Name,
Resources: mc.Resources,
Expand Down
6 changes: 2 additions & 4 deletions cmd/podman/machine/rm.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,14 +120,12 @@ func rm(_ *cobra.Command, args []string) error {
// All actual removal of files and vms should occur after this
//

// TODO Should this be a hard error?
if err := providerRm(); err != nil {
logrus.Errorf("failed to remove virtual machine from provider for %q", vmName)
logrus.Errorf("failed to remove virtual machine from provider for %q: %v", vmName, err)
}

// TODO Should this be a hard error?
jakecorrenti marked this conversation as resolved.
Show resolved Hide resolved
if err := genericRm(); err != nil {
logrus.Error("failed to remove machines files")
return fmt.Errorf("failed to remove machines files: %v", err)
}
newMachineEvent(events.Remove, events.Event{Name: vmName})
return nil
Expand Down
1 change: 0 additions & 1 deletion docs/source/markdown/podman-machine-inspect.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ Print results with a Go template.
| .ConfigDir ... | Machine configuration directory location |
| .ConnectionInfo ... | Machine connection information |
| .Created ... | Machine creation time (string, ISO3601) |
| .Image ... | Machine image config |
| .LastUp ... | Time when machine was last booted |
| .Name | Name of the machine |
| .Resources ... | Resources used by the machine |
Expand Down
1 change: 0 additions & 1 deletion pkg/machine/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ type InspectInfo struct {
ConfigDir define.VMFile
ConnectionInfo ConnectionConfig
Created time.Time
Image ImageConfig
LastUp time.Time
Name string
Resources vmconfigs.ResourceConfig
Expand Down
20 changes: 7 additions & 13 deletions pkg/machine/e2e/init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,16 +287,6 @@ var _ = Describe("podman machine init", func() {
Expect(err).ToNot(HaveOccurred())
cfgpth := filepath.Join(inspectSession.outputToString(), fmt.Sprintf("%s.json", name))

inspect = inspect.withFormat("{{.Image.IgnitionFile.Path}}")
inspectSession, err = mb.setCmd(inspect).run()
Expect(err).ToNot(HaveOccurred())
ign := inspectSession.outputToString()

inspect = inspect.withFormat("{{.Image.ImagePath.Path}}")
inspectSession, err = mb.setCmd(inspect).run()
Expect(err).ToNot(HaveOccurred())
img := inspectSession.outputToString()

rm := rmMachine{}
removeSession, err := mb.setCmd(rm.withForce()).run()
Expect(err).ToNot(HaveOccurred())
Expand All @@ -317,13 +307,17 @@ var _ = Describe("podman machine init", func() {
Expect(err).ToNot(HaveOccurred())
Expect(session).To(Exit(125))

// ensure files created by init are cleaned up on init failure
_, err = os.Stat(img)
imageSuffix := mb.imagePath[strings.LastIndex(mb.imagePath, "/")+1:]
imgPath := filepath.Join(testDir, ".local", "share", "containers", "podman", "machine", "qemu", mb.name+"_"+imageSuffix)
_, err = os.Stat(imgPath)
Expect(err).To(HaveOccurred())

cfgDir := filepath.Join(testDir, ".config", "containers", "podman", "machine", testProvider.VMType().String())
_, err = os.Stat(cfgpth)
Expect(err).To(HaveOccurred())

_, err = os.Stat(ign)
ignPath := filepath.Join(cfgDir, mb.name+".ign")
_, err = os.Stat(ignPath)
Expect(err).To(HaveOccurred())
}
})
Expand Down
15 changes: 3 additions & 12 deletions pkg/machine/e2e/rm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,16 +94,6 @@ var _ = Describe("podman machine rm", func() {
key := inspectSession.outputToString()
pubkey := key + ".pub"

inspect = inspect.withFormat("{{.Image.IgnitionFile.Path}}")
inspectSession, err = mb.setCmd(inspect).run()
Expect(err).ToNot(HaveOccurred())
ign := inspectSession.outputToString()

inspect = inspect.withFormat("{{.Image.ImagePath.Path}}")
inspectSession, err = mb.setCmd(inspect).run()
Expect(err).ToNot(HaveOccurred())
img := inspectSession.outputToString()

rm := rmMachine{}
removeSession, err := mb.setCmd(rm.withForce().withSaveIgnition().withSaveImage()).run()
Expect(err).ToNot(HaveOccurred())
Expand All @@ -122,10 +112,11 @@ var _ = Describe("podman machine rm", func() {

// WSL does not use ignition
if testProvider.VMType() != define.WSLVirt {
_, err = os.Stat(ign)
ignPath := filepath.Join(testDir, ".config", "containers", "podman", "machine", testProvider.VMType().String(), mb.name+".ign")
_, err = os.Stat(ignPath)
Expect(err).ToNot(HaveOccurred())
}
_, err = os.Stat(img)
_, err = os.Stat(mb.imagePath)
Expect(err).ToNot(HaveOccurred())
})

Expand Down
11 changes: 10 additions & 1 deletion pkg/machine/qemu/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"time"

"github.com/containers/common/pkg/config"
"github.com/containers/podman/v5/pkg/errorhandling"
"github.com/containers/podman/v5/pkg/machine/define"
"github.com/containers/podman/v5/pkg/machine/vmconfigs"
"github.com/digitalocean/go-qemu/qmp"
Expand Down Expand Up @@ -237,7 +238,15 @@ func (q *QEMUStubber) Remove(mc *vmconfigs.MachineConfig) ([]string, func() erro
}

return qemuRmFiles, func() error {
return nil
var errs []error
if err := mc.QEMUHypervisor.QEMUPidPath.Delete(); err != nil {
errs = append(errs, err)
}

if err := mc.QEMUHypervisor.QMPMonitor.Address.Delete(); err != nil {
errs = append(errs, err)
}
return errorhandling.JoinErrors(errs)
}, nil
}

Expand Down
25 changes: 15 additions & 10 deletions pkg/machine/vmconfigs/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"time"

define2 "github.com/containers/podman/v5/libpod/define"
"github.com/containers/podman/v5/pkg/errorhandling"
"github.com/containers/podman/v5/pkg/machine/connection"
"github.com/containers/podman/v5/pkg/machine/define"
"github.com/containers/podman/v5/pkg/machine/lock"
Expand Down Expand Up @@ -184,28 +185,32 @@ func (mc *MachineConfig) Remove(saveIgnition, saveImage bool) ([]string, func()
}

mcRemove := func() error {
var errs []error
if err := connection.RemoveConnections(mc.Name, mc.Name+"-root"); err != nil {
errs = append(errs, err)
}

if !saveIgnition {
if err := ignitionFile.Delete(); err != nil {
logrus.Error(err)
errs = append(errs, err)
}
}
if !saveImage {
if err := mc.ImagePath.Delete(); err != nil {
logrus.Error(err)
errs = append(errs, err)
}
}
if err := mc.configPath.Delete(); err != nil {
logrus.Error(err)
}
if err := readySocket.Delete(); err != nil {
logrus.Error()
errs = append(errs, err)
}
if err := logPath.Delete(); err != nil {
logrus.Error(err)
errs = append(errs, err)
}

if err := mc.configPath.Delete(); err != nil {
errs = append(errs, err)
}
// TODO This should be bumped up into delete and called out in the text given then
// are not technically files per'se
return connection.RemoveConnections(mc.Name, mc.Name+"-root")
return errorhandling.JoinErrors(errs)
}

return rmFiles, mcRemove, nil
Expand Down
Loading