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

make LIVE_PKG=<pkg> live #3842

Closed
wants to merge 6 commits into from
Closed

Conversation

rouming
Copy link
Contributor

@rouming rouming commented Apr 4, 2024

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 image
generations (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:

  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.

(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

@deitch
Copy link
Contributor

deitch commented Apr 4, 2024

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.

This had me in tears. 🤣

@deitch
Copy link
Contributor

deitch commented Apr 4, 2024

This PR is based on the previous not merged "Make ext4 great again", so
only 5 patches are on top of the #3830.

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.

@deitch
Copy link
Contributor

deitch commented Apr 4, 2024

Can you describe how this actually works (preferably in docs part of the commit)?

We understand that normal build is:

  1. Rebuild any packages
  2. make live (or raw) makes the tar for rootfs, then builds a squashfs out of it

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 dist/<arch>/<dirname that includes hash, etc.>/. If so, that would need to be updated?

@rouming
Copy link
Contributor Author

rouming commented Apr 4, 2024

@deitch first of all before any further discussion tell me, did you get the Marcel Proust award?

Copy link

codecov bot commented Apr 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 17.51%. Comparing base (d864826) to head (012ba71).
Report is 32 commits behind head on master.

❗ Current head 012ba71 differs from pull request most recent head 2c605ad. Consider uploading reports for the commit 2c605ad to get more accurate results

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.
📢 Have feedback on the report? Share it here.

@deitch
Copy link
Contributor

deitch commented Apr 4, 2024

did you get the Marcel Proust award

I sure did. 😄

So, yes, I did get it more or less. You basically are creating a different path. Where make live builds the image, a different path (let's call it make patch for this comment) patches an existing image.

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.

@rouming
Copy link
Contributor Author

rouming commented Apr 4, 2024

  1. make live (or raw) makes the tar for rootfs, then builds a squashfs out of it
  1. No squashfs for development daily work. ext4. Then we can patch qcow2 image on the flight (because ext4 is rw, squashds is ro).
  2. We generate tar for rootfs, but reduced, with only specified package. See examples here in this commit:
    8f106cf
    Linuxkit is very slow in generating everything. This reduces time.
  3. Then generated tarball is copied to the resulting qcow2 image with this oneliner:
guestfish -a live.qcow2 run : mount /dev/sda2 / : tar-in rootfs.tar /

@rouming
Copy link
Contributor Author

rouming commented Apr 4, 2024

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.

@deitch
Copy link
Contributor

deitch commented Apr 4, 2024

Please point me out if you know where exactly.

Probably BUILD.md, but there are some other candidates.

No squashfs for development daily work. ext4. Then we can patch qcow2 image on the flight (because ext4 is rw, squashds is ro).

Makes sense. So we use make live with ext4 rather than squashfs, and after that we can patch?

We generate tar for rootfs, but reduced, with only specified package. See examples here in this commit

Understood

Linuxkit is very slow in generating everything. This reduces time

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.

Then generated tarball is copied to the resulting qcow2 image with this oneliner
guestfish

guestfish!

In any case, all of this should go into docs properly.

@deitch
Copy link
Contributor

deitch commented Apr 4, 2024

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).

@deitch
Copy link
Contributor

deitch commented Apr 4, 2024

Separate question. The working assumption here is:

  1. We need a bootable disk image, i.e. qcow2
  2. It is 10-30x faster to patch something than to rebuild something
  3. squashfs takes a while to build (squash), and in any case is non-patchable (read-only)

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.

@deitch
Copy link
Contributor

deitch commented Apr 4, 2024

Oh, from the above. Should a patched qcow2 file be marked as such? We don't want people confused.

@rouming
Copy link
Contributor Author

rouming commented Apr 4, 2024

Is there some way we could launch directly from a tar file as the rootfs?

Not sure I understand that.

Oh, from the above. Should a patched qcow2 file be marked as such? We don't want people confused.

Why? Still do not understand where you are heading.

@deitch
Copy link
Contributor

deitch commented Apr 4, 2024

Not sure I understand that.

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.

Why? Still do not understand where you are heading

Run make live or make eve now. You end up with the artifacts in a directory named dist/<arch>/<release+commit>, e.g.

10.4.1-12-gbf81b337c-20-g0c123dcd5-482-ga93b85069

When you run this make patch, the file still is in that directory, but it no longer is correct, is it? The tree hash actually is out of date, might even be dirty.

@rouming
Copy link
Contributor Author

rouming commented Apr 4, 2024

Not sure I understand that.

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.

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.

Why? Still do not understand where you are heading

Run make live or make eve now. You end up with the artifacts in a directory named dist/<arch>/<release+commit>, e.g.

10.4.1-12-gbf81b337c-20-g0c123dcd5-482-ga93b85069

When you run this make patch, the file still is in that directory, but it no longer is correct, is it? The tree hash actually is out of date, might even be dirty.

make patch huh, that's how you call it? I'm bad in proper naming. Probably can emphasize the patching nature.

for patch mode different files are generated:

./images/out/rootfs-kvm-generic.yml.reduced
./dist/amd64/current/rootfs.reduced.tar

Others stay outdated.

@deitch
Copy link
Contributor

deitch commented Apr 4, 2024

Probably not this way, but there are other things can be done.

If it doesn't improve on what you already have here, not worth it. We can discuss though.

make patch huh, that's how you call it? I'm bad in proper naming. Probably can emphasize the patching nature

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?

for patch mode different files are generated:

./images/out/rootfs-kvm-generic.yml.reduced
./dist/amd64/current/rootfs.reduced.tar

OK, that part is nice and clean then, no accidental mixing things up.

but not the live.qcow2

I think it matters, but not sure. Let's let others weigh in?

rouming added 6 commits April 15, 2024 11:24
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>
@rouming rouming changed the title make LIVE_PKG=<p> live make LIVE_PKG=<pkg> live Apr 15, 2024
@rouming
Copy link
Contributor Author

rouming commented Apr 15, 2024

Rebased on latest master, improvements described in BUILD.md.

@deitch
Copy link
Contributor

deitch commented Apr 15, 2024

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.

  • this yq code is ugly, and ties it very tightly to understanding the structure of the yaml file. If and when that changes, this breaks.
  • rootfs.reduced.tar now represents a small part of the rootfs filesystem. If we run just that target again, we will get something else, e.g. now it includes pillar, tomorrow it includes xen-tools, day after etc. This means that the same tar file has multiple meanings at different times
  • Same comment about rootfs.yml.reduced, which can mean different things at different times.
  • Until now, rootfs.img was a reliable representation of the rootfs.tar, which itself is based on the known directory, e.g. ./dist/amd64/10.4.1-12-gbf81b337c-20-g0c123dcd5-577-g79dd19049/ - it includes the version, commits since, hash, etc. It also indicates if it is dirty, i.e. a source has changed. Now, the same file rootfs.img might or might not represent the directory that it is in.

I tried to see if we could get rid of yml.reduced or reduced.tar? How long could it take to just rebuild the whole tar file, once everything is in cache? On my relatively slow cloud VM, repeated runs with latest master commit average to 1m12s. That really is too long. It also would mean updating the entire ext4 filesystem each time by untarring it, also something that would take too long, as opposed to untarring a small subset (i.e. "reduced"), although untarring an entire rootfs.tar is about 2s, so that doesn't matter.

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 parse-pkgs.sh). I am not sure if it buys us enough, and it kind of violates linuxkit's immutable build philosophy, wherein it takes a file as-is on disk and processes it. Modifying it in-memory does not fit with it.

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 images/modifiers/.

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 PAXRecords tar header attribute. Then we could have something like the in-tar option, where we basically stream the files from one tar to the next, skip any that come from a removed package, add those for the replacement.

So the best I can come up with here is:

  1. Include the source packages for each file and directory in the rootfs.tar. This is a good thing anyways.
  2. If the target is --format tar, and we include an --in-tar option, then rather than just recreating the .tar from scratch, try instead to copy every unchanged file from the existing .tar, exclude ones whose packages are removed, add (in the usual way) those whose packages are added.
  3. Reapply the whole filesystem tar to the ext4 image - this is 2s, so not a big deal.

Is it worth all of this vs what we have? 🤷‍♂️

@rouming
Copy link
Contributor Author

rouming commented Apr 17, 2024

So the best I can come up with here is:

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 yq logic from the parse-pkgs is moved to the linuxkit itself:

linuxkit build --format tar ./foo.tar --only-pkg pillar ./rootfs.yml

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 yq. But this has to be carefully named, because we compare not the package, but image, using some simple regexp: 'pkg*', otherwise not all sections match.

@@ -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)))
Copy link
Contributor

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 ))

