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

bootc_disk: Add support for vmdk, ova #504

Merged
merged 3 commits into from
Mar 8, 2024
Merged

Conversation

cgwalters
Copy link
Contributor

@cgwalters cgwalters commented Mar 7, 2024

BIB PR using this: osbuild/bootc-image-builder#251


We want to support generating VMDK for bootc containers too.

@cgwalters
Copy link
Contributor Author

cgwalters commented Mar 7, 2024

So...I tried this out along with this patch for bib

From 2dd6c0ea3146191b16f6452f5cc3c3d6b191bc44 Mon Sep 17 00:00:00 2001
From: Colin Walters <walters@verbum.org>
Date: Wed, 6 Mar 2024 18:04:21 -0500
Subject: [PATCH] wip

---
 bib/cmd/bootc-image-builder/image.go | 8 +++++++-
 bib/cmd/bootc-image-builder/main.go  | 6 +++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/bib/cmd/bootc-image-builder/image.go b/bib/cmd/bootc-image-builder/image.go
index d80c38a..4f8d89f 100644
--- a/bib/cmd/bootc-image-builder/image.go
+++ b/bib/cmd/bootc-image-builder/image.go
@@ -48,7 +48,7 @@ func Manifest(c *ManifestConfig) (*manifest.Manifest, error) {
 	rng := createRand()
 
 	switch c.ImgType {
-	case "ami", "qcow2", "raw":
+	case "ami", "qcow2", "raw", "vmdk", "ova":
 		return manifestForDiskImage(c, rng)
 	case "anaconda-iso", "iso":
 		return manifestForISO(c, rng)
@@ -95,6 +95,12 @@ func manifestForDiskImage(c *ManifestConfig, rng *rand.Rand) (*manifest.Manifest
 	case "ami", "raw":
 		imageFormat = platform.FORMAT_RAW
 		filename = "disk.raw"
+	case "vmdk":
+		imageFormat = platform.FORMAT_VMDK
+		filename = "disk.vmdk"
+	case "ova":
+		imageFormat = platform.FORMAT_OVA
+		filename = "disk.ova"
 	}
 
 	switch c.Architecture {
diff --git a/bib/cmd/bootc-image-builder/main.go b/bib/cmd/bootc-image-builder/main.go
index 9423e48..fd03ce4 100644
--- a/bib/cmd/bootc-image-builder/main.go
+++ b/bib/cmd/bootc-image-builder/main.go
@@ -282,10 +282,14 @@ func cmdBuild(cmd *cobra.Command, args []string) error {
 		exports = []string{"qcow2"}
 	case "ami", "raw":
 		exports = []string{"image"}
+	case "vmdk":
+		exports = []string{"vmdk"}
+	case "ova":
+		exports = []string{"ovf"}
 	case "anaconda-iso", "iso":
 		exports = []string{"bootiso"}
 	default:
-		return fmt.Errorf("valid types are 'qcow2', 'ami', 'raw', 'anaconda-iso', not: '%s'", imgType)
+		return fmt.Errorf("valid types are 'qcow2', 'ami', 'raw', 'vmdk', 'anaconda-iso', not: '%s'", imgType)
 	}
 
 	manifestPath := filepath.Join(outputDir, manifest_fname)
-- 
2.43.0

And with that, I get this:

FileNotFoundError: [Errno 2] No such file or directory: 'qemu-img'

And yes, now I realize this is harder than I thought, because we need to either:

  • Add that into our base images and assume its presence (eww)
  • Add that into bib (edit: it's already there! thankfully), and rework the build pipeline to use the container as buildroot (I have no idea about doing this)

pkg/image/bootc_disk.go Outdated Show resolved Hide resolved
@cgwalters cgwalters changed the title bootc_disk: Add support for vmdk bootc_disk: Add support for vmdk, ova Mar 7, 2024
achilleas-k

This comment was marked as outdated.

@cgwalters cgwalters marked this pull request as ready for review March 7, 2024 21:49
@cgwalters
Copy link
Contributor Author

OK, if the remaining work is bib side and you think this is right - then lifting draft here.

pkg/image/bootc_disk.go Outdated Show resolved Hide resolved
@achilleas-k
Copy link
Member

  • Add that into bib (edit: it's already there! thankfully), and rework the build pipeline to use the container as buildroot (I have no idea about doing this)

See my in-line comment https://github.com/osbuild/images/pull/504/files#r1516881294

@achilleas-k achilleas-k self-requested a review March 7, 2024 21:52
Copy link
Member

@achilleas-k achilleas-k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make pipelines run outside the build root.

@achilleas-k
Copy link
Member

OK, if the remaining work is bib side and you think this is right - then lifting draft here.

Sorry for the confusion. I looked at the code before noticing the comment and it looked good but then realised the issue with the build root :)

Similar to eb24e7e which did
this for qcow2.

(Also fixing the typo `buidPipeline` in the ovf copy)
cgwalters and others added 2 commits March 7, 2024 17:30
We want to support generating these for bootc containers too.

As the TODO says there's some clearly sharable code with `disk.go`.
@cgwalters
Copy link
Contributor Author

OK yep updated, this generates an image! No idea if it works in vsphere yet, will ping about that.

Copy link
Contributor

@mvo5 mvo5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Let's land it as is and we (images team) will look into the suggested refactor (which makes a ton of sense) in a followup

pkg/image/bootc_disk.go Show resolved Hide resolved
@mvo5 mvo5 dismissed achilleas-k’s stale review March 8, 2024 08:29

This is addressed now, the pipeline runs now from the bib context.

@cgwalters cgwalters added this pull request to the merge queue Mar 8, 2024
Merged via the queue into osbuild:main with commit 6b51d80 Mar 8, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants