summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBrent Baude <bbaude@redhat.com>2022-03-25 14:42:25 -0500
committerBrent Baude <bbaude@redhat.com>2022-03-28 09:12:08 -0500
commit2ac897aa0dcda012f4fc038c84849d9429d421dc (patch)
treeac5e7906001e45d682c60b6644f612b60ec7040c
parent54f808e4dd10de0d3ceec7cadac776b119a293b1 (diff)
downloadpodman-2ac897aa0dcda012f4fc038c84849d9429d421dc.tar.gz
podman-2ac897aa0dcda012f4fc038c84849d9429d421dc.tar.bz2
podman-2ac897aa0dcda012f4fc038c84849d9429d421dc.zip
Machine refactor - part 1
the way machine was written was very adjunct and as such is in dire need of refactoring to better structures and structure methods where appropriate. the weekest part is specifically around all the files that machine requires and how some are just dynamically built on the fly. this pr defines a new machinefile type which allows us to work with the file and also takes into account the use of symlinks which are going to be needed on macos due to its relatively short file length restriction. also, added unit tests for new methods as well as anywhere else I saw a need. Signed-off-by: Brent Baude <bbaude@redhat.com>
-rw-r--r--cmd/podman/machine/init.go8
-rw-r--r--pkg/machine/config.go3
-rw-r--r--pkg/machine/config_test.go71
-rw-r--r--pkg/machine/qemu/config.go122
-rw-r--r--pkg/machine/qemu/config_test.go103
-rw-r--r--pkg/machine/qemu/machine.go24
6 files changed, 318 insertions, 13 deletions
diff --git a/cmd/podman/machine/init.go b/cmd/podman/machine/init.go
index 8fb17cf54..518e7490f 100644
--- a/cmd/podman/machine/init.go
+++ b/cmd/podman/machine/init.go
@@ -31,6 +31,10 @@ var (
now bool
)
+// maxMachineNameSize is set to thirty to limit huge machine names primarily
+// because macos has a much smaller file size limit.
+const maxMachineNameSize = 30
+
func init() {
registry.Commands = append(registry.Commands, registry.CliCommand{
Command: initCmd,
@@ -111,10 +115,12 @@ func initMachine(cmd *cobra.Command, args []string) error {
vm machine.VM
err error
)
-
provider := getSystemDefaultProvider()
initOpts.Name = defaultMachineName
if len(args) > 0 {
+ if len(args[0]) > maxMachineNameSize {
+ return errors.New("machine name must be 30 characters or less")
+ }
initOpts.Name = args[0]
}
if _, err := provider.LoadVMByName(initOpts.Name); err == nil {
diff --git a/pkg/machine/config.go b/pkg/machine/config.go
index aaf8da872..7e1561506 100644
--- a/pkg/machine/config.go
+++ b/pkg/machine/config.go
@@ -29,7 +29,7 @@ type InitOptions struct {
Username string
ReExec bool
Rootful bool
- // The numberical userid of the user that called machine
+ // The numerical userid of the user that called machine
UID string
}
@@ -128,6 +128,7 @@ type DistributionDownload interface {
}
func (rc RemoteConnectionType) MakeSSHURL(host, path, port, userName string) url.URL {
+ //TODO Should this function have input verification?
userInfo := url.User(userName)
uri := url.URL{
Scheme: "ssh",
diff --git a/pkg/machine/config_test.go b/pkg/machine/config_test.go
new file mode 100644
index 000000000..d9fc5425e
--- /dev/null
+++ b/pkg/machine/config_test.go
@@ -0,0 +1,71 @@
+package machine
+
+import (
+ "net"
+ "net/url"
+ "reflect"
+ "testing"
+)
+
+func TestRemoteConnectionType_MakeSSHURL(t *testing.T) {
+ var (
+ host = "foobar"
+ path = "/path/to/socket"
+ rc = "ssh"
+ username = "core"
+ )
+ type args struct {
+ host string
+ path string
+ port string
+ userName string
+ }
+ tests := []struct {
+ name string
+ rc RemoteConnectionType
+ args args
+ want url.URL
+ }{
+ {
+ name: "Good no port",
+ rc: "ssh",
+ args: args{
+ host: host,
+ path: path,
+ port: "",
+ userName: username,
+ },
+ want: url.URL{
+ Scheme: rc,
+ User: url.User(username),
+ Host: host,
+ Path: path,
+ ForceQuery: false,
+ },
+ },
+ {
+ name: "Good with port",
+ rc: "ssh",
+ args: args{
+ host: host,
+ path: path,
+ port: "222",
+ userName: username,
+ },
+ want: url.URL{
+ Scheme: rc,
+ User: url.User(username),
+ Host: net.JoinHostPort(host, "222"),
+ Path: path,
+ ForceQuery: false,
+ },
+ },
+ }
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ if got := tt.rc.MakeSSHURL(tt.args.host, tt.args.path, tt.args.port, tt.args.userName); !reflect.DeepEqual(got, tt.want) { //nolint: scopelint
+ t.Errorf("MakeSSHURL() = %v, want %v", got, tt.want) //nolint: scopelint
+ }
+ })
+ }
+}
diff --git a/pkg/machine/qemu/config.go b/pkg/machine/qemu/config.go
index 211d96ccb..408b33a33 100644
--- a/pkg/machine/qemu/config.go
+++ b/pkg/machine/qemu/config.go
@@ -4,12 +4,28 @@
package qemu
import (
+ "errors"
+ "os"
"time"
+
+ "github.com/sirupsen/logrus"
+)
+
+const (
+ // FCOS streams
+ // Testing FCOS stream
+ Testing string = "testing"
+ // Next FCOS stream
+ Next string = "next"
+ // Stable FCOS stream
+ Stable string = "stable"
)
type Provider struct{}
-type MachineVM struct {
+// Deprecated: MachineVMV1 is being deprecated in favor a more flexible and informative
+// structure
+type MachineVMV1 struct {
// CPUs to be assigned to the VM
CPUs uint64
// The command line representation of the qemu command
@@ -42,6 +58,74 @@ type MachineVM struct {
UID int
}
+type MachineVM struct {
+ // The command line representation of the qemu command
+ CmdLine []string
+ // HostUser contains info about host user
+ HostUser
+ // ImageConfig describes the bootable image
+ ImageConfig
+ // Mounts is the list of remote filesystems to mount
+ Mounts []Mount
+ // Name of VM
+ Name string
+ // PidFilePath is the where the PID file lives
+ PidFilePath MachineFile
+ // QMPMonitor is the qemu monitor object for sending commands
+ QMPMonitor Monitor
+ // ReadySocket tells host when vm is booted
+ ReadySocket MachineFile
+ // ResourceConfig is physical attrs of the VM
+ ResourceConfig
+ // SSHConfig for accessing the remote vm
+ SSHConfig
+}
+
+// ImageConfig describes the bootable image for the VM
+type ImageConfig struct {
+ IgnitionFilePath string
+ // ImageStream is the update stream for the image
+ ImageStream string
+ // ImagePath is the fq path to
+ ImagePath string
+}
+
+// HostUser describes the host user
+type HostUser struct {
+ // Whether this machine should run in a rootful or rootless manner
+ Rootful bool
+ // UID is the numerical id of the user that called machine
+ UID int
+}
+
+// SSHConfig contains remote access information for SSH
+type SSHConfig struct {
+ // IdentityPath is the fq path to the ssh priv key
+ IdentityPath string
+ // SSH port for user networking
+ Port int
+ // RemoteUsername of the vm user
+ RemoteUsername string
+}
+
+// ResourceConfig describes physical attributes of the machine
+type ResourceConfig struct {
+ // CPUs to be assigned to the VM
+ CPUs uint64
+ // Memory in megabytes assigned to the vm
+ Memory uint64
+ // Disk size in gigabytes assigned to the vm
+ DiskSize uint64
+}
+
+type MachineFile struct {
+ // Path is the fully qualified path to a file
+ Path string
+ // Symlink is a shortened version of Path by using
+ // a symlink
+ Symlink *string
+}
+
type Mount struct {
Type string
Tag string
@@ -52,7 +136,7 @@ type Mount struct {
type Monitor struct {
// Address portion of the qmp monitor (/tmp/tmp.sock)
- Address string
+ Address MachineFile
// Network portion of the qmp monitor (unix)
Network string
// Timeout in seconds for qmp monitor transactions
@@ -64,3 +148,37 @@ var (
// qmp monitor interactions.
defaultQMPTimeout time.Duration = 2 * time.Second
)
+
+// GetPath returns the working path for a machinefile. it returns
+// the symlink unless one does not exist
+func (m *MachineFile) GetPath() string {
+ if m.Symlink == nil {
+ return m.Path
+ }
+ return *m.Symlink
+}
+
+// Delete removes the machinefile symlink (if it exists) and
+// the actual path
+func (m *MachineFile) Delete() error {
+ if m.Symlink != nil {
+ if err := os.Remove(*m.Symlink); err != nil {
+ logrus.Errorf("unable to remove symlink %q", *m.Symlink)
+ }
+ }
+ return os.Remove(m.Path)
+}
+
+// NewMachineFile is a constructor for MachineFile
+func NewMachineFile(path string, symlink *string) (*MachineFile, error) {
+ if len(path) < 1 {
+ return nil, errors.New("invalid machine file path")
+ }
+ if symlink != nil && len(*symlink) < 1 {
+ return nil, errors.New("invalid symlink path")
+ }
+ return &MachineFile{
+ Path: path,
+ Symlink: symlink,
+ }, nil
+}
diff --git a/pkg/machine/qemu/config_test.go b/pkg/machine/qemu/config_test.go
new file mode 100644
index 000000000..e3e7437b5
--- /dev/null
+++ b/pkg/machine/qemu/config_test.go
@@ -0,0 +1,103 @@
+package qemu
+
+import (
+ "reflect"
+ "testing"
+)
+
+func TestMachineFile_GetPath(t *testing.T) {
+ path := "/var/tmp/podman/my.sock"
+ sym := "/tmp/podman/my.sock"
+ type fields struct {
+ Path string
+ Symlink *string
+ }
+ tests := []struct {
+ name string
+ fields fields
+ want string
+ }{
+ {
+ name: "Original path",
+ fields: fields{path, nil},
+ want: path,
+ },
+ {
+ name: "Symlink over path",
+ fields: fields{
+ Path: path,
+ Symlink: &sym,
+ },
+ want: sym,
+ },
+ }
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ m := &MachineFile{
+ Path: tt.fields.Path, //nolint: scopelint
+ Symlink: tt.fields.Symlink, //nolint: scopelint
+ }
+ if got := m.GetPath(); got != tt.want { //nolint: scopelint
+ t.Errorf("GetPath() = %v, want %v", got, tt.want) //nolint: scopelint
+ }
+ })
+ }
+}
+
+func TestNewMachineFile(t *testing.T) {
+ p := "/var/tmp/podman/my.sock"
+ sym := "/tmp/podman/my.sock"
+ empty := ""
+
+ m := MachineFile{
+ Path: p,
+ Symlink: nil,
+ }
+ type args struct {
+ path string
+ symlink *string
+ }
+ tests := []struct {
+ name string
+ args args
+ want *MachineFile
+ wantErr bool
+ }{
+ {
+ name: "Good",
+ args: args{path: p},
+ want: &m,
+ wantErr: false,
+ },
+ {
+ name: "Good with Symlink",
+ args: args{p, &sym},
+ want: &MachineFile{p, &sym},
+ wantErr: false,
+ },
+ {
+ name: "Bad path name",
+ args: args{empty, nil},
+ want: nil,
+ wantErr: true,
+ },
+ {
+ name: "Bad symlink name",
+ args: args{p, &empty},
+ want: nil,
+ wantErr: true,
+ },
+ }
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ got, err := NewMachineFile(tt.args.path, tt.args.symlink) //nolint: scopelint
+ if (err != nil) != tt.wantErr { //nolint: scopelint
+ t.Errorf("NewMachineFile() error = %v, wantErr %v", err, tt.wantErr) //nolint: scopelint
+ return
+ }
+ if !reflect.DeepEqual(got, tt.want) { //nolint: scopelint
+ t.Errorf("NewMachineFile() got = %v, want %v", got, tt.want) //nolint: scopelint
+ }
+ })
+ }
+}
diff --git a/pkg/machine/qemu/machine.go b/pkg/machine/qemu/machine.go
index f04625781..ac8e7d75c 100644
--- a/pkg/machine/qemu/machine.go
+++ b/pkg/machine/qemu/machine.go
@@ -111,7 +111,7 @@ func (p *Provider) NewMachine(opts machine.InitOptions) (machine.VM, error) {
return nil, err
}
vm.QMPMonitor = monitor
- cmd = append(cmd, []string{"-qmp", monitor.Network + ":/" + monitor.Address + ",server=on,wait=off"}...)
+ cmd = append(cmd, []string{"-qmp", monitor.Network + ":/" + monitor.Address.GetPath() + ",server=on,wait=off"}...)
// Add network
// Right now the mac address is hardcoded so that the host networking gives it a specific IP address. This is
@@ -134,7 +134,8 @@ func (p *Provider) NewMachine(opts machine.InitOptions) (machine.VM, error) {
// LoadByName reads a json file that describes a known qemu vm
// and returns a vm instance
func (p *Provider) LoadVMByName(name string) (machine.VM, error) {
- vm := &MachineVM{UID: -1} // posix reserves -1, so use it to signify undefined
+ vm := &MachineVM{Name: name}
+ vm.HostUser = HostUser{UID: -1} // posix reserves -1, so use it to signify undefined
vmConfigDir, err := machine.GetConfDir(vmtype)
if err != nil {
return nil, err
@@ -176,7 +177,7 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) {
v.Rootful = opts.Rootful
switch opts.ImagePath {
- case "testing", "next", "stable", "":
+ case Testing, Next, Stable, "":
// Get image as usual
v.ImageStream = opts.ImagePath
dd, err := machine.NewFcosDownloader(vmtype, v.Name, opts.ImagePath)
@@ -576,12 +577,12 @@ func (v *MachineVM) checkStatus(monitor *qmp.SocketMonitor) (machine.QemuMachine
func (v *MachineVM) Stop(name string, _ machine.StopOptions) error {
var disconnected bool
// check if the qmp socket is there. if not, qemu instance is gone
- if _, err := os.Stat(v.QMPMonitor.Address); os.IsNotExist(err) {
+ if _, err := os.Stat(v.QMPMonitor.Address.GetPath()); os.IsNotExist(err) {
// Right now it is NOT an error to stop a stopped machine
logrus.Debugf("QMP monitor socket %v does not exist", v.QMPMonitor.Address)
return nil
}
- qmpMonitor, err := qmp.NewSocketMonitor(v.QMPMonitor.Network, v.QMPMonitor.Address, v.QMPMonitor.Timeout)
+ qmpMonitor, err := qmp.NewSocketMonitor(v.QMPMonitor.Network, v.QMPMonitor.Address.GetPath(), v.QMPMonitor.Timeout)
if err != nil {
return err
}
@@ -684,20 +685,25 @@ func NewQMPMonitor(network, name string, timeout time.Duration) (Monitor, error)
if timeout == 0 {
timeout = defaultQMPTimeout
}
+ address, err := NewMachineFile(filepath.Join(rtDir, "qmp+"+name+".sock"), nil)
+ if err != nil {
+ return Monitor{}, err
+ }
monitor := Monitor{
Network: network,
- Address: filepath.Join(rtDir, "qmp_"+name+".sock"),
+ Address: *address,
Timeout: timeout,
}
return monitor, nil
}
+// Remove deletes all the files associated with a machine including ssh keys, the image itself
func (v *MachineVM) Remove(name string, opts machine.RemoveOptions) (string, func() error, error) {
var (
files []string
)
- // cannot remove a running vm
+ // cannot remove a running vm unless --force is used
running, err := v.isRunning()
if err != nil {
return "", nil, err
@@ -768,11 +774,11 @@ func (v *MachineVM) Remove(name string, opts machine.RemoveOptions) (string, fun
func (v *MachineVM) isRunning() (bool, error) {
// Check if qmp socket path exists
- if _, err := os.Stat(v.QMPMonitor.Address); os.IsNotExist(err) {
+ if _, err := os.Stat(v.QMPMonitor.Address.GetPath()); os.IsNotExist(err) {
return false, nil
}
// Check if we can dial it
- monitor, err := qmp.NewSocketMonitor(v.QMPMonitor.Network, v.QMPMonitor.Address, v.QMPMonitor.Timeout)
+ monitor, err := qmp.NewSocketMonitor(v.QMPMonitor.Network, v.QMPMonitor.Address.GetPath(), v.QMPMonitor.Timeout)
if err != nil {
// FIXME: this error should probably be returned
return false, nil // nolint: nilerr