From ceff5c5cfeaf211691f4d1156f358a940b5ef7b4 Mon Sep 17 00:00:00 2001
From: Maximilian Bosch <maximilian@mbosch.me>
Date: Mon, 12 Feb 2024 18:30:03 +0100
Subject: [PATCH 1/5] flake: fix gitea integration test

This is an integration test that confirms that jobset definitions from
git repositories are correctly built and status updates pushed to the
gitea instance. The following things needed to be fixed:

* We're still on 23.05 where gitea is marked as insecure. Not going to
  update nixpkgs right now, but going for the quick fix.
* Since gitea 1.19 tokens have scopes that describe what's possible.
  Not specifying the scope in the DB appears to imply that no
  permissions are granted.
* Apparently we have three status updates now (for three status hooks,
  queued/started/finished). No idea why that was broken before, but the
  behavior still looks correct.
---
 flake.nix | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/flake.nix b/flake.nix
index 306ed292..a6dfb977 100644
--- a/flake.nix
+++ b/flake.nix
@@ -180,13 +180,9 @@
                 root=d7f16a3412e01a43a414535b16007c6931d3a9c7
                 </gitea_authorization>
               '';
+              nixpkgs.config.permittedInsecurePackages = [ "gitea-1.19.4" ];
               nix = {
-                distributedBuilds = true;
-                buildMachines = [{
-                  hostName = "localhost";
-                  systems = [ system ];
-                }];
-                binaryCaches = [ ];
+                settings.substituters = [ ];
               };
               services.gitea = {
                 enable = true;
@@ -202,7 +198,7 @@
             testScript =
               let
                 scripts.mktoken = pkgs.writeText "token.sql" ''
-                  INSERT INTO access_token (id, uid, name, created_unix, updated_unix, token_hash, token_salt, token_last_eight) VALUES (1, 1, 'hydra', 1617107360, 1617107360, 'a930f319ca362d7b49a4040ac0af74521c3a3c3303a86f327b01994430672d33b6ec53e4ea774253208686c712495e12a486', 'XRjWE9YW0g', '31d3a9c7');
+                  INSERT INTO access_token (id, uid, name, created_unix, updated_unix, token_hash, token_salt, token_last_eight, scope) VALUES (1, 1, 'hydra', 1617107360, 1617107360, 'a930f319ca362d7b49a4040ac0af74521c3a3c3303a86f327b01994430672d33b6ec53e4ea774253208686c712495e12a486', 'XRjWE9YW0g', '31d3a9c7', 'all');
                 '';
 
                 scripts.git-setup = pkgs.writeShellScript "setup.sh" ''
@@ -357,9 +353,10 @@
 
                 response = json.loads(data)
 
-                assert len(response) == 2, "Expected exactly two status updates for latest commit!"
-                assert response[0]['status'] == "success", "Expected latest status to be success!"
-                assert response[1]['status'] == "pending", "Expected first status to be pending!"
+                assert len(response) == 3, "Expected exactly three status updates for latest commit (queued, started, finished)!"
+                assert response[0]['status'] == "success", "Expected finished status to be success!"
+                assert response[1]['status'] == "pending", "Expected started status to be pending!"
+                assert response[2]['status'] == "pending", "Expected queued status to be pending!"
 
                 machine.shutdown()
               '';

From 9db5d0a88daee61b8b4507e517d53cab7377a70d Mon Sep 17 00:00:00 2001
From: K900 <me@0upti.me>
Date: Fri, 23 Feb 2024 14:02:36 +0300
Subject: [PATCH 2/5] urlencode drv names when fetching logs

Otherwise names with special characters like + break things.
---
 src/lib/Hydra/Controller/Build.pm | 5 +++--
 src/lib/Hydra/Controller/Root.pm  | 3 ++-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/lib/Hydra/Controller/Build.pm b/src/lib/Hydra/Controller/Build.pm
index c3869838..6b25ff80 100644
--- a/src/lib/Hydra/Controller/Build.pm
+++ b/src/lib/Hydra/Controller/Build.pm
@@ -15,6 +15,7 @@ use Nix::Config;
 use List::SomeUtils qw(all);
 use Encode;
 use JSON::PP;
+use WWW::Form::UrlEncoded::PP qw();
 
 use feature 'state';
 
@@ -141,7 +142,7 @@ sub view_nixlog : Chained('buildChain') PathPart('nixlog') {
     $c->stash->{step} = $step;
 
     my $drvPath = $step->drvpath;
-    my $log_uri = $c->uri_for($c->controller('Root')->action_for("log"), [basename($drvPath)]);
+    my $log_uri = $c->uri_for($c->controller('Root')->action_for("log"), [WWW::Form::UrlEncoded::PP::url_encode(basename($drvPath))]);
     showLog($c, $mode, $log_uri);
 }
 
@@ -150,7 +151,7 @@ sub view_log : Chained('buildChain') PathPart('log') {
     my ($self, $c, $mode) = @_;
 
     my $drvPath = $c->stash->{build}->drvpath;
-    my $log_uri = $c->uri_for($c->controller('Root')->action_for("log"), [basename($drvPath)]);
+    my $log_uri = $c->uri_for($c->controller('Root')->action_for("log"), [WWW::Form::UrlEncoded::PP::url_encode(basename($drvPath))]);
     showLog($c, $mode, $log_uri);
 }
 
diff --git a/src/lib/Hydra/Controller/Root.pm b/src/lib/Hydra/Controller/Root.pm
index 548cfac3..e6c3049f 100644
--- a/src/lib/Hydra/Controller/Root.pm
+++ b/src/lib/Hydra/Controller/Root.pm
@@ -16,6 +16,7 @@ use List::Util qw[min max];
 use List::SomeUtils qw{any};
 use Net::Prometheus;
 use Types::Standard qw/StrMatch/;
+use WWW::Form::UrlEncoded::PP qw();
 
 use constant NARINFO_REGEX => qr{^([a-z0-9]{32})\.narinfo$};
 # e.g.: https://hydra.example.com/realisations/sha256:a62128132508a3a32eef651d6467695944763602f226ac630543e947d9feb140!out.doi
@@ -553,7 +554,7 @@ sub log :Local :Args(1) {
     my $logPrefix = $c->config->{log_prefix};
 
     if (defined $logPrefix) {
-        $c->res->redirect($logPrefix . "log/" . basename($drvPath));
+        $c->res->redirect($logPrefix . "log/" . WWW::Form::UrlEncoded::PP::url_encode(basename($drvPath)));
     } else {
         notFound($c, "The build log of $drvPath is not available.");
     }

From 669617ab54a667623fdbbc07dfd7354b5d66286b Mon Sep 17 00:00:00 2001
From: Maximilian Bosch <maximilian@mbosch.me>
Date: Thu, 7 Mar 2024 18:44:13 +0100
Subject: [PATCH 3/5] Use `submit` event in login form

It's a pet peeve from me when logging into my personal Hydra that I
always have to press the button rather than hitting Return after entering
my password.

Reason for that is that the form doesn't have a "submit" button, so far
it was always listened to the "click" event. Submit does that and you
can hit Return alternatively.
---
 src/root/auth.tt | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/root/auth.tt b/src/root/auth.tt
index d1539765..d49ba5bd 100644
--- a/src/root/auth.tt
+++ b/src/root/auth.tt
@@ -33,7 +33,7 @@
   <div id="hydra-signin" class="modal hide fade" tabindex="-1" role="dialog" aria-hidden="true">
     <div class="modal-dialog" role="document">
       <div class="modal-content">
-        <form>
+        <form id="signin-form">
           <div class="modal-body">
             <div class="form-group">
               <label for="username" class="col-form-label">User name</label>
@@ -45,7 +45,7 @@
             </div>
           </div>
           <div class="modal-footer">
-            <button id="do-signin" type="button" class="btn btn-primary">Sign in</button>
+            <button type="submit" class="btn btn-primary">Sign in</button>
             <button type="button" class="btn btn-secondary" data-dismiss="modal">Cancel</button>
           </div>
         </form>
@@ -57,10 +57,11 @@
 
     function finishSignOut() { }
 
-    $("#do-signin").click(function() {
+    $("#signin-form").submit(function(e) {
+      e.preventDefault();
       requestJSON({
         url: "[% c.uri_for('/login') %]",
-        data: $(this).parents("form").serialize(),
+        data: $(this).serialize(),
         type: 'POST',
         success: function(data) {
           window.location.reload();

From 806c375c338b4e6a1d276b96994018908784bf11 Mon Sep 17 00:00:00 2001
From: Maximilian Bosch <maximilian@mbosch.me>
Date: Fri, 8 Mar 2024 11:07:38 +0100
Subject: [PATCH 4/5] Don't send gitea status update when build is started

This was the source of a flaky test because sometimes hydra-notify was
quick enough to send out `buildStarted` and sometimes it apparently
wasn't which was quickly spottable with `nix build --rebuild`.

Removing that status update doesn't make a difference functionally,
gitea doesn't differentiate between "queued" and "running", so we send
the same status ("pending") out on both events, so we'd even safe one
avoidable request.
---
 flake.nix                           | 5 ++---
 src/lib/Hydra/Plugin/GiteaStatus.pm | 4 ----
 2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/flake.nix b/flake.nix
index a6dfb977..31a38416 100644
--- a/flake.nix
+++ b/flake.nix
@@ -353,10 +353,9 @@
 
                 response = json.loads(data)
 
-                assert len(response) == 3, "Expected exactly three status updates for latest commit (queued, started, finished)!"
+                assert len(response) == 2, "Expected exactly three status updates for latest commit (queued, finished)!"
                 assert response[0]['status'] == "success", "Expected finished status to be success!"
-                assert response[1]['status'] == "pending", "Expected started status to be pending!"
-                assert response[2]['status'] == "pending", "Expected queued status to be pending!"
+                assert response[1]['status'] == "pending", "Expected queued status to be pending!"
 
                 machine.shutdown()
               '';
diff --git a/src/lib/Hydra/Plugin/GiteaStatus.pm b/src/lib/Hydra/Plugin/GiteaStatus.pm
index 426c93f5..f3498941 100644
--- a/src/lib/Hydra/Plugin/GiteaStatus.pm
+++ b/src/lib/Hydra/Plugin/GiteaStatus.pm
@@ -88,10 +88,6 @@ sub buildQueued {
     common(@_, [], 0);
 }
 
-sub buildStarted {
-    common(@_, [], 1);
-}
-
 sub buildFinished {
     common(@_, 2);
 }

From 99afff03b06d6b9a3d464ede9724e29a03e91329 Mon Sep 17 00:00:00 2001
From: Maximilian Bosch <maximilian@mbosch.me>
Date: Thu, 14 Mar 2024 22:47:37 +0100
Subject: [PATCH 5/5] hydra-queue-runner: drop broken connections from pool

Closes #1336

When restarting postgresql, the connections are still reused in
`hydra-queue-runner` causing errors like this

    main thread: Lost connection to the database server.
    queue monitor: Lost connection to the database server.

and no more builds being processed.

`hydra-evaluator` doesn't have that issue since it crashes right away.
We could let it retry indefinitely as well (see below), but I don't
want to change too much.

If the DB is still unreachable 10s later, the process will stop with a
non-zero exit code because of a missing DB connection. This however
isn't such a big deal because it will be immediately restarted
afterwards. With the current configuration, Hydra will never give up,
but restart (and retry) infinitely. To me that seems reasonable, i.e. to
retry DB connections on a long-running process. If this doesn't work
out, the monitoring should fire anyways because the queue fills up, but
I'm open to discuss that.

Please note that this isn't reproducible with the DB and the queue
runner on the same machine when using `services.hydra-dev`, because of
the `Requires=` dependency `hydra-queue-runner.service` ->
`hydra-init.service` -> `postgresql.service` that causes the queue
runner to be restarted on `systemctl restart postgresql`.

Internally, Hydra uses Nix's pool data structure: it basically has N
slots (here DB connections) and whenever a new one is requested, an idle
slot is provided or a new one is created (when N slots are active, it'll
be waited until one slot is free). The issue in the code here is however
that whenever an error is encountered, the slot is released, however the
same broken connection will be reused the next time. By using
`Pool::Handle::markBad`, Nix will drop a broken slot. This is now being
done when `pqxx::broken_connection` was caught.
---
 src/hydra-queue-runner/hydra-queue-runner.cc | 15 ++++++---
 src/hydra-queue-runner/queue-monitor.cc      | 34 +++++++++++---------
 src/hydra-queue-runner/state.hh              |  2 +-
 3 files changed, 31 insertions(+), 20 deletions(-)

diff --git a/src/hydra-queue-runner/hydra-queue-runner.cc b/src/hydra-queue-runner/hydra-queue-runner.cc
index 0ee710cb..5ffa7fe6 100644
--- a/src/hydra-queue-runner/hydra-queue-runner.cc
+++ b/src/hydra-queue-runner/hydra-queue-runner.cc
@@ -926,10 +926,17 @@ void State::run(BuildID buildOne)
     while (true) {
         try {
             auto conn(dbPool.get());
-            receiver dumpStatus_(*conn, "dump_status");
-            while (true) {
-                conn->await_notification();
-                dumpStatus(*conn);
+            try {
+                receiver dumpStatus_(*conn, "dump_status");
+                while (true) {
+                    conn->await_notification();
+                    dumpStatus(*conn);
+                }
+            } catch (pqxx::broken_connection & connEx) {
+                printMsg(lvlError, "main thread: %s", connEx.what());
+                printMsg(lvlError, "main thread: Reconnecting in 10s");
+                conn.markBad();
+                sleep(10);
             }
         } catch (std::exception & e) {
             printMsg(lvlError, "main thread: %s", e.what());
diff --git a/src/hydra-queue-runner/queue-monitor.cc b/src/hydra-queue-runner/queue-monitor.cc
index 203f9f1d..1bd51f5a 100644
--- a/src/hydra-queue-runner/queue-monitor.cc
+++ b/src/hydra-queue-runner/queue-monitor.cc
@@ -10,8 +10,14 @@ using namespace nix;
 void State::queueMonitor()
 {
     while (true) {
+        auto conn(dbPool.get());
         try {
-            queueMonitorLoop();
+            queueMonitorLoop(*conn);
+        } catch (pqxx::broken_connection & e) {
+            printMsg(lvlError, "queue monitor: %s", e.what());
+            printMsg(lvlError, "queue monitor: Reconnecting in 10s");
+            conn.markBad();
+            sleep(10);
         } catch (std::exception & e) {
             printError("queue monitor: %s", e.what());
             sleep(10); // probably a DB problem, so don't retry right away
@@ -20,16 +26,14 @@ void State::queueMonitor()
 }
 
 
-void State::queueMonitorLoop()
+void State::queueMonitorLoop(Connection & conn)
 {
-    auto conn(dbPool.get());
-
-    receiver buildsAdded(*conn, "builds_added");
-    receiver buildsRestarted(*conn, "builds_restarted");
-    receiver buildsCancelled(*conn, "builds_cancelled");
-    receiver buildsDeleted(*conn, "builds_deleted");
-    receiver buildsBumped(*conn, "builds_bumped");
-    receiver jobsetSharesChanged(*conn, "jobset_shares_changed");
+    receiver buildsAdded(conn, "builds_added");
+    receiver buildsRestarted(conn, "builds_restarted");
+    receiver buildsCancelled(conn, "builds_cancelled");
+    receiver buildsDeleted(conn, "builds_deleted");
+    receiver buildsBumped(conn, "builds_bumped");
+    receiver jobsetSharesChanged(conn, "jobset_shares_changed");
 
     auto destStore = getDestStore();
 
@@ -39,17 +43,17 @@ void State::queueMonitorLoop()
     while (!quit) {
         localStore->clearPathInfoCache();
 
-        bool done = getQueuedBuilds(*conn, destStore, lastBuildId);
+        bool done = getQueuedBuilds(conn, destStore, lastBuildId);
 
         if (buildOne && buildOneDone) quit = true;
 
         /* Sleep until we get notification from the database about an
            event. */
         if (done && !quit) {
-            conn->await_notification();
+            conn.await_notification();
             nrQueueWakeups++;
         } else
-            conn->get_notifs();
+            conn.get_notifs();
 
         if (auto lowestId = buildsAdded.get()) {
             lastBuildId = std::min(lastBuildId, static_cast<unsigned>(std::stoul(*lowestId) - 1));
@@ -61,11 +65,11 @@ void State::queueMonitorLoop()
         }
         if (buildsCancelled.get() || buildsDeleted.get() || buildsBumped.get()) {
             printMsg(lvlTalkative, "got notification: builds cancelled or bumped");
-            processQueueChange(*conn);
+            processQueueChange(conn);
         }
         if (jobsetSharesChanged.get()) {
             printMsg(lvlTalkative, "got notification: jobset shares changed");
-            processJobsetSharesChange(*conn);
+            processJobsetSharesChange(conn);
         }
     }
 
diff --git a/src/hydra-queue-runner/state.hh b/src/hydra-queue-runner/state.hh
index cda238ae..5d242cdf 100644
--- a/src/hydra-queue-runner/state.hh
+++ b/src/hydra-queue-runner/state.hh
@@ -498,7 +498,7 @@ private:
 
     void queueMonitor();
 
-    void queueMonitorLoop();
+    void queueMonitorLoop(Connection & conn);
 
     /* Check the queue for new builds. */
     bool getQueuedBuilds(Connection & conn,