From 01d251c8d2cdb1c92502b58d3742f7b83d149e00 Mon Sep 17 00:00:00 2001
From: Michael Butler <butler@cockroachlabs.com>
Date: Thu, 28 Jul 2022 17:12:27 -0400
Subject: [PATCH] backupccl: check crdb_internal.invalid_objects during backup
 test cleanup

This patch adds a test helper function to easily check for invalid descriptors
during the cleanup of a test cluster created by any of the
backupRestoreTestSetup* helper funcs. This setup is used by many backup unit
tests (including all data driven tests), but a future PR should include this
clean up helper func in any backup unit test that doesn't include use
these helper funcs.

Informs #84757

Release note: none
---
 pkg/ccl/backupccl/backuputils/testutils.go | 17 +++++++++++++++++
 pkg/ccl/backupccl/datadriven_test.go       |  6 +++---
 pkg/ccl/backupccl/utils_test.go            | 10 ++++++++++
 3 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/pkg/ccl/backupccl/backuputils/testutils.go b/pkg/ccl/backupccl/backuputils/testutils.go
index e5eeb78617cd..205fe50ec887 100644
--- a/pkg/ccl/backupccl/backuputils/testutils.go
+++ b/pkg/ccl/backupccl/backuputils/testutils.go
@@ -172,3 +172,20 @@ func VerifyBackupRestoreStatementResult(
 
 	return nil
 }
+
+// CheckForInvalidDescriptors returns an error if there exists any descriptors in
+// the crdb_internal.invalid_objects virtual table.
+func CheckForInvalidDescriptors(sqlDB *gosql.DB) error {
+	rows, err := sqlDB.Query(`SELECT id FROM crdb_internal.invalid_objects`)
+	if err != nil {
+		return err
+	}
+	invalidIDs, err := sqlutils.RowsToDataDrivenOutput(rows)
+	if err != nil {
+		return err
+	}
+	if invalidIDs != "" {
+		return errors.Newf("The following descriptor ids are invalid %v", invalidIDs)
+	}
+	return nil
+}
diff --git a/pkg/ccl/backupccl/datadriven_test.go b/pkg/ccl/backupccl/datadriven_test.go
index 4a4a79fbd804..f16e8368f0ab 100644
--- a/pkg/ccl/backupccl/datadriven_test.go
+++ b/pkg/ccl/backupccl/datadriven_test.go
@@ -101,7 +101,7 @@ func newDatadrivenTestState() datadrivenTestState {
 	}
 }
 
-func (d *datadrivenTestState) cleanup(ctx context.Context) {
+func (d *datadrivenTestState) cleanup(ctx context.Context, t *testing.T) {
 	for _, db := range d.sqlDBs {
 		db.Close()
 	}
@@ -353,12 +353,12 @@ func TestDataDriven(t *testing.T) {
 	datadriven.Walk(t, testutils.TestDataPath(t, "backup-restore"), func(t *testing.T, path string) {
 		var lastCreatedServer string
 		ds := newDatadrivenTestState()
-		defer ds.cleanup(ctx)
+		defer ds.cleanup(ctx, t)
 		datadriven.RunTest(t, path, func(t *testing.T, d *datadriven.TestData) string {
 
 			switch d.Cmd {
 			case "reset":
-				ds.cleanup(ctx)
+				ds.cleanup(ctx, t)
 				ds = newDatadrivenTestState()
 				return ""
 
diff --git a/pkg/ccl/backupccl/utils_test.go b/pkg/ccl/backupccl/utils_test.go
index 324ec5230087..e565803b1ec4 100644
--- a/pkg/ccl/backupccl/utils_test.go
+++ b/pkg/ccl/backupccl/utils_test.go
@@ -12,6 +12,7 @@ import (
 	"context"
 	gosql "database/sql"
 	"fmt"
+	"github.com/cockroachdb/cockroach/pkg/ccl/backupccl/backuputils"
 	"io"
 	"io/ioutil"
 	"math/rand"
@@ -113,6 +114,9 @@ func backupRestoreTestSetupWithParams(
 	}
 
 	cleanupFn := func() {
+		if err := backuputils.CheckForInvalidDescriptors(tc.Conns[0]); err != nil {
+			t.Fatal(err)
+		}
 		tc.Stopper().Stop(ctx) // cleans up in memory storage's auxiliary dirs
 		dirCleanupFn()         // cleans up dir, which is the nodelocal:// storage
 	}
@@ -164,6 +168,9 @@ func backupRestoreTestSetupEmptyWithParams(
 	sqlDB = sqlutils.MakeSQLRunner(tc.Conns[0])
 
 	cleanupFn := func() {
+		if err := backuputils.CheckForInvalidDescriptors(tc.Conns[0]); err != nil {
+			t.Fatal(err)
+		}
 		tc.Stopper().Stop(ctx) // cleans up in memory storage's auxiliary dirs
 	}
 
@@ -186,6 +193,9 @@ func createEmptyCluster(
 	sqlDB = sqlutils.MakeSQLRunner(tc.Conns[0])
 
 	cleanupFn := func() {
+		if err := backuputils.CheckForInvalidDescriptors(tc.Conns[0]); err != nil {
+			t.Fatal(err)
+		}
 		tc.Stopper().Stop(ctx) // cleans up in memory storage's auxiliary dirs
 		dirCleanupFn()         // cleans up dir, which is the nodelocal:// storage
 	}