make-disk-image: change to be less VM-centric #24964

copumpkin · 2017-04-17T03:14:47Z

This changes much of the make-disk-image.nix logic (and thus most NixOS image building) to use LKL to set up the target directory structure rather than a Linux VM. The only work we still do in a VM is less IO-heavy stuff that while still time-consuming, is less of the overall load. The goal is to kill more of that stuff, but that will require deeper changes to NixOS activation scripts and switch-to-configuration.pl, and I don't want to bite off too much at once.

This is a new version of #21943, refactored to take advantage of the new nixos-prepare-root logic I introduce in #23026. There's still a couple of WIP things here but I wanted to get this out there to show what I've been talking about in the other PR.

Motivation for this change

Building disk images with VMs is miserable in EC2 and other cloud virtualization providers, and doesn't work at all in local VMWare due to some shady nested virtualization bugs.

Things done

Tested using sandboxing
(nix.useSandbox on NixOS,
or option build-use-sandbox in nix.conf
on non-NixOS)
Built on platform(s)
- NixOS
- macOS
- Linux
Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Fits CONTRIBUTING.md.

mention-bot · 2017-04-17T03:14:49Z

@copumpkin, thanks for your PR! By analyzing the history of the files in this pull request, we identified @domenkozar, @edolstra and @obadz to be potential reviewers.

domenkozar · 2017-04-17T14:21:36Z

How long does the build take on EC2 with this PR?

copumpkin · 2017-04-17T14:23:06Z

Under a minute for the default Amazon image module. I'll put up some stats later! Faster on bare metal too

…

On Mon, Apr 17, 2017 at 10:21 Domen Kožar ***@***.***> wrote: How long does the build take on EC2 with this PR? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#24964 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAKP3NeoDQDkDM8KjVtVXDklPXmHqdKks5rw3V2gaJpZM4M-1RU> .

domenkozar · 2017-04-17T14:24:52Z

I just had a 3 hour deploy where AMI handling was ~80min so this is highly appreciated!!

copumpkin · 2017-04-17T14:26:34Z

Sorry to hear that! I hope to get this PR in a mergeable state later today, so with any luck we'll all be fast by default very soon now :)

…

On Mon, Apr 17, 2017 at 10:24 Domen Kožar ***@***.***> wrote: I just had a 3 hour deploy where AMI handling was ~80min so this is highly appreciated!! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#24964 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAKP606LbGYs0GSDGZmIiWTH6mv1zi8ks5rw3Y3gaJpZM4M-1RU> .

7c6f434c · 2017-04-17T14:26:45Z

Hm, tools for cheap image builds sound like something useful for spinoffs, too…

copumpkin · 2017-04-17T15:30:47Z

nixos/lib/make-disk-image.nix

-pkgs.vmTools.runInLinuxVM (
+let
+  # Copied from https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/installer/cd-dvd/channel.nix
+  channelSources = pkgs.runCommand "nixos-${config.system.nixosVersion}" {} ''


This chunk might not actually be necessary...

copumpkin · 2017-04-17T17:07:54Z

@7c6f434c yes, I have a larger plan for all sorts of images that I start talking about a bit here: #23052

copumpkin · 2017-04-17T20:02:04Z

Pushed a few more changes that I think take this out of WIP land.

I also ran a few tests for speed in a loop just to get a sense for how this performs.

On (the same across all tries) physical hardware (with VT-X etc.):

Before I merged nixos-prepare-root (in case that affected it negatively):

real	3m39.639s
real	3m38.517s
real	3m40.729s
(I got sick of waiting)

This PR:

real	0m22.107s
real	0m21.942s
real	0m22.242s
real	0m22.511s
real	0m22.127s
real	0m22.318s
real	0m22.215s
real	0m22.328s
real	0m22.200s
real	0m22.235s

On EC2:

I don't have exact measurements handy but my EC2 image builds were in the 25-30 minute range and are now in the 30s-1m range.

layus · 2017-04-17T21:02:21Z

nixos/lib/make-disk-image.nix

+      exportReferencesGraph = [ "closure" metaClosure ];
+      postVM = ''
+        ${if format == "raw" then ''
+          mv $diskImage $out/nixos.img


This breaks nixos/modules/virtualisation/virtualbox-image.nix which relies on $diskImage being set.
The former code had diskImage=$out/nixos.${if format == "qcow2" then "qcow2" else "img"} in PreVM, which makes much sense.

Thanks! What's the best way to test that? Is there a good VM test or a nix-build invocation I can run to make sure it works?

Ah, I've been able to repro with this:

nix-build nixos/ --no-out-link -A config.system.build.virtualBoxOVA --arg configuration "{ imports = [ ./nixos/modules/virtualisation/virtualbox-image.nix ]; }"

@layus I don't think I can replicate that exact logic sensibly here, but I'll make it work for downstream consumers

Okay, I can build an OVA fine in the latest push. Takes about a minute to build on my machine.

I also tested that novaImage works fine, which seems to be the only other thing that uses make-disk-image.nix. An upcoming PR will consolidate all the other image builders (GCE, Azure, etc.) to use this script (making adjustments here to accommodate variations)

So you figured everything yourself. Nice :-).

Some years ago, I tried to build a VBox image using Nix{,OS}. It took one hour to make a trivial image. Now I took me about 8min, download included. There are still three copies where one would be optimal, but it's already way faster.

copumpkin · 2017-04-18T02:56:02Z

nixos/lib/make-disk-image.nix

-          done
+      # Set up core system link, GRUB, etc.
+      NIXOS_INSTALL_BOOTLOADER=1 chroot $mountPoint /nix/var/nix/profiles/system/bin/switch-to-configuration boot
+      chroot $mountPoint /nix/var/nix/profiles/system/activate


Should I be calling the activation script here?

As opposed to not calling activate at all? Was the activation script called in the previous flow?

copumpkin · 2017-04-18T02:56:35Z

@shlevy mind taking a look?

shlevy · 2017-04-18T09:19:14Z

@copumpkin probably not until this weekend, sorry

copumpkin · 2017-04-18T11:56:54Z

@volth I'd certainly be open to that, but probably not as part of this ticket, which is purely trying to refactor the implementation of the existing interface. Perhaps make a ticket talking about how you might use that? The thought of having builds not managed by Nix blows my mind so I can't imagine it 😄

copumpkin · 2017-04-19T04:43:26Z

It turns out I actually broke the old make-disk-image.nix in my last nixos-prepare-root PR. I'd rather not put effort into fixing it if we're going to replace it with this in the next day or two. Does anyone expect this PR to be particularly contentious?

cstrahan · 2017-04-19T05:05:15Z

Does anyone expect this PR to be particularly contentious?

Not I, FWIW.

cstrahan

Aside from the (open?) question regarding /nix/var/nix/profiles/system/activate, this looks good to me.

I think it's reasonable to refactor the filterFn and other channel bits in a follow up.

copumpkin · 2017-04-20T02:14:14Z

The old thing didn't call activate, nope. I'm not sure why I started doing that or whether it's the correct thing to do. It also forced me to start doing umount -R, which might suggest I'm doing the wrong thing.

cstrahan · 2017-04-21T18:19:38Z

@copumpkin Perhaps it's best to keep the activate part as it was, then. Also, I could be wrong, but I thought there was a use case in the context of AMIs that went something like this:

Create an image with a particular configuration
When it first boots, the activation script reads some info from the environment, and possibly pulls down a new configuration.nix, which it then switches to

That's essentially what one of your old PRs did (which I think I contributed a bit to, IIRC), right? My memory is a little foggy here.

I would think calling activate would make it difficult (or impossible) to do such first-boot setup.

copumpkin · 2017-04-21T21:46:24Z

Both of those work today with this configuration but you're right, I probably don't want the activation script in here. I'll take it out for now. The EC2 boot configuration thing is what actually runs in our EC2 VM tests and those still pass after this PR :)

…

On Apr 21, 2017, at 14:19, Charles Strahan ***@***.***> wrote: @copumpkin Perhaps it's best to keep the activate part as it was, then. Also, I could be wrong, but I thought there was a use case in the context of AMIs that went something like this: Create an image with a particular configuration When it first boots, the activation script reads some info from the environment, and possibly pulls down a new configuration.nix, which it then switches to That's essentially what one of your old PRs did (which I think I contributed a bit to, IIRC), right? My memory is a little foggy here. I would think calling activate would make it difficult (or impossible) to do such first-boot setup. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

shlevy · 2017-04-23T18:17:29Z

I'm not really all that familiar with this code and dont' really have an opinion either way. Fine by me if it works.

cstrahan · 2017-04-23T18:19:49Z

🚢 👍

copumpkin · 2017-04-23T19:25:51Z

Okay when I get back to my computer I'll remove the activation script invocation then merge. We can always revert if it's a disaster:)

…

On Apr 23, 2017, at 14:19, Charles Strahan ***@***.***> wrote: 🚢 👍 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

This changes much of the make-disk-image.nix logic (and thus most NixOS image building) to use LKL to set up the target directory structure rather than a Linux VM. The only work we still do in a VM is less IO-heavy stuff that while still time-consuming, is less of the overall load. The goal is to kill more of that stuff, but that will require deeper changes to NixOS activation scripts and switch-to-configuration.pl, and I don't want to bite off too much at once.

copumpkin · 2017-04-24T02:31:18Z

Okay, removed the call to activate, so I'll merge now. Going to be making a few more commits in next few days adding some tests and switching some of the other image builders to use this.

domenkozar · 2017-04-24T07:48:17Z

I'll test this shortly. Thanks!

copumpkin added the 6.topic: nixos label Apr 17, 2017

copumpkin requested review from shlevy, edolstra and obadz April 17, 2017 03:14

This was referenced Apr 17, 2017

[WIP, don't merge] make-disk-image: change to be less VM-centric #21943

Closed

Refactor nixos-install to separate out filesystem build logic #23026

Merged

copumpkin changed the base branch from copumpkin/nixos-install-refactor to master April 17, 2017 13:52

copumpkin commented Apr 17, 2017

View reviewed changes

copumpkin mentioned this pull request Apr 17, 2017

Does the EC2 image builder need to run in a VM? #6956

Closed

copumpkin added this to Improve speed & portability in Amazing images Apr 17, 2017

copumpkin force-pushed the make-disk-image-refactor branch from 2037fef to 27a49ef Compare April 17, 2017 20:01

copumpkin requested a review from domenkozar April 17, 2017 20:02

copumpkin changed the title ~~[WIP] make-disk-image: change to be less VM-centric~~ make-disk-image: change to be less VM-centric Apr 17, 2017

copumpkin requested a review from cstrahan April 17, 2017 20:47

layus reviewed Apr 17, 2017

View reviewed changes

copumpkin force-pushed the make-disk-image-refactor branch from 27a49ef to 9ce7d1d Compare April 17, 2017 21:27

copumpkin commented Apr 18, 2017

View reviewed changes

copumpkin force-pushed the make-disk-image-refactor branch 2 times, most recently from d4516ea to 7f8f168 Compare April 18, 2017 18:55

cstrahan approved these changes Apr 19, 2017

View reviewed changes

domenkozar mentioned this pull request Apr 20, 2017

nixos/lib/make-disk-image: performance regression since 16.03 #20471

Closed

copumpkin force-pushed the make-disk-image-refactor branch from 7f8f168 to f1708a9 Compare April 24, 2017 02:30

copumpkin merged commit 7025fc6 into NixOS:master Apr 24, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

make-disk-image: change to be less VM-centric #24964

make-disk-image: change to be less VM-centric #24964

copumpkin commented Apr 17, 2017 •

edited

mention-bot commented Apr 17, 2017

domenkozar commented Apr 17, 2017

copumpkin commented Apr 17, 2017 via email

domenkozar commented Apr 17, 2017

copumpkin commented Apr 17, 2017 via email

7c6f434c commented Apr 17, 2017

copumpkin Apr 17, 2017

copumpkin commented Apr 17, 2017

copumpkin commented Apr 17, 2017

layus Apr 17, 2017

copumpkin Apr 17, 2017

copumpkin Apr 17, 2017

copumpkin Apr 17, 2017

copumpkin Apr 17, 2017

copumpkin Apr 17, 2017

layus Apr 18, 2017

copumpkin Apr 18, 2017

cstrahan Apr 19, 2017

copumpkin commented Apr 18, 2017

shlevy commented Apr 18, 2017

copumpkin commented Apr 18, 2017

copumpkin commented Apr 19, 2017

cstrahan commented Apr 19, 2017

cstrahan left a comment

copumpkin commented Apr 20, 2017

cstrahan commented Apr 21, 2017

copumpkin commented Apr 21, 2017 via email

shlevy commented Apr 23, 2017

cstrahan commented Apr 23, 2017

copumpkin commented Apr 23, 2017 via email

copumpkin commented Apr 24, 2017

domenkozar commented Apr 24, 2017

make-disk-image: change to be less VM-centric #24964

make-disk-image: change to be less VM-centric #24964

Conversation

copumpkin commented Apr 17, 2017 • edited

Motivation for this change

Things done

mention-bot commented Apr 17, 2017

domenkozar commented Apr 17, 2017

copumpkin commented Apr 17, 2017 via email

domenkozar commented Apr 17, 2017

copumpkin commented Apr 17, 2017 via email

7c6f434c commented Apr 17, 2017

Choose a reason for hiding this comment

copumpkin commented Apr 17, 2017

copumpkin commented Apr 17, 2017

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

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

copumpkin commented Apr 18, 2017

shlevy commented Apr 18, 2017

copumpkin commented Apr 18, 2017

copumpkin commented Apr 19, 2017

cstrahan commented Apr 19, 2017

cstrahan left a comment

Choose a reason for hiding this comment

copumpkin commented Apr 20, 2017

cstrahan commented Apr 21, 2017

copumpkin commented Apr 21, 2017 via email

shlevy commented Apr 23, 2017

cstrahan commented Apr 23, 2017

copumpkin commented Apr 23, 2017 via email

copumpkin commented Apr 24, 2017

domenkozar commented Apr 24, 2017

copumpkin commented Apr 17, 2017 •

edited