Copy link
Contributor

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
Copy link
Contributor

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).
Copy link
Contributor

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).

Copy link
Contributor

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!"
Copy link
Contributor

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!"

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

@rouming
Copy link
Contributor Author

rouming commented Apr 18, 2024

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.

@deitch
Copy link
Contributor

deitch commented Apr 18, 2024

are you guys good with this dev approach or not?

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).

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.

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.

@deitch
Copy link
Contributor

deitch commented Apr 18, 2024

The first part in linuxkit is running CI, tagging each file inside the tar. Second one will be next.

@deitch
Copy link
Contributor

deitch commented Apr 19, 2024

@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.

@eriknordmark
Copy link
Contributor

@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?

@deitch
Copy link
Contributor

deitch commented Apr 21, 2024

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)"
Copy link
Contributor

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?

Comment on lines +212 to +213
echo " -p pkg - Removes everything which is not the <pkg> from " >&2
echo " the resulting output." >&2
Copy link
Contributor

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.

Copy link
Contributor

@eriknordmark eriknordmark left a 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.

@rouming rouming mentioned this pull request May 2, 2024
@rouming
Copy link
Contributor Author

rouming commented May 2, 2024

I close this in a favor of a slightly different approach: #3897

@rouming rouming closed this May 2, 2024
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.

4 participants