Skip to content

nixos/nordvpn: order nordvpnd after its socket - #580056

Merged
romildo merged 1 commit into
NixOS:masterfrom
novalkun:nordvpn-socket-requires
Oct 30, 2026
Merged

romildo merged 1 commit into
NixOS:masterfrom
novalkun:nordvpn-socket-requires

Conversation

@novalkun

Copy link
Copy Markdown
Contributor

Requires = "nordvpnd.socket" sat in serviceConfig, so it ended up in [Service], where systemd ignores it (Unknown key 'Requires' in section [Service], ignoring). Without a dependency on or ordering against its socket, and with wantedBy = [ "default.target" ], activation could start nordvpnd before nordvpnd.socket. The socket then refused to start (Socket service nordvpnd.service already active, refusing) and nixos-rebuild switch failed. This moves it to requires/after at the unit level.

Reproducer: systemctl stop nordvpnd.socket nordvpnd.service && systemctl start nordvpnd.service leaves the socket inactive before this change and active after it.

This touches the same module as #576413.

AI disclosure: the change, commit message and this description were drafted with Claude Code (Opus 5.5); I reviewed them and tested as below.

Things done

  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.
  • Follows the automation/AI policy.
  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • aarch64-darwin
  • Tested, as applicable:
    • Basic functionality of all binary files, usually in ./result/bin/.
    • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
    • Package tests at passthru.tests.
    • NixOS tests in nixos/tests.
    • Tests in lib/tests or pkgs/test for functions and "core" functionality.
    • Also tested on my machine: switch-to-configuration test to a system with this change, the reproducer above, and reverting.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.

@nixpkgs-ci nixpkgs-ci Bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 9.needs: reviewer This PR currently has no reviewers requested and needs attention. llm-assisted Contributions that include code generation with LLMs 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 Oct 28, 2026
@novalkun

Copy link
Copy Markdown
Contributor Author

@different-error could you take a look? This also explains the nordvpnd.socket failures after switching. Side note: the bot couldn’t request your review since you’re not in nixpkgs maintainer yet

@different-error

Copy link
Copy Markdown
Contributor

I think you can remove most of the description in your commit message. The commit's title contains sufficient information.

Remove:

`Requires = "nordvpnd.socket"` was set in serviceConfig, so it ended up in
the [Service] section, where systemd ignores it ("Unknown key 'Requires' in
section [Service], ignoring"). The service then had no dependency on or
ordering against its socket, and since it is also wanted by default.target,
switch-to-configuration could start nordvpnd before nordvpnd.socket. The
socket then refused to start ("Socket service nordvpnd.service already
active, refusing"), failing the activation.

Move it to the unit level as requires/after.

@novalkun
novalkun force-pushed the nordvpn-socket-requires branch from 3294c64 to e332112 Compare October 29, 2026 11:39
@novalkun

Copy link
Copy Markdown
Contributor Author

I think you can remove most of the description in your commit message. The commit's title contains sufficient information.

Remove:

`Requires = "nordvpnd.socket"` was set in serviceConfig, so it ended up in
the [Service] section, where systemd ignores it ("Unknown key 'Requires' in
section [Service], ignoring"). The service then had no dependency on or
ordering against its socket, and since it is also wanted by default.target,
switch-to-configuration could start nordvpnd before nordvpnd.socket. The
socket then refused to start ("Socket service nordvpnd.service already
active, refusing"), failing the activation.

Move it to the unit level as requires/after.

Done, thanks.

@different-error

Copy link
Copy Markdown
Contributor
Assisted-by: Claude Code (Claude Opus 5.5)

This appears as the more common pattern. Please replace.

@different-error

Copy link
Copy Markdown
Contributor
  • Verified correct coupling between nordvpnd and nordvpnd.socket: stopping/starting the former enforces the same for the latter.
  • Verified connection over all three protocols via cli and gui.

Ty.

Assisted-by: Claude Code (Claude Opus 5.5)
@novalkun
novalkun force-pushed the nordvpn-socket-requires branch from e332112 to 7100706 Compare October 29, 2026 12:52
@novalkun

Copy link
Copy Markdown
Contributor Author
  • Verified correct coupling between nordvpnd and nordvpnd.socket: stopping/starting the former enforces the same for the latter.

    • Verified connection over all three protocols via cli and gui.

Ty.

thanks. can it be approved?

@different-error different-error left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@nixpkgs-ci nixpkgs-ci Bot added 12.approvals: 1 This PR was reviewed and approved by one person. and removed 9.needs: reviewer This PR currently has no reviewers requested and needs attention. labels Oct 29, 2026
@romildo
romildo added this pull request to the merge queue Oct 30, 2026
Merged via the queue into NixOS:master with commit 3c404f1 Oct 30, 2026
29 checks passed
@novalkun
novalkun deleted the nordvpn-socket-requires branch October 30, 2026 01:04
@novalkun novalkun mentioned this pull request Oct 31, 2026
7 of 13 tasks
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. 12.approvals: 1 This PR was reviewed and approved by one person. llm-assisted Contributions that include code generation with LLMs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants