Dedup and refactor image acquisition

As promised in #19596, this pr deduplicates and refactors image
acquisition.  All virt providers that use FCOS as its default now use
the same code.

[NO NEW TESTS NEEDED]

Signed-off-by: Brent Baude <bbaude@redhat.com>

pkg/machine/applehv/config.go

@@ -34,7 +34,7 @@ type MMHardwareConfig struct {
 
 func VirtualizationProvider() machine.VirtProvider {
 	return &AppleHVVirtualization{
-		machine.NewVirtualization(machine.Metal, machine.Xz, machine.Raw),
+		machine.NewVirtualization(machine.AppleHV, machine.Xz, machine.Raw, vmtype),
 	}
 }
 

pkg/machine/applehv/machine.go

@@ -77,36 +77,6 @@ type MacMachine struct {
 	GvProxySock machine.VMFile
 }
 
-// acquireVMImage determines if the image is already in a FCOS stream. If so,
-// retrieves the image path of the uncompressed file. Otherwise, the user has
-// provided an alternative image, so we set the image path and download the image.
-func (m *MacMachine) acquireVMImage(opts machine.InitOptions, dataDir string) error {
-	// Acquire the image
-	switch opts.ImagePath {
-	case machine.Testing.String(), machine.Next.String(), machine.Stable.String(), "":
-		g, err := machine.NewGenericDownloader(vmtype, opts.Name, opts.ImagePath)
-		if err != nil {
-			return err
-		}
-
-		imagePath, err := machine.NewMachineFile(g.Get().GetLocalUncompressedFile(dataDir), nil)
-		if err != nil {
-			return err
-		}
-		m.ImagePath = *imagePath
-	default:
-		// The user has provided an alternate image which can be a file path
-		// or URL.
-		m.ImageStream = "custom"
-		imagePath, err := machine.AcquireAlternateImage(m.Name, vmtype, opts)
-		if err != nil {
-			return err
-		}
-		m.ImagePath = *imagePath
-	}
-	return nil
-}
-
 // setGVProxyInfo sets the VM's gvproxy pid and socket files
 func (m *MacMachine) setGVProxyInfo(runtimeDir string) error {
 	gvProxyPid, err := machine.NewMachineFile(filepath.Join(runtimeDir, "gvproxy.pid"), nil)
@@ -226,10 +196,20 @@ func (m *MacMachine) Init(opts machine.InitOptions) (bool, error) {
 		return false, err
 	}
 
-	if err := m.acquireVMImage(opts, dataDir); err != nil {
+	dl, err := VirtualizationProvider().NewDownload(m.Name)
+	if err != nil {
 		return false, err
 	}
 
+	imagePath, strm, err := dl.AcquireVMImage(opts.ImagePath)
+	if err != nil {
+		return false, err
+	}
+
+	// Set the values for imagePath and strm
+	m.ImagePath = *imagePath
+	m.ImageStream = strm.String()
+
 	logPath, err := machine.NewMachineFile(filepath.Join(dataDir, fmt.Sprintf("%s.log", m.Name)), nil)
 	if err != nil {
 		return false, err

pkg/machine/config.go

@@ -62,16 +62,18 @@ var (
 type Download struct {
 	Arch                  string
 	Artifact              Artifact
-	CompressionType       string
 	CacheDir              string
+	CompressionType       ImageCompression
+	DataDir               string
 	Format                ImageFormat
 	ImageName             string
 	LocalPath             string
 	LocalUncompressedFile string
 	Sha256sum             string
-	URL                   *url.URL
-	VMName                string
 	Size                  int64
+	URL                   *url.URL
+	VMKind                VMType
+	VMName                string
 }
 
 type ListOptions struct{}
@@ -430,7 +432,7 @@ func ParseVMType(input string, emptyFallback VMType) (VMType, error) {
 	}
 }
 
-type VirtProvider interface {
+type VirtProvider interface { //nolint:interfacebloat
 	Artifact() Artifact
 	CheckExclusiveActiveVM() (bool, string, error)
 	Compression() ImageCompression
@@ -439,6 +441,7 @@ type VirtProvider interface {
 	List(opts ListOptions) ([]*ListResponse, error)
 	LoadVMByName(name string) (VM, error)
 	NewMachine(opts InitOptions) (VM, error)
+	NewDownload(vmName string) (Download, error)
 	RemoveAndCleanMachines() error
 	VMType() VMType
 }
@@ -447,6 +450,7 @@ type Virtualization struct {
 	artifact    Artifact
 	compression ImageCompression
 	format      ImageFormat
+	vmKind      VMType
 }
 
 func (p *Virtualization) Artifact() Artifact {
@@ -461,11 +465,38 @@ func (p *Virtualization) Format() ImageFormat {
 	return p.format
 }
 
-func NewVirtualization(artifact Artifact, compression ImageCompression, format ImageFormat) Virtualization {
+func (p *Virtualization) VMType() VMType {
+	return p.vmKind
+}
+
+func (p *Virtualization) NewDownload(vmName string) (Download, error) {
+	cacheDir, err := GetCacheDir(p.VMType())
+	if err != nil {
+		return Download{}, err
+	}
+
+	dataDir, err := GetDataDir(p.VMType())
+	if err != nil {
+		return Download{}, err
+	}
+
+	return Download{
+		Artifact:        p.Artifact(),
+		CacheDir:        cacheDir,
+		CompressionType: p.Compression(),
+		DataDir:         dataDir,
+		Format:          p.Format(),
+		VMKind:          p.VMType(),
+		VMName:          vmName,
+	}, nil
+}
+
+func NewVirtualization(artifact Artifact, compression ImageCompression, format ImageFormat, vmKind VMType) Virtualization {
 	return Virtualization{
 		artifact,
 		compression,
 		format,
+		vmKind,
 	}
 }
 
@@ -493,3 +524,76 @@ func WaitAndPingAPI(sock string) {
 		logrus.Warn("API socket failed ping test")
 	}
 }
+
+func (dl Download) NewFcosDownloader(imageStream FCOSStream) (DistributionDownload, error) {
+	info, err := dl.GetFCOSDownload(imageStream)
+	if err != nil {
+		return nil, err
+	}
+	urlSplit := strings.Split(info.Location, "/")
+	dl.ImageName = urlSplit[len(urlSplit)-1]
+	downloadURL, err := url.Parse(info.Location)
+	if err != nil {
+		return nil, err
+	}
+
+	// Complete the download struct
+	dl.Arch = GetFcosArch()
+	// This could be eliminated as a struct and be a generated()
+	dl.LocalPath = filepath.Join(dl.CacheDir, dl.ImageName)
+	dl.Sha256sum = info.Sha256Sum
+	dl.URL = downloadURL
+	fcd := FcosDownload{
+		Download: dl,
+	}
+	dataDir, err := GetDataDir(dl.VMKind)
+	if err != nil {
+		return nil, err
+	}
+	fcd.Download.LocalUncompressedFile = fcd.GetLocalUncompressedFile(dataDir)
+	return fcd, nil
+}
+
+// AcquireVMImage determines if the image is already in a FCOS stream. If so,
+// retrieves the image path of the uncompressed file. Otherwise, the user has
+// provided an alternative image, so we set the image path and download the image.
+func (dl Download) AcquireVMImage(imagePath string) (*VMFile, FCOSStream, error) {
+	var (
+		err           error
+		imageLocation *VMFile
+		fcosStream    FCOSStream
+	)
+	switch imagePath {
+	// TODO these need to be re-typed as FCOSStreams
+	case Testing.String(), Next.String(), Stable.String(), "":
+		// Get image as usual
+		fcosStream, err = FCOSStreamFromString(imagePath)
+		if err != nil {
+			return nil, 0, err
+		}
+
+		dd, err := dl.NewFcosDownloader(fcosStream)
+		if err != nil {
+			return nil, 0, err
+		}
+
+		imageLocation, err = NewMachineFile(dd.Get().LocalUncompressedFile, nil)
+		if err != nil {
+			return nil, 0, err
+		}
+
+		if err := DownloadImage(dd); err != nil {
+			return nil, 0, err
+		}
+	default:
+		// The user has provided an alternate image which can be a file path
+		// or URL.
+		fcosStream = CustomStream
+		imgPath, err := dl.AcquireAlternateImage(imagePath)
+		if err != nil {
+			return nil, 0, err
+		}
+		imageLocation = imgPath
+	}
+	return imageLocation, fcosStream, nil
+}

pkg/machine/e2e/machine_test.go

@@ -44,8 +44,11 @@ func TestMachine(t *testing.T) {
 }
 
 var _ = BeforeSuite(func() {
-	qemuVP := qemu.VirtualizationProvider()
+	dd, err := qemu.VirtualizationProvider().NewDownload("")
-	fcd, err := machine.GetFCOSDownload(qemuVP, defaultStream)
+	if err != nil {
+		Fail("unable to create new download")
+	}
+	fcd, err := dd.GetFCOSDownload(defaultStream)
 	if err != nil {
 		Fail("unable to get virtual machine image")
 	}

pkg/machine/fcos.go

@@ -10,7 +10,6 @@ import (
 	"net/http"
 	url2 "net/url"
 	"os"
-	"path/filepath"
 	"runtime"
 	"strings"
 	"time"
@@ -40,7 +39,7 @@ const (
 
 	Qemu Artifact = iota
 	HyperV
-	Metal
+	AppleHV
 	None
 
 	Qcow ImageFormat = iota
@@ -61,8 +60,8 @@ func (a Artifact) String() string {
 	switch a {
 	case HyperV:
 		return "hyperv"
-	case Metal:
+	case AppleHV:
-		return "metal"
+		return "applehv"
 	}
 	return "qemu"
 }
@@ -74,7 +73,7 @@ func (imf ImageFormat) String() string {
 	case Tar:
 		return "tar.xz"
 	case Raw:
-		return "raw.xz"
+		return "raw.gz"
 	}
 	return "qcow2.xz"
 }
@@ -107,50 +106,12 @@ type FcosDownload struct {
 	Download
 }
 
-func NewFcosDownloader(vmType VMType, vmName string, imageStream FCOSStream, vp VirtProvider) (DistributionDownload, error) {
-	info, err := GetFCOSDownload(vp, imageStream)
-	if err != nil {
-		return nil, err
-	}
-	urlSplit := strings.Split(info.Location, "/")
-	imageName := urlSplit[len(urlSplit)-1]
-	url, err := url2.Parse(info.Location)
-	if err != nil {
-		return nil, err
-	}
-
-	cacheDir, err := GetCacheDir(vmType)
-	if err != nil {
-		return nil, err
-	}
-
-	fcd := FcosDownload{
-		Download: Download{
-			Arch:      GetFcosArch(),
-			Artifact:  Qemu,
-			CacheDir:  cacheDir,
-			Format:    Qcow,
-			ImageName: imageName,
-			LocalPath: filepath.Join(cacheDir, imageName),
-			Sha256sum: info.Sha256Sum,
-			URL:       url,
-			VMName:    vmName,
-		},
-	}
-	dataDir, err := GetDataDir(vmType)
-	if err != nil {
-		return nil, err
-	}
-	fcd.Download.LocalUncompressedFile = fcd.GetLocalUncompressedFile(dataDir)
-	return fcd, nil
-}
-
 func (f FcosDownload) Get() *Download {
 	return &f.Download
 }
 
 type FcosDownloadInfo struct {
-	CompressionType string
+	CompressionType ImageCompression
 	Location        string
 	Release         string
 	Sha256Sum       string
@@ -216,9 +177,8 @@ func getStreamURL(streamType FCOSStream) url2.URL {
 	return fedoracoreos.GetStreamURL(streamType.String())
 }
 
-// This should get Exported and stay put as it will apply to all fcos downloads
+// GetFCOSDownload parses fedoraCoreOS's stream and returns the image download URL and the release version
-// getFCOS parses fedoraCoreOS's stream and returns the image download URL and the release version
+func (dl Download) GetFCOSDownload(imageStream FCOSStream) (*FcosDownloadInfo, error) {
-func GetFCOSDownload(vp VirtProvider, imageStream FCOSStream) (*FcosDownloadInfo, error) {
 	var (
 		fcosstable stream.Stream
 		altMeta    release.Release
@@ -256,7 +216,7 @@ func GetFCOSDownload(vp VirtProvider, imageStream FCOSStream) (*FcosDownloadInfo
 		return &FcosDownloadInfo{
 			Location:        disk.Location,
 			Sha256Sum:       disk.Sha256,
-			CompressionType: vp.Compression().String(),
+			CompressionType: dl.CompressionType,
 		}, nil
 	}
 
@@ -271,17 +231,17 @@ func GetFCOSDownload(vp VirtProvider, imageStream FCOSStream) (*FcosDownloadInfo
 	if upstreamArtifacts == nil {
 		return nil, fmt.Errorf("unable to pull VM image: no artifact in stream")
 	}
-	upstreamArtifact, ok := upstreamArtifacts[vp.Artifact().String()]
+	upstreamArtifact, ok := upstreamArtifacts[dl.Artifact.String()]
 	if !ok {
-		return nil, fmt.Errorf("unable to pull VM image: no %s artifact in stream", vp.Artifact().String())
+		return nil, fmt.Errorf("unable to pull VM image: no %s artifact in stream", dl.Artifact.String())
 	}
 	formats := upstreamArtifact.Formats
 	if formats == nil {
 		return nil, fmt.Errorf("unable to pull VM image: no formats in stream")
 	}
-	formatType, ok := formats[vp.Format().String()]
+	formatType, ok := formats[dl.Format.String()]
 	if !ok {
-		return nil, fmt.Errorf("unable to pull VM image: no %s format in stream", vp.Format().String())
+		return nil, fmt.Errorf("unable to pull VM image: no %s format in stream", dl.Format.String())
 	}
 	disk := formatType.Disk
 	if disk == nil {
@@ -291,7 +251,7 @@ func GetFCOSDownload(vp VirtProvider, imageStream FCOSStream) (*FcosDownloadInfo
 		Location:        disk.Location,
 		Release:         upstreamArtifact.Release,
 		Sha256Sum:       disk.Sha256,
-		CompressionType: vp.Compression().String(),
+		CompressionType: dl.CompressionType,
 	}, nil
 }
 
@@ -307,6 +267,10 @@ const (
 	Stable
 	// Podman-Testing
 	PodmanTesting
+	// Unknown
+	UnknownStream
+	// Custom
+	CustomStream
 )
 
 // String is a helper func for fcos streams
@@ -318,20 +282,26 @@ func (st FCOSStream) String() string {
 		return "next"
 	case PodmanTesting:
 		return "podman-testing"
+	case Stable:
+		return "stable"
 	}
-	return "stable"
+	return "custom"
 }
 
-func FCOSStreamFromString(s string) FCOSStream {
+func FCOSStreamFromString(s string) (FCOSStream, error) {
 	switch s {
 	case Testing.String():
-		return Testing
+		return Testing, nil
 	case Next.String():
-		return Next
+		return Next, nil
 	case PodmanTesting.String():
-		return PodmanTesting
+		return PodmanTesting, nil
+	case Stable.String():
+		return Stable, nil
+	case CustomStream.String():
+		return CustomStream, nil
 	}
-	return Stable
+	return UnknownStream, fmt.Errorf("unknown fcos stream: %s", s)
 }
 
 func IsValidFCOSStreamString(s string) bool {
@@ -344,6 +314,8 @@ func IsValidFCOSStreamString(s string) bool {
 		fallthrough
 	case Stable.String():
 		return true
+	case CustomStream.String():
+		return true
 	}
 
 	return false

pkg/machine/fcos_test.go

@@ -1,6 +1,8 @@
 package machine
 
-import "testing"
+import (
+	"testing"
+)
 
 func Test_compressionFromFile(t *testing.T) {
 	type args struct {
@@ -164,9 +166,9 @@ func TestFCOSStream_String(t *testing.T) {
 			want: "next",
 		},
 		{
-			name: "default is stable",
+			name: "default is custom",
-			st:   99,
+			st:   CustomStream,
-			want: "stable",
+			want: "custom",
 		},
 	}
 	for _, tt := range tests {

pkg/machine/hyperv/config.go

@@ -23,7 +23,7 @@ type HyperVVirtualization struct {
 
 func VirtualizationProvider() machine.VirtProvider {
 	return &HyperVVirtualization{
-		machine.NewVirtualization(machine.HyperV, machine.Zip, machine.Vhdx),
+		machine.NewVirtualization(machine.HyperV, machine.Zip, machine.Vhdx, vmtype),
 	}
 }
 
@@ -139,15 +139,19 @@ func (v HyperVVirtualization) NewMachine(opts machine.InitOptions) (machine.VM,
 	}
 	m.GvProxyPid = *gvProxyPid
 
+	dl, err := VirtualizationProvider().NewDownload(m.Name)
+	if err != nil {
+		return nil, err
+	}
 	// Acquire the image
-	imagePath, imageStream, err := v.acquireVMImage(opts)
+	imagePath, imageStream, err := dl.AcquireVMImage(opts.ImagePath)
 	if err != nil {
 		return nil, err
 	}
 
 	// assign values to machine
 	m.ImagePath = *imagePath
-	m.ImageStream = imageStream
+	m.ImageStream = imageStream.String()
 
 	config := hypervctl.HardwareConfig{
 		CPUs:     uint16(opts.CPUS),
@@ -173,44 +177,6 @@ func (v HyperVVirtualization) NewMachine(opts machine.InitOptions) (machine.VM,
 	return v.LoadVMByName(opts.Name)
 }
 
-// acquireVMImage determines if the image is already in a FCOS stream. If so,
-// retrieves the image path of the uncompressed file. Otherwise, the user has
-// provided an alternative image, so we set the image path and download the image.
-func (v HyperVVirtualization) acquireVMImage(opts machine.InitOptions) (*machine.VMFile, string, error) {
-	imageStream := opts.ImagePath
-	var imagePath *machine.VMFile
-	switch opts.ImagePath {
-	// TODO these need to be re-typed as FCOSStreams
-	case machine.Testing.String(), machine.Next.String(), machine.Stable.String(), "":
-		// Get image as usual
-		vp := VirtualizationProvider()
-		dd, err := machine.NewFcosDownloader(machine.HyperVVirt, opts.Name, machine.FCOSStreamFromString(imageStream), vp)
-		if err != nil {
-			return nil, "", err
-		}
-
-		uncompressedFile, err := machine.NewMachineFile(dd.Get().LocalUncompressedFile, nil)
-		if err != nil {
-			return nil, "", err
-		}
-
-		imagePath = uncompressedFile
-		if err := machine.DownloadImage(dd); err != nil {
-			return nil, "", err
-		}
-	default:
-		// The user has provided an alternate image which can be a file path
-		// or URL.
-		imageStream = "custom"
-		altImagePath, err := machine.AcquireAlternateImage(opts.Name, vmtype, opts)
-		if err != nil {
-			return nil, "", err
-		}
-		imagePath = altImagePath
-	}
-	return imagePath, imageStream, nil
-}
-
 func (v HyperVVirtualization) RemoveAndCleanMachines() error {
 	// Error handling used here is following what qemu did
 	var (

pkg/machine/pull.go

@@ -87,12 +87,12 @@ func supportedURL(path string) (url *url2.URL) {
 	}
 }
 
-func (d Download) GetLocalUncompressedFile(dataDir string) string {
+func (dl Download) GetLocalUncompressedFile(dataDir string) string {
-	compressedFilename := d.VMName + "_" + d.ImageName
+	compressedFilename := dl.VMName + "_" + dl.ImageName
 	extension := compressionFromFile(compressedFilename)
 	uncompressedFile := strings.TrimSuffix(compressedFilename, fmt.Sprintf(".%s", extension.String()))
-	d.LocalUncompressedFile = filepath.Join(dataDir, uncompressedFile)
+	dl.LocalUncompressedFile = filepath.Join(dataDir, uncompressedFile)
-	return d.LocalUncompressedFile
+	return dl.LocalUncompressedFile
 }
 
 func (g GenericDownload) Get() *Download {
@@ -388,8 +388,8 @@ func RemoveImageAfterExpire(dir string, expire time.Duration) error {
 
 // 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) {
+func (dl Download) AcquireAlternateImage(inputPath string) (*VMFile, error) {
-	g, err := NewGenericDownloader(vmtype, name, opts.ImagePath)
+	g, err := NewGenericDownloader(dl.VMKind, dl.VMName, inputPath)
 	if err != nil {
 		return nil, err
 	}

pkg/machine/qemu/config.go

@@ -335,7 +335,7 @@ func (p *QEMUVirtualization) VMType() machine.VMType {
 
 func VirtualizationProvider() machine.VirtProvider {
 	return &QEMUVirtualization{
-		machine.NewVirtualization(machine.Qemu, machine.Xz, machine.Qcow),
+		machine.NewVirtualization(machine.Qemu, machine.Xz, machine.Qcow, vmtype),
 	}
 }
 

pkg/machine/qemu/machine.go

@@ -195,44 +195,6 @@ func migrateVM(configPath string, config []byte, vm *MachineVM) error {
 	return os.Remove(configPath + ".orig")
 }
 
-// acquireVMImage determines if the image is already in a FCOS stream. If so,
-// retrieves the image path of the uncompressed file. Otherwise, the user has
-// provided an alternative image, so we set the image path and download the image.
-func (v *MachineVM) acquireVMImage(opts machine.InitOptions) error {
-	switch opts.ImagePath {
-	// TODO these need to be re-typed as FCOSStreams
-	case machine.Testing.String(), machine.Next.String(), machine.Stable.String(), "":
-		// Get image as usual
-		v.ImageStream = opts.ImagePath
-		vp := VirtualizationProvider()
-
-		dd, err := machine.NewFcosDownloader(vmtype, v.Name, machine.FCOSStreamFromString(opts.ImagePath), vp)
-		if err != nil {
-			return err
-		}
-
-		uncompressedFile, err := machine.NewMachineFile(dd.Get().LocalUncompressedFile, nil)
-		if err != nil {
-			return err
-		}
-
-		v.ImagePath = *uncompressedFile
-		if err := machine.DownloadImage(dd); err != nil {
-			return err
-		}
-	default:
-		// The user has provided an alternate image which can be a file path
-		// or URL.
-		v.ImageStream = "custom"
-		imagePath, err := machine.AcquireAlternateImage(v.Name, vmtype, opts)
-		if err != nil {
-			return err
-		}
-		v.ImagePath = *imagePath
-	}
-	return nil
-}
-
 // addMountsToVM converts the volumes passed through the CLI into the specified
 // volume driver and adds them to the machine
 func (v *MachineVM) addMountsToVM(opts machine.InitOptions) error {
@@ -318,10 +280,20 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) {
 	v.IdentityPath = util.GetIdentityPath(v.Name)
 	v.Rootful = opts.Rootful
 
-	if err := v.acquireVMImage(opts); err != nil {
+	dl, err := VirtualizationProvider().NewDownload(v.Name)
+	if err != nil {
 		return false, err
 	}
 
+	imagePath, strm, err := dl.AcquireVMImage(opts.ImagePath)
+	if err != nil {
+		return false, err
+	}
+
+	// Assign values about the download
+	v.ImagePath = *imagePath
+	v.ImageStream = strm.String()
+
 	// Add arch specific options including image location
 	v.CmdLine = append(v.CmdLine, v.addArchOptions()...)
 
@@ -334,15 +306,14 @@ 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())
 
-	err := machine.AddSSHConnectionsToPodmanSocket(
+	if err := machine.AddSSHConnectionsToPodmanSocket(
 		v.UID,
 		v.Port,
 		v.IdentityPath,
 		v.Name,
 		v.RemoteUsername,
 		opts,
-	)
+	); err != nil {
-	if err != nil {
 		return false, err
 	}
 

pkg/machine/wsl/config.go

@@ -19,7 +19,7 @@ type WSLVirtualization struct {
 
 func VirtualizationProvider() machine.VirtProvider {
 	return &WSLVirtualization{
-		machine.NewVirtualization(machine.None, machine.Xz, machine.Tar),
+		machine.NewVirtualization(machine.None, machine.Xz, machine.Tar, vmtype),
 	}
 }