Skip to content

nixos/nix-daemon: Ensure continued availability of daemon socket#161059

Merged
dasJ merged 1 commit intoNixOS:masterfrom
hercules-ci:fix-nix-daemon-socket-availability
Feb 27, 2022
Merged

nixos/nix-daemon: Ensure continued availability of daemon socket#161059
dasJ merged 1 commit intoNixOS:masterfrom
hercules-ci:fix-nix-daemon-socket-availability

Conversation

@roberth
Copy link
Member

@roberth roberth commented Feb 20, 2022

Motivation for this change

This fixes the error messages

error: cannot connect to socket at '/nix/var/nix/daemon-socket/socket': Connection refused

and

error: opening a connection to remote store 'daemon' previously failed

in nix programs that are active during a NixOS switch.

As nix-daemon.service does not make use of ExecStop, we prefer to keep the socket up and available. This is important for machines that run Nix-based services, such as automated build, test, and deploy services, that expect the daemon socket to be available at all times.

See committed inline comment for further explanation.

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.05 Release Notes (or backporting 21.11 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.

cc @tomberek @dasJ

As `nix-daemon.service` does not make use of `ExecStop`, we prefer
to keep the socket up and available. This is important for machines
that run Nix-based services, such as automated build, test, and deploy
services, that expect the daemon socket to be available at all times.

See committed inline comment for further explanation.
@roberth roberth requested review from dasJ and tomberek February 20, 2022 12:49
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Feb 20, 2022
@roberth
Copy link
Member Author

roberth commented Feb 20, 2022

@ofborg test switchTest installer.simple

@roberth roberth marked this pull request as draft February 20, 2022 12:53
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Feb 20, 2022
@arianvp
Copy link
Member

arianvp commented Feb 20, 2022

Hmm I was under the assumption that the .socket unit would keep requests being buffered as the .service restarts. Are we accidentally restarting the .socket ? I know we had some bugs about that in the activation logic but I recall those were squashed recently.

@dasJ
Copy link
Member

dasJ commented Feb 20, 2022

@arianvp No, .socket triggers the .service unit and buffers until the .service unit is activated. We are not accidentially restarting sockets because that got reverted: https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/system/activation/switch-to-configuration.pl#L313

@roberth roberth marked this pull request as ready for review February 20, 2022 15:47
Copy link
Member

@dasJ dasJ left a comment

Choose a reason for hiding this comment

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

Looks good. I'll merge this in a few days when nobody has any strong point why I shouldn't

@dasJ
Copy link
Member

dasJ commented Feb 25, 2022

Can you confirm that the issue is also resolved by #161838?
Both PRs solve the same issue in different ways and are not conflicting with each other.
#161838 is preferrable IMO because it fixes the root cause of the problem for all services instead of just for nix-daemon.service

@github-actions
Copy link
Contributor

github-actions bot commented Mar 8, 2022

Successfully created backport PR #163323 for release-21.11.

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 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants