restore: fix skip_localities_check

Previously, there was no additional validation for
generating zone configurations during restore, so nothing
bad would happen if regions were missing during restore.
Later on, the zone config generation process had validation
to ensure all regions existed. This was problematic for
restores with skip_localities_check since we would still
incorrectly check localities.
To address this, this patch adds an option to skip validation
of regions when generating zone configurations.

Fixes: #100913

Release note (bug fix): Previously, RESTORE with skip_localities_check
would still fail with errors if regions were missing on a cluster.

pkg/ccl/backupccl/restore_job.go

@@ -1031,6 +1031,7 @@ func createImportingDescriptors(
 								txn,
 								p.ExecCfg(),
 								descsCol,
+								!details.SkipLocalitiesCheck,
 							); err != nil {
 								return err
 							}

pkg/ccl/backupccl/restore_planning.go

@@ -1862,10 +1862,11 @@ func doRestorePlan(
 		// compatability.
 		//
 		// TODO(msbutler): Delete in 23.1
-		RestoreSystemUsers: restoreStmt.DescriptorCoverage == tree.SystemUsers,
-		PreRewriteTenantId: oldTenantID,
-		SchemaOnly:         restoreStmt.Options.SchemaOnly,
-		VerifyData:         restoreStmt.Options.VerifyData,
+		RestoreSystemUsers:  restoreStmt.DescriptorCoverage == tree.SystemUsers,
+		PreRewriteTenantId:  oldTenantID,
+		SchemaOnly:          restoreStmt.Options.SchemaOnly,
+		VerifyData:          restoreStmt.Options.VerifyData,
+		SkipLocalitiesCheck: restoreStmt.Options.SkipLocalitiesCheck,
 	}
 
 	jr := jobs.Record{

pkg/ccl/backupccl/testdata/backup-restore/multiregion

@@ -63,6 +63,26 @@ RESTORE FROM LATEST IN 'nodelocal://0/full_cluster_backup/';
 pq: detected a mismatch in regions between the restore cluster and the backup cluster, missing regions detected: us-east-1, us-west-1.
 HINT: there are two ways you can resolve this issue: 1) update the cluster to which you're restoring to ensure that the regions present on the nodes' --locality flags match those present in the backup image, or 2) restore with the "skip_localities_check" option
 
+exec-sql
+RESTORE FROM LATEST IN 'nodelocal://1/full_cluster_backup/' WITH skip_localities_check;
+----
+
+exec-sql
+RESTORE DATABASE d FROM LATEST IN 'nodelocal://1/database_backup/' WITH skip_localities_check, new_db_name='d_new';
+----
+
+exec-sql
+DROP DATABASE d_new;
+----
+
+exec-sql
+DROP DATABASE d;
+----
+
+exec-sql
+DROP DATABASE data;
+----
+
 # Create a database with no regions to check default primary regions.
 exec-sql
 CREATE DATABASE no_region_db;

pkg/jobs/jobspb/jobs.proto

@@ -76,16 +76,16 @@ message StreamIngestionDetails {
   // StreamAddress locates the stream. It enables the client to find the
   // addresses of the stream's partitions.
   string stream_address = 1;
-  
+
   uint64 stream_id = 4 [(gogoproto.customname) = "StreamID"];
-  
+
   // Span is the keyspan into which this job will ingest KVs.
   //
   // The stream should emit all changes for a given span, and no changes outside
   // a span. Note that KVs received from the stream may need to be re-keyed into
   // this span.
   roachpb.Span span = 2 [(gogoproto.nullable) = false];
-  
+
   reserved 5;
   roachpb.TenantID tenant_id = 6 [(gogoproto.customname) = "TenantID", (gogoproto.nullable) = false];
 
@@ -267,7 +267,7 @@ message BackupProgress {
 }
 
 // DescriptorRewrite specifies a remapping from one descriptor ID to another for
-// use in rewritting descriptors themselves or things that reference them such 
+// use in rewritting descriptors themselves or things that reference them such
 // as is done during RESTORE or IMPORT.
 message DescriptorRewrite {
   uint32 id = 1 [
@@ -390,7 +390,18 @@ message RestoreDetails {
 
   bool VerifyData = 26;
 
-  // NEXT ID: 28.
+
+  // ProtectedTimestampRecord is the ID of the protected timestamp record
+  // corresponding to this job.
+  bytes protected_timestamp_record = 28 [
+    (gogoproto.customname) = "ProtectedTimestampRecord",
+    (gogoproto.customtype) = "github.com/cockroachdb/cockroach/pkg/util/uuid.UUID"
+  ];
+
+  // Disables loacality checking for zone configs.
+  bool SkipLocalitiesCheck = 29;
+
+  // NEXT ID: 30.
 }
 
 

pkg/sql/alter_database.go

@@ -790,6 +790,7 @@ func (n *alterDatabasePrimaryRegionNode) switchPrimaryRegion(params runParams) e
 		params.p.txn,
 		params.p.execCfg,
 		params.p.Descriptors(),
+		true, /* validateLocalities */
 	); err != nil {
 		return err
 	}
@@ -1165,6 +1166,7 @@ func (n *alterDatabaseSurvivalGoalNode) startExec(params runParams) error {
 		params.p.txn,
 		params.p.execCfg,
 		params.p.Descriptors(),
+		true, /* validateLocalities */
 	); err != nil {
 		return err
 	}
@@ -1295,6 +1297,7 @@ func (n *alterDatabasePlacementNode) startExec(params runParams) error {
 		params.p.txn,
 		params.p.execCfg,
 		params.p.Descriptors(),
+		true, /* validateLocalities */
 	); err != nil {
 		return err
 	}
@@ -1839,6 +1842,7 @@ func (n *alterDatabaseSecondaryRegion) startExec(params runParams) error {
 		params.p.txn,
 		params.p.execCfg,
 		params.p.Descriptors(),
+		true, /* validateLocalities */
 	); err != nil {
 		return err
 	}
@@ -1946,6 +1950,7 @@ func (n *alterDatabaseDropSecondaryRegion) startExec(params runParams) error {
 		params.p.txn,
 		params.p.execCfg,
 		params.p.Descriptors(),
+		true, /* validateLocalities */
 	); err != nil {
 		return err
 	}
@@ -2219,7 +2224,7 @@ func (n *alterDatabaseSetZoneConfigExtensionNode) startExec(params runParams) er
 
 	// Validate if the zone config extension is compatible with the database.
 	dbZoneConfig, err := generateAndValidateZoneConfigForMultiRegionDatabase(
-		params.ctx, params.ExecCfg(), updatedRegionConfig,
+		params.ctx, params.ExecCfg(), updatedRegionConfig, true, /* validateLocalities */
 	)
 	if err != nil {
 		return err

pkg/sql/database_region_change_finalizer.go

@@ -211,6 +211,7 @@ func (r *databaseRegionChangeFinalizer) updateDatabaseZoneConfig(
 		txn,
 		r.localPlanner.ExecCfg(),
 		r.localPlanner.Descriptors(),
+		true, /* validateLocalities */
 	)
 }
 

pkg/sql/region_util.go

@@ -33,6 +33,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
+	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/errors"
 	"github.com/gogo/protobuf/proto"
 )
@@ -877,7 +878,10 @@ func ApplyZoneConfigForMultiRegionTable(
 // generateAndValidateZoneConfigForMultiRegionDatabase returns a validated
 // zone config generated by the region config.
 func generateAndValidateZoneConfigForMultiRegionDatabase(
-	ctx context.Context, execConfig *ExecutorConfig, regionConfig multiregion.RegionConfig,
+	ctx context.Context,
+	execConfig *ExecutorConfig,
+	regionConfig multiregion.RegionConfig,
+	validateLocalities bool,
 ) (zonepb.ZoneConfig, error) {
 	// Build a zone config based on the RegionConfig information.
 	dbZoneConfig := zonepb.ZoneConfig{}
@@ -898,7 +902,13 @@ func generateAndValidateZoneConfigForMultiRegionDatabase(
 	}
 
 	if err := validateZoneAttrsAndLocalities(ctx, execConfig, &dbZoneConfig); err != nil {
-		return zonepb.ZoneConfig{}, err
+		// If we are validating localities this is fatal, otherwise let's log any
+		// errors as warnings.
+		if validateLocalities {
+			return zonepb.ZoneConfig{}, err
+		}
+		log.Warningf(ctx, "ignoring locality validation error for DB zone config %v", err)
+		err = nil
 	}
 
 	return dbZoneConfig, nil
@@ -913,9 +923,10 @@ func ApplyZoneConfigFromDatabaseRegionConfig(
 	txn *kv.Txn,
 	execConfig *ExecutorConfig,
 	descriptors *descs.Collection,
+	validateLocalities bool,
 ) error {
 	// Build a zone config based on the RegionConfig information.
-	dbZoneConfig, err := generateAndValidateZoneConfigForMultiRegionDatabase(ctx, execConfig, regionConfig)
+	dbZoneConfig, err := generateAndValidateZoneConfigForMultiRegionDatabase(ctx, execConfig, regionConfig, validateLocalities)
 	if err != nil {
 		return err
 	}
@@ -1152,7 +1163,9 @@ func (p *planner) maybeInitializeMultiRegionDatabase(
 		*regionConfig,
 		p.txn,
 		p.execCfg,
-		p.Descriptors()); err != nil {
+		p.Descriptors(),
+		true, /* validateLocalities */
+	); err != nil {
 		return err
 	}
 
@@ -1293,6 +1306,7 @@ func (p *planner) ResetMultiRegionZoneConfigsForDatabase(ctx context.Context, id
 		p.txn,
 		p.execCfg,
 		p.Descriptors(),
+		true, /* validateLocalities */
 	); err != nil {
 		return err
 	}