Merge pull request #167838 from tljuniper/test-driver-pipefail-fix

nixos/test-driver: shellopts with execute + timeout

nixos/doc/manual/development/writing-nixos-tests.section.md

···
 `execute`
 
 :   Execute a shell command, returning a list `(status, stdout)`.
+
+    Commands are run with `set -euo pipefail` set:
+
+    -   If several commands are separated by `;` and one fails, the
+        command as a whole will fail.
+
+    -   For pipelines, the last non-zero exit status will be returned
+        (if there is one; otherwise zero will be returned).
+
+    -   Dereferencing unset variables fails the command.
+
+    -   It will wait for stdout to be closed.
+
     If the command detaches, it must close stdout, as `execute` will wait
     for this to consume all output reliably. This can be achieved by
     redirecting stdout to stderr `>&2`, to `/dev/console`, `/dev/null` or
     a file. Examples of detaching commands are `sleep 365d &`, where the
     shell forks a new process that can write to stdout and `xclip -i`, where
     the `xclip` command itself forks without closing stdout.
+
     Takes an optional parameter `check_return` that defaults to `True`.
     Setting this parameter to `False` will not check for the return code
     and return -1 instead. This can be used for commands that shut down
     the VM and would therefore break the pipe that would be used for
     retrieving the return code.
 
+    A timeout for the command can be specified (in seconds) using the optional
+    `timeout` parameter, e.g., `execute(cmd, timeout=10)` or
+    `execute(cmd, timeout=None)`. The default is 900 seconds.
+
 `succeed`
 
 :   Execute a shell command, raising an exception if the exit status is
-    not zero, otherwise returning the standard output. Commands are run
-    with `set -euo pipefail` set:
-
-    -   If several commands are separated by `;` and one fails, the
-        command as a whole will fail.
-
-    -   For pipelines, the last non-zero exit status will be returned
-        (if there is one, zero will be returned otherwise).
-
-    -   Dereferencing unset variables fail the command.
-
-    -   It will wait for stdout to be closed. See `execute` for the
-        implications.
+    not zero, otherwise returning the standard output. Similar to `execute`,
+    except that the timeout is `None` by default. See `execute` for details on
+    command execution.
 
 `fail`
 
···
 `wait_until_succeeds`
 
 :   Repeat a shell command with 1-second intervals until it succeeds.
+    Has a default timeout of 900 seconds which can be modified, e.g.
+    `wait_until_succeeds(cmd, timeout=10)`. See `execute` for details on
+    command execution.
 
 `wait_until_fails`
 
-:   Repeat a shell command with 1-second intervals until it fails.
+:   Like `wait_until_succeeds`, but repeating the command until it fails.
 
 `wait_for_unit`
 

nixos/doc/manual/from_md/development/writing-nixos-tests.section.xml

···
         <listitem>
           <para>
             Execute a shell command, returning a list
-            <literal>(status, stdout)</literal>. If the command
-            detaches, it must close stdout, as
-            <literal>execute</literal> will wait for this to consume all
-            output reliably. This can be achieved by redirecting stdout
-            to stderr <literal>&gt;&amp;2</literal>, to
-            <literal>/dev/console</literal>,
-            <literal>/dev/null</literal> or a file. Examples of
-            detaching commands are <literal>sleep 365d &amp;</literal>,
-            where the shell forks a new process that can write to stdout
-            and <literal>xclip -i</literal>, where the
-            <literal>xclip</literal> command itself forks without
-            closing stdout. Takes an optional parameter
-            <literal>check_return</literal> that defaults to
-            <literal>True</literal>. Setting this parameter to
-            <literal>False</literal> will not check for the return code
-            and return -1 instead. This can be used for commands that
-            shut down the VM and would therefore break the pipe that
-            would be used for retrieving the return code.
+            <literal>(status, stdout)</literal>.
           </para>
-        </listitem>
-      </varlistentry>
-      <varlistentry>
-        <term>
-          <literal>succeed</literal>
-        </term>
-        <listitem>
           <para>
-            Execute a shell command, raising an exception if the exit
-            status is not zero, otherwise returning the standard output.
             Commands are run with <literal>set -euo pipefail</literal>
             set:
           </para>
···
             <listitem>
               <para>
                 For pipelines, the last non-zero exit status will be
-                returned (if there is one, zero will be returned
-                otherwise).
+                returned (if there is one; otherwise zero will be
+                returned).
               </para>
             </listitem>
             <listitem>
               <para>
-                Dereferencing unset variables fail the command.
+                Dereferencing unset variables fails the command.
               </para>
             </listitem>
             <listitem>
               <para>
-                It will wait for stdout to be closed. See
-                <literal>execute</literal> for the implications.
+                It will wait for stdout to be closed.
               </para>
             </listitem>
           </itemizedlist>
+          <para>
+            If the command detaches, it must close stdout, as
+            <literal>execute</literal> will wait for this to consume all
+            output reliably. This can be achieved by redirecting stdout
+            to stderr <literal>&gt;&amp;2</literal>, to
+            <literal>/dev/console</literal>,
+            <literal>/dev/null</literal> or a file. Examples of
+            detaching commands are <literal>sleep 365d &amp;</literal>,
+            where the shell forks a new process that can write to stdout
+            and <literal>xclip -i</literal>, where the
+            <literal>xclip</literal> command itself forks without
+            closing stdout.
+          </para>
+          <para>
+            Takes an optional parameter <literal>check_return</literal>
+            that defaults to <literal>True</literal>. Setting this
+            parameter to <literal>False</literal> will not check for the
+            return code and return -1 instead. This can be used for
+            commands that shut down the VM and would therefore break the
+            pipe that would be used for retrieving the return code.
+          </para>
+          <para>
+            A timeout for the command can be specified (in seconds)
+            using the optional <literal>timeout</literal> parameter,
+            e.g., <literal>execute(cmd, timeout=10)</literal> or
+            <literal>execute(cmd, timeout=None)</literal>. The default
+            is 900 seconds.
+          </para>
+        </listitem>
+      </varlistentry>
+      <varlistentry>
+        <term>
+          <literal>succeed</literal>
+        </term>
+        <listitem>
+          <para>
+            Execute a shell command, raising an exception if the exit
+            status is not zero, otherwise returning the standard output.
+            Similar to <literal>execute</literal>, except that the
+            timeout is <literal>None</literal> by default. See
+            <literal>execute</literal> for details on command execution.
+          </para>
         </listitem>
       </varlistentry>
       <varlistentry>
···
         <listitem>
           <para>
             Repeat a shell command with 1-second intervals until it
-            succeeds.
+            succeeds. Has a default timeout of 900 seconds which can be
+            modified, e.g.
+            <literal>wait_until_succeeds(cmd, timeout=10)</literal>. See
+            <literal>execute</literal> for details on command execution.
           </para>
         </listitem>
       </varlistentry>
···
         </term>
         <listitem>
           <para>
-            Repeat a shell command with 1-second intervals until it
-            fails.
+            Like <literal>wait_until_succeeds</literal>, but repeating
+            the command until it fails.
           </para>
         </listitem>
       </varlistentry>

nixos/lib/test-driver/test_driver/machine.py

···
         self.run_callbacks()
         self.connect()
 
+        # Always run command with shell opts
+        command = f"set -euo pipefail; {command}"
+
+        timeout_str = ""
         if timeout is not None:
-            command = "timeout {} sh -c {}".format(timeout, shlex.quote(command))
+            timeout_str = f"timeout {timeout}"
+
+        out_command = (
+            f"{timeout_str} sh -c {shlex.quote(command)} | (base64 --wrap 0; echo)\n"
+        )
 
-        out_command = f"( set -euo pipefail; {command} ) | (base64 --wrap 0; echo)\n"
         assert self.shell
         self.shell.send(out_command.encode())