Take all backup dbs into account for checkup + remote uninstall (#1756)

ee/agent/storage/bbolt/backup.go

@@ -86,7 +86,7 @@ func (d *databaseBackupSaver) backupDb() error {
 	}
 
 	// Take backup
-	backupLocation := BackupLauncherDbLocation(d.knapsack.RootDirectory())
+	backupLocation := backupLauncherDbLocation(d.knapsack.RootDirectory())
 	if err := d.knapsack.BboltDB().View(func(tx *bbolt.Tx) error {
 		return tx.CopyFile(backupLocation, 0600)
 	}); err != nil {
@@ -110,7 +110,7 @@ func (d *databaseBackupSaver) backupDb() error {
 }
 
 func (d *databaseBackupSaver) rotate() error {
-	baseBackupPath := BackupLauncherDbLocation(d.knapsack.RootDirectory())
+	baseBackupPath := backupLauncherDbLocation(d.knapsack.RootDirectory())
 
 	// Delete the oldest backup, if it exists
 	oldestBackupPath := fmt.Sprintf("%s.%d", baseBackupPath, numberOfOldBackupsToRetain)
@@ -201,12 +201,30 @@ func LauncherDbLocation(rootDir string) string {
 	return filepath.Join(rootDir, "launcher.db")
 }
 
-func BackupLauncherDbLocation(rootDir string) string {
+func backupLauncherDbLocation(rootDir string) string {
 	return filepath.Join(rootDir, "launcher.db.bak")
 }
 
+func BackupLauncherDbLocations(rootDir string) []string {
+	backupLocations := make([]string, 0)
+
+	backupLocation := backupLauncherDbLocation(rootDir)
+	if exists, _ := nonEmptyFileExists(backupLocation); exists {
+		backupLocations = append(backupLocations, backupLocation)
+	}
+
+	for i := 1; i <= numberOfOldBackupsToRetain; i += 1 {
+		currentBackupLocation := fmt.Sprintf("%s.%d", backupLocation, i)
+		if exists, _ := nonEmptyFileExists(currentBackupLocation); exists {
+			backupLocations = append(backupLocations, currentBackupLocation)
+		}
+	}
+
+	return backupLocations
+}
+
 func latestBackupDb(rootDir string) string {
-	backupLocation := BackupLauncherDbLocation(rootDir)
+	backupLocation := backupLauncherDbLocation(rootDir)
 	if exists, _ := nonEmptyFileExists(backupLocation); exists {
 		return backupLocation
 	}

ee/agent/storage/bbolt/backup_test.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"os"
+	"path/filepath"
 	"testing"
 	"time"
 
@@ -54,7 +55,7 @@ func TestUseBackupDbIfNeeded(t *testing.T) {
 			// Set up test databases
 			tempRootDir := t.TempDir()
 			originalDbFileLocation := LauncherDbLocation(tempRootDir)
-			backupDbFileLocation := BackupLauncherDbLocation(tempRootDir)
+			backupDbFileLocation := backupLauncherDbLocation(tempRootDir)
 			if tt.originalDbExists {
 				createNonEmptyBboltDb(t, originalDbFileLocation)
 			}
@@ -114,7 +115,7 @@ func Test_rotate(t *testing.T) {
 
 	// Set up test root dir
 	tempRootDir := t.TempDir()
-	backupDbFileLocation := BackupLauncherDbLocation(tempRootDir)
+	backupDbFileLocation := backupLauncherDbLocation(tempRootDir)
 
 	// Set up backup saver
 	testKnapsack := typesmocks.NewKnapsack(t)
@@ -153,6 +154,93 @@ func Test_rotate(t *testing.T) {
 	require.NoError(t, d.rotate(), "must be able to rotate even when launcher.db.bak does not exist")
 }
 
+func TestBackupLauncherDbLocations(t *testing.T) {
+	t.Parallel()
+
+	for _, tt := range []struct {
+		testName         string
+		expectedDbStates map[string]bool
+	}{
+		{
+			testName: "all backup dbs exist",
+			expectedDbStates: map[string]bool{
+				"launcher.db.bak":   true,
+				"launcher.db.bak.1": true,
+				"launcher.db.bak.2": true,
+				"launcher.db.bak.3": true,
+			},
+		},
+		{
+			testName: "only primary backup exists",
+			expectedDbStates: map[string]bool{
+				"launcher.db.bak":   true,
+				"launcher.db.bak.1": false,
+				"launcher.db.bak.2": false,
+				"launcher.db.bak.3": false,
+			},
+		},
+		{
+			testName: "primary backup exists, an older one is missing",
+			expectedDbStates: map[string]bool{
+				"launcher.db.bak":   true,
+				"launcher.db.bak.1": true,
+				"launcher.db.bak.2": false,
+				"launcher.db.bak.3": true,
+			},
+		},
+		{
+			testName: "primary backup does not exist, an older one is missing",
+			expectedDbStates: map[string]bool{
+				"launcher.db.bak":   false,
+				"launcher.db.bak.1": false,
+				"launcher.db.bak.2": true,
+				"launcher.db.bak.3": true,
+			},
+		},
+		{
+			testName: "no backup dbs",
+			expectedDbStates: map[string]bool{
+				"launcher.db.bak":   false,
+				"launcher.db.bak.1": false,
+				"launcher.db.bak.2": false,
+				"launcher.db.bak.3": false,
+			},
+		},
+	} {
+		tt := tt
+		t.Run(tt.testName, func(t *testing.T) {
+			t.Parallel()
+
+			// Set up test root dir and backup dbs
+			tempRootDir := t.TempDir()
+			for dbPath, exists := range tt.expectedDbStates {
+				if !exists {
+					continue
+				}
+				createNonEmptyBboltDb(t, filepath.Join(tempRootDir, dbPath))
+			}
+
+			// Validate we didn't return any paths that we didn't expect
+			foundBackupDbs := BackupLauncherDbLocations(tempRootDir)
+			actualDbs := make(map[string]bool)
+			for _, foundBackupDb := range foundBackupDbs {
+				db := filepath.Base(foundBackupDb)
+				require.Contains(t, tt.expectedDbStates, db, "backup db found not in original list")
+				require.True(t, tt.expectedDbStates[db], "found backup db that should not have been created")
+				actualDbs[db] = true
+			}
+
+			// Validate that we don't have any dbs missing from actualDbs that we expected to have
+			for expectedDb, exists := range tt.expectedDbStates {
+				if !exists {
+					continue
+				}
+				require.Contains(t, actualDbs, expectedDb, "missing db from results")
+			}
+		})
+	}
+}
+
 func TestInterrupt_Multiple(t *testing.T) {
 	t.Parallel()
 

ee/debug/checkups/bboltdb.go

@@ -59,30 +59,35 @@ func (c *bboltdbCheckup) Run(_ context.Context, extraFH io.Writer) error {
 	return nil
 }
 
-func (c *bboltdbCheckup) backupStats() (map[string]any, error) {
-	backupStatsMap := make(map[string]any)
-
-	backupDbLocation := agentbbolt.BackupLauncherDbLocation(c.k.RootDirectory())
-	if _, err := os.Stat(backupDbLocation); err != nil {
-		return nil, fmt.Errorf("backup db not found at %s: %w", backupDbLocation, err)
-	}
-
-	// Open a connection to the backup, since we don't have one available yet
-	boltOptions := &bbolt.Options{Timeout: time.Duration(30) * time.Second}
-	backupDb, err := bbolt.Open(backupDbLocation, 0600, boltOptions)
-	if err != nil {
-		return nil, fmt.Errorf("could not open backup db at %s: %w", backupDbLocation, err)
-	}
-	defer backupDb.Close()
-
-	// Gather stats
-	backupStats, err := agent.GetStats(backupDb)
-	if err != nil {
-		return nil, fmt.Errorf("could not get backup db stats: %w", err)
-	}
-
-	for k, v := range backupStats.Buckets {
-		backupStatsMap[k] = v
+func (c *bboltdbCheckup) backupStats() (map[string]map[string]any, error) {
+	backupStatsMap := make(map[string]map[string]any)
+
+	backupDbLocations := agentbbolt.BackupLauncherDbLocations(c.k.RootDirectory())
+
+	for _, backupDbLocation := range backupDbLocations {
+		if _, err := os.Stat(backupDbLocation); err != nil {
+			continue
+		}
+
+		backupStatsMap[backupDbLocation] = make(map[string]any)
+
+		// Open a connection to the backup, since we don't have one available yet
+		boltOptions := &bbolt.Options{Timeout: time.Duration(30) * time.Second}
+		backupDb, err := bbolt.Open(backupDbLocation, 0600, boltOptions)
+		if err != nil {
+			return nil, fmt.Errorf("could not open backup db at %s: %w", backupDbLocation, err)
+		}
+		defer backupDb.Close()
+
+		// Gather stats
+		backupStats, err := agent.GetStats(backupDb)
+		if err != nil {
+			return nil, fmt.Errorf("could not get backup db stats: %w", err)
+		}
+
+		for k, v := range backupStats.Buckets {
+			backupStatsMap[backupDbLocation][k] = v
+		}
 	}
 
 	return backupStatsMap, nil

ee/uninstall/uninstall.go

@@ -35,12 +35,14 @@ func Uninstall(ctx context.Context, k types.Knapsack, exitOnCompletion bool) {
 		)
 	}
 
-	backupDbPath := agentbbolt.BackupLauncherDbLocation(k.RootDirectory())
-	if err := os.Remove(backupDbPath); err != nil {
-		slogger.Log(ctx, slog.LevelError,
-			"removing backup database",
-			"err", err,
-		)
+	backupDbPaths := agentbbolt.BackupLauncherDbLocations(k.RootDirectory())
+	for _, db := range backupDbPaths {
+		if err := os.Remove(db); err != nil {
+			slogger.Log(ctx, slog.LevelError,
+				"removing backup database",
+				"err", err,
+			)
+		}
 	}
 
 	if !exitOnCompletion {

ee/uninstall/uninstall_test.go

@@ -10,7 +10,6 @@ import (
 
 	"github.com/kolide/launcher/ee/agent"
 	"github.com/kolide/launcher/ee/agent/storage"
-	agentbbolt "github.com/kolide/launcher/ee/agent/storage/bbolt"
 	storageci "github.com/kolide/launcher/ee/agent/storage/ci"
 	"github.com/kolide/launcher/ee/agent/types"
 	"github.com/kolide/launcher/ee/agent/types/mocks"
@@ -46,11 +45,17 @@ func TestUninstall(t *testing.T) {
 
 			// create a backup database to delete
 			tempRootDir := t.TempDir()
-			backupDbLocation := agentbbolt.BackupLauncherDbLocation(tempRootDir)
+			backupDbLocation := filepath.Join(tempRootDir, "launcher.db.bak")
 			db, err := bbolt.Open(backupDbLocation, 0600, &bbolt.Options{Timeout: time.Duration(5) * time.Second})
 			require.NoError(t, err, "creating db")
 			require.NoError(t, db.Close(), "closing db")
 
+			// create an older backup db to delete
+			olderBackupDbLocation := fmt.Sprintf("%s.2", backupDbLocation)
+			db2, err := bbolt.Open(olderBackupDbLocation, 0600, &bbolt.Options{Timeout: time.Duration(5) * time.Second})
+			require.NoError(t, err, "creating db")
+			require.NoError(t, db2.Close(), "closing db")
+
 			k := mocks.NewKnapsack(t)
 			k.On("EnrollSecretPath").Return(enrollSecretPath)
 			k.On("Slogger").Return(multislogger.NewNopLogger())
@@ -117,6 +122,10 @@ func TestUninstall(t *testing.T) {
 			// check that the backup database was removed
 			_, err = os.Stat(backupDbLocation)
 			require.True(t, os.IsNotExist(err), "checking that launcher.db.bak does not exist, and error is not ErrNotExist")
+
+			// check that the older backup database was removed
+			_, err = os.Stat(olderBackupDbLocation)
+			require.True(t, os.IsNotExist(err), "checking that launcher.db.bak does not exist, and error is not ErrNotExist")
 		})
 	}
 }