nixos/switch-to-configuration: restart changed socket units by fpletz · Pull Request #50340 · NixOS/nixpkgs

fpletz · 2018-11-14T03:12:40Z

Motivation for this change

Previously, socket units wouldn't be restarted if they were changed. To restart the socket, the service the socket is attached to needs to be stopped first before the socket can be restarted.

Discovered while converting fcgiwrap to full socket activation support and writing a test for it. These changes are also part of this PR as is the test that checks using fcgiwrap if sockets are properly restarted.

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
Built on platform(s)
- NixOS
- macOS
- other Linux distributions
Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Determined the impact on package closure size (by running nix path-info -S before and after)
Fits CONTRIBUTING.md.

Previously, socket units wouldn't be restarted if they were changed. To restart the socket, the service the socket is attached to needs to be stopped first before the socket can be restarted.

fpletz · 2018-11-14T03:26:00Z

@GrahamcOfBorg test fcgiwrap systemd

GrahamcOfBorg · 2018-11-14T03:26:21Z

No attempt on x86_64-linux (full log)

The following builds were skipped because they don't evaluate on x86_64-linux: tests.fcgiwrap, tests.systemd

Partial log (click to expand)

error: while evaluating 'recursiveUpdate' at /var/lib/gc-of-borg/.nix-test-rs/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/rbvermaa-spot/lib/attrsets.nix:415:26, called from /var/lib/gc-of-borg/.nix-test-rs/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/rbvermaa-spot/lib/attrsets.nix:148:28:
while evaluating 'recursiveUpdateUntil' at /var/lib/gc-of-borg/.nix-test-rs/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/rbvermaa-spot/lib/attrsets.nix:384:37, called from /var/lib/gc-of-borg/.nix-test-rs/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/rbvermaa-spot/lib/attrsets.nix:416:5:
while evaluating 'zipAttrsWith' at /var/lib/gc-of-borg/.nix-test-rs/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/rbvermaa-spot/lib/attrsets.nix:347:21, called from /var/lib/gc-of-borg/.nix-test-rs/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/rbvermaa-spot/lib/attrsets.nix:394:8:
while evaluating 'zipAttrsWithNames' at /var/lib/gc-of-borg/.nix-test-rs/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/rbvermaa-spot/lib/attrsets.nix:332:33, called from /var/lib/gc-of-borg/.nix-test-rs/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/rbvermaa-spot/lib/attrsets.nix:347:27:
while evaluating anonymous function at /var/lib/gc-of-borg/.nix-test-rs/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/rbvermaa-spot/lib/lists.nix:113:41, called from /var/lib/gc-of-borg/.nix-test-rs/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/rbvermaa-spot/lib/attrsets.nix:347:46:
while evaluating the attribute 'systemd' at /var/lib/gc-of-borg/.nix-test-rs/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/rbvermaa-spot/nixos/tests/all-tests.nix:198:3:
while evaluating 'handleTest' at /var/lib/gc-of-borg/.nix-test-rs/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/rbvermaa-spot/nixos/tests/all-tests.nix:17:22, called from /var/lib/gc-of-borg/.nix-test-rs/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/rbvermaa-spot/nixos/tests/all-tests.nix:198:13:
while evaluating 'discoverTests' at /var/lib/gc-of-borg/.nix-test-rs/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/rbvermaa-spot/nixos/tests/all-tests.nix:13:19, called from /var/lib/gc-of-borg/.nix-test-rs/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/rbvermaa-spot/nixos/tests/all-tests.nix:18:5:
access to path '/nix/store/g6rn7lxamwwl6rwn2xqsnq6gbqrw336c-rbvermaa-spot' is forbidden in restricted mode

GrahamcOfBorg · 2018-11-14T03:26:26Z

No attempt on aarch64-linux (full log)

The following builds were skipped because they don't evaluate on aarch64-linux: tests.fcgiwrap, tests.systemd

Partial log (click to expand)

Cannot nix-instantiate `tests.systemd' because:
error: while evaluating 'recursiveUpdate' at /var/lib/gc-of-borg/nix-test-rs-32/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-32/lib/attrsets.nix:415:26, called from /var/lib/gc-of-borg/nix-test-rs-32/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-32/lib/attrsets.nix:148:28:
while evaluating 'recursiveUpdateUntil' at /var/lib/gc-of-borg/nix-test-rs-32/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-32/lib/attrsets.nix:384:37, called from /var/lib/gc-of-borg/nix-test-rs-32/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-32/lib/attrsets.nix:416:5:
while evaluating 'zipAttrsWith' at /var/lib/gc-of-borg/nix-test-rs-32/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-32/lib/attrsets.nix:347:21, called from /var/lib/gc-of-borg/nix-test-rs-32/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-32/lib/attrsets.nix:394:8:
while evaluating 'zipAttrsWithNames' at /var/lib/gc-of-borg/nix-test-rs-32/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-32/lib/attrsets.nix:332:33, called from /var/lib/gc-of-borg/nix-test-rs-32/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-32/lib/attrsets.nix:347:27:
while evaluating the attribute 'systemd' at /var/lib/gc-of-borg/nix-test-rs-32/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-32/nixos/tests/all-tests.nix:198:3:
while evaluating 'handleTest' at /var/lib/gc-of-borg/nix-test-rs-32/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-32/nixos/tests/all-tests.nix:17:22, called from /var/lib/gc-of-borg/nix-test-rs-32/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-32/nixos/tests/all-tests.nix:198:13:
while evaluating 'discoverTests' at /var/lib/gc-of-borg/nix-test-rs-32/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-32/nixos/tests/all-tests.nix:13:19, called from /var/lib/gc-of-borg/nix-test-rs-32/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-32/nixos/tests/all-tests.nix:18:5:
access to path '/nix/store/i7qg1qsz4idr6a68pfiz9dk1m34vp31x-grahamc-aarch64-community-32' is forbidden in restricted mode

fpletz · 2018-11-14T03:28:17Z

Tests work fine for me. Seems to be an ofborg glitch. cc @grahamc

GrahamcOfBorg · 2018-11-14T03:36:41Z

Success on x86_64-linux (full log)

Attempted: fcgiwrap

Partial log (click to expand)

moving /nix/store/vrs3ppxmq302la93xi6w02k1f3ab0x78-fcgiwrap-1.1.0/man to /nix/store/vrs3ppxmq302la93xi6w02k1f3ab0x78-fcgiwrap-1.1.0/share/man
shrinking RPATHs of ELF executables and libraries in /nix/store/vrs3ppxmq302la93xi6w02k1f3ab0x78-fcgiwrap-1.1.0
shrinking /nix/store/vrs3ppxmq302la93xi6w02k1f3ab0x78-fcgiwrap-1.1.0/sbin/fcgiwrap
gzipping man pages under /nix/store/vrs3ppxmq302la93xi6w02k1f3ab0x78-fcgiwrap-1.1.0/share/man/
strip is /nix/store/vcc4svb8gy29g4pam2zja6llkbcwsyiq-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/vrs3ppxmq302la93xi6w02k1f3ab0x78-fcgiwrap-1.1.0/sbin
patching script interpreter paths in /nix/store/vrs3ppxmq302la93xi6w02k1f3ab0x78-fcgiwrap-1.1.0
checking for references to /build in /nix/store/vrs3ppxmq302la93xi6w02k1f3ab0x78-fcgiwrap-1.1.0...
moving /nix/store/vrs3ppxmq302la93xi6w02k1f3ab0x78-fcgiwrap-1.1.0/sbin/* to /nix/store/vrs3ppxmq302la93xi6w02k1f3ab0x78-fcgiwrap-1.1.0/bin
/nix/store/vrs3ppxmq302la93xi6w02k1f3ab0x78-fcgiwrap-1.1.0

GrahamcOfBorg · 2018-11-14T03:36:46Z

No attempt on x86_64-darwin (full log)

The following builds were skipped because they don't evaluate on x86_64-darwin: fcgiwrap

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.nix.

GrahamcOfBorg · 2018-11-14T03:36:57Z

Success on aarch64-linux (full log)

Attempted: fcgiwrap

Partial log (click to expand)

moving /nix/store/la4910f8rrlc304y9qpqizyw9s11s3vn-fcgiwrap-1.1.0/man to /nix/store/la4910f8rrlc304y9qpqizyw9s11s3vn-fcgiwrap-1.1.0/share/man
shrinking RPATHs of ELF executables and libraries in /nix/store/la4910f8rrlc304y9qpqizyw9s11s3vn-fcgiwrap-1.1.0
shrinking /nix/store/la4910f8rrlc304y9qpqizyw9s11s3vn-fcgiwrap-1.1.0/sbin/fcgiwrap
gzipping man pages under /nix/store/la4910f8rrlc304y9qpqizyw9s11s3vn-fcgiwrap-1.1.0/share/man/
strip is /nix/store/p9akxn2sfy4wkhqdqa3li97pc6jaz3r1-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/la4910f8rrlc304y9qpqizyw9s11s3vn-fcgiwrap-1.1.0/sbin
patching script interpreter paths in /nix/store/la4910f8rrlc304y9qpqizyw9s11s3vn-fcgiwrap-1.1.0
checking for references to /build in /nix/store/la4910f8rrlc304y9qpqizyw9s11s3vn-fcgiwrap-1.1.0...
moving /nix/store/la4910f8rrlc304y9qpqizyw9s11s3vn-fcgiwrap-1.1.0/sbin/* to /nix/store/la4910f8rrlc304y9qpqizyw9s11s3vn-fcgiwrap-1.1.0/bin
/nix/store/la4910f8rrlc304y9qpqizyw9s11s3vn-fcgiwrap-1.1.0

lheckemann · 2018-11-14T13:29:03Z

For the ofborg problem: https://logs.nix.samueldr.com/nixos-borg/2018-11-13#1542105665-1542108034; (caused by #50233)

lheckemann

Overall LGTM!

lheckemann · 2018-11-15T11:21:32Z

nixos/doc/manual/release-notes/rl-1903.xml

+        addresses as defined in
+        <citerefentry><refentrytitle>systemd.socket</refentrytitle><manvolnum>5</manvolnum></citerefentry>.
+      </para>
+    </listitem>


Should there also be a note about socket restarts in here?

Hmm, yeah. you're right. Adding socking restart support is kind of a bugfix though. I'll add a paragraph.

nixos/modules/services/web-servers/fcgiwrap.nix

lheckemann · 2018-11-15T11:25:49Z

nixos/modules/services/web-servers/fcgiwrap.nix

+      socketMode = mkOption {
+        type = types.str;
+        default = "0660";
+        description = "File mode of the socket if it is defined as a Unix socket.";


I suggest we don't bikeshed this PR on this discussion, but…

Would it maybe make sense to make a socket type which contains all these options, since it may well be used in multiple places? It doesn't seem that nice to have these settings which are essentially aliases for systemd.sockets.fcgiwrap.socketConfig.*.

nixos/modules/system/activation/switch-to-configuration.pl

mmahut · 2019-08-13T08:13:18Z

Are there any updates on this pull request, please?

domenkozar · 2019-11-26T12:39:54Z

Done in #73871, fcgiwrap needs a new PR :)

fpletz added 0.kind: enhancement Add something new or improve an existing system. 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS labels Nov 14, 2018

fpletz requested a review from infinisil as a code owner November 14, 2018 03:12

GrahamcOfBorg added the 8.has: module (update) This PR changes an existing module in `nixos/` label Nov 14, 2018

fpletz added 3 commits November 14, 2018 04:23

nixos/switch-to-configuration: restart changed socket units

e4cba5a

Previously, socket units wouldn't be restarted if they were changed. To restart the socket, the service the socket is attached to needs to be stopped first before the socket can be restarted.

fcgiwrap module: switch to full socket activation

c282114

fcgiwrap: add patch to support changing workdir

c831e81

fpletz force-pushed the fix/socket-change-activation branch from 795266e to c831e81 Compare November 14, 2018 03:25

fpletz added the 8.has: changelog This PR adds or changes release notes label Nov 14, 2018

GrahamcOfBorg 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 Nov 14, 2018

GrahamcOfBorg added the 8.has: documentation This PR adds or changes documentation label Nov 14, 2018

globin approved these changes Nov 14, 2018

View reviewed changes

lheckemann reviewed Nov 15, 2018

View reviewed changes

aneeshusa reviewed Nov 18, 2018

View reviewed changes

nixos/modules/system/activation/switch-to-configuration.pl Show resolved Hide resolved

lheckemann mentioned this pull request Nov 21, 2019

nixos/switch-to-configuration: restart changed socket units #73871

Merged

3 tasks

domenkozar closed this Nov 26, 2019

fpletz deleted the fix/socket-change-activation branch December 7, 2019 11:18

Uh oh!

Conversation

fpletz commented Nov 14, 2018

Motivation for this change

Things done

Uh oh!

fpletz commented Nov 14, 2018

Uh oh!

GrahamcOfBorg commented Nov 14, 2018

Uh oh!

GrahamcOfBorg commented Nov 14, 2018

Uh oh!

fpletz commented Nov 14, 2018

Uh oh!

GrahamcOfBorg commented Nov 14, 2018

Uh oh!

GrahamcOfBorg commented Nov 14, 2018

Uh oh!

GrahamcOfBorg commented Nov 14, 2018

Uh oh!

lheckemann commented Nov 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lheckemann left a comment

Choose a reason for hiding this comment

Uh oh!

lheckemann Nov 15, 2018

Choose a reason for hiding this comment

Uh oh!

fpletz Nov 15, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lheckemann Nov 15, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mmahut commented Aug 13, 2019

Uh oh!

domenkozar commented Nov 26, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

lheckemann commented Nov 14, 2018 •

edited

Loading