Add convert command using bib #58

germag · 2024-07-05T12:20:11Z

Add convert command that uses bootc image builder (bib) to create disk images

It's marked as draft because it requires some refactoring to cache the results. And complete the AMI options
The bib doc (https://github.com/osbuild/bootc-image-builder?tab=readme-ov-file#%EF%B8%8F-cloud-uploaders) show how to pass some of the AWS configuration using env vars and the aws directory:

  -v $HOME/.aws:/root/.aws:ro \
  --env AWS_PROFILE=default \

$ cat aws.secrets
AWS_ACCESS_KEY_ID=AKIAIOSFODNN7EXAMPLE
AWS_SECRET_ACCESS_KEY=wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY

..
 --env-file=aws.secrets \
..

So, I'm not sure if adding a generic --env-file or more specific ones: --aws-env-vars and --aws-cfg-dir

I think there is a bug in osbuild, the export function (that calls "cp --reflink=auto ...") returns an error if stdin is not connected to the container, that is the reason --quiet shows an error even after succeeding, and for some reason that error doesn't happen if podman machine uses virtiofsd.

vrothberg · 2024-07-08T07:21:24Z

I'd be tempted to ignore the AWS-specific features. I am sure there AWS-specific CLI tools which can do that already, so just converting to an AMI checks the box for me.

germag · 2024-07-08T09:17:03Z

I'd be tempted to ignore the AWS-specific features. I am sure there AWS-specific CLI tools which can do that already, so just converting to an AMI checks the box for me.

Do you mean also removing --aws-ami-name, --aws-bucket, and --aws-region ?

vrothberg · 2024-07-08T09:28:02Z

Do you mean also removing --aws-ami-name, --aws-bucket, and --aws-region ?

For now, yes. Let's focus on converting first. AWS integration is up to debate IMO.

germag · 2024-07-08T09:53:19Z

Do you mean also removing --aws-ami-name, --aws-bucket, and --aws-region ?

For now, yes. Let's focus on converting first. AWS integration is up to debate IMO.

I almost forgot, at DevConf.cz I talked to @ondrejbudai, and he told me that he intends to use podman-bootc convert to test boot-image-builder as part of their CI/CD. That is the reason behind the PODMAN_BOOTC_BIB_IMAGE env var, so maybe we can add a PODMAN_BOOTC_BIB_EXTRA_ARGS env var so they can pass any arguments even if we don't support it (it will not work for -v, --env and --env-file, tho)

vrothberg · 2024-07-08T10:58:42Z

SGTM 👍

cmd/convert.go

rhatdan · 2024-07-08T16:52:04Z

If we are going to support uploads, I would prefer these options to become more generic.
--aws-ami-name, --aws-bucket, and --aws-region ?

I would figure users would want similar support for Azure and Google Cloud ...

germag · 2024-07-08T18:52:35Z

If we are going to support uploads, I would prefer these options to become more generic. --aws-ami-name, --aws-bucket, and --aws-region ?

I would figure users would want similar support for Azure and Google Cloud ...

I'm worried about an explosion of options, maybe instead of podman-bootc --format ami|qcow2|etc ... we need to do something like:

podman-bootc convert-to ami --name --bucket --reagion ...

so each "format" is a sub-command with its own options if it's needed, o none like

podman-bootc convert-to qcow2 ...

cgwalters · 2024-07-08T19:02:53Z

This strongly relates to https://gitlab.com/fedora/bootc/tracker/-/issues/2 which argues that in a nutshell:

bootc-image-builder becomes bootc-sdk
podman-bootc becomes a thin wrapper to execute code in bootc-sdk

In this world it's actually bootc-sdk that has sub-verbs, not the tool here.

germag · 2024-07-09T13:48:23Z

This strongly relates to https://gitlab.com/fedora/bootc/tracker/-/issues/2 which argues that in a nutshell:
* bootc-image-builder becomes bootc-sdk

* podman-bootc becomes a thin wrapper to execute code in bootc-sdk
In this world it's actually bootc-sdk that has sub-verbs, not the tool here.

Agreed, but in the meantime we need to do something here to support bib. I think going to a sub-command per format is nicer from a UI perspective.

I still need to understand how the sdk will work, I thought it will be more like a library, in any case I agree that podma-bootc should be a wrapper around it

vrothberg · 2024-07-09T13:53:22Z

I still need to understand how the sdk will work, I thought it will be more like a library, in any case I agree that podma-bootc should be a wrapper around it

👍 podman-bootc convert is something we need short term. The SDK idea needs more cooking beyond August.

germag · 2024-07-09T15:19:01Z

Removed all the AMI specific options
Read an env var PODMAN_BOOTC_BIB_EXTRA for additional arguments for BIB (should I also add that for podman?)
It uses the BIB defined in the bootc.diskimage-builder label, if present

Although I prefer podman-bootc convert-to <format> than podman-bootc convert --format <format> I think we can go with this I change that later

Add convert command that uses bootc image builder (bib) to create disk images Signed-off-by: German Maglione <gmaglione@redhat.com>

vrothberg · 2024-07-10T09:06:44Z

cmd/convert.go

+	RootCmd.AddCommand(convertCmd)
+	convertCmd.Flags().BoolVar(&quiet, "quiet", false, "Suppress output from disk image creation")
+	convertCmd.Flags().StringVar(&options.Config, "config", "", "Image builder config file")
+	convertCmd.Flags().StringVar(&options.Output, "output", ".", "output directory (default \".\")")


Suggested change

convertCmd.Flags().StringVar(&options.Output, "output", ".", "output directory (default \".\")")

convertCmd.Flags().StringVar(&options.Output, "output", ".", "output directory")

To avoid it being displayed twice.

vrothberg · 2024-07-10T09:09:27Z

pkg/bib/build.go

+
+	_, err = os.Stat(buildOption.Config)
+	if err != nil {
+		return fmt.Errorf("config file %s: %w", buildOption.Config, err)


Suggested change

return fmt.Errorf("config file %s: %w", buildOption.Config, err)

return fmt.Errorf("config file %q: %w", buildOption.Config, err)

vrothberg · 2024-07-10T09:09:37Z

pkg/bib/build.go

+func Build(ctx context.Context, user user.User, imageNameOrId string, quiet bool, buildOption BuildOption) error {
+	outputInfo, err := os.Stat(buildOption.Output)
+	if err != nil {
+		return fmt.Errorf("output directory %s: %w", buildOption.Output, err)


Suggested change

return fmt.Errorf("output directory %s: %w", buildOption.Output, err)

return fmt.Errorf("output directory %q: %w", buildOption.Output, err)

vrothberg · 2024-07-10T09:10:21Z

pkg/bib/build.go

+	BibExtraArgs      []string
+}
+
+func Build(ctx context.Context, user user.User, imageNameOrId string, quiet bool, buildOption BuildOption) error {


I think we should error out early here when --output or --config haven't been set.

For the config, we could consider creating an empty temp file and pass that on?

vrothberg · 2024-07-10T09:10:32Z

pkg/bib/build.go

+	}
+
+	if !outputInfo.IsDir() {
+		return fmt.Errorf("%s is not a directory ", buildOption.Output)


Suggested change

return fmt.Errorf("%s is not a directory ", buildOption.Output)

return fmt.Errorf("%q is not a directory ", buildOption.Output)

cdrage

Awesome work! 🚀

cdrage · 2024-07-11T15:14:18Z

cmd/convert.go

+	// the image
+	convertCmd.Flags().StringVar(&options.Filesystem, "filesystem", "", "Overrides the root filesystem (e.g. xfs, btrfs, ext4)")
+	// Corresponds to bib '--type', using '--format' to be consistent with podman
+	convertCmd.Flags().StringVar(&options.Format, "format", "qcow2", "Disk image type (ami, anaconda-iso, iso, qcow2, raw, vmdk) [default: qcow2]")


should iso be dropped as it's an alias for anaconda-iso?

it was also removed from the README, see: https://github.com/osbuild/bootc-image-builder?tab=readme-ov-file#-image-types

cdrage · 2024-07-11T15:17:09Z

pkg/bib/build.go

+
+func bibArguments(imageNameOrId string, buildOption BuildOption) []string {
+	args := []string{
+		"--local", // we pull the image if necessary, so don't pull it from a registry


--local is always true now and is being deprecated, so no need for this.

cdrage · 2024-07-11T15:20:18Z

pkg/bib/build.go

+				},
+				{
+					Source:      "/var/lib/containers/storage",
+					Destination: "/var/lib/containers/storage",


I'm assuming that this command is always running in sudo / root and we shouldn't use ~/.local/share/containers/storage/ right?

This would cause an issue if a user did podman pull not sudo podman pull before running podman-bootc.

Unsure to be honest how this should be worked around / I guess we assume that podman-bootc is always running privileged?

I guess we assume that podman-bootc is always running privileged?

No; we assume the container is running privileged inside podman machine.

vrothberg · 2024-08-26T12:17:21Z

I'd still shoot for a convert command, see https://issues.redhat.com/browse/BIFROST-353

germag force-pushed the add-convert-cmd branch from f398e86 to 9e327cb Compare July 8, 2024 09:45

cgwalters reviewed Jul 8, 2024

View reviewed changes

cmd/convert.go Outdated Show resolved Hide resolved

germag force-pushed the add-convert-cmd branch from 9e327cb to 33716b4 Compare July 9, 2024 15:15

germag force-pushed the add-convert-cmd branch from 33716b4 to 430f87c Compare July 9, 2024 15:19

Add convert command using bib

3413e52

Add convert command that uses bootc image builder (bib) to create disk images Signed-off-by: German Maglione <gmaglione@redhat.com>

germag force-pushed the add-convert-cmd branch from 430f87c to 3413e52 Compare July 9, 2024 18:05

vrothberg reviewed Jul 10, 2024

View reviewed changes

cdrage mentioned this pull request Jul 11, 2024

Get rid of Linux Podman Machine requirement and instead run escalated privileged CLI command. podman-desktop/extension-bootc#623

Closed

cdrage reviewed Jul 11, 2024

View reviewed changes

germag mentioned this pull request Jul 11, 2024

Does not work on Linux with Podman Machine (errors with cp: error writing '/output/image/./disk.raw') osbuild/bootc-image-builder#540

Closed

cdrage mentioned this pull request Jul 30, 2024

feat: use native podman build for linux podman-desktop/extension-bootc#684

Merged

cgwalters mentioned this pull request Aug 21, 2024

Implement the bootc provision plugin teemtee/tmt#3161

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add convert command using bib #58

Add convert command using bib #58

germag commented Jul 5, 2024 •

edited

Loading

vrothberg commented Jul 8, 2024

germag commented Jul 8, 2024

vrothberg commented Jul 8, 2024

germag commented Jul 8, 2024

vrothberg commented Jul 8, 2024

rhatdan commented Jul 8, 2024

germag commented Jul 8, 2024 •

edited

Loading

cgwalters commented Jul 8, 2024

germag commented Jul 9, 2024 •

edited

Loading

vrothberg commented Jul 9, 2024

germag commented Jul 9, 2024 •

edited

Loading

vrothberg Jul 10, 2024

vrothberg Jul 10, 2024

vrothberg Jul 10, 2024

vrothberg Jul 10, 2024

vrothberg Jul 10, 2024

vrothberg Jul 10, 2024

cdrage left a comment

cdrage Jul 11, 2024

cdrage Jul 11, 2024

cdrage Jul 11, 2024

cdrage Jul 11, 2024

cgwalters Jul 11, 2024

vrothberg commented Aug 26, 2024

	convertCmd.Flags().StringVar(&options.Output, "output", ".", "output directory (default \".\")")
	convertCmd.Flags().StringVar(&options.Output, "output", ".", "output directory")

	return fmt.Errorf("config file %s: %w", buildOption.Config, err)
	return fmt.Errorf("config file %q: %w", buildOption.Config, err)

	return fmt.Errorf("output directory %s: %w", buildOption.Output, err)
	return fmt.Errorf("output directory %q: %w", buildOption.Output, err)

	return fmt.Errorf("%s is not a directory ", buildOption.Output)
	return fmt.Errorf("%q is not a directory ", buildOption.Output)

Add convert command using bib #58

Are you sure you want to change the base?

Add convert command using bib #58

Conversation

germag commented Jul 5, 2024 • edited Loading

vrothberg commented Jul 8, 2024

germag commented Jul 8, 2024

vrothberg commented Jul 8, 2024

germag commented Jul 8, 2024

vrothberg commented Jul 8, 2024

rhatdan commented Jul 8, 2024

germag commented Jul 8, 2024 • edited Loading

cgwalters commented Jul 8, 2024

germag commented Jul 9, 2024 • edited Loading

vrothberg commented Jul 9, 2024

germag commented Jul 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cdrage left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vrothberg commented Aug 26, 2024

germag commented Jul 5, 2024 •

edited

Loading

germag commented Jul 8, 2024 •

edited

Loading

germag commented Jul 9, 2024 •

edited

Loading

germag commented Jul 9, 2024 •

edited

Loading