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

nixosTests.nscd: init, remove nixosTests.resolv #194916

Merged
merged 3 commits into from
Oct 14, 2022

Conversation

flokli
Copy link
Contributor

@flokli flokli commented Oct 7, 2022

nixosTests.systemd is quite heavy, it requires a full graphical system, which is quite a big of a rebuild if the only thing you want to test is whether dynamic users work.

This is now moved to an nscd test, which tests various NSS lookups, making extra sure that the nscd path is tested, not the fallback path (by hiding /etc/nsswitch.conf and /etc/hosts for getent).

nixosTests.resolv is removed. It didn't check for reverse lookups, didn't catch nscd breaking halfway in between, and also had an unabiguous reverse lookup - 192.0.2.1 could either reverse lookup to host-ipv4.example.net, or host-dual.example.net.

This also tests unscd, at least the parts that are known to work (see #193535).

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 22.11 Release Notes (or backporting 22.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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@flokli flokli marked this pull request as ready for review October 7, 2022 10:58
@github-actions github-actions bot added the 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS label Oct 7, 2022
@flokli flokli requested a review from erikarvstedt October 7, 2022 11:01
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Oct 7, 2022
nixosTests.systemd is quite heavy, it requires a full graphical system,
which is quite a big of a rebuild if the only thing you want to test is
whether dynamic users work.

This is now moved to an `nscd` test, which tests various NSS lookups,
making extra sure that the nscd path is tested, not the fallback path
(by hiding /etc/nsswitch.conf and /etc/hosts for getent).

nixosTests.resolv is removed. It didn't check for reverse lookups,
didn't catch nscd breaking halfway in between, and also had an
ambiguous reverse lookup - 192.0.2.1 could either reverse lookup to
host-ipv4.example.net, or host-dual.example.net.
@github-actions github-actions bot added the 8.has: module (update) This PR changes an existing module in `nixos/` label Oct 10, 2022
@ofborg ofborg bot added 10.rebuild-linux: 1-10 and removed 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Oct 10, 2022
@github-actions github-actions bot removed the 8.has: module (update) This PR changes an existing module in `nixos/` label Oct 10, 2022
@ofborg ofborg bot added 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux and removed 10.rebuild-linux: 1-10 labels Oct 10, 2022
@flokli flokli requested a review from ctheune October 13, 2022 20:19
This shows that external nss module resolution is broken with unscd.
@picnoir
Copy link
Member

picnoir commented Oct 14, 2022

@GrahamcOfBorg test nscd

@picnoir
Copy link
Member

picnoir commented Oct 14, 2022

Hmm, the _gateway hostname resolution seems to fail on aarch64.

Could it be related to STRCASE_IN_SET https://github.com/systemd/systemd/blob/0da2bb7414a53957287aacc5811fc7c52c13b730/src/basic/hostname-util.h#L55 ?

[Edit]: it the getent call does not fail on a aarch64 host but does fail in a aarch64 VM running on that very same host. 🤔

Copy link
Member

@picnoir picnoir left a comment

Choose a reason for hiding this comment

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

We have an issue with AARCH64 don't merge yet.

@picnoir
Copy link
Member

picnoir commented Oct 14, 2022

@GrahamcOfBorg test nscd

@flokli
Copy link
Contributor Author

flokli commented Oct 14, 2022

Apparently, the getent' ahosts _gateway fails, but only when run via the non-interactive test runner, from the test script, and only on aarch64. 🤔

It's especially confusing because the other requests before, including those for dynamic user support do work, and because it also works fine when querying manually via driverInteractive.

@picnoir
Copy link
Member

picnoir commented Oct 14, 2022

Okay, it's actually flaky, not always failing.

@GrahamcOfBorg test nscd

@flokli
Copy link
Contributor Author

flokli commented Oct 14, 2022

Running this multiple times exposed it seems to be some flakyness in that lookup call, and it might not be limited to aarch64.

This has shown to be flaky in the VM test, at least when running on
the aarch64 ofborg builder(s).

I assume it's some flakyness in systemd-networkd not being fully up, or
at least not up to the point that it properly replies to the _gateway
request.

This part of the test is supposed to test external (non-glibc) nss
module lookup for the host database works, which is already sufficiently
covered in the previous checks (for *.localhost). Drop these redundant
checks. We're not integration-testing networkd here.
@picnoir
Copy link
Member

picnoir commented Oct 14, 2022

@GrahamcOfBorg test nscd

@picnoir picnoir merged commit 8e3b02d into NixOS:master Oct 14, 2022
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 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants