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

switch-to-configuration: Better handling of socket-activated units #359724

Merged

Conversation

antifuchs
Copy link
Contributor

@antifuchs antifuchs commented Nov 28, 2024

Previously, if any unit had a socket associated with it, stc-ng counted it as "socket-activated", meaning that the unit would get stopped and the socket get restarted. That can wreak havoc on units like systemd-udevd and systemd-networkd.

Instead, let units set the new flag notSocketActivated, which sets a boolean on the unit indicating to stc-ng that the unit wants to be treated like any other non-socket-activated unit instead. That will stop/start or restart these units on upgrades, without unnecessarily tearing down any machinery that the system needs to run.

This addresses what @ElvishJerricco and I thought was a systemd bug (systemd/systemd#35371) but is really a bug in how socket-activated services are handled in stc (and -ng).

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.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
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

Previously, if any unit had a socket associated with it, stc-ng
counted it as "socket-activated", meaning that the unit would get
stopped and the socket get restarted. That can wreak havoc on units
like systemd-udevd and systemd-networkd.

Instead, let units set the new flag notSocketActivated, which sets a
boolean on the unit indicating to stc-ng that the unit wants to be
treated like any other non-socket-activated unit instead. That will
stop/start or restart these units on upgrades, without unnecessarily
tearing down any machinery that the system needs to run.
@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 Nov 28, 2024
@antifuchs
Copy link
Contributor Author

I believe this doesn't 100% entirely resolve the issue: While on one machine, explicitly starting udevd & networkd again did allow the switch to go through without issue, on another, the original problem occurred: tailscale0 turned pending (from unmanaged).

Maybe the logic around stopping/starting these critical services is wrong entirely?

@jmbaur
Copy link
Contributor

jmbaur commented Nov 29, 2024

Can you please implement the perl side of this as well? We will need to have both prior to merge.

@antifuchs
Copy link
Contributor Author

Can you please implement the perl side of this as well? We will need to have both prior to merge.

Added, thanks for the note!

@antifuchs antifuchs changed the title switch-to-configuration-ng: Better handling of socket-activated units switch-to-configuration: Better handling of socket-activated units Nov 29, 2024
@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Dec 1, 2024
Copy link
Contributor

@ElvishJerricco ElvishJerricco 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, but probably needs to update nixos/doc/manual/development/unit-handling.section.md.

@antifuchs
Copy link
Contributor Author

antifuchs commented Dec 9, 2024

Looks good, but probably needs to update nixos/doc/manual/development/unit-handling.section.md.

Updated, thanks for the pointer!

@github-actions github-actions bot added the 8.has: documentation This PR adds or changes documentation label Dec 9, 2024
@wegank wegank removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Dec 9, 2024
Copy link
Contributor

@ElvishJerricco ElvishJerricco left a comment

Choose a reason for hiding this comment

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

Just a few little nitpicks but otherwise seems fine to me.

nixos/modules/services/hardware/udev.nix Outdated Show resolved Hide resolved
nixos/lib/systemd-lib.nix Outdated Show resolved Hide resolved
nixos/lib/systemd-unit-options.nix Outdated Show resolved Hide resolved
@ElvishJerricco ElvishJerricco merged commit 15be453 into NixOS:master Jan 4, 2025
41 of 42 checks passed
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: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants