(nixos/system/boot): make stage-2 respect boot.initrd.verbose flag

[mainrs]

Fixes #32555

Motivation for this change

This should remove any output of the stage 2 boot script, making boots silent. A lot of people have asked for silent boots for some time now.

Things that need to be done

I think there still needs to be a boot.verbose flag added to stage-2.nix, similar to how it's done in stage-1.nix. Maybe it's a good idea to re-use the value of boot.initrd.verbose? I left this change out to get some feedback!

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:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• 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.

[Synthetica9]

Would be nice if this had a NixOS test

[mainrs]

I think there still needs to be a boot.verbose flag added to stage-2.nix, similar to how it's done in stage-1.nix. Maybe it's a good idea to re-use the value of boot.initrd.verbose? I left this change out to get some feedback!

[mainrs]

Would be nice if this had a NixOS test

I’ve taken a look at the boot tests but none of them seems to somehow check for the output of the script? They all seem to spin up some machine and check if it booted correctly. Is there documentation for the test framework?

Maybe someone more experienced can chime in and write a test instead of me having to tinker around for two hours to make it work :)

[Synthetica9]

There is definitely documentation, but I found that the best way to make something work is to just poke around.

I'll see if I can build something.

[Synthetica9]

Okay that was a bit more difficult than I thought and required some changes to the NixOS test driver 😅. I'll throw that into it's own PR. Here are the diffs, couldn't push to your branch. (had to rebase because the requires #146905 )

commit 90d6e9a986938fc91eb11e84dce4e971d5e8306b
Author: Patrick Hilhorst <git@hilhorst.be>
Date:   Wed Jan 19 22:31:48 2022 +0100

    nixosTests.silent-boot: init

diff --git a/nixos/tests/all-tests.nix b/nixos/tests/all-tests.nix
index b2f223e7ccd..6d7b875aef0 100644
--- a/nixos/tests/all-tests.nix
+++ b/nixos/tests/all-tests.nix
@@ -433,6 +433,7 @@ in
   shattered-pixel-dungeon = handleTest ./shattered-pixel-dungeon.nix {};
   shiori = handleTest ./shiori.nix {};
   signal-desktop = handleTest ./signal-desktop.nix {};
+  silent-boot = handleTest ./silent-boot.nix {};
   simple = handleTest ./simple.nix {};
   slurm = handleTest ./slurm.nix {};
   smokeping = handleTest ./smokeping.nix {};
diff --git a/nixos/tests/silent-boot.nix b/nixos/tests/silent-boot.nix
new file mode 100644
index 00000000000..c8121d936ac
--- /dev/null
+++ b/nixos/tests/silent-boot.nix
@@ -0,0 +1,23 @@
+import ./make-test-python.nix ({ pkgs, lib, ... }: {
+  name = "silent-boot";
+
+  meta.maintainers = with lib.maintainers; [ synthetica ];
+
+  machine = { ... }: {
+    boot.initrd.verbose = false;
+  };
+
+  testScript = ''
+    @polling_condition
+    def multi_user_reached():
+        return not (
+            machine.connected and
+            machine.get_unit_info("multi-user.target")["ActiveState"] == "active"
+        )
+
+
+    with must_raise("multi_user_reached"), multi_user_reached:
+        start_all()
+        machine.wait_for_console_text("\<\<\< NixOS Stage 2 \>\>\>")
+  '';
+})

commit 9431fcdd838bfb9753a3adce5e128c9173ed0ff8
Author: Patrick Hilhorst <git@hilhorst.be>
Date:   Wed Jan 19 22:23:01 2022 +0100

    nixos/boot/stage-2: inherit verbose flag from intrd

diff --git a/nixos/modules/system/boot/stage-2.nix b/nixos/modules/system/boot/stage-2.nix
index f6b6a8e4b0b..94881cce085 100644
--- a/nixos/modules/system/boot/stage-2.nix
+++ b/nixos/modules/system/boot/stage-2.nix
@@ -11,6 +11,7 @@ let
     shellDebug = "${pkgs.bashInteractive}/bin/bash";
     shell = "${pkgs.bash}/bin/bash";
     inherit (config.boot) systemdExecutable extraSystemdUnitPaths;
+    inherit (config.boot.initrd) verbose;
     isExecutable = true;
     inherit (config.nix) readOnlyStore;
     inherit useHostResolvConf;

commit eb4d81cbee50889f21b2f718784f822db1461c92
Author: Patrick Hilhorst <git@hilhorst.be>
Date:   Wed Jan 19 22:14:46 2022 +0100

    nixos/test-driver: add and export Driver.must_raise

diff --git a/nixos/lib/test-driver/test_driver/driver.py b/nixos/lib/test-driver/test_driver/driver.py
index 49a42fe5fb4..cb5b4883fc4 100644
--- a/nixos/lib/test-driver/test_driver/driver.py
+++ b/nixos/lib/test-driver/test_driver/driver.py
@@ -2,6 +2,7 @@ from contextlib import contextmanager
 from pathlib import Path
 from typing import Any, Dict, Iterator, List, Union, Optional, Callable, ContextManager
 import os
+import re
 import tempfile
 
 from test_driver.logger import rootlog
@@ -10,6 +11,20 @@ from test_driver.vlan import VLan
 from test_driver.polling_condition import PollingCondition
 
 
+@contextmanager
+def must_raise(
+    regex: str,
+    exception: type = Exception,
+) -> Iterator[None]:
+    with rootlog.nested(f"Waiting for exception {regex!r}"):
+        try:
+            yield
+        except exception as e:  # type: ignore
+            if re.search(regex, str(e)):
+                return
+        raise Exception(f"Excpected exception {regex!r}")
+
+
 class Driver:
     """A handle to the driver that sets up the environment
     and runs the tests"""
@@ -91,6 +106,7 @@ class Driver:
             serial_stdout_on=self.serial_stdout_on,
             polling_condition=self.polling_condition,
             Machine=Machine,  # for typing
+            must_raise=must_raise,
         )
         machine_symbols = {m.name: m for m in self.machines}
         # If there's exactly one machine, make it available under the name

commit 1d790b39691e8a4d78be8adacd2061cf0f80bca2
Author: Patrick Hilhorst <git@hilhorst.be>
Date:   Wed Jan 19 22:14:04 2022 +0100

    nixos/test-driver: return match from Machine.wait_for_console_text

diff --git a/nixos/lib/test-driver/test_driver/machine.py b/nixos/lib/test-driver/test_driver/machine.py
index e8893137792..aa7589e6813 100644
--- a/nixos/lib/test-driver/test_driver/machine.py
+++ b/nixos/lib/test-driver/test_driver/machine.py
@@ -805,7 +805,7 @@ class Machine:
         with self.nested("waiting for {} to appear on screen".format(regex)):
             retry(screen_matches)
 
-    def wait_for_console_text(self, regex: str) -> None:
+    def wait_for_console_text(self, regex: str) -> re.Match[str]:
         with self.nested("waiting for {} to appear on console".format(regex)):
             # Buffer the console output, this is needed
             # to match multiline regexes.
@@ -820,7 +820,7 @@ class Machine:
                 console.seek(0)
                 matches = re.search(regex, console.read())
                 if matches is not None:
-                    return
+                    return matches
 
     def send_key(self, key: str) -> None:
         key = CHAR_TO_KEY.get(key, key)

commit b74bac5e18fef9923d597eb86faa2b348d111594
Author: Patrick Hilhorst <git@hilhorst.be>
Date:   Wed Jan 19 22:13:12 2022 +0100

    nixos/test-driver: run callbacks during Machine.wait_for_console_text

diff --git a/nixos/lib/test-driver/test_driver/machine.py b/nixos/lib/test-driver/test_driver/machine.py
index e050cbd7d99..e8893137792 100644
--- a/nixos/lib/test-driver/test_driver/machine.py
+++ b/nixos/lib/test-driver/test_driver/machine.py
@@ -812,8 +812,9 @@ class Machine:
             console = io.StringIO()
             while True:
                 try:
-                    console.write(self.last_lines.get())
+                    console.write(self.last_lines.get(timeout=1))
                 except queue.Empty:
+                    self.run_callbacks()
                     self.sleep(1)
                     continue
                 console.seek(0)

commit be05619c4458dfde9c3d090f63c6afe66dc086b1
Author: mainrs <5113257+mainrs@users.noreply.github.com>
Date:   Wed Jan 19 13:24:32 2022 +0100

    (nixos/system/boot): make stage-2 respect boot.initrd.verbose flag
    
    Fixes #32555

diff --git a/nixos/modules/system/boot/stage-2-init.sh b/nixos/modules/system/boot/stage-2-init.sh
index a90f58042d2..bbc50e64dda 100755
--- a/nixos/modules/system/boot/stage-2-init.sh
+++ b/nixos/modules/system/boot/stage-2-init.sh
@@ -1,9 +1,15 @@
 #! @shell@
 
 systemConfig=@systemConfig@
+verbose="@verbose@"
 
 export HOME=/root PATH="@path@"
 
+info() {
+    if [[ -n "$verbose" ]]; then
+        echo -e "$@"
+    fi
+}
 
 # Process the kernel command line.
 for o in $(</proc/cmdline); do
@@ -21,9 +27,9 @@ done
 
 
 # Print a greeting.
-echo
-echo -e "\e[1;32m<<< NixOS Stage 2 >>>\e[0m"
-echo
+info ""
+info "\e[1;32m<<< NixOS Stage 2 >>>\e[0m"
+info ""
 
 
 # Normally, stage 1 mounts the root filesystem read/writable.
@@ -103,7 +109,7 @@ ln -s /run/lock /var/lock
 
 # Clear the resume device.
 if test -n "$resumeDevice"; then
-    mkswap "$resumeDevice" || echo 'Failed to clear saved image.'
+    mkswap "$resumeDevice" || info 'Failed to clear saved image.'
 fi
 
 
@@ -129,7 +135,7 @@ fi
 
 # Run the script that performs all configuration activation that does
 # not have to be done at boot time.
-echo "running activation script..."
+info "running activation script..."
 $systemConfig/activate
 
 
@@ -168,7 +174,7 @@ exec {logOutFd}>&- {logErrFd}>&-
 
 
 # Start systemd.
-echo "starting systemd..."
+info "starting systemd..."
 
 PATH=/run/current-system/systemd/lib/systemd:@fsPackagesPath@ \
     LOCALE_ARCHIVE=/run/current-system/sw/lib/locale/locale-archive @systemdUnitPathEnvVar@ \

[mainrs]

@Synthetica9

Good that I didn't start. Pretty sure that would have ended in a dead end for me :)
Is it OK to use the same configuration (boot.initrd.verbose) for stage 2? Technically, stage 2 doesn't happen in initrd. But I thought that there are probably no cases where one wants verbosity in only one of them.

[Synthetica9]

Good that I didn't start. Pretty sure that would have ended in a dead end for me :)

Haha yeah, probably, but I guess it's good to identify cases where the actual driver is lacking like this, because the test itself isn't super complicated now.

Is it OK to use the same configuration (boot.initrd.verbose) for stage 2? Technically, stage 2 doesn't happen in initrd. But I thought that there are probably no cases where one wants verbosity in only one of them.

Neither can I, but I'm not super deep into the boot process. Maybe someone like @grahamc knows what's appropriate here?

[mainrs]

Blocked on #155730 (afaik)

[Synthetica9]

Yes, but only for the tests. In the mean time I'm still not sure if we should re-use the boot.initrd.verbose flag or make a new flag (or have a `boot.verbose` flag which sets the default for initrd and stage2?)

…

On Sat, 22 Jan 2022, 10:31 mainrs, ***@***.***> wrote: Blocked on #155730 <#155730> (afaik) — Reply to this email directly, view it on GitHub <#155622 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABV7PJ3NWM5MELVC3XD7M2DUXJ2ONANCNFSM5MJZXJ7Q> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[dasJ]

I'd go for a new flag because it seems weird to be able to enable non-initrd logging using an initrd option

[arbv]

not stale

[RaitoBezarius]

Is there something I can do to help so this PR's contents can be merged?

[aacebedo]

Any way to get this merged?

[RaitoBezarius]

Any way to get this merged?

I suggest taking those changes and redo the PR if you are in a hurry of seeing this merged.

[pyrotek45]

any day now.....

[qweered]

any progress..

[emilazy]

Like #209173 (comment), this pull request is unmergeable: the fork was deleted but the PR somehow remains open. (It seems the author later created another fork of Nixpkgs, but it does not contain this branch, and the same symptoms as in the comment I linked to persist.)