backupccl: remove support for pre-22.1 backups

pkg/ccl/backupccl/backup_test.go

@@ -53,7 +53,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/cloud/cloudpb"
 	"github.com/cockroachdb/cockroach/pkg/cloud/gcp"
 	_ "github.com/cockroachdb/cockroach/pkg/cloud/impl" // register cloud storage providers
-	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/config"
 	"github.com/cockroachdb/cockroach/pkg/config/zonepb"
 	"github.com/cockroachdb/cockroach/pkg/jobs"
@@ -67,7 +66,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/security"
 	"github.com/cockroachdb/cockroach/pkg/security/username"
-	"github.com/cockroachdb/cockroach/pkg/server"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/spanconfig"
 	"github.com/cockroachdb/cockroach/pkg/sql"
@@ -10150,67 +10148,6 @@ func TestBackupNoOverwriteLatest(t *testing.T) {
 	require.NotEqual(t, firstLatest, thirdLatest)
 }
 
-// TestBackupLatestInBaseDirectory tests to see that a LATEST
-// file in the base directory can be properly read when one is not found
-// in metadata/latest. This can occur when an older version node creates
-// the backup.
-func TestBackupLatestInBaseDirectory(t *testing.T) {
-	defer leaktest.AfterTest(t)()
-	defer log.Scope(t).Close(t)
-
-	disableUpgradeCh := make(chan struct{})
-	const numAccounts = 1
-	const userfile = "'userfile:///a'"
-	args := base.TestClusterArgs{
-		ServerArgs: base.TestServerArgs{
-			Knobs: base.TestingKnobs{
-				Server: &server.TestingKnobs{
-					BinaryVersionOverride:          clusterversion.ByKey(clusterversion.BackupDoesNotOverwriteLatestAndCheckpoint - 1),
-					DisableAutomaticVersionUpgrade: disableUpgradeCh,
-				},
-			},
-		},
-	}
-
-	tc, sqlDB, _, cleanupFn := backupRestoreTestSetupWithParams(t, singleNode, numAccounts, InitManualReplication, args)
-	defer cleanupFn()
-	execCfg := tc.Server(0).ExecutorConfig().(sql.ExecutorConfig)
-	ctx := context.Background()
-	store, err := execCfg.DistSQLSrv.ExternalStorageFromURI(ctx, "userfile:///a", username.RootUserName())
-	require.NoError(t, err)
-
-	query := fmt.Sprintf("BACKUP INTO %s", userfile)
-	sqlDB.Exec(t, query)
-
-	// Confirm that the LATEST file was written to the base directory.
-	r, err := store.ReadFile(ctx, backupbase.LatestFileName)
-	require.NoError(t, err)
-	r.Close(ctx)
-
-	// Close the channel so that the cluster version is upgraded.
-	close(disableUpgradeCh)
-	// Check the cluster version is bumped to newVersion.
-	testutils.SucceedsSoon(t, func() error {
-		var version string
-		sqlDB.QueryRow(t, "SELECT value FROM system.settings WHERE name = 'version'").Scan(&version)
-		var v clusterversion.ClusterVersion
-		if err := protoutil.Unmarshal([]byte(version), &v); err != nil {
-			return err
-		}
-		version = v.String()
-		if version != clusterversion.TestingBinaryVersion.String() {
-			return errors.Errorf("cluster version is still %s, should be %s", version, clusterversion.TestingBinaryVersion.String())
-		}
-		return nil
-	})
-
-	// Take an incremental backup on the new version using the latest file
-	// written by the old version in the base directory.
-	query = fmt.Sprintf("BACKUP INTO LATEST IN %s", userfile)
-	sqlDB.Exec(t, query)
-
-}
-
 // TestBackupRestoreTelemetryEvents tests that BACKUP and RESTORE correctly
 // publishes telemetry events.
 func TestBackupRestoreTelemetryEvents(t *testing.T) {

pkg/ccl/backupccl/backupdest/BUILD.bazel

@@ -48,9 +48,7 @@ go_test(
         "//pkg/ccl/utilccl",
         "//pkg/cloud",
         "//pkg/cloud/impl:cloudimpl",
-        "//pkg/clusterversion",
         "//pkg/jobs/jobspb",
-        "//pkg/roachpb",
         "//pkg/security/securityassets",
         "//pkg/security/securitytest",
         "//pkg/security/username",

pkg/ccl/backupccl/backupdest/backup_destination.go

@@ -325,13 +325,6 @@ func FindLatestFile(
 func WriteNewLatestFile(
 	ctx context.Context, settings *cluster.Settings, exportStore cloud.ExternalStorage, suffix string,
 ) error {
-	// If the cluster is still running on a mixed version, we want to write
-	// to the base directory instead of the metadata/latest directory. That
-	// way an old node can still find the LATEST file.
-	if !settings.Version.IsActive(ctx, clusterversion.BackupDoesNotOverwriteLatestAndCheckpoint) {
-		return cloud.WriteFile(ctx, exportStore, backupbase.LatestFileName, strings.NewReader(suffix))
-	}
-
 	// HTTP storage does not support listing and so we cannot rely on the
 	// above-mentioned List method to return us the most recent latest file.
 	// Instead, we disregard write once semantics and always read and write

pkg/ccl/backupccl/backupdest/backup_destination_test.go

@@ -21,9 +21,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/ccl/backupccl/backuputils"
 	"github.com/cockroachdb/cockroach/pkg/cloud"
 	_ "github.com/cockroachdb/cockroach/pkg/cloud/impl" // register cloud storage providers
-	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
-	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/security/username"
 	"github.com/cockroachdb/cockroach/pkg/sql"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
@@ -298,7 +296,6 @@ func TestBackupRestoreResolveDestination(t *testing.T) {
 				inc8Time := full3Time.Add(time.Minute * 30)
 				inc9Time := inc8Time.Add(time.Minute * 30)
 				full4Time := inc9Time.Add(time.Minute * 30)
-				inc10Time := full4Time.Add(time.Minute * 30)
 
 				// firstBackupChain is maintained throughout the tests as the history of
 				// backups that were taken based on the initial full backup.
@@ -311,14 +308,12 @@ func TestBackupRestoreResolveDestination(t *testing.T) {
 				// separate path relative to the full backup in their chain. Otherwise,
 				// it should be an empty array of strings
 				noIncrementalStorage := []string(nil)
-				incrementalBackupSubdirEnabled := true
-				notIncrementalBackupSubdirEnabled := false
 
 				firstRemoteBackupChain := []string(nil)
 
 				testCollectionBackup := func(t *testing.T, backupTime time.Time,
 					expectedDefault, expectedSuffix, expectedIncDir string, expectedPrevBackups []string,
-					appendToLatest bool, subdir string, incrementalTo []string, incrementalBackupSubdirEnabled bool) {
+					appendToLatest bool, subdir string, incrementalTo []string) {
 
 					endTime := hlc.Timestamp{WallTime: backupTime.UnixNano()}
 					incrementalFrom := []string(nil)
@@ -336,17 +331,6 @@ func TestBackupRestoreResolveDestination(t *testing.T) {
 						_, localityCollections, err = backupdest.GetURIsByLocalityKV(incrementalTo, "")
 						require.NoError(t, err)
 					}
-					currentVersion := execCfg.Settings.Version.ActiveVersion(ctx)
-					if !incrementalBackupSubdirEnabled {
-						// Downgrading is disallowed in normal operation, but fine for testing here.
-						err = execCfg.Settings.Version.SetActiveVersion(ctx, clusterversion.ClusterVersion{
-							Version: roachpb.Version{
-								Major: 21,
-								Minor: 2,
-							},
-						})
-						require.NoError(t, err)
-					}
 
 					fullBackupExists := false
 					if expectedIncDir != "" {
@@ -360,9 +344,7 @@ func TestBackupRestoreResolveDestination(t *testing.T) {
 						incrementalFrom,
 						&execCfg,
 					)
-					clusterErr := execCfg.Settings.Version.SetActiveVersion(ctx, currentVersion)
 					require.NoError(t, err)
-					require.NoError(t, clusterErr)
 
 					localityDests := make(map[string]string, len(localityCollections))
 					for locality, localityDest := range localityCollections {
@@ -389,7 +371,7 @@ func TestBackupRestoreResolveDestination(t *testing.T) {
 
 					testCollectionBackup(t, fullTime,
 						expectedDefault, expectedSuffix, expectedIncDir, firstBackupChain,
-						false /* intoLatest */, noExplicitSubDir, noIncrementalStorage, incrementalBackupSubdirEnabled)
+						false /* intoLatest */, noExplicitSubDir, noIncrementalStorage)
 					firstBackupChain = append(firstBackupChain, expectedDefault)
 					firstRemoteBackupChain = append(firstRemoteBackupChain, expectedDefault)
 					writeManifest(t, expectedDefault)
@@ -406,7 +388,7 @@ func TestBackupRestoreResolveDestination(t *testing.T) {
 
 					testCollectionBackup(t, inc1Time,
 						expectedDefault, expectedSuffix, expectedIncDir, firstBackupChain,
-						true /* intoLatest */, noExplicitSubDir, defaultIncrementalTo, incrementalBackupSubdirEnabled)
+						true /* intoLatest */, noExplicitSubDir, defaultIncrementalTo)
 					firstBackupChain = append(firstBackupChain, expectedDefault)
 					writeManifest(t, expectedDefault)
 				}
@@ -420,7 +402,7 @@ func TestBackupRestoreResolveDestination(t *testing.T) {
 
 					testCollectionBackup(t, inc2Time,
 						expectedDefault, expectedSuffix, expectedIncDir, firstBackupChain,
-						true /* intoLatest */, noExplicitSubDir, defaultIncrementalTo, incrementalBackupSubdirEnabled)
+						true /* intoLatest */, noExplicitSubDir, defaultIncrementalTo)
 					firstBackupChain = append(firstBackupChain, expectedDefault)
 					writeManifest(t, expectedDefault)
 				}
@@ -435,7 +417,7 @@ func TestBackupRestoreResolveDestination(t *testing.T) {
 
 					testCollectionBackup(t, full2Time,
 						expectedDefault, expectedSuffix, expectedIncDir, []string(nil),
-						false /* intoLatest */, noExplicitSubDir, noIncrementalStorage, incrementalBackupSubdirEnabled)
+						false /* intoLatest */, noExplicitSubDir, noIncrementalStorage)
 					writeManifest(t, expectedDefault)
 					// We also wrote a new full backup, so let's update the latest.
 					writeLatest(t, collectionLoc, expectedSuffix)
@@ -450,7 +432,7 @@ func TestBackupRestoreResolveDestination(t *testing.T) {
 
 					testCollectionBackup(t, inc3Time,
 						expectedDefault, expectedSuffix, expectedIncDir, []string{backup2Location},
-						true /* intoLatest */, noExplicitSubDir, defaultIncrementalTo, incrementalBackupSubdirEnabled)
+						true /* intoLatest */, noExplicitSubDir, defaultIncrementalTo)
 					writeManifest(t, expectedDefault)
 				}
 
@@ -463,7 +445,7 @@ func TestBackupRestoreResolveDestination(t *testing.T) {
 
 					testCollectionBackup(t, inc4Time,
 						expectedDefault, expectedSuffix, expectedIncDir, firstBackupChain,
-						false /* intoLatest */, expectedSubdir, defaultIncrementalTo, incrementalBackupSubdirEnabled)
+						false /* intoLatest */, expectedSubdir, defaultIncrementalTo)
 					writeManifest(t, expectedDefault)
 				}
 
@@ -479,7 +461,7 @@ func TestBackupRestoreResolveDestination(t *testing.T) {
 
 					testCollectionBackup(t, inc5Time,
 						expectedDefault, expectedSuffix, expectedIncDir, firstRemoteBackupChain,
-						false /* intoLatest */, expectedSubdir, customIncrementalTo, incrementalBackupSubdirEnabled)
+						false /* intoLatest */, expectedSubdir, customIncrementalTo)
 					writeManifest(t, expectedDefault)
 
 					firstRemoteBackupChain = append(firstRemoteBackupChain, expectedDefault)
@@ -497,7 +479,7 @@ func TestBackupRestoreResolveDestination(t *testing.T) {
 
 					testCollectionBackup(t, inc6Time,
 						expectedDefault, expectedSuffix, expectedIncDir, firstRemoteBackupChain,
-						false /* intoLatest */, expectedSubdir, customIncrementalTo, incrementalBackupSubdirEnabled)
+						false /* intoLatest */, expectedSubdir, customIncrementalTo)
 					writeManifest(t, expectedDefault)
 				}
 
@@ -514,7 +496,7 @@ func TestBackupRestoreResolveDestination(t *testing.T) {
 
 					testCollectionBackup(t, inc7Time,
 						expectedDefault, expectedSuffix, expectedIncDir, []string{backup2Location},
-						true /* intoLatest */, expectedSubdir, customIncrementalTo, incrementalBackupSubdirEnabled)
+						true /* intoLatest */, expectedSubdir, customIncrementalTo)
 					writeManifest(t, expectedDefault)
 				}
 
@@ -528,7 +510,7 @@ func TestBackupRestoreResolveDestination(t *testing.T) {
 
 					testCollectionBackup(t, full3Time,
 						expectedDefault, expectedSuffix, expectedIncDir, []string(nil),
-						false /* intoLatest */, noExplicitSubDir, noIncrementalStorage, incrementalBackupSubdirEnabled)
+						false /* intoLatest */, noExplicitSubDir, noIncrementalStorage)
 					writeManifest(t, expectedDefault)
 					// We also wrote a new full backup, so let's update the latest.
 					writeLatest(t, collectionLoc, expectedSuffix)
@@ -557,42 +539,23 @@ func TestBackupRestoreResolveDestination(t *testing.T) {
 
 					testCollectionBackup(t, inc9Time,
 						expectedDefault, expectedSuffix, expectedIncDir, []string{backup3Location, oldStyleDefault},
-						true /* intoLatest */, expectedSubdir, noIncrementalStorage, incrementalBackupSubdirEnabled)
+						true /* intoLatest */, expectedSubdir, noIncrementalStorage)
 					writeManifest(t, expectedDefault)
 				}
 
 				// A new full backup: BACKUP INTO collection
-				var backup4Location string
 				{
 					expectedSuffix := "/2020/12/25-120000.00"
 					expectedIncDir := ""
 					expectedDefault := fmt.Sprintf("nodelocal://1/%s%s?AUTH=implicit", t.Name(), expectedSuffix)
-					backup4Location = expectedDefault
 
 					testCollectionBackup(t, full4Time,
 						expectedDefault, expectedSuffix, expectedIncDir, []string(nil),
-						false /* intoLatest */, noExplicitSubDir, noIncrementalStorage, notIncrementalBackupSubdirEnabled)
+						false /* intoLatest */, noExplicitSubDir, noIncrementalStorage)
 					writeManifest(t, expectedDefault)
 					// We also wrote a new full backup, so let's update the latest.
 					writeLatest(t, collectionLoc, expectedSuffix)
 				}
-
-				// An automatic incremental into the fourth full backup: BACKUP INTO LATEST
-				// IN collection, BUT simulating an old cluster that doesn't support
-				// dedicated incrementals subdir by default yet.
-				{
-					expectedSuffix := "/2020/12/25-120000.00"
-					expectedIncDir := "/20201225/123000.00"
-					expectedSubdir := expectedSuffix
-					expectedDefault := fmt.Sprintf("nodelocal://1/%s%s%s?AUTH=implicit",
-						t.Name(),
-						expectedSuffix, expectedIncDir)
-
-					testCollectionBackup(t, inc10Time,
-						expectedDefault, expectedSuffix, expectedIncDir, []string{backup4Location},
-						true /* intoLatest */, expectedSubdir, noIncrementalStorage, notIncrementalBackupSubdirEnabled)
-					writeManifest(t, expectedDefault)
-				}
 			})
 		})
 	}

pkg/ccl/backupccl/backupdest/incrementals.go

@@ -18,7 +18,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/ccl/backupccl/backupbase"
 	"github.com/cockroachdb/cockroach/pkg/ccl/backupccl/backuputils"
 	"github.com/cockroachdb/cockroach/pkg/cloud"
-	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/security/username"
 	"github.com/cockroachdb/cockroach/pkg/sql"
 	"github.com/cockroachdb/errors"
@@ -166,9 +165,8 @@ func ResolveIncrementalsBackupLocation(
 				"Please choose a location manually with the `incremental_location` parameter.")
 	}
 
-	// If the cluster isn't fully migrated, or we have backups in the old default
-	// location, continue to use the old location.
-	if len(prevOld) > 0 || !execCfg.Settings.Version.IsActive(ctx, clusterversion.IncrementalBackupSubdir) {
+	// If we have backups in the old default location, continue to use the old location.
+	if len(prevOld) > 0 {
 		return resolvedIncrementalsBackupLocationOld, nil
 	}
 

pkg/ccl/backupccl/backupinfo/BUILD.bazel

@@ -18,7 +18,6 @@ go_library(
         "//pkg/ccl/storageccl",
         "//pkg/cloud",
         "//pkg/cloud/cloudpb",
-        "//pkg/clusterversion",
         "//pkg/jobs/jobspb",
         "//pkg/keys",
         "//pkg/roachpb",

pkg/ccl/backupccl/backupinfo/manifest_handling.go

@@ -26,7 +26,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/ccl/storageccl"
 	"github.com/cockroachdb/cockroach/pkg/cloud"
 	"github.com/cockroachdb/cockroach/pkg/cloud/cloudpb"
-	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/security/username"
@@ -984,26 +983,6 @@ func WriteBackupManifestCheckpoint(
 		}
 	}
 
-	// If the cluster is still running on a mixed version, we want to write
-	// to the base directory instead of the progress directory. That way if
-	// an old node resumes a backup, it doesn't have to start over.
-	if !execCfg.Settings.Version.IsActive(ctx, clusterversion.BackupDoesNotOverwriteLatestAndCheckpoint) {
-		// We want to overwrite the latest checkpoint in the base directory,
-		// just write to the non versioned BACKUP-CHECKPOINT file.
-		err = cloud.WriteFile(ctx, defaultStore, BackupManifestCheckpointName, bytes.NewReader(descBuf))
-		if err != nil {
-			return err
-		}
-
-		checksum, err := GetChecksum(descBuf)
-		if err != nil {
-			return err
-		}
-
-		return cloud.WriteFile(ctx, defaultStore, BackupManifestCheckpointName+
-			BackupManifestChecksumSuffix, bytes.NewReader(checksum))
-	}
-
 	// We timestamp the checkpoint files in order to enforce write once backups.
 	// When the job goes to read these timestamped files, it will List
 	// the checkpoints and pick the file whose name is lexicographically