Skip to content

profiles/minimal: reduce size#173661

Merged
Lassulus merged 7 commits into
NixOS:masterfrom
Izorkin:profiles-reduce
Dec 9, 2022
Merged

profiles/minimal: reduce size#173661
Lassulus merged 7 commits into
NixOS:masterfrom
Izorkin:profiles-reduce

Conversation

@Izorkin

@Izorkin Izorkin commented May 19, 2022

Copy link
Copy Markdown
Contributor
Description of changes

Minimize size netboot and iso images.
Splitted this PR - #170460

cc @SuperSandro2000

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 22.05 Release Notes (or backporting 21.11 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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@github-actions github-actions Bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels May 19, 2022
@ofborg ofborg Bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels May 19, 2022
@Mindavi Mindavi added the 6.topic: closure size The final size of a derivation, including its dependencies label May 19, 2022
@SuperSandro2000 SuperSandro2000 requested a review from Artturin May 19, 2022 23:19
@Izorkin

Izorkin commented Jun 27, 2022

Copy link
Copy Markdown
Contributor Author

What does it take for this PR to be merged?

Comment thread nixos/modules/installer/cd-dvd/installation-cd-minimal.nix Outdated
Comment thread nixos/modules/profiles/minimal.nix Outdated
@Izorkin Izorkin force-pushed the profiles-reduce branch 3 times, most recently from 70e6a1e to c6f28bd Compare July 5, 2022 09:11
@Izorkin

Izorkin commented Jul 5, 2022

Copy link
Copy Markdown
Contributor Author

Rebased PR.
Removing commit 'nixos/installer/netboot: use makeInitrdNG' - #176796
cc @ajs124 @Lassulus @lheckemann

@SuperSandro2000 SuperSandro2000 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nixos/modules/profiles/minimal.nix is also used for containers, right? Then I don't think we should disable man pages there. Also we should think about removing environment.noXlibs as it can cause quite weird problems when building certain packages and you didn't think about it while debugging.

@Izorkin

Izorkin commented Jul 5, 2022

Copy link
Copy Markdown
Contributor Author

@SuperSandro2000 virtualisation.containers does not use nixos/modules/profiles/minimal.nix.

@Izorkin

Izorkin commented Jul 5, 2022

Copy link
Copy Markdown
Contributor Author

Didn't not found mysself any problems with environment.noXlibs.

@SuperSandro2000

Copy link
Copy Markdown
Member

@SuperSandro2000 virtualisation.containers does not use nixos/modules/profiles/minimal.nix.

nspawn/nixos containers, not oci containers.

The last case is https://discourse.nixos.org/t/unable-to-install-paperless-ngx/19962

@lheckemann

Copy link
Copy Markdown
Member

This should have a release notes entry, since it may require changes to people's configurations.

@github-actions github-actions Bot added 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation labels Jul 12, 2022
@Izorkin

Izorkin commented Jul 12, 2022

Copy link
Copy Markdown
Contributor Author

This should have a release notes entry, since it may require changes to people's configurations.

Added release notes.

Comment thread nixos/modules/profiles/minimal.nix Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That could be really dangerous and fill up your disk when logs in containers get no longer rotated. Also many people don't even know that the container uses this profile.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This module is used only in tests and in minimal ISO image. Should not affect working to containers. And it was enabled by default recently.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This module is used only in tests and in minimal ISO image.

No, it is also included for nixos-containers because otherwise the stupid environment.noXlibs option wouldn't mess with some software builds.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I tested with this configuration:

  containers.test-man = {
    autoStart = true;
    privateNetwork = true;
    hostBridge = "br0";
    localAddress = "10.11.0.1/24";
    config = { pkgs, config, ... }: {
      system.extraDependencies = [ pkgs.hello ];
      services.openssh = {
        enable = true;
      };
    };
  };

Changes to nixos/modules/profiles/minimal.nix did not affect the container

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🤔 maybe I was doing something wonky but lxc container imports minimal profile which I was using for the container https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/virtualisation/lxc-container.nix#L54

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@SuperSandro2000 add this patch?

diff --git a/nixos/doc/manual/from_md/release-notes/rl-2211.section.xml b/nixos/doc/manual/from_md/release-notes/rl-2211.section.xml
index 6a7032e1963..392f79cb51e 100644
--- a/nixos/doc/manual/from_md/release-notes/rl-2211.section.xml
+++ b/nixos/doc/manual/from_md/release-notes/rl-2211.section.xml
@@ -368,6 +368,11 @@
           <literal>nixos/modules/profiles/minimal.nix</literal> profile.
         </para>
       </listitem>
+      <listitem>
+        <para>
+          Now lxc containers don't use minimal profile.
+        </para>
+      </listitem>
       <listitem>
         <para>
           There is a new module for the <literal>xfconf</literal>
diff --git a/nixos/doc/manual/release-notes/rl-2211.section.md b/nixos/doc/manual/release-notes/rl-2211.section.md
index b56be16127b..817262affd8 100644
--- a/nixos/doc/manual/release-notes/rl-2211.section.md
+++ b/nixos/doc/manual/release-notes/rl-2211.section.md
@@ -134,6 +134,8 @@ Use `configure.packages` instead.

 - The minimal ISO image now use `nixos/modules/profiles/minimal.nix` profile.

+- Now lxc containers don't use minimal profile.
+
 - There is a new module for the `xfconf` program (the Xfce configuration storage system), which has a dbus service.

 <!-- To avoid merge conflicts, consider adding your item at an arbitrary place in the list instead. -->
diff --git a/nixos/modules/virtualisation/lxc-container.nix b/nixos/modules/virtualisation/lxc-container.nix
index d3a2e0ed151..b6b744dfbca 100644
--- a/nixos/modules/virtualisation/lxc-container.nix
+++ b/nixos/modules/virtualisation/lxc-container.nix
@@ -51,7 +51,6 @@ in
 {
   imports = [
     ../installer/cd-dvd/channel.nix
-    ../profiles/minimal.nix
     ../profiles/clone-config.nix
   ];

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Or this variant:

diff --git a/nixos/modules/virtualisation/lxc-container.nix b/nixos/modules/virtualisation/lxc-container.nix
index d3a2e0ed151..b6b4dcff1d0 100644
--- a/nixos/modules/virtualisation/lxc-container.nix
+++ b/nixos/modules/virtualisation/lxc-container.nix
@@ -55,6 +55,8 @@ in
     ../profiles/clone-config.nix
   ];

+  environment.noXlibs = lib.mkOverride 500 false;
+
   options = {
     virtualisation.lxc = {
       templates = mkOption {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@SuperSandro2000 Is there a consensus on this remark?

@SuperSandro2000 SuperSandro2000 Dec 9, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

noXlibs should not be enabled by default. It is an advanced setting that required multiple overrides to build on all my machines, which is not acceptable for normal users.

logrotation should not be disabled in lxc otherwise your disk can completely fill up which should be avoided with good defaults.

@Izorkin

Izorkin commented Oct 19, 2022

Copy link
Copy Markdown
Contributor Author

Rebased PR.
What is needed to get this PR merged? :)

@Artturin

Artturin commented Oct 19, 2022

Copy link
Copy Markdown
Member

rebased the pr on master(git rebase --onto master HEAD~7)

and built the iso (nix build -f ./nixos/release-combined.nix nixos.iso_minimal.x86_64-linux) on master with and without this pr

but the size seems to be larger?

now

du -shc --apparent-size $(readlink -f result/iso/nixos-minimal-22.11pre56789.gfedcba-x86_64-linux.iso)
3.8G	/nix/store/w6aalmkc9w9mvmlmvcc4a6567h5ncrqg-nixos-minimal-22.11pre56789.gfedcba-x86_64-linux.iso/iso/nixos-minimal-22.11pre56789.gfedcba-x86_64-linux.iso
3.8G	total

before

$ du -shc --apparent-size $(readlink -f result/iso/nixos-minimal-22.11pre56789.gfedcba-x86_64-linux.iso)
2.2G	/nix/store/6lby9i5kx22qqgpin23klwpbf6nn6vxm-nixos-minimal-22.11pre56789.gfedcba-x86_64-linux.iso/iso/nixos-minimal-22.11pre56789.gfedcba-x86_64-linux.iso
2.2G	total

@Izorkin

Izorkin commented Oct 29, 2022

Copy link
Copy Markdown
Contributor Author

Fix conflicts.

@ck3d ck3d left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@SuperSandro2000 SuperSandro2000 removed their request for review October 30, 2022 18:01
@RaitoBezarius

Copy link
Copy Markdown
Member

Can you move the release notes to 23.05 ? Think it's good to merge now.

@Izorkin

Izorkin commented Dec 1, 2022

Copy link
Copy Markdown
Contributor Author

Can you move the release notes to 23.05 ? Think it's good to merge now.

Updated.

@Artturin

Artturin commented Dec 3, 2022

Copy link
Copy Markdown
Member

From somewhere, are added nixpkgs sources with git history to image:

du -d 1 -hc  /nix/.ro-store/5q1kw0svyl0j3p1jwc0yv20x1sa63bw5-source/
1.2G    /nix/.ro-store/5q1kw0svyl0j3p1jwc0yv20x1sa63bw5-source/.git
53K     /nix/.ro-store/5q1kw0svyl0j3p1jwc0yv20x1sa63bw5-source/.github
788K    /nix/.ro-store/5q1kw0svyl0j3p1jwc0yv20x1sa63bw5-source/doc
479K    /nix/.ro-store/5q1kw0svyl0j3p1jwc0yv20x1sa63bw5-source/lib
563K    /nix/.ro-store/5q1kw0svyl0j3p1jwc0yv20x1sa63bw5-source/maintainers
13M     /nix/.ro-store/5q1kw0svyl0j3p1jwc0yv20x1sa63bw5-source/nixos
109M    /nix/.ro-store/5q1kw0svyl0j3p1jwc0yv20x1sa63bw5-source/pkgs
1.3G    /nix/.ro-store/5q1kw0svyl0j3p1jwc0yv20x1sa63bw5-source/
1.3G    total

Will fix it
#204178

@Izorkin

Izorkin commented Dec 7, 2022

Copy link
Copy Markdown
Contributor Author

Resolve conflicts.

Comment thread nixos/modules/installer/cd-dvd/installation-cd-minimal.nix Outdated
@Izorkin

Izorkin commented Dec 9, 2022

Copy link
Copy Markdown
Contributor Author

Resolve conflicts.
Who can merge PR?

@Lassulus Lassulus merged commit 05c1c92 into NixOS:master Dec 9, 2022
@Izorkin

Izorkin commented Dec 9, 2022

Copy link
Copy Markdown
Contributor Author

Thanks!

@Izorkin Izorkin deleted the profiles-reduce branch December 9, 2022 10:18
@ncfavier

ncfavier commented Dec 9, 2022

Copy link
Copy Markdown
Member

Note that the discussion about disabling logrotate in LXC containers is still open.

@Izorkin

Izorkin commented Dec 9, 2022

Copy link
Copy Markdown
Contributor Author

Note that the discussion about disabling logrotate in LXC containers is still open.

I will try to test the lxc containers a little later.

@SuperSandro2000

Copy link
Copy Markdown
Member

Note that the discussion about disabling logrotate in LXC containers is still open.

We should fix this regression ASAP because it could fill up the disk completely if things go really south.

@Izorkin

Izorkin commented Dec 9, 2022

Copy link
Copy Markdown
Contributor Author

We should fix this regression ASAP because it could fill up the disk completely if things go really south.

Created PR - #205346

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: closure size The final size of a derivation, including its dependencies 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.