Merge #153907

153907: backup: teach ContainsManifest to use the up to date manifest file r=kev-cao a=msbutler

In addition, this patch adds some todos on the plan to delete the deprecated
backup manifest.

Informs #139159

Release note: none

Co-authored-by: Michael Butler <[email protected]>

Author: craig[bot]

pkg/backup/backup_job.go

@@ -428,7 +428,10 @@ func backup(
 	}
 
 	// TODO(msbutler): version gate writing the old manifest once we can guarantee
-	// a cluster version that will not read the old manifest.
+	// a cluster version that will not read the old manifest. This will occur when we delete
+	// LegacyFindPriorBackups and the fallback path in
+	// ListFullBackupsInCollection, which can occur when we completely rely on the
+	// backup index.
 	if err := backupinfo.WriteBackupManifest(ctx, defaultStore, backupbase.DeprecatedBackupManifestName,
 		encryption, &kmsEnv, backupManifest); err != nil {
 		return roachpb.RowCount{}, 0, err

pkg/backup/backupdest/backup_destination.go

@@ -52,7 +52,7 @@ const (
 // On some cloud storage platforms (i.e. GS, S3), backups in a base bucket may
 // omit a leading slash. However, backups in a subdirectory of a base bucket
 // will contain one.
-var backupPathRE = regexp.MustCompile("^/?[^\\/]+/[^\\/]+/[^\\/]+/" + backupbase.DeprecatedBackupManifestName + "$")
+var deprecatedBackupPathRE = regexp.MustCompile("^/?[^\\/]+/[^\\/]+/[^\\/]+/" + backupbase.DeprecatedBackupManifestName + "$")
 
 // ResolvedDestination encapsulates information that is populated while
 // resolving the destination of a backup.
@@ -483,7 +483,7 @@ func ListFullBackupsInCollection(
 
 	var backupPaths []string
 	if err := store.List(ctx, "", backupbase.ListingDelimDataSlash, func(f string) error {
-		if backupPathRE.MatchString(f) {
+		if deprecatedBackupPathRE.MatchString(f) {
 			backupPaths = append(backupPaths, f)
 		}
 		return nil

pkg/backup/backupdest/incrementals.go

@@ -122,6 +122,9 @@ func FindAllIncrementalPaths(
 //
 // Note: store should be rooted at the directory containing the incremental
 // backups (i.e. gs://my-bucket/backup/incrementals/2025/07/29-123456.00/)
+//
+// TODO (msbutler): we didn't bother to update this to use the up to date
+// manifest as this will eventually be deleted when all reads use the index.
 func LegacyFindPriorBackups(
 	ctx context.Context, store cloud.ExternalStorage, includeManifest bool,
 ) ([]string, error) {

pkg/backup/backupinfo/manifest_handling.go

@@ -169,15 +169,22 @@ func ReadBackupManifestFromStore(
 }
 
 func ContainsManifest(ctx context.Context, exportStore cloud.ExternalStorage) (bool, error) {
-	r, _, err := exportStore.ReadFile(ctx, backupbase.DeprecatedBackupManifestName, cloud.ReadOptions{NoFileSize: true})
-	if err != nil {
-		if errors.Is(err, cloud.ErrFileDoesNotExist) {
-			return false, nil
+	containsManifestWithFilename := func(filename string) (bool, error) {
+		r, _, err := exportStore.ReadFile(ctx, filename, cloud.ReadOptions{NoFileSize: true})
+		if err != nil {
+			if errors.Is(err, cloud.ErrFileDoesNotExist) {
+				return false, nil
+			}
+			return false, err
 		}
-		return false, err
+		r.Close(ctx)
+		return true, nil
+	}
+	exists, err := containsManifestWithFilename(backupbase.BackupMetadataName)
+	if exists || err != nil {
+		return exists, err
 	}
-	r.Close(ctx)
-	return true, nil
+	return containsManifestWithFilename(backupbase.DeprecatedBackupManifestName)
 }
 
 // compressData compresses data buffer and returns compressed
@@ -1310,13 +1317,13 @@ func CheckForPreviousBackup(
 	exists, err := ContainsManifest(ctx, defaultStore)
 	if err != nil {
 		return errors.Wrapf(err,
-			"%s returned an unexpected error when checking for the existence of %s file",
-			redactedURI, backupbase.DeprecatedBackupManifestName)
+			"%s returned an unexpected error when checking for the existence of manifest",
+			redactedURI)
 	}
 	if exists {
 		return pgerror.Newf(pgcode.FileAlreadyExists,
-			"%s already contains a %s file",
-			redactedURI, backupbase.DeprecatedBackupManifestName)
+			"%s already contains a manifest",
+			redactedURI)
 	}
 
 	// Check for the presence of a BACKUP-LOCK file with a job ID different from