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

lib/test-driver: fix reading command output for other distros #382260

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

Conversation

am-on
Copy link
Contributor

@am-on am-on commented Feb 15, 2025

When running the test-driver on other distros (e.g., via https://github.com/numtide/nix-vm-test/) executing a command can fail if the driver receives both stdout and stderr.

The test-driver’s logic for reading output currently assumes it will only read base64 encoded stdout, so any extra stderr content leads to unexpected failures.

This commit fixes the issue by redirecting the stderr to /dev/null, ensuring the driver only sees stdout.

The tradeoff is losing stderr messages in the machine logs. Users can work around this by redirecting stderr to stdout in the command they are sending to the machine:

machine.execute("some_command 2>&1")

Related issues:

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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.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.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: testing Tooling for automated testing of packages and modules labels Feb 15, 2025
@nix-owners nix-owners bot requested a review from tfc February 15, 2025 08:30
@am-on am-on changed the title lib.test-driver: fix reading command output for other distros lib/test-driver: fix reading command output for other distros Feb 15, 2025
@am-on am-on force-pushed the fix/test-driver-other-distros branch 3 times, most recently from 8039b0f to 6e91025 Compare February 15, 2025 09:11
When running the test-driver on other distros (e.g., via
[https://github.com/numtide/nix-vm-test/](nix-vm-test)) executing a command
can fail if the driver receives both stdout and stderr.

The test-driver’s logic for reading output currently assumes it will only read
base64 encoded stdout, so any extra stderr content leads to unexpected
failures.

This commit fixes the issue by redirecting the stderr to /dev/null, ensuring
the driver only sees stdout.

The tradeoff is losing stderr messages in the machine logs. Users can work
around this by redirecting stderr to stdout in the command they are sending
to the machine:

```python
machine.execute("some_command 2>&1")
```

Related issues:
- numtide/nix-vm-test#84
- numtide/nix-vm-test#5
@am-on am-on force-pushed the fix/test-driver-other-distros branch from 6e91025 to d81d119 Compare February 15, 2025 09:14
@am-on
Copy link
Contributor Author

am-on commented Feb 15, 2025

Worked on this during Thaiger sprint: Thaigersprint/thaigersprint-2025#1

@tfc
Copy link
Contributor

tfc commented Feb 15, 2025

@ofborg test nat.firewall networking.scripted.link installer.simpleProvided installer.separateBootFat networking.scripted.virtual printing keymap.azerty keymap.dvorak-programmer boot-stage1 installer.swraid keymap.neo nfs4.simple i3wm udisks2 networking.networkd.loopback containers-ip ecryptfs login installer.simpleLabels php.httpd zfs.installer predictable-interface-names.unpredictable predictable-interface-names.unpredictableNetworkd mutableUsers

@tfc
Copy link
Contributor

tfc commented Feb 15, 2025

@ofborg test pantheon boot.biosCdrom boot.uefiUsb networking.networkd.static latestKernel.login networking.networkd.dhcpSimple networking.networkd.sit env switchTest hibernate predictable-interface-names.predictable gnome networking.networkd.bridge networking.networkd.privacy plasma5 systemd-networkd-ipv6-prefix-delegation lightdm installer.simple installer.btrfsSimple installer.btrfsSubvols sway networking.networkd.link networking.scripted.privacy

@tfc
Copy link
Contributor

tfc commented Feb 15, 2025

@ofborg test nfs3.simple php.fpm fontconfig-default-fonts networking.scripted.bond nano keymap.dvorak installer.luksroot networking.scripted.bridge docker simple chromium networking.scripted.dhcpOneIf networking.networkd.bond php.pcre nat.standalone openssh networking.scripted.vlan ipv6 installer.separateBoot misc predictable-interface-names.predictableNetworkd networking.scripted.loopback keymap.colemak installer.btrfsSubvolDefault

Copy link
Contributor

@jfly jfly left a comment

Choose a reason for hiding this comment

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

I still think the previous behavior is better. IMO, the real fix is to make sure we're always invoking qemu/using the serial console in the same way (namely, where stderr gets printed to qemu's output, rather than the socket the test driver reads).

@jfly
Copy link
Contributor

jfly commented Feb 15, 2025

Dug into this a bit with @Mic92's help:

I tried spinning up a nix-vm-test myself (following these instructions) and was unable to reproduce the issue you're hitting, so I'm a little confused. Would be nice to have a simple repro to poke at.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: testing Tooling for automated testing of packages and modules 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants