Unverified Commit bc9b484d authored by Janne Heß's avatar Janne Heß Committed by GitHub
Browse files

Merge pull request #232230 from oddlama/fix-activation-template-unit-specializations 

Fix detection of changed template unit specializations in switch-to-configuration.pl
parents 1d482993 da6c61cc

nixos/doc/manual/administration/service-mgmt.chapter.md

+30 −0
Original line number Diff line number Diff line
@@ -118,3 +118,33 @@ the symlink, and this path is in `/nix/store/.../lib/systemd/user/`. 
Hence [garbage collection](#sec-nix-gc) will remove that file and you

will wind up with a broken symlink in your systemd configuration, which

in turn will not make the service / timer start on login.



## Template units {#sect-nixos-systemd-template-units}



systemd supports templated units where a base unit can be started multiple

times with a different parameter. The syntax to accomplish this is

`service-name@instance-name.service`. Units get the instance name passed to

them (see `systemd.unit(5)`). NixOS has support for these kinds of units and

for template-specific overrides. A service needs to be defined twice, once

for the base unit and once for the instance. All instances must include

`overrideStrategy = "asDropin"` for the change detection to work. This

example illustrates this:

```nix

{

  systemd.services = {

    "base-unit@".serviceConfig = {

      ExecStart = "...";

      User = "...";

    };

    "base-unit@instance-a" = {

      overrideStrategy = "asDropin"; # needed for templates to work

      wantedBy = [ "multi-user.target" ]; # causes NixOS to manage the instance

    };

    "base-unit@instance-b" = {

      overrideStrategy = "asDropin"; # needed for templates to work

      wantedBy = [ "multi-user.target" ]; # causes NixOS to manage the instance

      serviceConfig.User = "root"; # also override something for this specific instance

    };

  };

}

```

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

+35 −23
Original line number Diff line number Diff line
@@ -253,16 +253,24 @@ sub parse_systemd_ini { 
# If a directory with the same basename ending in .d exists next to the unit file, it will be

# assumed to contain override files which will be parsed as well and handled properly.

sub parse_unit {

    my ($unit_path) = @_;

    my ($unit_path, $base_unit_path) = @_;



    # Parse the main unit and all overrides

    my %unit_data;

    # Replace \ with \\ so glob() still works with units that have a \ in them

    # Valid characters in unit names are ASCII letters, digits, ":", "-", "_", ".", and "\"

    $base_unit_path =~ s/\\/\\\\/gmsx;

    $unit_path =~ s/\\/\\\\/gmsx;

    foreach (glob("${unit_path}{,.d/*.conf}")) {



    foreach (glob("${base_unit_path}{,.d/*.conf}")) {

        parse_systemd_ini(\%unit_data, "$_")

    }

    # Handle drop-in template-unit instance overrides

    if ($unit_path ne $base_unit_path) {

        foreach (glob("${unit_path}.d/*.conf")) {

            parse_systemd_ini(\%unit_data, "$_")

        }

    }

    return %unit_data;

}
@@ -423,7 +431,7 @@ sub compare_units { ## no critic(Subroutines::ProhibitExcessComplexity) 
# Called when a unit exists in both the old systemd and the new system and the units

# differ. This figures out of what units are to be stopped, restarted, reloaded, started, and skipped.

sub handle_modified_unit { ## no critic(Subroutines::ProhibitManyArgs, Subroutines::ProhibitExcessComplexity)

    my ($unit, $base_name, $new_unit_file, $new_unit_info, $active_cur, $units_to_stop, $units_to_start, $units_to_reload, $units_to_restart, $units_to_skip) = @_;

    my ($unit, $base_name, $new_unit_file, $new_base_unit_file, $new_unit_info, $active_cur, $units_to_stop, $units_to_start, $units_to_reload, $units_to_restart, $units_to_skip) = @_;



    if ($unit eq "sysinit.target" || $unit eq "basic.target" || $unit eq "multi-user.target" || $unit eq "graphical.target" || $unit =~ /\.path$/msx || $unit =~ /\.slice$/msx) {

        # Do nothing.  These cannot be restarted directly.
@@ -442,7 +450,7 @@ sub handle_modified_unit { ## no critic(Subroutines::ProhibitManyArgs, Subroutin 
        # Revert of the attempt: https://github.com/NixOS/nixpkgs/pull/147609

        # More details: https://github.com/NixOS/nixpkgs/issues/74899#issuecomment-981142430

    } else {

        my %new_unit_info = $new_unit_info ? %{$new_unit_info} : parse_unit($new_unit_file);

        my %new_unit_info = $new_unit_info ? %{$new_unit_info} : parse_unit($new_unit_file, $new_base_unit_file);

        if (parse_systemd_bool(\%new_unit_info, "Service", "X-ReloadIfChanged", 0) and not $units_to_restart->{$unit} and not $units_to_stop->{$unit}) {

            $units_to_reload->{$unit} = 1;

            record_unit($reload_list_file, $unit);
@@ -538,31 +546,33 @@ my %units_to_filter; # units not shown 


my $active_cur = get_active_units();

while (my ($unit, $state) = each(%{$active_cur})) {

    my $base_unit = $unit;

    my $cur_unit_file = "/etc/systemd/system/$unit";

    my $new_unit_file = "$toplevel/etc/systemd/system/$unit";



    my $cur_unit_file = "/etc/systemd/system/$base_unit";

    my $new_unit_file = "$toplevel/etc/systemd/system/$base_unit";

    my $base_unit = $unit;

    my $cur_base_unit_file = $cur_unit_file;

    my $new_base_unit_file = $new_unit_file;



    # Detect template instances.

    if (!-e $cur_unit_file && !-e $new_unit_file && $unit =~ /^(.*)@[^\.]*\.(.*)$/msx) {

      $base_unit = "$1\@.$2";

      $cur_unit_file = "/etc/systemd/system/$base_unit";

      $new_unit_file = "$toplevel/etc/systemd/system/$base_unit";

      $cur_base_unit_file = "/etc/systemd/system/$base_unit";

      $new_base_unit_file = "$toplevel/etc/systemd/system/$base_unit";

    }



    my $base_name = $base_unit;

    $base_name =~ s/\.[[:lower:]]*$//msx;



    if (-e $cur_unit_file && ($state->{state} eq "active" || $state->{state} eq "activating")) {

        if (! -e $new_unit_file || abs_path($new_unit_file) eq "/dev/null") {

            my %cur_unit_info = parse_unit($cur_unit_file);

    if (-e $cur_base_unit_file && ($state->{state} eq "active" || $state->{state} eq "activating")) {

        if (! -e $new_base_unit_file || abs_path($new_base_unit_file) eq "/dev/null") {

            my %cur_unit_info = parse_unit($cur_unit_file, $cur_base_unit_file);

            if (parse_systemd_bool(\%cur_unit_info, "Unit", "X-StopOnRemoval", 1)) {

                $units_to_stop{$unit} = 1;

            }

        }



        elsif ($unit =~ /\.target$/msx) {

            my %new_unit_info = parse_unit($new_unit_file);

            my %new_unit_info = parse_unit($new_unit_file, $new_base_unit_file);



            # Cause all active target units to be restarted below.

            # This should start most changed units we stop here as
@@ -596,11 +606,11 @@ while (my ($unit, $state) = each(%{$active_cur})) { 
        }



        else {

            my %cur_unit_info = parse_unit($cur_unit_file);

            my %new_unit_info = parse_unit($new_unit_file);

            my %cur_unit_info = parse_unit($cur_unit_file, $cur_base_unit_file);

            my %new_unit_info = parse_unit($new_unit_file, $new_base_unit_file);

            my $diff = compare_units(\%cur_unit_info, \%new_unit_info);

            if ($diff == 1) {

                handle_modified_unit($unit, $base_name, $new_unit_file, \%new_unit_info, $active_cur, \%units_to_stop, \%units_to_start, \%units_to_reload, \%units_to_restart, \%units_to_skip);

                handle_modified_unit($unit, $base_name, $new_unit_file, $new_base_unit_file, \%new_unit_info, $active_cur, \%units_to_stop, \%units_to_start, \%units_to_reload, \%units_to_restart, \%units_to_skip);

            } elsif ($diff == 2 and not $units_to_restart{$unit}) {

                $units_to_reload{$unit} = 1;

                record_unit($reload_list_file, $unit);
@@ -710,13 +720,14 @@ if ($action eq "dry-activate") { 
    # Handle the activation script requesting the restart or reload of a unit.

    foreach (split(/\n/msx, read_file($dry_restart_by_activation_file, err_mode => "quiet") // "")) {

        my $unit = $_;

        my $new_unit_file = "$toplevel/etc/systemd/system/$unit";

        my $base_unit = $unit;

        my $new_unit_file = "$toplevel/etc/systemd/system/$base_unit";

        my $new_base_unit_file = $new_unit_file;



        # Detect template instances.

        if (!-e $new_unit_file && $unit =~ /^(.*)@[^\.]*\.(.*)$/msx) {

          $base_unit = "$1\@.$2";

          $new_unit_file = "$toplevel/etc/systemd/system/$base_unit";

          $new_base_unit_file = "$toplevel/etc/systemd/system/$base_unit";

        }



        my $base_name = $base_unit;
@@ -728,7 +739,7 @@ if ($action eq "dry-activate") { 
            next;

        }



        handle_modified_unit($unit, $base_name, $new_unit_file, undef, $active_cur, \%units_to_restart, \%units_to_restart, \%units_to_reload, \%units_to_restart, \%units_to_skip);

        handle_modified_unit($unit, $base_name, $new_unit_file, $new_base_unit_file, undef, $active_cur, \%units_to_restart, \%units_to_restart, \%units_to_reload, \%units_to_restart, \%units_to_skip);

    }

    unlink($dry_restart_by_activation_file);
@@ -782,13 +793,14 @@ system("$out/activate", "$out") == 0 or $res = 2; 
# Handle the activation script requesting the restart or reload of a unit.

foreach (split(/\n/msx, read_file($restart_by_activation_file, err_mode => "quiet") // "")) {

    my $unit = $_;

    my $new_unit_file = "$toplevel/etc/systemd/system/$unit";

    my $base_unit = $unit;

    my $new_unit_file = "$toplevel/etc/systemd/system/$base_unit";

    my $new_base_unit_file = $new_unit_file;



    # Detect template instances.

    if (!-e $new_unit_file && $unit =~ /^(.*)@[^\.]*\.(.*)$/msx) {

      $base_unit = "$1\@.$2";

      $new_unit_file = "$toplevel/etc/systemd/system/$base_unit";

      $new_base_unit_file = "$toplevel/etc/systemd/system/$base_unit";

    }



    my $base_name = $base_unit;
@@ -801,7 +813,7 @@ foreach (split(/\n/msx, read_file($restart_by_activation_file, err_mode => "quie 
        next;

    }



    handle_modified_unit($unit, $base_name, $new_unit_file, undef, $active_cur, \%units_to_restart, \%units_to_restart, \%units_to_reload, \%units_to_restart, \%units_to_skip);

    handle_modified_unit($unit, $base_name, $new_unit_file, $new_base_unit_file, undef, $active_cur, \%units_to_restart, \%units_to_restart, \%units_to_reload, \%units_to_restart, \%units_to_skip);

}

# We can remove the file now because it has been propagated to the other restart/reload files

unlink($restart_by_activation_file);
@@ -859,7 +871,7 @@ if (scalar(keys(%units_to_reload)) > 0) { 
    for my $unit (keys(%units_to_reload)) {

        if (!unit_is_active($unit)) {

            # Figure out if we need to start the unit

            my %unit_info = parse_unit("$toplevel/etc/systemd/system/$unit");

            my %unit_info = parse_unit("$toplevel/etc/systemd/system/$unit", "$toplevel/etc/systemd/system/$unit");

            if (!(parse_systemd_bool(\%unit_info, "Unit", "RefuseManualStart", 0) || parse_systemd_bool(\%unit_info, "Unit", "X-OnlyManualStart", 0))) {

                $units_to_start{$unit} = 1;

                record_unit($start_list_file, $unit);

nixos/tests/switch-test.nix

+153 −9
Original line number Diff line number Diff line 
# Test configuration switching.



import ./make-test-python.nix ({ pkgs, ...} : let

import ./make-test-python.nix ({ lib, pkgs, ...} : let



  # Simple service that can either be socket-activated or that will

  # listen on port 1234 if not socket-activated.
@@ -279,6 +279,28 @@ in { 
          systemd.services.test-service.unitConfig.RefuseManualStart = true;

        };



        unitWithTemplate.configuration = {

          systemd.services."instantiated@".serviceConfig = {

            Type = "oneshot";

            RemainAfterExit = true;

            ExecStart = "${pkgs.coreutils}/bin/true";

            ExecReload = "${pkgs.coreutils}/bin/true";

          };

          systemd.services."instantiated@one" = {

            wantedBy = [ "multi-user.target" ];

            overrideStrategy = "asDropin";

          };

          systemd.services."instantiated@two" = {

            wantedBy = [ "multi-user.target" ];

            overrideStrategy = "asDropin";

          };

        };



        unitWithTemplateModified.configuration = {

          imports = [ unitWithTemplate.configuration ];

          systemd.services."instantiated@".serviceConfig.X-Test = "test";

        };



        restart-and-reload-by-activation-script.configuration = {

          systemd.services = rec {

            simple-service = {
@@ -290,29 +312,50 @@ in { 
                ExecReload = "${pkgs.coreutils}/bin/true";

              };

            };

            "templated-simple-service@" = simple-service;

            "templated-simple-service@instance".overrideStrategy = "asDropin";



            simple-restart-service = simple-service // {

              stopIfChanged = false;

            };

            "templated-simple-restart-service@" = simple-restart-service;

            "templated-simple-restart-service@instance".overrideStrategy = "asDropin";



            simple-reload-service = simple-service // {

              reloadIfChanged = true;

            };

            "templated-simple-reload-service@" = simple-reload-service;

            "templated-simple-reload-service@instance".overrideStrategy = "asDropin";



            no-restart-service = simple-service // {

              restartIfChanged = false;

            };

            "templated-no-restart-service@" = no-restart-service;

            "templated-no-restart-service@instance".overrideStrategy = "asDropin";



            reload-triggers = simple-service // {

              wantedBy = [ "multi-user.target" ];

            };

            "templated-reload-triggers@" = simple-service;

            "templated-reload-triggers@instance" = {

              overrideStrategy = "asDropin";

              wantedBy = [ "multi-user.target" ];

            };



            reload-triggers-and-restart-by-as = simple-service;

            "templated-reload-triggers-and-restart-by-as@" = reload-triggers-and-restart-by-as;

            "templated-reload-triggers-and-restart-by-as@instance".overrideStrategy = "asDropin";



            reload-triggers-and-restart = simple-service // {

              stopIfChanged = false; # easier to check for this

              wantedBy = [ "multi-user.target" ];

            };

            "templated-reload-triggers-and-restart@" = simple-service;

            "templated-reload-triggers-and-restart@instance" = {

              overrideStrategy = "asDropin";

              stopIfChanged = false; # easier to check for this

              wantedBy = [ "multi-user.target" ];

            };

          };



          system.activationScripts.restart-and-reload-test = {
@@ -332,12 +375,20 @@ in { 
              simple-reload-service.service

              no-restart-service.service

              reload-triggers-and-restart-by-as.service

              templated-simple-service@instance.service

              templated-simple-restart-service@instance.service

              templated-simple-reload-service@instance.service

              templated-no-restart-service@instance.service

              templated-reload-triggers-and-restart-by-as@instance.service

              EOF



              cat <<EOF >> "$g"

              reload-triggers.service

              reload-triggers-and-restart-by-as.service

              reload-triggers-and-restart.service

              templated-reload-triggers@instance.service

              templated-reload-triggers-and-restart-by-as@instance.service

              templated-reload-triggers-and-restart@instance.service

              EOF

            '';

          };
@@ -346,6 +397,10 @@ in { 
        restart-and-reload-by-activation-script-modified.configuration = {

          imports = [ restart-and-reload-by-activation-script.configuration ];

          systemd.services.reload-triggers-and-restart.serviceConfig.X-Modified = "test";

          systemd.services."templated-reload-triggers-and-restart@instance" = {

            overrideStrategy = "asDropin";

            serviceConfig.X-Modified = "test";

          };

        };



        simple-socket.configuration = {
@@ -507,6 +562,10 @@ in { 
      set -o pipefail

      exec env -i "$@" | tee /dev/stderr

    '';



    # Returns a comma separated representation of the given list in sorted

    # order, that matches the output format of switch-to-configuration.pl

    sortedUnits = xs: lib.concatStringsSep ", " (builtins.sort builtins.lessThan xs);

  in /* python */ ''

    def switch_to_specialisation(system, name, action="test", fail=False):

        if name == "":
@@ -733,6 +792,16 @@ in { 
        assert_contains(out, "\nstarting the following units: required-service.service\n")

        assert_lacks(out, "the following new units were started:")



        # Ensure templated units are restarted when the base unit changes

        switch_to_specialisation("${machine}", "unitWithTemplate")

        out = switch_to_specialisation("${machine}", "unitWithTemplateModified")

        assert_contains(out, "stopping the following units: instantiated@one.service, instantiated@two.service\n")

        assert_lacks(out, "NOT restarting the following changed units:")

        assert_lacks(out, "reloading the following units:")

        assert_lacks(out, "\nrestarting the following units:")

        assert_contains(out, "\nstarting the following units: instantiated@one.service, instantiated@two.service\n")

        assert_lacks(out, "the following new units were started:")



    with subtest("failing units"):

        # Let the simple service fail

        switch_to_specialisation("${machine}", "simpleServiceModified")
@@ -896,15 +965,62 @@ in { 
        assert_lacks(out, "NOT restarting the following changed units:")

        assert_lacks(out, "reloading the following units:")

        assert_lacks(out, "restarting the following units:")

        assert_contains(out, "\nstarting the following units: no-restart-service.service, reload-triggers-and-restart-by-as.service, simple-reload-service.service, simple-restart-service.service, simple-service.service\n")

        assert_contains(out, "the following new units were started: no-restart-service.service, reload-triggers-and-restart-by-as.service, reload-triggers-and-restart.service, reload-triggers.service, simple-reload-service.service, simple-restart-service.service, simple-service.service\n")

        assert_contains(out, "\nstarting the following units: ${sortedUnits [

          "no-restart-service.service"

          "reload-triggers-and-restart-by-as.service"

          "simple-reload-service.service"

          "simple-restart-service.service"

          "simple-service.service"

          "templated-no-restart-service@instance.service"

          "templated-reload-triggers-and-restart-by-as@instance.service"

          "templated-simple-reload-service@instance.service"

          "templated-simple-restart-service@instance.service"

          "templated-simple-service@instance.service"

        ]}\n")

        assert_contains(out, "the following new units were started: ${sortedUnits [

          "no-restart-service.service"

          "reload-triggers-and-restart-by-as.service"

          "reload-triggers-and-restart.service"

          "reload-triggers.service"

          "simple-reload-service.service"

          "simple-restart-service.service"

          "simple-service.service"

          "system-templated\\\\x2dno\\\\x2drestart\\\\x2dservice.slice"

          "system-templated\\\\x2dreload\\\\x2dtriggers.slice"

          "system-templated\\\\x2dreload\\\\x2dtriggers\\\\x2dand\\\\x2drestart.slice"

          "system-templated\\\\x2dreload\\\\x2dtriggers\\\\x2dand\\\\x2drestart\\\\x2dby\\\\x2das.slice"

          "system-templated\\\\x2dsimple\\\\x2dreload\\\\x2dservice.slice"

          "system-templated\\\\x2dsimple\\\\x2drestart\\\\x2dservice.slice"

          "system-templated\\\\x2dsimple\\\\x2dservice.slice"

          "templated-no-restart-service@instance.service"

          "templated-reload-triggers-and-restart-by-as@instance.service"

          "templated-reload-triggers-and-restart@instance.service"

          "templated-reload-triggers@instance.service"

          "templated-simple-reload-service@instance.service"

          "templated-simple-restart-service@instance.service"

          "templated-simple-service@instance.service"

        ]}\n")

        # Switch to the same system where the example services get restarted

        # and reloaded by the activation script

        out = switch_to_specialisation("${machine}", "restart-and-reload-by-activation-script")

        assert_lacks(out, "stopping the following units:")

        assert_lacks(out, "NOT restarting the following changed units:")

        assert_contains(out, "reloading the following units: reload-triggers-and-restart.service, reload-triggers.service, simple-reload-service.service\n")

        assert_contains(out, "restarting the following units: reload-triggers-and-restart-by-as.service, simple-restart-service.service, simple-service.service\n")

        assert_contains(out, "reloading the following units: ${sortedUnits [

          "reload-triggers-and-restart.service"

          "reload-triggers.service"

          "simple-reload-service.service"

          "templated-reload-triggers-and-restart@instance.service"

          "templated-reload-triggers@instance.service"

          "templated-simple-reload-service@instance.service"

        ]}\n")

        assert_contains(out, "restarting the following units: ${sortedUnits [

          "reload-triggers-and-restart-by-as.service"

          "simple-restart-service.service"

          "simple-service.service"

          "templated-reload-triggers-and-restart-by-as@instance.service"

          "templated-simple-restart-service@instance.service"

          "templated-simple-service@instance.service"

        ]}\n")

        assert_lacks(out, "\nstarting the following units:")

        assert_lacks(out, "the following new units were started:")

        # Switch to the same system and see if the service gets restarted when it's modified
@@ -912,16 +1028,44 @@ in { 
        out = switch_to_specialisation("${machine}", "restart-and-reload-by-activation-script-modified")

        assert_lacks(out, "stopping the following units:")

        assert_lacks(out, "NOT restarting the following changed units:")

        assert_contains(out, "reloading the following units: reload-triggers.service, simple-reload-service.service\n")

        assert_contains(out, "restarting the following units: reload-triggers-and-restart-by-as.service, reload-triggers-and-restart.service, simple-restart-service.service, simple-service.service\n")

        assert_contains(out, "reloading the following units: ${sortedUnits [

          "reload-triggers.service"

          "simple-reload-service.service"

          "templated-reload-triggers@instance.service"

          "templated-simple-reload-service@instance.service"

        ]}\n")

        assert_contains(out, "restarting the following units: ${sortedUnits [

          "reload-triggers-and-restart-by-as.service"

          "reload-triggers-and-restart.service"

          "simple-restart-service.service"

          "simple-service.service"

          "templated-reload-triggers-and-restart-by-as@instance.service"

          "templated-reload-triggers-and-restart@instance.service"

          "templated-simple-restart-service@instance.service"

          "templated-simple-service@instance.service"

        ]}\n")

        assert_lacks(out, "\nstarting the following units:")

        assert_lacks(out, "the following new units were started:")

        # The same, but in dry mode

        out = switch_to_specialisation("${machine}", "restart-and-reload-by-activation-script", action="dry-activate")

        assert_lacks(out, "would stop the following units:")

        assert_lacks(out, "would NOT stop the following changed units:")

        assert_contains(out, "would reload the following units: reload-triggers.service, simple-reload-service.service\n")

        assert_contains(out, "would restart the following units: reload-triggers-and-restart-by-as.service, reload-triggers-and-restart.service, simple-restart-service.service, simple-service.service\n")

        assert_contains(out, "would reload the following units: ${sortedUnits [

          "reload-triggers.service"

          "simple-reload-service.service"

          "templated-reload-triggers@instance.service"

          "templated-simple-reload-service@instance.service"

        ]}\n")

        assert_contains(out, "would restart the following units: ${sortedUnits [

          "reload-triggers-and-restart-by-as.service"

          "reload-triggers-and-restart.service"

          "simple-restart-service.service"

          "simple-service.service"

          "templated-reload-triggers-and-restart-by-as@instance.service"

          "templated-reload-triggers-and-restart@instance.service"

          "templated-simple-restart-service@instance.service"

          "templated-simple-service@instance.service"

        ]}\n")

        assert_lacks(out, "\nwould start the following units:")



    with subtest("socket-activated services"):