eve icon indicating copy to clipboard operation
eve copied to clipboard

introduce new mechanism of composing rootfs templates

Open zededa-yuri opened this issue 2 years ago • 8 comments

This is preparation for introducing the development builds. The main goal of the patch is to be able to combine different build flavors together, by sequentially applying patches to the rootfs Configuration.

At the moment the final rootfs configuration is built by applying patches on the base configuration. To instruct the build to mutate rootfs, the user needs to pass the HV variable to the make. For example

   make HV=acrn #to build the acrn-based Eve
   make HV=zfs-kvm #to build kvm-based with zfs as default file system

Currently, the build system would look under the images folder for the corresponding patch. For example images/rootfs-acrn.yml.in.patch. This means for each build flavor we want to support we need a separate *.patch file.

This becomes a problem once we need to mix flavors. Particularly, with upcoming “development” build flavor, which is orthogonal to the hypervisor type and can be applied to kvm, xen and acrn. If we keep this approach, to add a dev build we would have to introduce 3 patches under the images directory. This approach clearly does not scale.

In this patch the HV string is tokenized, using minus (‘-’) as a separator. And each token represents a separate patch under the images directory. For example for the HV=acrn-dev, 2 patches would be applied to the base configuration:

  • ‘images/acrn.patch’
  • ‘images/dev.patch’ (will be introduced later)

For now, since this new mechanism is not used, we have the luxury of a transition period. This patch still keeps the old approach and the both mechanisms co-exist: in addition to rootfs-.yml.in also the rootfs-.yml.in.new file is generated and the context is compared. Bail if they do not match.

If any broken build configurations are overlooked during testing, keeping both mechanisms will let us catch any broken builds before they cause any damage.

zededa-yuri avatar May 13 '22 16:05 zededa-yuri

I see some reference to necessary linuxkit changes so tagging @deitch

eriknordmark avatar May 18 '22 18:05 eriknordmark

I see some reference to necessary linuxkit changes so tagging @deitch

Where? Changing linuxkit should not be necessary yet.

zededa-yuri avatar May 18 '22 18:05 zededa-yuri

@zededa-yuri I don't quite get what this is supposed to change.

  1. How do I invoke this new build?
  2. What is the net result difference of doing a "new build" vs an "old build" (or more correctly, a "dev build" vs a "regular build")?
  3. If the build process changes - not just the resultant bootable eve image, but the process of how we get that eve image - what is that change?

May I recommend a docs update as part of this PR, so people coming in do not need to reverse-engineer by reading the Makefile and scripts (or worse, calling Yuri with lots of questions :-) )? That would also help with this.

deitch avatar May 19 '22 00:05 deitch

How do I invoke this new build?

Same way as the old one - nothing is changed here. However the new build will be an extension of the old - allows flavour mixing

What is the net result difference of doing a "new build" vs an "old build" (or more correctly, a "dev build" vs a "regular build")?

This patch does not introduce dev-build (yet). But it enables the build system to have it.

If the build process changes - not just the resultant bootable eve image, but the process of how we get that eve image - what is that change?

The intention of this patch is to have exactly the same image as we currently have. To make sure the new approach does not break existent builds. In the following steps the old approach will be removed

zededa-yuri avatar May 23 '22 15:05 zededa-yuri

@zededa-yuri let me see if I get this right.

The current build system has just one patch. When you run make HV=something eve, it will use the base rootfs yml, and then apply a single patch file whose name is rootfs-${HV}.yml.in.patch (or something similar).

Since there are multiple variants, each of which can have options, we have an exponential problem:

  • hypervisor (kvm, xen, acrn)
  • filesystem (zfs, ext4)
  • console colour (blue, red, green) - I made this one up 😁

Since I can pick one of each of these, I might sanely want to have kvm+zfs+blue or kvm+zfs+green or kvm+ext4+green, etc. In the above examples, there are 18 possible permutations (323).

That would mean we would need 18 individual patch files:

  • rootfs-kvm-zfs-blue.yml.in.patch
  • rootfs-kvm-zfs-green.yml.in.patch
  • rootfs-kvm-ext4-blue.yml.in.patch etc. etc.

Your proposal is to break it into separate patch files, where HV= applies multiple patch files in sequence.

So if I want kvm+zfs+green, all I need is 3 patch files:

  • kvm.yml.in.patch - includes just the changes for kvm
  • zfs.yml.in.patch - includes just the changes for zfs
  • green.yml.in.patch - includes just the changes for green

When I run make HV=kvm-zfs-green, it applies the kvm patch, then the zfs, then the green.

This allows us to have exactly one patch file per sub variant, a total of 8 for the above.

Is that it?

If I understood correctly, I have a few suggestions.

  1. Can you write that up as part of the PR? Add that description to a doc file? Let people understand.
  2. Should this be a single variable HV, as opposed to multiple? Why is this better than make eve HV=kvm FS=zfs COLOR=green?
  3. If, for some reason, it has to be a single variable, HV (from "hypervisor") truly no longer represents it; maybe make FLAVOR=kvm-zfs-green or make PATCHES=kvm-zfs-green?

If I did understand correctly, this makes a ton of sense, very much on board with it. Just needs a clear write-up in docs (not just the PR comment), so people can see how it is built; and needs to think if the right thing is single variable (with a better name) or multiple vars.

deitch avatar May 24 '22 05:05 deitch

Obsoleted by #2666

zededa-yuri avatar Jun 13 '22 18:06 zededa-yuri

Obsoleted by #2666

@zededa-yuri does that mean this PR should be closed?

eriknordmark avatar Jul 21 '22 09:07 eriknordmark

not really. @deitch convinced me that the other PR is still needed as it makes our build a little bit cleaner. I'll brash it up once I have some free time.

zededa-yuri avatar Jul 21 '22 10:07 zededa-yuri

@deitch any interest to take this work over? Or we close?

rouming avatar Nov 09 '22 10:11 rouming

I like what @zededa-yuri proposed here. Does he have time to move it forward?

deitch avatar Nov 13 '22 13:11 deitch

@deitch he left the company

rouming avatar Nov 13 '22 13:11 rouming

Ah oops. Good point! It is not high on my priority list, much as I like the idea. Let's see what @eriknordmark has to say about the priority of this?

deitch avatar Nov 13 '22 13:11 deitch

@deitch please look at my https://github.com/lf-edge/eve/pull/2912 After build flavor tokenization we can generate a set of YQ rules to apply to original rootfs.yml.in Comparing to patches rules are independent so it solves the problem of exponentiation explosion of number of combinations

mikem-zed avatar Nov 16 '22 19:11 mikem-zed

@eriknordmark @mikem-zed can we close this then?

rouming avatar Nov 24 '22 13:11 rouming

I think we can. The only thing missing in #2912 is a description of how to use it. I will add a comment to that PR.

deitch avatar Nov 25 '22 10:11 deitch

Closing since #2912 is now merged.

eriknordmark avatar Nov 29 '22 11:11 eriknordmark