nginx service: reload instead of restart

nixos/modules/services/web-servers/nginx/default.nix

@@ -415,19 +415,39 @@ in
       description = "Nginx Web Server";
       after = [ "network.target" ];
       wantedBy = [ "multi-user.target" ];
-      stopIfChanged = false;
       preStart =
         ''
         mkdir -p ${cfg.stateDir}/logs
         chmod 700 ${cfg.stateDir}
         chown -R ${cfg.user}:${cfg.group} ${cfg.stateDir}
-        '';
+        ln -sf ${configFile} /run/nginx/config
+        ln -sf ${cfg.package} /run/nginx/package
+      '';
+      reload = ''
+        # Check if the new config is valid
+        ${cfg.package}/bin/nginx -t -c ${configFile} -p ${cfg.stateDir}
+
+        # Check if the package changed
+        if [[ `readlink /run/nginx/package` != ${cfg.package} ]]; then
+          # If it changed, we need to restart nginx. So we kill nginx
+          # gracefully. We can't send a restart to systemd while in the
+          # reload script. Nginx will be restarted by systemd automatically.
+          ${pkgs.coreutils}/bin/kill -QUIT $MAINPID
+          exit 0
+        fi
+
+        # We only need to change the configuration, so update it and reload nginx
+        ln -sf ${configFile} /run/nginx/config
+        ${pkgs.coreutils}/bin/kill -HUP $MAINPID
+      '';
+      restartTriggers = [ configFile ];
+      reloadIfChanged = true;
       serviceConfig = {
-        ExecStart = "${cfg.package}/bin/nginx -c ${configFile} -p ${cfg.stateDir}";
-        ExecReload = "${pkgs.coreutils}/bin/kill -HUP $MAINPID";
+        ExecStart = "${cfg.package}/bin/nginx -c /run/nginx/config -p ${cfg.stateDir}";
         Restart = "always";
-        RestartSec = "10s";
+        RestartSec = "1s";
         StartLimitInterval = "1min";
+        RuntimeDirectory = "nginx";
       };
     };
 

nixos/tests/nginx.nix

@@ -1,42 +1,72 @@
 # verifies:
 #   1. nginx generates config file with shared http context definitions above
 #      generated virtual hosts config.
+#   2. configuration reload works and nginx keeps running with old confi on
+#      syntax errors
 
 import ./make-test.nix ({ pkgs, ...} : {
-  name = "jenkins";
+  name = "nginx";
   meta = with pkgs.stdenv.lib.maintainers; {
-    maintainers = [ mbbx6spp ];
+    maintainers = [ mbbx6spp fpletz ];
   };
 
-  nodes = {
-    webserver =
-      { config, pkgs, ... }:
-      { services.nginx.enable = true;
-        services.nginx.commonHttpConfig = ''
-        log_format ceeformat '@cee: {"status":"$status",'
-          '"request_time":$request_time,'
-          '"upstream_response_time":$upstream_response_time,'
-          '"pipe":"$pipe","bytes_sent":$bytes_sent,'
-          '"connection":"$connection",'
-          '"remote_addr":"$remote_addr",'
-          '"host":"$host",'
-          '"timestamp":"$time_iso8601",'
-          '"request":"$request",'
-          '"http_referer":"$http_referer",'
-          '"upstream_addr":"$upstream_addr"}';
-        '';
-        services.nginx.virtualHosts."0.my.test" = {
-          extraConfig = ''
+  nodes = let
+    commonConfig = customCfg:
+      { lib, ... }:
+      lib.mkMerge [
+        { services.nginx.enable = true;
+          services.nginx.commonHttpConfig = ''
+          log_format ceeformat '@cee: {"status":"$status",'
+            '"request_time":$request_time,'
+            '"upstream_response_time":$upstream_response_time,'
+            '"pipe":"$pipe","bytes_sent":$bytes_sent,'
+            '"connection":"$connection",'
+            '"remote_addr":"$remote_addr",'
+            '"host":"$host",'
+            '"timestamp":"$time_iso8601",'
+            '"request":"$request",'
+            '"http_referer":"$http_referer",'
+            '"upstream_addr":"$upstream_addr"}';
+          '';
+          services.nginx.virtualHosts."_".extraConfig = ''
             access_log syslog:server=unix:/dev/log,facility=user,tag=mytag,severity=info ceeformat;
           '';
-        };
+        }
+        customCfg
+      ];
+    in {
+      webserver = commonConfig {
+        services.nginx.virtualHosts."_".extraConfig = "return 200 works;";
       };
-  };
+      webserver_new = commonConfig {
+        services.nginx.virtualHosts."_".extraConfig = "return 200 new-content;";
+        services.nginx.package = pkgs.nginxMainline;
+      };
+      webserver_invalid = commonConfig {
+        services.nginx.appendHttpConfig = "foo";
+      };
+    };
+
+  testScript = { nodes, ... }: let
+      webserver_new = nodes.webserver_new.config.system.build.toplevel;
+      webserver_invalid = nodes.webserver_invalid.config.system.build.toplevel;
+    in ''
+    $webserver->start;
+
+    $webserver->waitForUnit("nginx");
+    $webserver->waitForOpenPort("80");
+    $webserver->succeed("[[ `curl http://localhost/` == \"works\" ]]");
+
+    $webserver->succeed("${webserver_new}/bin/switch-to-configuration test");
+
+    $webserver->waitForUnit("nginx");
+    $webserver->waitForOpenPort("80");
+    $webserver->succeed("[[ `curl http://localhost/` == \"new-content\" ]]");
 
-  testScript = ''
-    startAll;
+    $webserver->fail("${webserver_invalid}/bin/switch-to-configuration test");
 
     $webserver->waitForUnit("nginx");
     $webserver->waitForOpenPort("80");
+    $webserver->succeed("[[ `curl http://localhost/` == \"new-content\" ]]");
   '';
 })