feat: use native podman build for linux #684

cdrage · 2024-07-30T16:02:29Z

feat: use native podman build for linux

What does this PR do?

Switches to using native podman building for Linux rather than using
podman machine
Tested against using a manifest as well as a normal image.
Uses CLI commands the equivalant of doing sudo podman run. PD does
not support running / viewing / using sudo root connections. So we use
the CLI instead
Uses CLI commands for saving the image / importing as well. The
reasoning is that importing requires sudo / privileged and
retrieving via image ID does not work for saving via the API.

Screenshot / video of UI

What issues does this PR fix or reference?

Closes #623

How to test this PR?

Try on Linux (Fedora 40 or above)
Go to build and it should ask for credentials after a few moments of
building
Successful image build

Signed-off-by: Charlie Drage [email protected]

cdrage · 2024-07-30T16:09:52Z

@deboer-tim @benoitf

Before reviewing there was a lengthy discussion on the issue regarding why we need to run sudo podman run commands on the CLI.

The main reasoning being that:

PD does not support running root containers as a normal user
bootc-image-builder must run as root.

This is a short-term solution until podman-bootc CLI is developed more / krun support is in, or when bootc-image-builder supports krun itself.

This PR will unblock the currently broken (and cumbersome) Linux support by solving issue osbuild/bootc-image-builder#540 as well as removing the podman machine restriction which has been causing issues for users (specifically users running Fedora Silverblue).

See referenced issues and discussions:

cdrage · 2024-07-31T19:04:58Z

~~Update: Fixing a test, but otherwise ready for review 👍~~ Fixed!

axel7083 · 2024-08-01T13:13:10Z

packages/backend/src/build-disk-image.ts

-          build.buildContainerId = containerId;
-          await history.addOrUpdateBuildInfo(build);
+            // Step 2. Check if there are any previous builds and remove them
+            progress.report({ increment: 5 });


It feels a bit weird as I had the progress stuck on 50 for most of the time, I feel like an indeterminate would be more appropriate maybe ?

