Support more than 99 nodes in NixOS tests & export toHex and toBase #85635
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@GrahamcOfBorg test networking |
|
@GrahamcOfBorg test networking.dhcpSimple |
|
@GrahamcOfBorg test networking.networkd |
`toHex` converts the given positive integer to a string of the hexadecimal representation of that integer. For example: ``` toHex 0 => "0" toHex 16 => "10" toHex 250 => "FA" ``` `toBase base i` converts the positive integer `i` to a list of it digits in the given `base`. For example: ``` toBase 10 123 => [ 1 2 3 ] toBase 2 6 => [ 1 1 0 ] toBase 16 250 => [ 15 10 ] ```
This modifies the `router` to not give out a range of IP addresses but only give out a fixed address based on the MAC address using the `services.dhcpd4.machines` option. To get access to the MAC address the `qemuNicMac` function is defined and exported from `qemu-flags.nix`.
This makes the type of these functions more apparent from the name.
basvandijk
force-pushed
the
support-bigger-nixos-tests
branch
from
c12a9d9
to
6e7822b
Compare
|
@GrahamcOfBorg test networking.networkd |
|
@GrahamcOfBorg test networking.networkd.dhcpSimple |
|
Maybe it's better to assert that there aren't more than 99 nodes? If you have that many nodes you have a big problem, since it's going to be superslow and take a huge amount of memory. |
|
No. as Bas already noted this starts breaking at 10 nodes; not 100 without this fix. Adding an assertion |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation for this change
The current
zeroPadfunction, which determines the MAC address of each QEMU VM in NixOS tests, returns a MAC address component in decimal notation while it should return it in hexadecimal. This means that you get inconsistent MAC addresses once you have 10 nodes or more. Worse: when you have 100 or more nodes the MAC address will look like52:54:00:12:01:100which is invalid.Things done
This PR fixes the
zeroPadfunction to return a proper hexadecimal component which means we can now have up to 254 nodes.The implementation needed the
toHexandtoBasefunctions which have been added to theliband come with unit tests.Finally the
dhcpSimpleNixOS test from thenetworkingtest suite has been modified to give out fixed IP addresses (instead of a range) based on the MAC address.sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)