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

nixos/etc-overlay: avoid rebuilding the initrd every time the etc contents change #340722

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

r-vdp
Copy link
Contributor

@r-vdp r-vdp commented Sep 9, 2024

Description of changes

Before this change, the hash of the etc metadata image was included in the mount unit that's responsible for mounting this metadata image in the initrd.
And because this metadata image changes with every change to the etc contents, the initrd would be rebuild every time as well.
This can lead to a lot of rebuilds (especially when revision info is included in /etc/os-release) and all these initrd archives use up a lot of space on the ESP.

With this change, we instead include a symlink to the metadata image in the top-level directory, in the same way as we already do for things like init and prepare-root, and we deduce the store path from the init= kernel parameter, in the same way as we already do to find the path to init and prepare-root.

Doing so avoids rebuilding the initrd all the time.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@r-vdp
Copy link
Contributor Author

r-vdp commented Sep 9, 2024

@ofborg test activation-etc-overlay-mutable activation-etc-overlay-immutable

Comment on lines +107 to +133
set -uo pipefail

# Figure out what closure to boot
closure=
for o in $(< /proc/cmdline); do
case $o in
init=*)
IFS='=' read -r -a initParam <<< "$o"
closure="''${initParam[1]}"
;;
esac
done

# Sanity check
if [ -z "''${closure:-}" ]; then
echo 'No init= parameter on the kernel command line' >&2
exit 1
fi

# Resolve symlinks in the init parameter
closure="$(chroot /sysroot ${lib.getExe' pkgs.coreutils "realpath"} "$closure")"
# Assume the directory containing the init script is the closure.
closure="$(dirname "$closure")"
Copy link
Contributor Author

@r-vdp r-vdp Sep 9, 2024

Choose a reason for hiding this comment

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

This is basically the same exact logic as what we have for the activation service, where we find the path to prepare-root in the same way.
Maybe it would make sense to create a common service that does this once and makes the closure available in the initrd so that other units can use it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ElvishJerricco I'd like to hear your thoughts on this. Would it make sense to pull this into a separate service that creates a link to the closure so that we can reuse this elsewhere?

@@ -23,6 +23,15 @@
specialisation.new-generation.configuration = {
environment.etc."newgen".text = "newgen";
};

assertions = lib.mkIf (lib.length (lib.attrNames config.specialisation) > 0) [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This mkIf is needed because otherwise this assertion is also added to the specialisations, which leads to an error because they don't have any specialisations themselves.

@r-vdp r-vdp force-pushed the etc-overlay-no-initrd-rebuilds branch from 6c6e66d to 08f7609 Compare September 25, 2024 22:08
@r-vdp
Copy link
Contributor Author

r-vdp commented Oct 3, 2024

@nikstur would you be able to give this a review? I think it would be nice to get this in before 24.11 branch-off.

…tents change

Before this change, the hash of the etc metadata image was included in
the mount unit that's responsible for mounting this metadata image in the
initrd.
And because this metadata image changes with every change to the etc
contents, the initrd would be rebuild every time as well.
This can lead to a lot of rebuilds (especially when revision info is
included in /etc/os-release) and all these initrd archives use up a lot of
space on the ESP.

With this change, we instead include a symlink to the metadata image in the
top-level directory, in the same way as we already do for things like init and
prepare-root, and we deduce the store path from the init= kernel parameter,
in the same way as we already do to find the path to init and prepare-root.

Doing so avoids rebuilding the initrd all the time.
@r-vdp r-vdp force-pushed the etc-overlay-no-initrd-rebuilds branch from 08f7609 to fd801cf Compare October 5, 2024 08:15
@r-vdp r-vdp changed the title etc-overlay: avoid rebuilding the initrd every time the etc contents change nixos/etc-overlay: avoid rebuilding the initrd every time the etc contents change Oct 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant