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/tests: fix switchTest #161859

Closed
wants to merge 2 commits into from
Closed

Conversation

ncfavier
Copy link
Member

Currently the test-watch.service gets started in a loop as long as
/testpath exists, so `rm /testpath /testpath-modified` runs into a race
condition where if the service was just getting activated, it will
create /testpath-modified and make the test fail.

This is fixed by making the service RemainAfterExit so that it only
starts once, and stopping it manually after we remove /testpath.
@ncfavier ncfavier requested a review from dasJ February 25, 2022 17:07
@github-actions github-actions bot added the 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS label Feb 25, 2022
@ncfavier
Copy link
Member Author

@ofborg test switchTest

@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 Feb 25, 2022
@dasJ
Copy link
Member

dasJ commented Feb 25, 2022

Is this also fixed by #161838 ?

@ncfavier
Copy link
Member Author

Nope, both issues still present.

@vcunat vcunat added the 1.severity: channel blocker Blocks a channel label Feb 25, 2022
@vcunat
Copy link
Member

vcunat commented Feb 25, 2022

Well, I'm still getting another problem even with this PR:

machine # [   55.922029] nixos[2645]: finished switching to system configuration /nix/store/ach2gr8ss9mxvkjy050qm7q77iqm53a3-nixos-system-machine-22.05.git.35ea9035c1d
(finished: must succeed: /nix/store/nd0g7xz7hwpl011s7kjzx69r1hym6hs4-nixos-system-machine-22.05.git.35ea9035c1d/specialisation/unitWithBackslashModified/bin/switch-to-configuration test 2>&1, in 4.32 seconds)
The haystack that will cause the following exception is:
---
stopping the following units: escaped\x2ddash.service
activating the configuration...
setting up /etc...
setting up tmpfiles
starting the following units: escaped\x2ddash.service
the following new units were started: logrotate.service
---
Test "services" failed with error: "Unexpected string 'the following new units were started:' was found"
cleanup
kill machine (pid 9)
machine # qemu-kvm: terminating on signal 15 from pid 6 (/nix/store/afi0ysqw20yiiw2gr2d28dx40bc4ddf8-python3-3.9.10/bin/python3.9)
(finished: cleanup, in 0.03 seconds)
kill vlan (pid 7)
kill vlan (pid 8)

EDIT: maybe it's just something on my side (even though repeated), as I see OfBorg succeeding and you also surely tested this.

@ncfavier
Copy link
Member Author

I only removed the first occurrence of assert_lacks(out, "the following new units were started:") because that seemed to make the test succeed on my end, but clearly this is an imperfect workaround. Maybe the easiest is just to disable the logrotate service in the entire test?

@martinetd
Copy link
Member

It illustrates a real problem with the service (in my opinion), let's fix logrotate :)

The following fixes it for me:

diff --git a/nixos/modules/services/logging/logrotate.nix b/nixos/modules/services/logging/logrotate.nix
index 77e4fc395981..082cf92ff4ef 100644
--- a/nixos/modules/services/logging/logrotate.nix
+++ b/nixos/modules/services/logging/logrotate.nix
@@ -167,7 +167,6 @@ in
 
     systemd.services.logrotate = {
       description = "Logrotate Service";
-      wantedBy = [ "multi-user.target" ];
       startAt = "hourly";
 
       serviceConfig = {

And logrotate.timer is still enabled, so the service will run every hour as expected... It can still start randomly during the test, but you won't get the "new units has been started" as it won't be started directly by the switch so I think test won't fail.

I'll submit this as a PR with a new test in nixosTests.logrotate to make sure the timer stays enabled, just need to figure out how to wait for the service to have run automatically after a date change...

@ncfavier
Copy link
Member Author

Closing in favour of #161929

@ncfavier ncfavier closed this Feb 26, 2022
@ncfavier ncfavier deleted the fix-switch-test branch March 13, 2022 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.severity: channel blocker Blocks a channel 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.

4 participants