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.

Sign up for GitHub

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

Jump to bottom

nixos/tests: add predictable-interface-names.nix #34305

Merged
merged 4 commits into from Feb 9, 2018
Merged

nixos/tests: add predictable-interface-names.nix #34305

merged 4 commits into from Feb 9, 2018

Conversation

symphorien
Copy link
Member

Motivation for this change

#33738

Things done

I tested that the revision incriminated in the issue makes the test fail, and that the test succeeds on current revision.

I think this test should be added to "release critical tests" as well, but I don't know how to do it (if it is in
nixpkgs).

  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@fpletz
Copy link
Member

fpletz commented Jan 26, 2018

Should we also test this with networkd enabled?

@symphorien
Copy link
Member Author

Honestly, no idea. But if someone thinks it is a good idea I can add it.

@symphorien
Copy link
Member Author

Well i pushed a commit to test with networkd as well

@adisbladis
Copy link
Member

Thanks for this! I would like this test added to nixos/release-small.nix.
What do you think @fpletz?

@fpletz
Copy link
Member

fpletz commented Jan 28, 2018

@adisbladis Oh, yeah, totally!

@symphorien
Copy link
Member Author

Here is my attempt at adding this test to release-small.nix as suggested.
My file evaluates as an attrset whose values are the actual tests like the chromium test.
Yet, when I try to build release-small.nix I get this error:

nix-build --no-out-link nixos/release-small.nix --show-trace
error: while evaluating ‘hydraJob’ at /home/symphorien/src/nixpkgs/lib/customisation.nix:167:14, called from /home/symphorien/src/nixpkgs/nixos/release-small.nix:81:12:
while evaluating the attribute ‘drvPath’ at /home/symphorien/src/nixpkgs/lib/customisation.nix:184:13:
while evaluating the attribute ‘drvPath’ at /home/symphorien/src/nixpkgs/lib/customisation.nix:146:13:
while evaluating the attribute ‘constituents’ of the derivation ‘nixos-18.03pre56789.gfedcba’ at /home/symphorien/src/nixpkgs/pkgs/stdenv/generic/make-derivation.nix:148:11:
while evaluating ‘collect’ at /home/symphorien/src/nixpkgs/lib/attrsets.nix:169:19, called from /home/symphorien/src/nixpkgs/nixos/release-small.nix:92:10:
while evaluating ‘concatMap’ at /home/symphorien/src/nixpkgs/lib/lists.nix:102:18, called from /home/symphorien/src/nixpkgs/lib/attrsets.nix:173:7:
while evaluating ‘collect’ at /home/symphorien/src/nixpkgs/lib/attrsets.nix:169:19, called from undefined position:
while evaluating ‘concatMap’ at /home/symphorien/src/nixpkgs/lib/lists.nix:102:18, called from /home/symphorien/src/nixpkgs/lib/attrsets.nix:173:7:
while evaluating ‘collect’ at /home/symphorien/src/nixpkgs/lib/attrsets.nix:169:19, called from undefined position:
while evaluating ‘concatMap’ at /home/symphorien/src/nixpkgs/lib/lists.nix:102:18, called from /home/symphorien/src/nixpkgs/lib/attrsets.nix:173:7:
while evaluating ‘collect’ at /home/symphorien/src/nixpkgs/lib/attrsets.nix:169:19, called from undefined position:
while evaluating ‘isDerivation’ at /home/symphorien/src/nixpkgs/lib/attrsets.nix:295:18, called from /home/symphorien/src/nixpkgs/lib/attrsets.nix:170:8:
while evaluating anonymous function at /home/symphorien/src/nixpkgs/nixos/release.nix:19:72, called from /home/symphorien/src/nixpkgs/lib/attrsets.nix:282:43:
while evaluating ‘hydraJob’ at /home/symphorien/src/nixpkgs/lib/customisation.nix:167:14, called from /home/symphorien/src/nixpkgs/nixos/release.nix:19:80:
while evaluating the attribute ‘meta’ at /home/symphorien/src/nixpkgs/lib/customisation.nix:172:24:
attribute ‘meta’ missing, at /home/symphorien/src/nixpkgs/lib/customisation.nix:172:10

