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

iso: avoid systemd service startup #202

Merged
merged 5 commits into from
Mar 22, 2021
Merged

iso: avoid systemd service startup #202

merged 5 commits into from
Mar 22, 2021

Conversation

blaggacao
Copy link
Contributor

@blaggacao blaggacao commented Mar 18, 2021

fixes #194
alternative to #197

Manual Tests

was unrelated
  • flk install NixOS --impure correctly onto /mnt ❎ (looks like no profile is present)

Issue: #204
Upstream Issue: NixOS/nixpkgs#116938

  • acceptable build time / closure size ca 850MB (for a simple base OS) ✔️
  • local profile with cage service is disabled, that is: boots into terminal ✔️
  • success: air gapped / offline devshell enter ✔️
  • failure: aire gapped target install: ← non blocking bonus item ❎
$ flk install POS
warning: you don't have internet access; disabling some network-dependent features
building the flake in path:/iso/devos?narHash=sha265-...
warning: you don't have internet access; disabling some network-dependent features
error: unable to download 'https://api.github.com/repos/NixOS/nixpkgs/df8e3...': Couldn't resolve host name (6)

→ detailed rationale in the commit messages

❤️ @Pacman99 for the excellent and detailed discussions in #197 and the may ideas, suggestions and code.

lib/devos/devosSystem.nix Outdated Show resolved Hide resolved
@blaggacao blaggacao force-pushed the da/fix-194 branch 4 times, most recently from 12cfb76 to adcd7f0 Compare March 18, 2021 23:46
@blaggacao

This comment has been minimized.

@blaggacao blaggacao force-pushed the da/fix-194 branch 5 times, most recently from 13af8dd to 54654c0 Compare March 19, 2021 04:27
David Arnold added 5 commits March 18, 2021 23:46
This is required so that filtering via lib.remove works against 
modules.core and similar which are of path type.

It is also a prerequisite for disabledModules to match by module.key
instead of path string relative to nixpkgs' modulePath.
IN order to avoid random startup of systemd services, filter out all
profiles, except for core and user profiles.

This works becasue of a fundamental devos contract, that modules
only define configuration, but don't implement them and profiles
only implement confguration but don't define them. So only ever an
activated profile is expected to effectively start up a systemd service.

closes: divnix#194
This ensures that all builds of activated profiles are included into
the iso cache and don't require rebuilding within the live installer
environment.
This is just for convenience, since the closuers are already in the
store. It might be helpful to be able to test out some things
of those deactivated profiles een on the iso isntaller.
@blaggacao blaggacao marked this pull request as ready for review March 19, 2021 04:47
@blaggacao blaggacao requested a review from Pacman99 March 19, 2021 04:48
@blaggacao

This comment has been minimized.

@Pacman99
Copy link
Member

Also apparently getting a segmentation faullt while building the iso is a known issue with nix that happens sometimes. A workaround is to preface the build command with GC_DONT_GC=1

@blaggacao

This comment has been minimized.

@bors
Copy link
Contributor

bors bot commented Mar 19, 2021

try

Already running a review

@blaggacao blaggacao marked this pull request as draft March 19, 2021 14:26
@blaggacao

This comment has been minimized.

@blaggacao

This comment has been minimized.

@blaggacao blaggacao marked this pull request as ready for review March 19, 2021 16:22
Copy link
Member

@Pacman99 Pacman99 left a comment

Choose a reason for hiding this comment

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

Looks great

@Pacman99
Copy link
Member

failure: aire gapped target install: ← non blocking bonus item

Just wondering, is this because of not importing drvPath? Perhaps you could add another build for an offline install iso vs a network install iso. But as you said thats a bonus

@blaggacao
Copy link
Contributor Author

Just wondering, is this because of not importing drvPath?

If I remember correctly, it was trying to download nixpkgs source. — which made me wonder about the registry pinning. But I think to fully understand, we would have to dig through https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/installer/tools/nixos-install.sh

