make real installer image, as own package #4248

deitch · 2024-09-12T11:28:44Z

Complete revamp of installer build.

This is a fairly large PR. Unfortunately, I could not really break it into multiple.

In the past, the installer and verifier were heavily duplicated, and did a lot of "ugly hacking" to start the usual rootfs and inject the right installer/verifier container in before others. It was brittle, and made redoing the installer terribly painful and changing a lot of things. Even changing unrelated things could break them.

This PR completely revamps that.

Now we have two main parts:

images/installer.yml.in, which builds a dedicated installer image, just like rootfs.yml.in
pkg/verifier, which is the container that does install and verify. This is included in installer.yml.in just like pkg/uefi or pkg/pillar are included in rootfs.yml.in

If you want to change how install works, or verification, or add a GUI or TUI, or options, or whatever, just change pkg/verifier and rebuild.

The only links between pkg/verifier and the outside are the "debug" options. The installer reads /proc/cmdline to determine if a certain setting is enabled, and if so, runs in debug mode and all of its output to the console. This is set in a grub menu option, which is in pkg/eve/installer/grub_installer.cfg.

Also redoes how grub files include each other. It was necessary to make all of the scenarios work, but is much cleaner.
Rather than getting pkg/eve/installer/grub.cfg to be called sometimes, and maybe accidentally overwrite the main grub.cfg, and worry about duplicate functions, etc., the main grub.cfg always is called, but it knows how to include grub_include.cfg. For installer images, that will be present, injected as part of installer.yml.in. We can expand this facility in the future if we need. But the execution chain is much simpler now. Always the same starting point of grub.cfg (whose source is pkg/grub/rootfs.cfg), and it knows to include others if they are found.

Running net installer locally over qemu, via make run-installer-net, used to extract the installer.net into the $(BUILD_DIR)/installer directory. That could (and did) overwrite existing things by accident, and then you worry about why future things won't run. Instead, it now creates a new dir $(BUILD_DIR)/netboot and boots from there.

Because verification image no longer exists, removed all of the references to it in Makefile.

Added docs, notably to pkg/installer/README.md and detailed the boot chain in docs/BOOT-INSTALLER.md. Hopefully no one will have to go through the weeks I went through to figure it all out.

deitch · 2024-09-12T11:29:31Z

I would ask no one to approve this until multiple people have commented. I think eden tests are a waste of resources until then.

@jsfakian has commented that he has tried it, and he thinks the installer might work on raw but not iso. Comment here please.

deitch · 2024-09-12T11:32:21Z

What does this mean in yetus errors?

  ./out has been created
  Modes: GitHubActions Robot InContainer ResetRepo UnitTests
  Processing: GH:4248
  GITHUB PR #4248 is being downloaded from
  https://api.github.com/repos/lf-edge/eve/pulls/4248
    JSON data at Thu Sep 12 11:29:42 AM UTC 2024
    Patch data at Thu Sep 12 11:29:43 AM UTC 2024
  ERROR: Unsure how to process GH:4248. Permissions missing?

OhmSpectator · 2024-09-12T11:59:23Z

@yash-zededa, could you please help understand why Yetus failed to run?

