fix(migration): resume the settings reshape after a partial run on mysql

MySQL implicitly commits around every ALTER TABLE, so the transaction
wrapping the reshape is real only on sqlite and postgres. On MySQL an
interrupted run could stop between the two legacy column drops, and the
rerun keyed its already-migrated check on the cluster column alone --
leaving the subdomain column behind forever.

Key the entry check on either legacy column remaining, run the backfill
and its loud-failure check only while the cluster column exists (they
provably completed before any drop), and skip drops for columns already
gone. No data-loss window existed in any state: the backfill commits
before the first drop on every engine.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

management/server/migration/migration_agentnetwork.go

@@ -33,6 +33,15 @@ func (agentNetworkSettingsMigration) TableName() string { return "agent_network_
 // bootstrap always wrote both, so such a row indicates corruption and the
 // migration fails loudly rather than leaving an empty domain to collide with
 // the unique index confusingly.
+//
+// The transaction is real only on sqlite and postgres, where DDL is
+// transactional. MySQL implicitly commits around every ALTER TABLE, so there
+// each step stands alone; what makes an interrupted run resumable on MySQL is
+// that every step is guarded by the schema state it changes — the entry check
+// fires while either legacy column remains, the adds skip existing columns,
+// the backfill and its loud-failure check run only while the legacy cluster
+// column exists (they provably completed before any drop), and each drop
+// skips what is already gone.
 func MigrateAgentNetworkSettingsToDomain(ctx context.Context, db *gorm.DB) error {
 	model := &agentNetworkSettingsMigration{}
 	migrator := db.Migrator()
@@ -40,7 +49,8 @@ func MigrateAgentNetworkSettingsToDomain(ctx context.Context, db *gorm.DB) error
 	if !migrator.HasTable(model) {
 		return nil
 	}
-	if !migrator.HasColumn(model, "cluster") {
+	hasCluster := migrator.HasColumn(model, "cluster")
+	if !hasCluster && !migrator.HasColumn(model, "subdomain") {
 		// Fresh schema or already migrated — nothing to reshape.
 		return nil
 	}
@@ -55,27 +65,33 @@ func MigrateAgentNetworkSettingsToDomain(ctx context.Context, db *gorm.DB) error
 			}
 		}
 
-		concat := "subdomain || '.' || cluster"
-		if tx.Name() == "mysql" {
-			concat = "CONCAT(subdomain, '.', cluster)"
-		}
-		res := tx.Exec(fmt.Sprintf(
-			"UPDATE agent_network_settings SET domain = %s, proxy_address = cluster WHERE (domain IS NULL OR domain = '') AND cluster <> '' AND subdomain <> ''",
-			concat,
-		))
-		if res.Error != nil {
-			return fmt.Errorf("backfill agent_network_settings domain: %w", res.Error)
-		}
+		if hasCluster {
+			concat := "subdomain || '.' || cluster"
+			if tx.Name() == "mysql" {
+				concat = "CONCAT(subdomain, '.', cluster)"
+			}
+			res := tx.Exec(fmt.Sprintf(
+				"UPDATE agent_network_settings SET domain = %s, proxy_address = cluster WHERE (domain IS NULL OR domain = '') AND cluster <> '' AND subdomain <> ''",
+				concat,
+			))
+			if res.Error != nil {
+				return fmt.Errorf("backfill agent_network_settings domain: %w", res.Error)
+			}
 
-		var unmigratable int64
-		if err := tx.Model(model).Where("domain IS NULL OR domain = ''").Count(&unmigratable).Error; err != nil {
-			return fmt.Errorf("count unmigratable agent_network_settings rows: %w", err)
-		}
-		if unmigratable > 0 {
-			return fmt.Errorf(
-				"%d agent_network_settings row(s) have no cluster/subdomain to derive an endpoint from; resolve them manually before upgrading",
-				unmigratable,
-			)
+			var unmigratable int64
+			if err := tx.Model(model).Where("domain IS NULL OR domain = ''").Count(&unmigratable).Error; err != nil {
+				return fmt.Errorf("count unmigratable agent_network_settings rows: %w", err)
+			}
+			if unmigratable > 0 {
+				return fmt.Errorf(
+					"%d agent_network_settings row(s) have no cluster/subdomain to derive an endpoint from; resolve them manually before upgrading",
+					unmigratable,
+				)
+			}
+
+			if res.RowsAffected > 0 {
+				log.WithContext(ctx).Infof("migrated %d agent_network_settings row(s) to domain/proxy_address", res.RowsAffected)
+			}
 		}
 
 		if txMigrator.HasIndex(model, "idx_agent_network_settings_cluster_subdomain") {
@@ -84,14 +100,13 @@ func MigrateAgentNetworkSettingsToDomain(ctx context.Context, db *gorm.DB) error
 			}
 		}
 		for _, field := range []string{"Cluster", "Subdomain"} {
-			if err := txMigrator.DropColumn(model, field); err != nil {
-				return fmt.Errorf("drop legacy agent_network_settings column %s: %w", field, err)
+			if txMigrator.HasColumn(model, field) {
+				if err := txMigrator.DropColumn(model, field); err != nil {
+					return fmt.Errorf("drop legacy agent_network_settings column %s: %w", field, err)
+				}
 			}
 		}
 
-		if res.RowsAffected > 0 {
-			log.WithContext(ctx).Infof("migrated %d agent_network_settings row(s) to domain/proxy_address", res.RowsAffected)
-		}
 		return nil
 	})
 }