Maybe we can open an issue for air-gapped install, and try to make it work over time.

@Pacman99
Copy link
Member

I think it might try and download the registry anyways and perhaps try to update flake. I think theres a --offline flag that you could try and see how far you get.

@@ -27,7 +27,7 @@ let mkProfileAttrs =
f = n: _:
lib.optionalAttrs
(lib.pathExists "${dir}/${n}/default.nix")
{ default = "${dir}/${n}"; }
{ default = /. + "${dir}/${n}"; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the motivation here?

Copy link
Member

Choose a reason for hiding this comment

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

for the expression in devosSystem lib.remove modules.core suite.allProfiles. For the matching to work all items in the list must be the same type. And in general its better to pass around modules as paths rather than strings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But also.most importantly for disabledModules to filter.on module.key proper instead of a rleative to.modulePath string.

@@ -6,27 +6,42 @@ lib.nixosSystem (args // {
let
moduleList = builtins.attrValues modules;
modpath = "nixos/modules";
cd = "installer/cd-dvd/installation-cd-minimal-new-kernel.nix";

fullHostConfig = (lib.nixosSystem (args // { modules = moduleList; })).config;

isoConfig = (lib.nixosSystem
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would really love to see the bonus goal of your PR implemented. Is it possible that we could create an isoConfig' that is everything that the current isoConfig defines with storeContents abstracted out into a separate variable, while the new isoConfig would just be something like:

{
  isoConfig = isoConfig' // {  isoImage.storeContents = storeContents ++ [ isoConfig' ];
}

I'm trying to think of a way to do this without triggering infinite recursion, although perhaps I haven't thought this through enough and the above does cause and infinite loop. I'll have more time to test tomorrow and see if I can work it out.

Copy link
Contributor Author

@blaggacao blaggacao Mar 20, 2021

Choose a reason for hiding this comment

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

Within the devshell of a live iso, it is ultimately upstream nixos-install at this line that is hindering us, specifically: nix flake metadata evaluates very greedily. Couldn't we obtain that .url — which seemed to be a store path — more cheaply?

On the other hand, the build itself is already fully cached (auto?trusted=1) courtesy of the fullHostConfig's toplevel. That extends to the system profile, which is loaded locally from the build's output

After that there is no potential network call left. So the culprit must be indeed nix flake metadata...

Copy link
Collaborator

@nrdxp nrdxp Mar 20, 2021

Choose a reason for hiding this comment

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

If it is only nixos-install that's blocking us (and it has other problems with flakes), perhaps we could just do what's mentioned in that comment, or something similar to work around it. I personally had to do a legacy nix-build via compat dir to install onto my laptop. Something about my configuration triggered the error mentioned in that thread.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We just moved to a new appartment and, for a varying operational reasons, I probably will be unavailable to follow trough on this bonus item any time soon. (No inet, no setup, obly cell-phone, life getting in the way, ...)

Can we carve this out into an issue while moving on with this PR in its current state?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sure. I'd like to have a crack at it first though, if you don't mind, as I think it would be pretty straight-forward. If I get it working, I'll pushing a PR to your branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suspect the --offline option would get us quite some mileage (combined with a small patch to nixos-install / flakes-first reconception of it), unfortunately I havn't been able to test this in time.

Copy link
Collaborator

Choose a reason for hiding this comment

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

seems I don't have as much time as I'd hoped. I'll go ahead and merge this and we will try to work it out later.
bors r+

@bors
Copy link
Contributor

bors bot commented Mar 22, 2021

Build succeeded:

@bors bors bot merged commit 870897a into divnix:core Mar 22, 2021
@bors
Copy link
Contributor

bors bot commented Mar 23, 2021

try

Merge conflict.

@blaggacao blaggacao deleted the da/fix-194 branch April 22, 2021 00:10
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.

iso: systemd service startup
3 participants