multipass icon indicating copy to clipboard operation
multipass copied to clipboard

Blueprints v2 with loading from URL

Open luis4a0 opened this issue 3 years ago • 13 comments

This PR introduces fetching images from URL on Blueprints. Since the images are only intended to be created in AMD64 and ARM64, adding this feature to Blueprints makes the YAML format used in the old Blueprints incompatible. For this, we introduce a new version of Blueprints, v2.

Fixes #2671.

luis4a0 avatar Sep 14 '22 13:09 luis4a0

Codecov Report

Merging #2745 (c2cd92c) into main (7e6e020) will increase coverage by 0.03%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2745      +/-   ##
==========================================
+ Coverage   86.49%   86.52%   +0.03%     
==========================================
  Files         230      230              
  Lines       11693    11744      +51     
==========================================
+ Hits        10114    10162      +48     
- Misses       1579     1582       +3     
Impacted Files Coverage Δ
include/multipass/vm_image_vault.h 100.00% <ø> (ø)
...ueprint_provider/default_vm_blueprint_provider.cpp 99.08% <100.00%> (-0.92%) :arrow_down:
src/daemon/daemon.cpp 66.73% <100.00%> (+0.13%) :arrow_up:
src/daemon/default_vm_image_vault.cpp 85.51% <100.00%> (+0.08%) :arrow_up:
src/platform/backends/lxd/lxd_vm_image_vault.cpp 94.40% <100.00%> (+0.04%) :arrow_up:
src/sshfs_mount/sshfs_mount_handler.cpp 14.43% <0.00%> (-1.04%) :arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov[bot] avatar Sep 14 '22 13:09 codecov[bot]

If I may chime in, does this really require a bump to v2? Can't you instead maintain backwards compatibility?

Saviq avatar Sep 14 '22 14:09 Saviq

Can't you instead maintain backwards compatibility?

Sure, it's possible, but it would add lots of complexity for Blueprints maintainers and for Multipass. The notion of Blueprint-specific images will only apply to amd64 and arm64 and after team discussion, we felt that separating the Blueprints that either support the v1 way such as the base cloud image, lots of bootstrapping of cloud-init, etc. should be separate from specifying an image and only allowing the Blueprints to run on those two architectures. It's a clean break between the two. Also, doing this will allow a corresponding v1 Blueprint that runs on a different architecture to still exist without muddying up the Blueprint with all sorts of logic to do things depending on if there a Blueprint-specific image or not.

townsend2010 avatar Sep 14 '22 14:09 townsend2010

Blueprint-specific images will only apply to amd64 and arm64

Shouldn't that be a per-blueprint thing? I mean, if someone comes in and provides an armhf image, that should be supported?

doing this will allow a corresponding v1 Blueprint that runs on a different architecture to still exist

That I grok, but only if that doesn't mean that amd64/arm64 will only support v2, and others only support v1?

Saviq avatar Sep 14 '22 15:09 Saviq

Hey @Saviq! Imagine the situation with our Docker Blueprint. We want to provide only AMD64 and ARM64 builds, while generating the VM from a plain jammy image for the other architectures. So we'll have a cloud-init for AMD64, ARM64, and a different cloud-init for the other archs. If we do all the definitions on the same YAML, even modifying the syntax for that, would be a mess. Introducing v2 will tidy things up in this case.

luis4a0 avatar Sep 14 '22 15:09 luis4a0

@luis4a0 sure, I get why it's cleaner this way, but IMO limiting, if you can't "escape" it. If Multipass parses both the v1 and v2, and selects the highest one supporting the current arch, that allows for both a v1/foo.yaml and a v2/foo.yaml to exist. This way you can gradually introduce bespoke images. But you have to maintain two blueprints, even if a lot of them will be the same (that obviously depends on what the blueprint actually does - if most of it can be moved to the bespoke image, then the v2 will be much simpler).

For reference, Snapcraft has the same problem, and they solved it with advanced grammar, so there is prior art: https://snapcraft.io/docs/snapcraft-advanced-grammar. You could think of adopting a similar approach long-term.

Saviq avatar Sep 14 '22 15:09 Saviq

Maybe restricting the v2 Blueprints to just amd64 and arm64 is not necessary although we did some analysis and this affects only Linux and only 0.5% of those users are potentially affected (and only a subset of those users will use a Blueprint), so drawing a line in the sand is not really that far off base.

I really think introducing v2 here is better for the reasons @luis4a0 pointed out, it keeps it neat and tidy. We could do something like this though:

  1. If v2 Blueprint exists and has image for host architecture, use it.
  2. If not, then check if a corresponding v1 Blueprint exists. If so, check runs-on with host architecture and use it if there is a match.
  3. Otherwise, Blueprint doesn't work and it won't be available.

As you said, if most of a Blueprints bootstrapping can moved into an image, then v2 will be simpler. But if it can't, there is really no reason for a v2 Blueprint and image and the Blueprint should just stay v1.

Regarding the Snapcraft advanced grammar, we didn't consider this and IMO, it muddies the Blueprint again if we introduce something like that. There is a trade-off no matter which path we go down, but I think we are trying to cater to a very, very small section of users and I'd like to keep things as clean as we can.

townsend2010 avatar Sep 14 '22 16:09 townsend2010

We could do something like this though:

  1. If v2 Blueprint exists and has image for host architecture, use it.
  2. If not, then check if a corresponding v1 Blueprint exists. If so, check runs-on with host architecture and use it if there is a match.
  3. Otherwise, Blueprint doesn't work and it won't be available.

Right, that's the backwards compatibility I had in mind. Just clarify one thing for me - can you still use image aliases in v2?

Regarding the Snapcraft advanced grammar, we didn't consider this and IMO, it muddies the Blueprint again

That is true, it doesn't make for an easy read.

Saviq avatar Sep 14 '22 16:09 Saviq

Right, that's the backwards compatibility I had in mind. Just clarify one thing for me - can you still use image aliases in v2?

There really shouldn't be a need for that. If you want to use an alias, that is what v1 would be for. I see v2 as "Here's an image that does all of the otherwise heavylifting of installing packages that you see in v1 Blueprints." Without a bespoke image in v2, there really isn't a point. But if you have an idea I'm not thinking off, I'm all, uh, eyes.

townsend2010 avatar Sep 14 '22 16:09 townsend2010

There really shouldn't be a need for that. If you want to use an alias, that is what v1 would be for. I see v2 as "Here's an image that does all of the otherwise heavylifting of installing packages that you see in v1 Blueprints." Without a bespoke image in v2, there really isn't a point. But if you have an idea I'm not thinking off, I'm all, uh, eyes.

That's not what's versions are in my mind… You shouldn't be stuck with two "active" versions for too long. And if someone can only maintain an image for either amd64 or arm64, they'll need to maintain two blueprints, even if they could encode the logic in just one (conditional on e.g. what's preinstalled or not - they can do it just fine within the commands ran, most of them will even be idempotent). I just worry you'll end up stuck with the two modes forever and have to explain / document / otherwise deal with it.

Which is why I ultimately think it should still be "v1", just with a new grammar for images supported, and aliases supported in there as well. Let the blueprint author decide how they approach it.

It's your call of course, just my ¢2.

Saviq avatar Sep 16 '22 08:09 Saviq

I certainly understand your point and thought about this too. It's not just about the images grammar- that's actually the uncomplicated part. The complicated part is the handling of the cloud-init:vendor-data part. We just pass this in directly to cloud-init, but there would be many conditional parts in this section. We would basically have to have two separate vendor-data's depending on whether a bespoke image is available or not. That would get very messy in the Blueprint IMO.

Also, v2 is just an arbitrary name. We could call it something else like v1/image-based/foo.yaml and it would accomplish the same thing and not look like a version jump.

In the end, there's going to be a compromise with this. The question that needs to be answered is do we want to still use a unified Blueprint that supports all of this (which will be complicated for the author) or have the option of two separate Blueprints in which the Blueprints themselves will be much cleaner?

One thing to keep in mind is I feel the vast majority of the Blueprints that use bespoke images will be for amd64 and arm64 only. It's worth pointing out again that the other architectures only account for 0.5% of installs on Linux only. Most of our users are on Windows and macOS and there is no other option than those two architectures.

Edit: And one very last point in all of this- we really, really want to encourage Blueprint authors to provide an image with the Blueprint when it makes sense. Really, if it doesn't make sense to include an image, then just providing a URL based cloud-init config would probably be better than creating a whole Blueprint. I can see at some point deprecating alias based Blueprints with all of the cloud-init bootstrapping and the author will have to decide what architectures they want to support with the images.

townsend2010 avatar Sep 16 '22 11:09 townsend2010

ACK on all points. I suppose I just dislike the inevitable duplication, and the alternative of only supporting a{md,rm}64 in a given blueprint (which is what I imagine this split will lead to). You clearly thought this through and I'm outvoted, so will not push this more :)

Saviq avatar Sep 16 '22 14:09 Saviq

I suppose I just dislike the inevitable duplication

I don't really like it either, but I think the alternative of having a complicated Blueprint is worse, but yes, I certainly understand you on this.

the alternative of only supporting a{md,rm}64 in a given blueprint

Well, that won't be a restriction after thinking about what you said as we will allow authors to use bespoke images for other architectures. Yes, I don't think there will be too many takers on maintaining a plethora of images, but images will be a very important factor for the best user experience when using Blueprints and we want to encourage this.

Anyways, I always appreciate your comments and it's good for discussion :wink:

townsend2010 avatar Sep 16 '22 14:09 townsend2010

Timed out.

bors[bot] avatar Feb 03 '23 23:02 bors[bot]

Integration tests actually passed: https://github.com/canonical/multipass/actions/runs/4088953335

Merging manually...\o/

townsend2010 avatar Feb 04 '23 02:02 townsend2010