-
Notifications
You must be signed in to change notification settings - Fork 165
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
make LIVE_PKG=<pkg> live #3842
make LIVE_PKG=<pkg> live #3842
Conversation
This had me in tears. 🤣 |
I was wondering about that. Many of the changes looked familiar like I had seen you do them before. We should get that one in, and then this one. Keep each PR to a more narrowly-defined change. |
Can you describe how this actually works (preferably in docs part of the commit)? We understand that normal build is:
I am not clear on how I would use this, or how it actually works. I get the benefit: you want to make it possible to make a change to a single package, and then update an existing eve image with that updated package. How I do so (the exact steps) and how it works under the covers is not clear in the docs. Also, does this affect the name of the built image? We usually build them into |
@deitch first of all before any further discussion tell me, did you get the Marcel Proust award? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3842 +/- ##
=======================================
Coverage 17.51% 17.51%
=======================================
Files 3 3
Lines 805 805
=======================================
Hits 141 141
Misses 629 629
Partials 35 35 ☔ View full report in Codecov by Sentry. |
I sure did. 😄 So, yes, I did get it more or less. You basically are creating a different path. Where I was trying to say that I didn't full get how it worked, and I didn't see any of it in the docs, so that the user who wants to just read docs, and developer who wants to update it, know how to use it or how it works. |
|
I will look a good place in docs to describe this. Please point me out if you know where exactly. |
Probably BUILD.md, but there are some other candidates.
Makes sense. So we use
Understood
Of course, because it has to find all of the images in the yaml file, and if not in cache download them, etc. The actual tar speed is about the same as any tar implementation; it is the "let's tar 15 OCI images" vs "let's tar just one" that makes what you are doing faster.
guestfish! In any case, all of this should go into docs properly. |
By the way, if I didn't say it before, this is a neat overall approach. During development cycles, instead of rebuilding the whole thing, patch (repeat 10-50 times). Once it does what you need when patched, try rebuilding (just one to a few times). |
Separate question. The working assumption here is:
So we use ext4 inside qcow2 image, and then patch it. Is there some way we could launch directly from a tar file as the rootfs? I don't know how we could do that using qemu (or any other boot structure), but if we could, this gets even simpler. |
Oh, from the above. Should a patched qcow2 file be marked as such? We don't want people confused. |
Not sure I understand that.
Why? Still do not understand where you are heading. |
To get this to work, you need to create a tar patch, then apply it to a filesystem in a disk image. Much easier if you could just do step 1. It might not be possible, but how nice it would be.
Run
When you run this |
We can discuss that in details next week (are you coming to berlin?), how things can be improved. Probably not this way, but there are other things can be done.
for
Others stay outdated. |
If it doesn't improve on what you already have here, not worth it. We can discuss though.
I do not know if that is the right naming for it. It just came out as I was typing. it does emphasize that it is a different process, but let's let others weigh in?
OK, that part is nice and clean then, no accidental mixing things up.
I think it matters, but not sure. Let's let others weigh in? |
We need this extra space for updating packages while building live image: make LIVE_PKG=<p> live. The whole implementation follows. Stay tuned. Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
By -p <pkg> parameter the resulting yml will be reduced to exactly the package specified, for example if input contains kernel, init, onboot and services sections and "pillar" is specified as a package, then the resulting yml file will contain only elements and sections which mention "pillar", and not other packages. Examples of reduced output based on rootfs-kvm-generic.yml.in input: $ ./tools/parse-pkgs.sh -p pkg/grub images/out/rootfs-kvm-generic.yml.in init: - lfedge/eve-grub:d16eeeb340b3f2ff8cb6e60be65c586e72de9aae-amd64 $ ./tools/parse-pkgs.sh -p pkg/kernel images/out/rootfs-kvm-generic.yml.in kernel: image: docker.io/lfedge/eve-kernel:eve-kernel-amd64-v6.1.38-generic-ae347d3a26ec-gcc cmdline: "rootdelay=3" $ ./tools/parse-pkgs.sh -p pkg/pillar images/out/rootfs-kvm-generic.yml.in services: - name: pillar image: lfedge/eve-pillar:39a33fb49dfb6c17ee8b373dd4f934f151c25136-amd64 cgroupsPath: /eve/services/pillar oomScoreAdj: -999 onboot: # If you change the order of pillar-onboot don't forget to # change /containers/onboot/005-pillar-onboot/lower in pkg/mkimage-raw-efi accordingly: # 005-pillar-onboot must follow the order number of pillar-onboot # onboot part of pillar to prepare services to start - name: pillar-onboot image: lfedge/eve-pillar:39a33fb49dfb6c17ee8b373dd4f934f151c25136-amd64 command: ["/opt/zededa/bin/onboot.sh"] Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
Targets `images/out` and `build-tools` should be put as order-only-prerequisites: https://www.gnu.org/savannah-checkouts/gnu/make/manual/html_node/Prerequisite-Types.html#Prerequisite-Types Because `images/out` is a folder, which timestamp is always changes if new files are modified inside and `build-tools` is .PHONY target. This patch moves `images/out` and `build-tools` to the order-only-prerequisites to avoid constant yml* regeneration, which forces rebuild images. The force of rebuilding is controlled by the RESCAN_DEPS variable, which is set to FORCE (is .PHONY target), which means the default behavior won't break, but gives us freedom to reset the RESCAN_DEPS and do not generate live image if it was already generated. This 'do-not-generate' live image logic will be used in the next patch for the 'LIVE_PKG' update. Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
The main problem with the EVE build system (with its other features) is its complete inability to detect changes and create images from what has actually been changed, which leads to complete rebuild of the live image every time you made a small change and takes: $ time make live real 3m15.571s user 1m7.485s sys 0m5.301s So much waste of time for daily developer work. This can be slightly improved by generating the ext4 rootfs image: $ time make ROOTFS_FORMAT=ext4 live real 2m8.087s user 1m45.201s sys 0m7.096s Yeah, gets better now. But not ideal. For daily developer routine we don't need to rebuild the whole image if you've modified only one component, which is usually the case. Let's say your daily work is to tweak pillar and live qcow2 image has been already generated with ext4 rootfs, so why the image has to be regenerated each time you did a change in pillar? No reason at all. This change enables the LIVE_PKG=<pkg> mode for qcow2 live image generations (with ext4 as rootfs, this is the requirement). This mode does not generates the whole qcow2 image from the scratch, but mounts rootfs and copies prepared tarball. Easy. To make things even faster prepared tarball contains only the required package, not the whole rootfs. This is made by the reducing rootfs yml (see previous patches). What it gives us? Not speed of light, unfortunately, but tens of seconds, look at the numbers: time make LIVE_PKG=pkg/dom0-ztools live real 0m9.387s user 0m4.919s sys 0m3.216s time make LIVE_PKG=pkg/grub live real 0m7.059s user 0m3.050s sys 0m2.307s time make LIVE_PKG=pkg/pillar live real 0m22.470s user 0m19.443s sys 0m3.696s (yeah, pillar is a fat guy) Limitations? Yes, a couple: 1. live qcow2 should be generated once with the ROOTFS_FORMAT=ext4 2. Don't forget what package you've changed, otherwise your image will be patched with something different. 3. Probably something does not work, but will be found and fixed if more people use that. 4. Probably something is broken. Same, will be fixed. Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
make LIVE_PKG=<pkg> live updates existing qcow2 disk image of EVE with RW rootfs (ext4) by only copying specified package. This significantly reduced overall build time of the disk image. Used by developers only! Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
1. Creating a live image with an EXT4 root filesystem: `make ROOTFS_FORMAT=ext4 live` 2. Patch the EXT4 partition of the live image with only the necessary package, e.g. pkg/pillar: `make LIVE_PKG=pkg/pillar live` Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
Rebased on latest master, improvements described in BUILD.md. |
There are a few things here that I don't like, but (as discussed privately with @rouming), I don't know if we have a better way to do them.
I tried to see if we could get rid of I also looked into removing all of that yq by having linuxkit be able to process just a subset of the yaml. For example: linuxkit build --format tar --pkgs lfedge/eve-pillar:abcd ./rootfs.yml
# or
linuxkit build --format tar --pkg-paths ./pkg/pillar ./rootfs.yml Sort of a "give me a patch mode". I think that is doable, and would save all of the ugly yq (or any changes to The one advantage to that approach is that linuxkit already knows its structure, so no need to try and figure it out from within parse-pkgs.sh and then pass it through yq. Of course, we already do that in I also looked at something like giving linuxkit "incremental" powers. E.g. linuxkit build --format tar --in-tar ./foo.tar ./rootfs.yml This would say, "go ahead and build the output tar file. However, before reading the images from your cache, first check the existing tar file. If an image is unchanged in rootfs.yml vs what is in the in-tar, just copy its contents from the in-tar, rather than figuring it out from your cache." This is the cleanest, as it sticks with the existing philosophy, always creating reproducible and immutable tar files, and doesn't change much from our current build style. The hard part here was how to understand what the existing tar contains. We already include the build.yml in the tar file, so we could read that and compare to the new one, which would give us just what changed. But how do we know which files in the existing tar come from which packages, without parsing all of the packages in the cache again, which I think is just as slow? We do not maintain a mapping for each file, "I came from this package". We could add them to each file as we put them in with So the best I can come up with here is:
Is it worth all of this vs what we have? 🤷♂️ |
What really matters in comparison of different approaches is numbers. Will any other approach reduce overall time on live image build? I reached ~20 seconds for pkg/pillar, which is significant improvement to ~3 min. That makes me much less nervous about wasted time and cpu cycles. Personally, I do not like when any philosophies make someone suffer. For producing official images - sure the build procedure can be more constrained and sophisticated - machine does not care. For end developers who should have control over the whole build, time really matters. I do not see any harm if someone can speedup their local build, even though this breaks some of the ideological philosophies - who cares how is the local live image for qemu generated? Unfortunately I do not see how linuxkit can differentiate what has been changed in tar, that is exactly what you've mentioned. Only with the developer help we can make this happen, because developer knows what package was recently changed, thus 'LIVE_PKG=< p >' argument. This is ugly and nasty, I clearly understand that, but having all the machinery around the linuxkit, I could not come up with anything better than in this PR (I tried really hard to squeeze in between ideologies, constraints and philosophies we have :) If we don't like the yq (and we don't!), can we provide the same '--only-pkg' argument to the linuxkit, so the linuxkit can reduce the resulting tarball by including only the package specified? So basically the whole
This still breaks the "always creating reproducible and immutable tar files" philosophy, but at least we get rid of the yq queries, which no one can fix :) And you proposed that as well, but with different option. This is good idea, I would like to have this instead of |
@@ -41,6 +41,9 @@ IMGFILE=/rootfs.img | |||
ROOTFS_FILE_SIZE_KB=$(du -sk . | awk '{print $1}') | |||
ROOTFS_PART_BLOCKS=$(( $ROOTFS_FILE_SIZE_KB / 4 + $ROOTFS_PART_HEADROOM_BLOCKS )) | |||
ROOTFS_PART_SIZE=$(( $ROOTFS_PART_BLOCKS * $ROOTFS_BLOCKSZ )) | |||
# Increase fs size on 200MB. We need extra room for updating packages while | |||
# building live image. See 'LIVE_PKG=<p> live' in the Makefile. | |||
ROOTFS_PART_SIZE=$((ROOTFS_PART_SIZE + (200<<20))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of reassign the variable you could use something like that:
# Additional 200MB space. We need extra room for updating packages while
# building live image. See 'LIVE_PKG=<p> live' in the Makefile.
ROOTFS_PART_EXTRA_SPACE=$((200<<20))
ROOTFS_PART_SIZE=$(( ($ROOTFS_PART_BLOCKS * $ROOTFS_BLOCKSZ) + $ROOTFS_PART_EXTRA_SPACE ))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
@@ -377,6 +377,22 @@ The current process uses the control knob `PLATFORM` in the following places in | |||
* As the first argument to [prepare-platform.sh](../tools/prepare-platform.sh) in the Makefile targets for `live.*`, `installer.*` and `verification.*`, which, in turn, uses it to add specific files to the build and output directories prior to making the final image. | |||
* In the final eve image `lf-edge/eve`, specifically the entrypoint [runme.sh](../pkg/eve/runme.sh), where the platform is passed as `-p` argument when calling `docker run`, and is used to modify the final layout. | |||
|
|||
### Note for a developer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: I think "Notes for developers" sounds better...
|
||
`make LIVE_PKG=pkg/pillar live` | ||
|
||
The command generates the rootfs tarball consisting only from the `pkg/pillar` and patches the live image using the `guestfish` one-liner (check the main Makefile for details). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion:
.... and patches the live image using the guestfish
one-liner tool (which must be installed in the system, check the Makefile for details).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw, will this work if run on a cloud provider? AWS for example doesn't support nested virtualization; doesn't guestfish spin up a VM?
@echo " pkg/XXX builds XXX EVE package" | ||
@echo " rootfs builds default EVE rootfs image (upload it to the cloud as BaseImage)" | ||
@echo " live builds a full disk image of EVE which can be function as a virtual device" | ||
@echo " LIVE_PKG=<pkg> live updates existing qcow2 disk image of EVE with RW rootfs (ext4) by only copying specified package. This significantly reduced overall build time of the disk image. Used by developers only!" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is by far bigger than others from the help. Does it make sense to break it into two @echo lines to be more terminal friendly?
@echo " LIVE_PKG=<pkg> live updates existing qcow2 disk image of EVE with RW rootfs (ext4) by only copying specified package."
@echo " This significantly reduced overall build time of the disk image. Used by developers only!"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
Before I address comments from @rene I would like to ask main stakeholder @eriknordmark and @deitch of EVE build system: are you guys good with this dev approach or not? I feel I kind of step in the area of ideological discussion (or put it simple: there are some implied philosophies which this PR violates), and this is not my intention. Also, Avi, do you still want to pull the 'yq' reducing logic to the linuxkit? I mean do I wait for your linixkit changes, and then modify this PR and put it on top of your changes, or we commit this first and then once linuxkit is ready I make another PR? Or we close this? Want to understand my next steps. |
Yes. I really would prefer if we could find a way to make the whole rebuild be super-quick, so that you wouldn't need to do some of this unhappy stuff, but I know of no such way right now. So I accept it as the best alternative to date (and salute you for coming up with it).
I am going to add those basic steps I mentioned to linuxkit today. It won't solve your ext4 update problem. My suggestion would be, let's see if I can get this in in the next 24 hours. If I can, then it saves you some of the headache. If not, just go ahead. |
The first part in linuxkit is running CI, tagging each file inside the tar. Second one will be next. |
@rouming I updated linuxkit to support patching. It can use an existing tar file as an input. If it finds the yaml manifest in the tar file at the same location, it compares the manifests. Whatever is identical, is just copied from input tar to output tar; whatever is different, it reads from the usual cache or registry. At this point, it is up to you. If leveraging it helps you enough, go for it. If not, go ahead with this. |
@deitch did you concern about a given EVE version string being reused as the code changes get resolved? |
Well enough. This is only in development, so we can live with it. |
@echo " live builds a full disk image of EVE which can be function as a virtual device" | ||
@echo " LIVE_PKG=<pkg> live updates existing qcow2 disk image of EVE with RW rootfs (ext4) by only copying specified package. This significantly reduced overall build time of the disk image. Used by developers only!" | ||
|
||
@echo " live-XXX builds a particular kind of EVE live image (raw, qcow2, gcp, vdi, parallels)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this line be immediately below the "live" line?
echo " -p pkg - Removes everything which is not the <pkg> from " >&2 | ||
echo " the resulting output." >&2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to support a single package being patched? Can this be used to update more than one package?
If not, is the recommended workflow to run it multiple times? e.g.
make LIVE_PKG=pkg/wwan live
make LIVE_PKG=pkg/pillar live
If that is the case it would be good to include that in the documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but comments/questions/suggestions from the reviewers to respond to.
I close this in a favor of a slightly different approach: #3897 |
Progress surges. Speed increases. Performance amplifies. Humanity yearns
for more. Numbers dominate. Waiting becomes obsolete. Acceleration is
relentless. Efficiency reigns supreme. Innovation fuels perpetual motion.
Expectations soar. Advancement knows no bounds. Society craves constant
improvement. Technology evolves rapidly, shaping daily lives. Competition
drives relentless innovation cycles. Markets demand faster, better solutions.
Individuals seek instant gratification, always. Patience wanes, urgency becomes
norm. Evolutionary pressure fuels exponential growth. Time compresses,
deadlines shrink, expectations rise. Pressure cooker of progress never cools.
Enough, ChatGPT. Now to the point.
The main problem with the EVE build system (with its other features)
is its complete inability to detect changes and create images from
what has actually been changed, which leads to complete rebuild
of the live image every time you made a small change and takes:
$ time make live
real 3m15.571s
user 1m7.485s
sys 0m5.301s
So much waste of time for daily developer work. This can be slightly
improved by generating the ext4 rootfs image:
$ time make ROOTFS_FORMAT=ext4 live
real 2m8.087s
user 1m45.201s
sys 0m7.096s
Yeah, gets better now. But not ideal. For daily developer routine
we don't need to rebuild the whole image if you've modified only
one component, which is usually the case. Let's say your daily
work is to tweak pillar and live qcow2 image has been already
generated with ext4 rootfs, so why the image has to be regenerated
each time you did a change in pillar? No reason at all.
This change enables the
LIVE_PKG=<p>
mode for qcow2 live imagegenerations (with ext4 as rootfs, this is the requirement). This
mode does not generate the whole qcow2 image from the scratch,
but mounts rootfs and copies prepared tarball. Easy.
To make things even faster prepared tarball contains only the
required package, not the whole rootfs. This is made by the
reducing rootfs yml (see previous patches).
What it gives us? Not speed of light, unfortunately, but tens
of seconds, look at the numbers:
time make LIVE_PKG=pkg/dom0-ztools live
real 0m9.387s
user 0m4.919s
sys 0m3.216s
time make LIVE_PKG=pkg/grub live
real 0m7.059s
user 0m3.050s
sys 0m2.307s
time make LIVE_PKG=pkg/pillar live
real 0m22.470s
user 0m19.443s
sys 0m3.696s
(yeah, pillar is a fat guy)
Limitations? Yes, a couple:
will be patched with something different.
if more people use that.
(Whoever read this far receives a Marcel Proust award)
This PR is based on the previous not merged "Make ext4 great again", so
only 5 patches are on top of the #3830.
cc: @deitch