yeah :( I will open up another issue for this.

the best path will probably open the image-build.log in a different synchronous process and do continuous reading of it similar to how we stream the container logs.

i'll follow this up in a different PR so this one isn't as big with issue: #698

axel7083

Tested on fedora 40. Works as expected, it requested the sudo authentification, that I provided and was able to complete the build with success.

Small concern, the output I had to delete the disk.raw using sudo rm as the file could not be deleted otherwise.

cdrage · 2024-08-01T13:21:54Z

Tested on fedora 40. Works as expected, it requested the sudo authentification, that I provided and was able to complete the build with success.

Small concern, the output I had to delete the disk.raw using sudo rm as the file could not be deleted otherwise.

Thank you so much for testing and happy it works as well on your side.

Having to delete with sudo rm will be solved with: #695 so that you can remove after without having to run sudo.

Your testing made me realize that's why they added that feature haha! I was wondering why it wasn't working on macOS or Windows.

deboer-tim · 2024-08-01T18:33:50Z

packages/shared/src/BootcAPI.ts

@@ -35,6 +35,7 @@ export abstract class BootcApi {
  abstract openFolder(folder: string): Promise<boolean>;
  abstract generateUniqueBuildID(name: string): Promise<string>;
  abstract openLink(link: string): Promise<void>;
+  abstract isLinux(): Promise<boolean>;


Will node:os not work in frontend? It feels odd for our API to need to expose a method for the front end to know this.

It doesn't and unfortunatley causes issues (can't detect "in time" before onMount), so using it here was best

deboer-tim · 2024-08-01T18:35:06Z

packages/backend/src/build-disk-image.ts

@@ -140,7 +140,9 @@ export async function buildDiskImage(build: BootcBuildInfo, history: History, ov
        const buildImageContainer = createBuilderImageOptions(containerName, build, builder);
        logData += JSON.stringify(buildImageContainer, undefined, 2);
        logData += '\n----------\n';
-        logData += createPodmanRunCommand(buildImageContainer);
+        // Output new line with `\` added at end for each in the array.
+        // logData += createPodmanCLIRunCommand(buildImageContainer);


Earlier code commented out but not deleted?

deboer-tim · 2024-08-01T18:42:29Z

packages/backend/src/machine-utils.ts

-    return 'Podman v5.0 or higher is required to build disk images.';
-  }
+  // Podman Machine checks are applicable to non-Linux platforms only
+  if (!isLinux()) {


It's pretty unlikely someone gets to the bootc extension on a Linux machine without podman install, but maybe we should still have a basic check?

should still have this check here since we use checkPrereqs() from the frontend.

the reason why this is in the backend rather than frontend is that we do too much loading in onMount() in build.svelte, specifically with the update() function which causes issues trying to pre-detect Linux.

So it's better to check when executing the backend commands instead if we are on Linux

I meant even on Linux shouldn't we confirm podman is installed? (vs this code which has no prereq checking for linux)

Ohhh! There's no check anyways because the Build page will already error out with no Podman installed / never reach this point.

We should actually add this to the dashboard / have this higher up?

I opened up: #701

cgwalters · 2024-08-01T21:19:34Z

This is a short-term solution until podman-bootc CLI is developed more / krun support is in,

But wouldn't it make more sense to help with that?

cdrage · 2024-08-01T23:19:34Z

This is a short-term solution until podman-bootc CLI is developed more / krun support is in,

But wouldn't it make more sense to help with that?

I investigated and tried a few different ways, including pulling the podman-bootc project and seeing what I could do in Go, but hit a bit of a wall fighting /proc issues getting it working. I started working on using a custom kernel / custom filesystem as per the comment here: containers/podman-bootc#9 (comment) but then ran into time constraints while deep diving through linux kernel documentation.

I also opened up osbuild/bootc-image-builder#569 to see if we could even get it working via non-root.

The solution in this PR mimics the exact CLI commands which are being ran currently by https://github.com/osbuild/bootc-image-builder so it's been quite reliable testing it the past week.

When containers/podman-bootc#9 is complete we can integrate that into the podman desktop bootc extension?

### What does this PR do? * Switches to using native podman building for Linux rather than using podman machine * Tested against using a manifest as well as a normal image. * Uses CLI commands the equivalant of doing `sudo podman run`. PD does not support running / viewing / using sudo root connections. So we use the CLI instead * Uses CLI commands for saving the image / importing as well. The reasoning is that importing requires `sudo` / privileged and retrieving via image ID does not work for saving via the API. ### Screenshot / video of UI  ### What issues does this PR fix or reference?  Closes podman-desktop#623 ### How to test this PR?  1. Try on Linux (Fedora 40 or above) 2. Go to build and it should ask for credentials after a few moments of building 3. Successful image build Signed-off-by: Charlie Drage <[email protected]>

Signed-off-by: Charlie Drage <[email protected]>

axel7083

LGTM feature wise 🚀 I can't give a proper code review as I am not familiar with the code base of the bootc extension. But take my approval as it works !

deboer-tim

Thanks for making changes. As discussed elsewhere I hope this is a stopgap towards podman-bootc, but better usability and unknown timeline makes this worthwhile in the meantime.

cdrage requested a review from a team as a code owner July 30, 2024 16:02

cdrage force-pushed the linux-support-local-cli branch 5 times, most recently from f78cffb to 6692200 Compare July 31, 2024 18:33

cdrage mentioned this pull request Jul 31, 2024

feat: add chown (linux only) #695

Merged

cdrage force-pushed the linux-support-local-cli branch from 6692200 to a7bcf63 Compare July 31, 2024 19:03

cdrage force-pushed the linux-support-local-cli branch 3 times, most recently from cf6826c to 5de5222 Compare July 31, 2024 20:10

axel7083 reviewed Aug 1, 2024

View reviewed changes

cdrage mentioned this pull request Aug 1, 2024

Implement correct progress for Linux builds #698

Open

deboer-tim reviewed Aug 1, 2024

View reviewed changes

cdrage force-pushed the linux-support-local-cli branch from 5de5222 to 8c01f4c Compare August 1, 2024 23:24

cdrage added 2 commits August 1, 2024 19:24

remove not used code

7899d91

Signed-off-by: Charlie Drage <[email protected]>

cdrage force-pushed the linux-support-local-cli branch from 8c01f4c to 7899d91 Compare August 1, 2024 23:24

axel7083 approved these changes Aug 6, 2024

View reviewed changes

deboer-tim approved these changes Aug 6, 2024

View reviewed changes

cdrage merged commit b750cc3 into podman-desktop:main Aug 6, 2024
5 checks passed

feat: use native podman build for linux #684

feat: use native podman build for linux #684

Uh oh!

Conversation

cdrage commented Jul 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Screenshot / video of UI

What issues does this PR fix or reference?

How to test this PR?

Uh oh!

cdrage commented Jul 30, 2024

Uh oh!

cdrage commented Jul 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

axel7083 left a comment

Choose a reason for hiding this comment

Uh oh!

cdrage commented Aug 1, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cdrage Aug 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cgwalters commented Aug 1, 2024

Uh oh!

cdrage commented Aug 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

axel7083 left a comment

Choose a reason for hiding this comment

Uh oh!

deboer-tim left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cdrage commented Jul 30, 2024 •

edited

Loading

cdrage commented Jul 31, 2024 •

edited

Loading

cdrage Aug 1, 2024 •

edited

Loading

cdrage commented Aug 1, 2024 •

edited

Loading