Upd. Just restarting the workflow does not help =(

yash-zededa · 2024-09-12T12:10:54Z

@yash-zededa, could you please help understand why Yetus failed to run?

Upd. Just restarting the workflow does not help =(

Just checked. GHA throws an error

ERROR: Unsure how to process GH:4248. Permissions missing?

OhmSpectator · 2024-09-12T12:13:15Z

@yash-zededa, could you please help understand why Yetus failed to run?
Upd. Just restarting the workflow does not help =(

Just checked. GHA throws an error
ERROR: Unsure how to process GH:4248. Permissions missing?

Yep, we saw it... But how come?

yash-zededa · 2024-09-12T12:15:05Z

@yash-zededa, could you please help understand why Yetus failed to run?
Upd. Just restarting the workflow does not help =(

Just checked. GHA throws an error
ERROR: Unsure how to process GH:4248. Permissions missing?
Yep, we saw it... But how come?

Permission shouldn't be a problem. I've never encountered this error before. I'm looking into it to get more details.

OhmSpectator · 2024-09-12T12:19:14Z

The new docs and README files would be helpful to put into https://github.com/lf-edge/eve/blob/master/docs/mkdocs/mkdocs.yml, as the team members use it for easier documentation exploration.

deitch · 2024-09-12T12:27:49Z

@OhmSpectator done, good point, thank you.

deitch · 2024-09-12T12:28:09Z

Permission shouldn't be a problem. I've never encountered this error before. I'm looking into it to get more details

I think we saw it once before, but don't rely on my memory

deitch · 2024-09-12T12:34:36Z

Of course, now that I split it into 10 distinct commits, to make it easier to review and understand each one, lint commit complains about the short commit messages. If you put it together, like the original single commit, it is quite large and detailed. Now it is 10 small and detailed.

I am ignoring that complaint.

christoph-zededa · 2024-09-12T12:54:01Z

pkg/installer/src/tpmmgr/tpmmgr.go

@@ -0,0 +1,1279 @@
+// Copyright (c) 2024 Zededa, Inc.


Is this file a copy of pillar/cmd/tpmmgr/tpmmgr.go?

Almost. I trimmed it down a lot, just to what we need in installer. And to run standalone.

This all should be replaced soon enough by @jsfakian, I believe.

OhmSpectator · 2024-09-12T13:14:11Z

Is it ok? I ran make pkg/installer eve

#17 [raw 1/2] RUN rm /initrd.img /installer.img
#17 0.061 rm: cannot remove '/installer.img': No such file or directory
#17 ERROR: process "/bin/ash -eo pipefail -c rm /initrd.img /installer.img" did not complete successfully: exit code: 1
------
 > [raw 1/2] RUN rm /initrd.img /installer.img:
0.061 rm: cannot remove '/installer.img': No such file or directory
------
Error: error building "lfedge/eve:0.0.0-installer-img-db02f1ea-kvm": error building for arch amd64: failed to solve: process "/bin/ash -eo pipefail -c rm /initrd.img /installer.img" did not complete successfully: exit code: 1
2024/09/12 15:03:16 error during command execution: error building "lfedge/eve:0.0.0-installer-img-db02f1ea-kvm": error building for arch amd64: failed to solve: process "/bin/ash -eo pipefail -c rm /initrd.img /installer.img" did not complete successfully: exit code: 1
make: *** [Makefile:844: eve] Error 1
rm images/out/installer-kvm-generic.yml.in images/out/rootfs-kvm-generic.yml.in

deitch · 2024-09-12T13:17:43Z

Is it ok? I ran make pkg/installer eve

I will check it.

OhmSpectator · 2024-09-12T13:18:13Z

pkg/installer/Dockerfile

+# SPDX-License-Identifier: Apache-2.0
+
+# Dockerfile to build installer img initrd
+FROM lfedge/eve-alpine:c114cf1d3ea51534f061f9aa949beb6ac5c12fb3 AS build


We use 82df60e43ab9f8c935584b8c7b4d0a4b0271d608 now

I will bump.

Are you sure?

$ lkt pkg show-tag ./pkg/alpine lfedge/eve-alpine:2ed33236f1eea746532f612d6c4b7779c5da2193

Never mind, I will take yours. If they have to be update, we will do them all.

Yep, @rene updated it not that long ago: 72e392c

deitch · 2024-09-12T13:25:03Z

Yup, CI caught the same thing. That is what it is there for 😄

deitch · 2024-09-12T13:25:19Z

Is it ok? I ran make pkg/installer eve

Yeah, good catch. The eve build is fine, but still assumes some older stuff is there. I will update it.

OhmSpectator · 2024-09-12T13:30:36Z

pkg/installer/Dockerfile

+ADD http://sources.buildroot.net/edid-decode/edid-decode-188950472c19492547e298b27f9da0d72cf826df.tar.gz /out/edid-decode-188950472c19492547e298b27f9da0d72cf826df.tar.gz
+ADD https://github.com/linuxhw/build-stuff/releases/download/1.6/hw-probe-1.6-AI.tar.gz /out/hw-probe-1.6-AI.tar.gz
+# Temporarily removed ubuntu VM image to make the verification image smaller.
+#ADD https://cloud-images.ubuntu.com/minimal/releases/jammy/release/ubuntu-22.04-minimal-cloudimg-amd64.img /out/ubuntu-22.04-minimal-cloudimg-amd64.img


Do we still need this line?

I have no idea. 🤷‍♂️

We do. These are tools used in the verifier.

We can remove 34 and 35 if you want.

Yeah, I was talking about the commented lines...

deitch · 2024-09-12T13:32:08Z

OK, this is going to take some work to get working with make eve. Just when you thought you were out of the woods!

It turns out that pkg/eve/runme.sh not only has lots of duplication of stuff from other builds, but assumes a lot about the internal structure.

Probably a day or so to figure this out.

OhmSpectator · 2024-09-12T13:36:40Z

pkg/installer/Dockerfile

+    wireless-tools libxrandr eudev-libs libusb sudo fio iperf3 sysstat \
+    lm-sensors acpi iw libdrm hwinfo dhclient dmidecode smartmontools \
+    pciutils libgcc pixman glib libvncserver musl-utils dhclient \
+    qemu-system-x86_64 tpm2-tss-dev tpm2-tss-esys tpm2-tss-fapi tpm2-tss-rc \


Do we really need qemu-system-x86_64 in the installer?...

I don't know. But I don't want to change too much from how current installer+verifier works or what it has. The goal in this one is to replace the process for getting a working installer image (and clean up all of the complexity on the way, which includes how we build and grub and ipxe etc)

If this was there before, and doesn't break anything, let's leave it. We always can do another PR that just touches pkg/verifier when done.

Mmm... It wasn't obvious to me that this came from an old version. I thought it was a new file.

Yeah, you are right, it wasn't clear. It consolidates everything that was in both the old installer and the old verification.

Can you maybe describe in the commit message where you introduce a new package which part of it is taken from the old one?

OhmSpectator · 2024-09-12T13:39:10Z

OK, this is going to take some work to get working with make eve. Just when you thought you were out of the woods!

It turns out that pkg/eve/runme.sh not only has lots of duplication of stuff from other builds, but assumes a lot about the internal structure.

Probably a day or so to figure this out.

Ok, could we mark the PR as a draft for now? I review it with a lower priority then. Ok?

deitch · 2024-09-17T06:59:03Z

How do I get the eden tests to run again?

OhmSpectator · 2024-09-17T09:25:42Z

How do I get the eden tests to run again?

I reapproved to trigger the tests again.

eriknordmark · 2024-09-17T10:02:57Z

I don't understand this part:

pkg/verifier, which is the container that does install and verify. This is included in installer.yml.in just like pkg/uefi or pkg/pillar are included in rootfs.yml.in

What is the impact on the verifier image? Will that be included in the installer image?
The verifier might include guest VM images, hence can be significantly larger than the EVE installer image.

deitch · 2024-09-17T10:28:35Z

I'm not sure what that means. My understanding is that verifier is basically installer plus a bunch of verification tests. Tons of duplication. I dug into the code and that's what I saw. So I merged them.

What does "verification image plus guest VMs" mean? It didn't fit with anything I saw?

jsfakian · 2024-09-17T10:43:04Z

I'm not sure what that means. My understanding is that verifier is basically installer plus a bunch of verification tests. Tons of duplication. I dug into the code and that's what I saw. So I merged them.

What does "verification image plus guest VMs" mean? It didn't fit with anything I saw?

@eriknordmark, @deitch, Initially, with the verifier, I wanted to include various tests with guest VMs with different parameters. To achieve that, I included a small Ubuntu image (~250 MB) in the verifier. This caused some problems with the verification process, and I temporarily removed that Ubuntu image. So, Erik asks what we are going to do if we decide to add guest images to the verifier in the future (which is now the same as the installer). Are we okay with having a huge installer image?

My idea was to add a very small Alpine VM (~10MB), which would be okay. What do you think?

deitch · 2024-09-17T10:52:54Z

My idea was to add a very small Alpine VM (~10MB), which would be okay. What do you think?

To my mind, that would be fine. Minimal impact.

We aren't going to start having dozens of different verifier images, nor one with 20 VMs in it, making it many GBs of size. So either it will be static and small with no VMs (or 1-2 really small ones, like @jsfakian said), or dynamic, which will pull things down from somewhere (Internet? attached drive?), which means a whole new design for verifier than what we have today.

Either way, we should be fine with this.

deitch · 2024-09-17T10:53:20Z

@OhmSpectator it doesn't look to me like eden tests ran?

OhmSpectator · 2024-09-17T11:38:07Z

@OhmSpectator it doesn't look to me like eden tests ran?

strange... @yash-zededa, can you check, please?

deitch · 2024-09-17T13:49:47Z

I tried rerunning it manually by selecting, "run all jobs" (i.e. those that were skipped), it does another run, but immediately says, "this job was skipped", not clear why, though.

rene · 2024-09-17T14:18:57Z

I tried rerunning it manually by selecting, "run all jobs" (i.e. those that were skipped), it does another run, but immediately says, "this job was skipped", not clear why, though.

I think that's because of Yetus failure....

rene · 2024-09-17T14:19:56Z

@deitch , have you opened the ticket on LF's Jira for the creation of pkg/installer at dockerhub? We need to have this repository in place before merging.....

deitch · 2024-09-17T14:20:53Z

Of course, no one seems to know why yetus is failing (i.e. not even running).

./out has been created
  Modes: GitHubActions Robot InContainer ResetRepo UnitTests
  Processing: GH:4248
  GITHUB PR #4248 is being downloaded from
  https://api.github.com/repos/lf-edge/eve/pulls/4248
    JSON data at Mon Sep 16 05:35:02 PM UTC 2024
    Patch data at Mon Sep 16 05:35:03 PM UTC 2024
  ERROR: Unsure how to process GH:4248. Permissions missing?

rene

@deitch , thanks for this hard work, the installer booting and overall process is much clear now... and I hope it can solve the installation for RPi (didn't test it yet)....

deitch · 2024-09-17T14:21:57Z

installer booting and overall process is much clear now

and documented. Funny, that is what I am most proud of.

and I hope it can solve the installation for RPi (didn't test it yet)

If not, we can focus on fixing it just on a small part.

deitch · 2024-09-17T14:22:17Z

have you opened the ticket on LF's Jira for the creation of pkg/installer at dockerhub?

Yup, all done yesterday. Docker Hub repo in place.

deitch · 2024-09-17T15:17:28Z

Rate pull limits!

deitch · 2024-09-17T17:47:54Z

eden is happy! Can we get request for changes, or approval (written) so we can merge?

eriknordmark · 2024-09-18T09:56:59Z

@deitch , thanks for this hard work, the installer booting and overall process is much clear now... and I hope it can solve the installation for RPi (didn't test it yet)....

@rene will you test the RPi installation before we merge this?

rene · 2024-09-18T16:14:00Z

@deitch , thanks for this hard work, the installer booting and overall process is much clear now... and I hope it can solve the installation for RPi (didn't test it yet)....

@rene will you test the RPi installation before we merge this?

@eriknordmark , we will test it tomorrow, if you want to wait, I can merge it if everything works....

deitch · 2024-09-18T16:17:31Z

Yeah let's wait the extra day.

rene · 2024-09-19T14:23:05Z

Tested on:

Pollux board
Raspberry Pi 4

Thanks @christoph-zededa for the help 👍

deitch requested review from zedi-pramodh, rene and eriknordmark as code owners September 12, 2024 11:28

deitch force-pushed the installer-img branch from c31f516 to 9b2e945 Compare September 12, 2024 11:32

github-actions bot requested review from jsfakian and rucoder September 12, 2024 11:37

deitch force-pushed the installer-img branch from 9b2e945 to 30662be Compare September 12, 2024 11:39

deitch force-pushed the installer-img branch from 30662be to ff0a0c4 Compare September 12, 2024 12:09

deitch force-pushed the installer-img branch from ff0a0c4 to db02f1e Compare September 12, 2024 12:27

christoph-zededa reviewed Sep 12, 2024

View reviewed changes

OhmSpectator reviewed Sep 12, 2024

View reviewed changes

deitch force-pushed the installer-img branch from db02f1e to 615b38a Compare September 12, 2024 14:26

OhmSpectator self-requested a review September 17, 2024 09:24

OhmSpectator approved these changes Sep 17, 2024

View reviewed changes

rene approved these changes Sep 17, 2024

View reviewed changes

OhmSpectator approved these changes Sep 18, 2024

View reviewed changes

rene approved these changes Sep 19, 2024

View reviewed changes

rene merged commit 6169cf9 into lf-edge:master Sep 19, 2024
81 of 95 checks passed

deitch deleted the installer-img branch September 19, 2024 14:24

This was referenced Sep 19, 2024

Added interactive installer package in pkg/installer #4055

Merged

mkimage-raw-efi references lower-level serial container dir directly and shouldn't #4056

Closed

deitch mentioned this pull request Dec 11, 2024

installer ISO incorrect when created via docker run #4475

Closed

make real installer image, as own package #4248

make real installer image, as own package #4248

Conversation

deitch commented Sep 12, 2024

deitch commented Sep 12, 2024

deitch commented Sep 12, 2024

OhmSpectator commented Sep 12, 2024 • edited Loading

yash-zededa commented Sep 12, 2024

OhmSpectator commented Sep 12, 2024 • edited Loading

yash-zededa commented Sep 12, 2024

OhmSpectator commented Sep 12, 2024

deitch commented Sep 12, 2024

deitch commented Sep 12, 2024

deitch commented Sep 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OhmSpectator commented Sep 12, 2024

deitch commented Sep 12, 2024

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

deitch commented Sep 12, 2024

deitch commented Sep 12, 2024

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

deitch commented Sep 12, 2024

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

OhmSpectator commented Sep 12, 2024

deitch commented Sep 17, 2024

OhmSpectator commented Sep 17, 2024

eriknordmark commented Sep 17, 2024

deitch commented Sep 17, 2024

jsfakian commented Sep 17, 2024

deitch commented Sep 17, 2024

deitch commented Sep 17, 2024

OhmSpectator commented Sep 17, 2024

deitch commented Sep 17, 2024

rene commented Sep 17, 2024

rene commented Sep 17, 2024

deitch commented Sep 17, 2024

rene left a comment

Choose a reason for hiding this comment

deitch commented Sep 17, 2024

deitch commented Sep 17, 2024

deitch commented Sep 17, 2024

deitch commented Sep 17, 2024

eriknordmark commented Sep 18, 2024

rene commented Sep 18, 2024

deitch commented Sep 18, 2024

rene commented Sep 19, 2024

OhmSpectator commented Sep 12, 2024 •

edited

Loading

OhmSpectator commented Sep 12, 2024 •

edited

Loading