management/server/migration/migration_test.go

@@ -818,3 +818,43 @@ func TestMigrateAgentNetworkSettingsToDomain_FailsOnUnmigratableRow(t *testing.T
 	require.Error(t, err, "a row with no identity to derive an endpoint from must fail the migration")
 	assert.Contains(t, err.Error(), "resolve them manually", "the error must tell the operator what to do")
 }
+
+// partialAgentNetworkSettings models the one non-atomic state a MySQL run can
+// be interrupted in: DDL auto-commits there, so a crash between the two legacy
+// column drops leaves subdomain behind while cluster (and the completed
+// backfill) are already committed.
+type partialAgentNetworkSettings struct {
+	AccountID    string `gorm:"primaryKey"`
+	Subdomain    string
+	Domain       string `gorm:"type:varchar(255)"`
+	ProxyAddress string `gorm:"type:varchar(255)"`
+}
+
+func (partialAgentNetworkSettings) TableName() string { return "agent_network_settings" }
+
+// TestMigrateAgentNetworkSettingsToDomain_ResumesAfterPartialDrop pins MySQL
+// resumability: a rerun over the interrupted state must remove the leftover
+// subdomain column without re-running the backfill (the cluster column that
+// feeds it is gone) and without touching the migrated values.
+func TestMigrateAgentNetworkSettingsToDomain_ResumesAfterPartialDrop(t *testing.T) {
+	ctx := context.Background()
+	db := setupDatabase(t)
+	require.NoError(t, db.Migrator().DropTable(&partialAgentNetworkSettings{}))
+	require.NoError(t, db.AutoMigrate(&partialAgentNetworkSettings{}))
+	require.NoError(t, db.Create(&partialAgentNetworkSettings{
+		AccountID: "acct-1", Subdomain: "violet",
+		Domain: "violet.eu.proxy.netbird.io", ProxyAddress: "eu.proxy.netbird.io",
+	}).Error)
+
+	require.NoError(t, migration.MigrateAgentNetworkSettingsToDomain(ctx, db),
+		"a rerun over a partially-dropped schema must resume, not error")
+
+	migrator := db.Migrator()
+	assert.False(t, migrator.HasColumn(&partialAgentNetworkSettings{}, "subdomain"),
+		"the leftover legacy column must be dropped on resume")
+
+	var row agentNetworkTypes.Settings
+	require.NoError(t, db.First(&row, "account_id = ?", "acct-1").Error)
+	assert.Equal(t, "violet.eu.proxy.netbird.io", row.Domain, "migrated values must be untouched")
+	assert.Equal(t, "eu.proxy.netbird.io", row.ProxyAddress, "migrated values must be untouched")
+}