From 43eb35a7722b7284b56146a051b38afdfca41ae0 Mon Sep 17 00:00:00 2001 From: Brent Baude Date: Mon, 20 Feb 2023 13:39:42 -0600 Subject: [PATCH] Machine refactor for QEMU/AppleHV in preparation for adding hyper as a machine option, several common functions needed to be moved specifically from qemu to a common area in pkg/machine. this usually involved functions and variables related to using fcos as a machine image as well as its compression, artifact, and image format. [NO NEW TESTS NEEEDED] Signed-off-by: Brent Baude --- pkg/machine/applehv/config.go | 52 ++++++++++++++ pkg/machine/applehv/machine.go | 39 ++--------- pkg/machine/config.go | 15 ++-- pkg/machine/e2e/machine_test.go | 6 +- pkg/machine/fcos.go | 118 +++++++++++++++++++------------- pkg/machine/fcos_test.go | 48 +++++++++++-- pkg/machine/qemu/config.go | 19 ++--- pkg/machine/qemu/machine.go | 26 +++++-- pkg/machine/wsl/fedora.go | 4 +- pkg/machine/wsl/machine.go | 25 ++++++- pkg/machine/wsl/util_windows.go | 6 +- 11 files changed, 241 insertions(+), 117 deletions(-) create mode 100644 pkg/machine/applehv/config.go diff --git a/pkg/machine/applehv/config.go b/pkg/machine/applehv/config.go new file mode 100644 index 0000000000..6cab16b53f --- /dev/null +++ b/pkg/machine/applehv/config.go @@ -0,0 +1,52 @@ +//go:build arm64 && darwin +// +build arm64,darwin + +package applehv + +import "github.com/containers/podman/v4/pkg/machine" + +type Virtualization struct { + artifact machine.Artifact + compression machine.ImageCompression + format machine.ImageFormat +} + +func (v Virtualization) Artifact() machine.Artifact { + return machine.None +} + +func (v Virtualization) CheckExclusiveActiveVM() (bool, string, error) { + return false, "", machine.ErrNotImplemented +} + +func (v Virtualization) Compression() machine.ImageCompression { + return v.compression +} + +func (v Virtualization) Format() machine.ImageFormat { + return v.format +} + +func (v Virtualization) IsValidVMName(name string) (bool, error) { + return false, machine.ErrNotImplemented +} + +func (v Virtualization) List(opts machine.ListOptions) ([]*machine.ListResponse, error) { + return nil, machine.ErrNotImplemented +} + +func (v Virtualization) LoadVMByName(name string) (machine.VM, error) { + return nil, machine.ErrNotImplemented +} + +func (v Virtualization) NewMachine(opts machine.InitOptions) (machine.VM, error) { + return nil, machine.ErrNotImplemented +} + +func (v Virtualization) RemoveAndCleanMachines() error { + return machine.ErrNotImplemented +} + +func (v Virtualization) VMType() string { + return vmtype +} diff --git a/pkg/machine/applehv/machine.go b/pkg/machine/applehv/machine.go index 742bff6d51..8bdff227d1 100644 --- a/pkg/machine/applehv/machine.go +++ b/pkg/machine/applehv/machine.go @@ -1,5 +1,5 @@ -//go:build arm64 && !windows && !linux -// +build arm64,!windows,!linux +//go:build arm64 && darwin +// +build arm64,darwin package applehv @@ -9,16 +9,17 @@ import ( "github.com/containers/podman/v4/pkg/machine" ) -type Provider struct{} - var ( - hvProvider = &Provider{} // vmtype refers to qemu (vs libvirt, krun, etc). vmtype = "apple" ) func GetVirtualizationProvider() machine.VirtProvider { - return hvProvider + return &Virtualization{ + artifact: machine.None, + compression: machine.Xz, + format: machine.Qcow, + } } const ( @@ -41,30 +42,4 @@ const ( dockerGlobal ) -func (p *Provider) NewMachine(opts machine.InitOptions) (machine.VM, error) { - return nil, machine.ErrNotImplemented -} - -func (p *Provider) LoadVMByName(name string) (machine.VM, error) { - return nil, machine.ErrNotImplemented -} - -func (p *Provider) List(opts machine.ListOptions) ([]*machine.ListResponse, error) { - return nil, machine.ErrNotImplemented -} - -func (p *Provider) IsValidVMName(name string) (bool, error) { - return false, machine.ErrNotImplemented -} - -func (p *Provider) CheckExclusiveActiveVM() (bool, string, error) { - return false, "", machine.ErrNotImplemented -} - -func (p *Provider) RemoveAndCleanMachines() error { - return machine.ErrNotImplemented -} - -func (p *Provider) VMType() string { - return vmtype } diff --git a/pkg/machine/config.go b/pkg/machine/config.go index 1d77ad4cc6..035ba8434a 100644 --- a/pkg/machine/config.go +++ b/pkg/machine/config.go @@ -48,11 +48,14 @@ const ( ) type VirtProvider interface { - NewMachine(opts InitOptions) (VM, error) - LoadVMByName(name string) (VM, error) - List(opts ListOptions) ([]*ListResponse, error) - IsValidVMName(name string) (bool, error) + Artifact() Artifact CheckExclusiveActiveVM() (bool, string, error) + Compression() ImageCompression + Format() ImageFormat + IsValidVMName(name string) (bool, error) + List(opts ListOptions) ([]*ListResponse, error) + LoadVMByName(name string) (VM, error) + NewMachine(opts InitOptions) (VM, error) RemoveAndCleanMachines() error VMType() string } @@ -72,10 +75,10 @@ var ( type Download struct { Arch string - Artifact artifact + Artifact Artifact CompressionType string CacheDir string - Format imageFormat + Format ImageFormat ImageName string LocalPath string LocalUncompressedFile string diff --git a/pkg/machine/e2e/machine_test.go b/pkg/machine/e2e/machine_test.go index dd5031fde2..a2c6d9f3d9 100644 --- a/pkg/machine/e2e/machine_test.go +++ b/pkg/machine/e2e/machine_test.go @@ -12,6 +12,7 @@ import ( "time" "github.com/containers/podman/v4/pkg/machine" + "github.com/containers/podman/v4/pkg/machine/qemu" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" ) @@ -21,7 +22,7 @@ func TestMain(m *testing.M) { } const ( - defaultStream string = "testing" + defaultStream machine.FCOSStream = machine.Testing ) var ( @@ -43,7 +44,8 @@ func TestMachine(t *testing.T) { } var _ = BeforeSuite(func() { - fcd, err := machine.GetFCOSDownload(defaultStream, machine.Xz) + qemuVP := qemu.GetVirtualizationProvider() + fcd, err := machine.GetFCOSDownload(qemuVP, defaultStream) if err != nil { Fail("unable to get virtual machine image") } diff --git a/pkg/machine/fcos.go b/pkg/machine/fcos.go index 309235840a..8b8e4ce30b 100644 --- a/pkg/machine/fcos.go +++ b/pkg/machine/fcos.go @@ -22,9 +22,9 @@ import ( "github.com/sirupsen/logrus" ) -type imageCompression int64 -type artifact int64 -type imageFormat int64 +type ImageCompression int64 +type Artifact int64 +type ImageFormat int64 const ( // Used for testing the latest podman in fcos @@ -33,17 +33,17 @@ const ( PodmanTestingHost = "fedorapeople.org" PodmanTestingURL = "groups/podman/testing" - Xz imageCompression = iota + Xz ImageCompression = iota Zip Gz Bz2 - Qemu artifact = iota + Qemu Artifact = iota HyperV None - qcow imageFormat = iota - vhdx + Qcow ImageFormat = iota + Vhdx Tar ) @@ -55,16 +55,16 @@ const ( // typed strongly // -func (a artifact) String() string { +func (a Artifact) String() string { if a == HyperV { return "hyperv" } return "qemu" } -func (imf imageFormat) String() string { +func (imf ImageFormat) String() string { switch imf { - case vhdx: + case Vhdx: return "vhdx.zip" case Tar: return "tar.xz" @@ -72,7 +72,7 @@ func (imf imageFormat) String() string { return "qcow2.xz" } -func (c imageCompression) String() string { +func (c ImageCompression) String() string { switch c { case Gz: return "gz" @@ -84,7 +84,7 @@ func (c imageCompression) String() string { return "xz" } -func compressionFromFile(path string) imageCompression { +func compressionFromFile(path string) ImageCompression { switch { case strings.HasSuffix(path, Bz2.String()): return Bz2 @@ -100,8 +100,8 @@ type FcosDownload struct { Download } -func NewFcosDownloader(vmType, vmName, imageStream string) (DistributionDownload, error) { - info, err := GetFCOSDownload(imageStream, Xz) +func NewFcosDownloader(vmType, vmName string, imageStream FCOSStream, vp VirtProvider) (DistributionDownload, error) { + info, err := GetFCOSDownload(vp, imageStream) if err != nil { return nil, err } @@ -122,7 +122,7 @@ func NewFcosDownloader(vmType, vmName, imageStream string) (DistributionDownload Arch: GetFcosArch(), Artifact: Qemu, CacheDir: cacheDir, - Format: qcow, + Format: Qcow, ImageName: imageName, LocalPath: filepath.Join(cacheDir, imageName), Sha256sum: info.Sha256Sum, @@ -194,41 +194,28 @@ func GetFcosArch() string { // getStreamURL is a wrapper for the fcos.GetStream URL // so that we can inject a special stream and url for // testing podman before it merges into fcos builds -func getStreamURL(streamType string) url2.URL { +func getStreamURL(streamType FCOSStream) url2.URL { // For the podmanTesting stream type, we point to // a custom url on fedorapeople.org - if streamType == podmanTesting { + if streamType == PodmanTesting { return url2.URL{ Scheme: "https", Host: PodmanTestingHost, Path: fmt.Sprintf("%s/%s.json", PodmanTestingURL, "podman4"), } } - return fedoracoreos.GetStreamURL(streamType) + return fedoracoreos.GetStreamURL(streamType.String()) } // This should get Exported and stay put as it will apply to all fcos downloads // getFCOS parses fedoraCoreOS's stream and returns the image download URL and the release version -func GetFCOSDownload(imageStream string, compression imageCompression) (*FcosDownloadInfo, error) { +func GetFCOSDownload(vp VirtProvider, imageStream FCOSStream) (*FcosDownloadInfo, error) { var ( fcosstable stream.Stream altMeta release.Release - streamType string ) - switch imageStream { - case "podman-testing": - streamType = "podman-testing" - case "testing", "": - streamType = fedoracoreos.StreamTesting - case "next": - streamType = fedoracoreos.StreamNext - case "stable": - streamType = fedoracoreos.StreamStable - default: - return nil, fmt.Errorf("invalid stream %s: valid streams are `testing` and `stable`", imageStream) - } - streamurl := getStreamURL(streamType) + streamurl := getStreamURL(imageStream) resp, err := http.Get(streamurl.String()) if err != nil { return nil, err @@ -242,7 +229,7 @@ func GetFCOSDownload(imageStream string, compression imageCompression) (*FcosDow logrus.Error(err) } }() - if imageStream == podmanTesting { + if imageStream == PodmanTesting { if err := json.Unmarshal(body, &altMeta); err != nil { return nil, err } @@ -251,7 +238,7 @@ func GetFCOSDownload(imageStream string, compression imageCompression) (*FcosDow if !ok { return nil, fmt.Errorf("unable to pull VM image: no targetArch in stream") } - qcow2, ok := arches.Media.Qemu.Artifacts[qcow.String()] + qcow2, ok := arches.Media.Qemu.Artifacts[Qcow.String()] if !ok { return nil, fmt.Errorf("unable to pull VM image: no qcow2.xz format in stream") } @@ -260,7 +247,7 @@ func GetFCOSDownload(imageStream string, compression imageCompression) (*FcosDow return &FcosDownloadInfo{ Location: disk.Location, Sha256Sum: disk.Sha256, - CompressionType: compression.String(), + CompressionType: vp.Compression().String(), }, nil } @@ -271,30 +258,69 @@ func GetFCOSDownload(imageStream string, compression imageCompression) (*FcosDow if !ok { return nil, fmt.Errorf("unable to pull VM image: no targetArch in stream") } - artifacts := arch.Artifacts - if artifacts == nil { + upstreamArtifacts := arch.Artifacts + if upstreamArtifacts == nil { return nil, fmt.Errorf("unable to pull VM image: no artifact in stream") } - qemu, ok := artifacts[Qemu.String()] + upstreamArtifact, ok := upstreamArtifacts[vp.Artifact().String()] if !ok { - return nil, fmt.Errorf("unable to pull VM image: no qemu artifact in stream") + return nil, fmt.Errorf("unable to pull VM image: no %s artifact in stream", vp.Artifact().String()) } - formats := qemu.Formats + formats := upstreamArtifact.Formats if formats == nil { return nil, fmt.Errorf("unable to pull VM image: no formats in stream") } - qcow, ok := formats[qcow.String()] + formatType, ok := formats[vp.Format().String()] if !ok { - return nil, fmt.Errorf("unable to pull VM image: no qcow2.xz format in stream") + return nil, fmt.Errorf("unable to pull VM image: no %s format in stream", vp.Format().String()) } - disk := qcow.Disk + disk := formatType.Disk if disk == nil { return nil, fmt.Errorf("unable to pull VM image: no disk in stream") } return &FcosDownloadInfo{ Location: disk.Location, - Release: qemu.Release, + Release: upstreamArtifact.Release, Sha256Sum: disk.Sha256, - CompressionType: compression.String(), + CompressionType: vp.Compression().String(), }, nil } + +type FCOSStream int64 + +const ( + // FCOS streams + // Testing FCOS stream + Testing FCOSStream = iota + // Next FCOS stream + Next + // Stable FCOS stream + Stable + // Podman-Testing + PodmanTesting +) + +// String is a helper func for fcos streams +func (st FCOSStream) String() string { + switch st { + case Testing: + return "testing" + case Next: + return "next" + case PodmanTesting: + return "podman-testing" + } + return "stable" +} + +func FCOSStreamFromString(s string) FCOSStream { + switch s { + case Testing.String(): + return Testing + case Next.String(): + return Next + case PodmanTesting.String(): + return PodmanTesting + } + return Stable +} diff --git a/pkg/machine/fcos_test.go b/pkg/machine/fcos_test.go index b9a8932baf..841a78cbe3 100644 --- a/pkg/machine/fcos_test.go +++ b/pkg/machine/fcos_test.go @@ -9,7 +9,7 @@ func Test_compressionFromFile(t *testing.T) { var tests = []struct { name string args args - want imageCompression + want ImageCompression }{ { name: "xz", @@ -52,7 +52,7 @@ func Test_compressionFromFile(t *testing.T) { func TestImageCompression_String(t *testing.T) { tests := []struct { name string - c imageCompression + c ImageCompression want string }{ { @@ -93,17 +93,17 @@ func TestImageCompression_String(t *testing.T) { func TestImageFormat_String(t *testing.T) { tests := []struct { name string - imf imageFormat + imf ImageFormat want string }{ { name: "vhdx.zip", - imf: vhdx, + imf: Vhdx, want: "vhdx.zip", }, { name: "qcow2", - imf: qcow, + imf: Qcow, want: "qcow2.xz", }, } @@ -119,7 +119,7 @@ func TestImageFormat_String(t *testing.T) { func Test_artifact_String(t *testing.T) { tests := []struct { name string - a artifact + a Artifact want string }{ { @@ -141,3 +141,39 @@ func Test_artifact_String(t *testing.T) { }) } } + +func TestFCOSStream_String(t *testing.T) { + tests := []struct { + name string + st FCOSStream + want string + }{ + { + name: "testing", + st: Testing, + want: "testing", + }, + { + name: "stable", + st: Stable, + want: "stable", + }, + { + name: "next", + st: Next, + want: "next", + }, + { + name: "default is stable", + st: 99, + want: "stable", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.st.String(); got != tt.want { + t.Errorf("String() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/pkg/machine/qemu/config.go b/pkg/machine/qemu/config.go index 79f90ed901..730dc4f038 100644 --- a/pkg/machine/qemu/config.go +++ b/pkg/machine/qemu/config.go @@ -6,20 +6,11 @@ import ( "github.com/containers/podman/v4/pkg/machine" ) -const ( - // FCOS streams - // Testing FCOS stream - Testing string = "testing" - // Next FCOS stream - Next string = "next" - // Stable FCOS stream - Stable string = "stable" - - // Max length of fully qualified socket path - -) - -type Virtualization struct{} +type Virtualization struct { + artifact machine.Artifact + compression machine.ImageCompression + format machine.ImageFormat +} // Deprecated: MachineVMV1 is being deprecated in favor a more flexible and informative // structure diff --git a/pkg/machine/qemu/machine.go b/pkg/machine/qemu/machine.go index e87e6e789c..5b759cc88a 100644 --- a/pkg/machine/qemu/machine.go +++ b/pkg/machine/qemu/machine.go @@ -35,13 +35,17 @@ import ( ) var ( - qemuProvider = &Virtualization{} // vmtype refers to qemu (vs libvirt, krun, etc). + // Could this be moved into Provider vmtype = "qemu" ) func GetVirtualizationProvider() machine.VirtProvider { - return qemuProvider + return &Virtualization{ + artifact: machine.Qemu, + compression: machine.Xz, + format: machine.Qcow, + } } const ( @@ -252,10 +256,12 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) { v.Rootful = opts.Rootful switch opts.ImagePath { - case Testing, Next, Stable, "": + // 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 - dd, err := machine.NewFcosDownloader(vmtype, v.Name, opts.ImagePath) + vp := GetVirtualizationProvider() + dd, err := machine.NewFcosDownloader(vmtype, v.Name, machine.FCOSStreamFromString(opts.ImagePath), vp) if err != nil { return false, err @@ -1221,6 +1227,18 @@ func (p *Virtualization) CheckExclusiveActiveVM() (bool, string, error) { return false, "", nil } +func (p *Virtualization) Artifact() machine.Artifact { + return p.artifact +} + +func (p *Virtualization) Compression() machine.ImageCompression { + return p.compression +} + +func (p *Virtualization) Format() machine.ImageFormat { + return p.format +} + // startHostNetworking runs a binary on the host system that allows users // to set up port forwarding to the podman virtual machine func (v *MachineVM) startHostNetworking() (string, apiForwardingState, error) { diff --git a/pkg/machine/wsl/fedora.go b/pkg/machine/wsl/fedora.go index 6888486383..83920cca85 100644 --- a/pkg/machine/wsl/fedora.go +++ b/pkg/machine/wsl/fedora.go @@ -1,5 +1,5 @@ -//go:build amd64 || arm64 -// +build amd64 arm64 +//go:build windows +// +build windows package wsl diff --git a/pkg/machine/wsl/machine.go b/pkg/machine/wsl/machine.go index 0775837ad0..dfe04c7c2d 100644 --- a/pkg/machine/wsl/machine.go +++ b/pkg/machine/wsl/machine.go @@ -28,7 +28,6 @@ import ( ) var ( - wslProvider = &Virtualization{} // vmtype refers to qemu (vs libvirt, krun, etc) vmtype = "wsl" ) @@ -204,7 +203,23 @@ const ( globalPipe = "docker_engine" ) -type Virtualization struct{} +type Virtualization struct { + artifact machine.Artifact + compression machine.ImageCompression + format machine.ImageFormat +} + +func (p *Virtualization) Artifact() machine.Artifact { + return p.artifact +} + +func (p *Virtualization) Compression() machine.ImageCompression { + return p.compression +} + +func (p *Virtualization) Format() machine.ImageFormat { + return p.format +} type MachineVM struct { // ConfigPath is the path to the configuration file @@ -236,7 +251,11 @@ func (e *ExitCodeError) Error() string { } func GetWSLProvider() machine.VirtProvider { - return wslProvider + return &Virtualization{ + artifact: machine.None, + compression: machine.Xz, + format: machine.Tar, + } } // NewMachine initializes an instance of a wsl machine diff --git a/pkg/machine/wsl/util_windows.go b/pkg/machine/wsl/util_windows.go index 759061c124..8672f2593f 100644 --- a/pkg/machine/wsl/util_windows.go +++ b/pkg/machine/wsl/util_windows.go @@ -1,3 +1,6 @@ +//go:build windows +// +build windows + package wsl import ( @@ -12,11 +15,10 @@ import ( "unicode/utf16" "unsafe" + "github.com/containers/storage/pkg/homedir" "github.com/sirupsen/logrus" "golang.org/x/sys/windows" "golang.org/x/sys/windows/registry" - - "github.com/containers/storage/pkg/homedir" ) // nolint