I have no idea how to fix this so some help would be greatly appreciated :)

adisbladis
adisbladis reviewed Jan 29, 2018
View reviewed changes
nixos/tests/predictable-interface-names.nix Outdated
let boolToString = x: if x then "yes" else "no"; in
let testWhenSetTo = predictable: withNetworkd:
makeTest {
name = "predictableInterfaceNames-${boolToString predictable}-with-networkd-${boolToString withNetworkd}";
Copy link
Member

Choose a reason for hiding this comment

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

I would like these attribute names to be more descriptive.
Instead of yes and no I think we should do something along these lines:

"predictableInterfaceNames-${lib.optionalString (!predictable) "un"}predictable${lib.optionalString withNetworkd "-with-networkd"}"

@adisbladis
Copy link
Member

adisbladis commented Jan 29, 2018 •
edited

@symphorien It was just a matter of replacing callTest with callSubTests.
Fixed up the test, squashed it with your wip commit and removed the wip label.

In my mind this one is good to go as soon as my comment about the drv/attribute names is resolved.

I do want one of the release managers to merge this as we touch release-critical infrastructure.

@symphorien
Copy link
Member Author

I tried to address @adisbladis 's request to change the name.
I get a strange failure when I do so:

starting VDE switch for network 1
running the VM test script
machine: must succeed: ip link
machine: waiting for the VM to finish booting
machine: starting vm
machine# Formatting '/tmp/nix-build-vm-test-run-predictableInterfaceNames-predictable-with-networkd.drv-0/vm-state-machine/machine.qcow2', fmt=qcow2 size=536870912 cluster_size=65536 lazy_refcounts=6
machine# qemu-system-x86_64: -usbdevice tablet: '-usbdevice' is deprecated, please use '-device usb-...' instead
machine# qemu-system-x86_64: -monitor unix:./monitor: Failed to connect socket ./monitor: No such file or directory
error: Can't use an undefined value as a symbol reference at /nix/store/22rg6kabcnqjadb6ajfdxqk8sq415dq3-nixos-test-driver/lib/perl5/site_perl/Machine.pm line 275.
Can't use an undefined value as a symbol reference at /nix/store/22rg6kabcnqjadb6ajfdxqk8sq415dq3-nixos-test-driver/lib/perl5/site_perl/Machine.pm line 275.

This behavior depends on the name of the derivation and is deterministic (I already had encountered such behavior when I was using "true" and "false" instead of "yes" and "no").
I know nothing about perl, and I couldn't find anything meaningful and close to @adisbladis suggestion
which builds sucessfully so once again I hope someone can help :)

@symphorien
Copy link
Member Author

since this is independent from predictable interface names, I filed an issue: #34393

@symphorien
Copy link
Member Author

@adisbladis I changed from your suggestion to "${if predictable then "" else "un"}predictableInterfaceNames${if withNetworkd then "-with-networkd" else ""}"
since apparently a shorter name was needed.
Now it builds :)

@adisbladis
Copy link
Member

adisbladis commented Feb 1, 2018 •
edited

Feeling pretty good about this now. 👍
@fpletz Can you take a look?

@adisbladis
Copy link
Member

ping @fpletz @globin

@fpletz fpletz merged commit 0146074 into NixOS:master Feb 9, 2018
@fpletz
Copy link
Member

fpletz commented Feb 9, 2018

Thanks!

@symphorien symphorien deleted the predictableifacetest branch May 18, 2019 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adisbladis adisbladis adisbladis approved these changes

@edolstra edolstra Awaiting requested review from edolstra

@nbp nbp Awaiting requested review from nbp

Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@symphorien @fpletz @adisbladis @GrahamcOfBorg