From e838ad86b889b4901bbf2e715b729d8d7e43c8ff Mon Sep 17 00:00:00 2001 From: Brent Baude Date: Mon, 20 Feb 2023 10:40:13 -0600 Subject: [PATCH 1/2] machine refactoring preparations for hyperv before we can support hyperv as a virtualization option for podman machine, several areas in machine will require cleanup. this is the first pass of these changes to keep the review burden low. changes include: * convert artifact, format (image format) and compression to enums with string methods * rename Provider interface to VirtProvider * change Provider implementation in QEMU to QEMUVirt * change Provider implementation in WSL to WSLVirt as mentioned earlier, there will be several more of these refactoring PRs because assumptions were made about associations of platforms and virt providers as well as compression and image formats. Signed-off-by: Brent Baude --- pkg/machine/pull.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/machine/pull.go b/pkg/machine/pull.go index f59f681e1c..7498223f1c 100644 --- a/pkg/machine/pull.go +++ b/pkg/machine/pull.go @@ -212,7 +212,7 @@ func Decompress(localPath, uncompressedPath string) error { return decompressEverythingElse(localPath, uncompressedFileWriter) } -// Will error out if file without .xz already exists +// Will error out if file without .Xz already exists // Maybe extracting then renaming is a good idea here.. // depends on Xz: not pre-installed on mac, so it becomes a brew dependency func decompressXZ(src string, output io.WriteCloser) error { From 43eb35a7722b7284b56146a051b38afdfca41ae0 Mon Sep 17 00:00:00 2001 From: Brent Baude Date: Mon, 20 Feb 2023 13:39:42 -0600 Subject: [PATCH 2/2] 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