From 7b57ab21cb9be3bc0df946b77769f363a1118eef Mon Sep 17 00:00:00 2001
From: nexxo <nexxo@clupilot.local>
Date: Sun, 26 Jul 2026 00:01:16 +0200
Subject: [PATCH] fix(hosts): retry a failed DNS registration before advancing
 without a name

The failure may be a lost response to a request that did create the record.
Advancing straight away stored no id, so PurgeHost could never remove it and the
host's management address stayed published. The upsert is idempotent, so a retry
recovers the id; only after the attempts run out does the onboarding continue
without a name.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 app/Provisioning/Steps/Host/RegisterHostDns.php | 11 +++++++++++
 tests/Feature/Provisioning/HostStepsTest.php    | 17 ++++++++++++-----
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/app/Provisioning/Steps/Host/RegisterHostDns.php b/app/Provisioning/Steps/Host/RegisterHostDns.php
index 665da3b..50a82b3 100644
--- a/app/Provisioning/Steps/Host/RegisterHostDns.php
+++ b/app/Provisioning/Steps/Host/RegisterHostDns.php
@@ -47,6 +47,17 @@ class RegisterHostDns extends HostStep
         try {
             $recordId = $this->dns->upsertRecord($fqdn, 'A', $host->wg_ip);
         } catch (Throwable $e) {
+            // Retried first, because the failure may be a lost response to a
+            // request that DID create the record: advancing then leaves a
+            // published address with no id, so PurgeHost could never remove it.
+            // The upsert is idempotent, so a retry recovers the id.
+            $allowed = min(3, (int) $run->max_attempts);
+            if ($run->attempt + 1 < $allowed) {
+                return StepResult::retry(30, 'DNS unavailable: '.$e->getMessage());
+            }
+
+            // Out of attempts: a name is convenience, and a host that is
+            // otherwise ready should not be stranded for the want of one.
             $run->events()->create([
                 'step' => $this->key(),
                 'outcome' => 'info',
diff --git a/tests/Feature/Provisioning/HostStepsTest.php b/tests/Feature/Provisioning/HostStepsTest.php
index 8f94c79..7ff769c 100644
--- a/tests/Feature/Provisioning/HostStepsTest.php
+++ b/tests/Feature/Provisioning/HostStepsTest.php
@@ -287,15 +287,22 @@ it('numbers hosts per datacenter and never reuses a number', function () {
     expect($host->fresh()->dns_name)->toBe('fsn-03');
 });
 
-it('does not strand an onboarding because DNS was unreachable', function () {
+it('retries a failed DNS registration before giving up on the name', function () {
     $s = fakeServices();
     $s['dns']->failUpsert = true;
     $host = App\Models\Host::factory()->active()->create(['datacenter' => 'fsn', 'wg_ip' => '10.66.0.40', 'dns_name' => null]);
-    $run = App\Models\ProvisioningRun::factory()->forHost($host)->create(["pipeline" => "host"]);
+    $run = App\Models\ProvisioningRun::factory()->forHost($host)->create(['pipeline' => 'host', 'attempt' => 0]);
 
-    // A name is convenience; the host works without it.
-    expect((new App\Provisioning\Steps\Host\RegisterHostDns($s['dns']))->execute($run)->type)->toBe("advance");
-    expect($run->events()->where('step', 'register_host_dns')->exists())->toBeTrue();
+    $step = new App\Provisioning\Steps\Host\RegisterHostDns($s['dns']);
+
+    // The failure may be a lost response to a request that did create the
+    // record — advancing straight away would orphan it.
+    expect($step->execute($run)->type)->toBe('retry');
+
+    // Out of attempts, the host still finishes: a name is convenience.
+    $run->update(['attempt' => 5]);
+    expect($step->execute($run)->type)->toBe('advance')
+        ->and($run->events()->where('step', 'register_host_dns')->exists())->toBeTrue();
 });
 
 it('takes the host DNS record with it when the host is purged', function () {