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 Integration Tests on macOS: Remove some obstacles (ulimit, requiredSystemFeatures) #261694

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions nixos/lib/testing/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ let
./legacy.nix
./meta.nix
./name.nix
./macos-host.nix
./network.nix
./nodes.nix
./pkgs.nix
Expand Down
19 changes: 19 additions & 0 deletions nixos/lib/testing/macos-host.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{ config, lib, hostPkgs, ... }:

{
config = lib.mkIf hostPkgs.stdenv.hostPlatform.isDarwin {
defaults = {
virtualisation.host.pkgs = hostPkgs;
Copy link
Member

Choose a reason for hiding this comment

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

This line doesn't seem darwin-specific.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so what do you think is the best way to do it? use !...isLinux ?

Copy link
Member

Choose a reason for hiding this comment

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

I think this could be set in this module, here

virtualisation.qemu.package = testModuleArgs.config.qemu.package;

I can't think of a situation where host.pkgs and hostPkgs should not have the same value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure how you mean that... use hostPkgs instead of ...config.qemu.package? I guess you mean that the default value for that should be hostpkgs but then in the file where this is defined?

Copy link
Member

Choose a reason for hiding this comment

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

I meant it like

 virtualisation.qemu.package = testModuleArgs.config.qemu.package;
+virtualisation.host.pkgs = hostPkgs;

ie move the line away from here

Suggested change
virtualisation.host.pkgs = hostPkgs;

and into lib/testing/nodes.nix.

The virtualisation.qemu.package line is unchanged and unrelated.


# On non-darwin, mounting the whole nix store from the host is the default because
# this generally makes rebuilds of the VMs faster and the VMs smaller, while reducing
# disk I/O.
# However, on macOS it currently leads to many crashing guest processes.
virtualisation.useNixStoreImage = true;
# Without this, the store image uses "raw" instead of "qcow2" and
# that throws some errors on macOS on startup of the VM
# TODO: remove when `raw` errors are fixed.
virtualisation.writableStore = true;
tfc marked this conversation as resolved.
Show resolved Hide resolved
};
};
}
4 changes: 3 additions & 1 deletion nixos/lib/testing/run.nix
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ in
derivation = hostPkgs.stdenv.mkDerivation {
name = "vm-test-run-${config.name}";

requiredSystemFeatures = [ "kvm" "nixos-test" ];
requiredSystemFeatures = [ "nixos-test" ]
++ lib.optionals hostPkgs.stdenv.hostPlatform.isLinux [ "kvm" ]
++ lib.optionals hostPkgs.stdenv.hostPlatform.isDarwin [ "apple-virt" ];

buildCommand = ''
mkdir -p $out
Expand Down
4 changes: 4 additions & 0 deletions nixos/modules/virtualisation/qemu-vm.nix
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,10 @@ let
idx=$((idx + 1))
'')}

${lib.optionalString (hostPkgs.stdenv.hostPlatform.isDarwin && !config.virtualisation.useNixStoreImage) ''
ulimit -n 12288
''}

# Start QEMU.
exec ${qemu-common.qemuBinary qemu} \
-name ${config.system.name} \
Expand Down
14 changes: 10 additions & 4 deletions pkgs/build-support/testers/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -97,17 +97,23 @@
# See doc/builders/testers.chapter.md or
# https://nixos.org/manual/nixpkgs/unstable/#tester-runNixOSTest
runNixOSTest =
let nixos = import ../../../nixos/lib {
inherit lib;
};
let
toGuest = builtins.replaceStrings [ "darwin" ] [ "linux" ];
guestPkgs =
if (!pkgs.stdenv.hostPlatform.isDarwin)
then pkgs
else import pkgs.path { system = toGuest pkgs.system; };
Comment on lines +102 to +105
Copy link
Member

Choose a reason for hiding this comment

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

Would be nice to have pkgsLinux, like we have pkgsStatic or pkgsCross.<x> (although this wouldn't be cross; just a related platform that's built natively)

Copy link
Member

Choose a reason for hiding this comment

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

Or pkgsNativePlatform.linux or something, with native referring both to not being cross and being able to run on the same hardware that runs pkgs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True. Although this would blow up this PR... can we discuss this offline after this PR how to improve it?

nixos = import ../../../nixos/lib {
inherit lib;
};
in testModule:
nixos.runTest {
_file = "pkgs.runNixOSTest implementation";
imports = [
(lib.setDefaultModuleLocation "the argument that was passed to pkgs.runNixOSTest" testModule)
];
hostPkgs = pkgs;
node.pkgs = pkgs;
node.pkgs = guestPkgs;
roberth marked this conversation as resolved.
Show resolved Hide resolved
};

# See doc/builders/testers.chapter.md or
Expand Down
Loading