diff --git a/gpbackman/COMMANDS.md b/gpbackman/COMMANDS.md index c67947a3..d9e43d47 100644 --- a/gpbackman/COMMANDS.md +++ b/gpbackman/COMMANDS.md @@ -72,7 +72,7 @@ For non local backups the following logic are applied: The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database will be searched in the current directory. +If the --history-db option is not specified, the history database is looked for in the current directory. Pass `--auto-load-history-db` to resolve it from `$COORDINATOR_DATA_DIRECTORY` instead. Usage: gpbackman backup-clean [flags] @@ -158,7 +158,7 @@ For non local backups the following logic are applied: The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database will be searched in the current directory. +If the --history-db option is not specified, the history database is looked for in the current directory. Pass `--auto-load-history-db` to resolve it from `$COORDINATOR_DATA_DIRECTORY` instead. Usage: gpbackman backup-delete [flags] @@ -256,7 +256,7 @@ To display the "object filtering details" column for all backups without using - The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database will be searched in the current directory. +If the --history-db option is not specified, the history database is looked for in the current directory. Pass `--auto-load-history-db` to resolve it from `$COORDINATOR_DATA_DIRECTORY` instead. Usage: gpbackman backup-info [flags] @@ -455,7 +455,7 @@ Only --older-than-days or --before-timestamp option must be specified, not both. The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database will be searched in the current directory. +If the --history-db option is not specified, the history database is looked for in the current directory. Pass `--auto-load-history-db` to resolve it from `$COORDINATOR_DATA_DIRECTORY` instead. Usage: gpbackman history-clean [flags] @@ -524,7 +524,7 @@ It is not necessary to use the --plugin-report-file-path flag for the following The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database will be searched in the current directory. +If the --history-db option is not specified, the history database is looked for in the current directory. Pass `--auto-load-history-db` to resolve it from `$COORDINATOR_DATA_DIRECTORY` instead. Usage: gpbackman report-info [flags] diff --git a/gpbackman/README.md b/gpbackman/README.md index 388ff898..d9bd9ae8 100644 --- a/gpbackman/README.md +++ b/gpbackman/README.md @@ -53,6 +53,7 @@ Available Commands: Flags: -h, --help help for gpbackman + --auto-load-history-db resolve gpbackup_history.db from $COORDINATOR_DATA_DIRECTORY when --history-db is unset --history-db string full path to the gpbackup_history.db file --log-file string full path to log file directory, if not specified, the log file will be created in the $HOME/gpAdminLogs directory --log-level-console string level for console logging (error, info, debug, verbose) (default "info") diff --git a/gpbackman/cmd/backup_clean.go b/gpbackman/cmd/backup_clean.go index 4f5e2179..46c9548b 100644 --- a/gpbackman/cmd/backup_clean.go +++ b/gpbackman/cmd/backup_clean.go @@ -77,7 +77,7 @@ For non local backups the following logic are applied: The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database will be searched in the current directory.`, +If the --history-db option is not specified, the history database is looked for in the current directory. To resolve it from $COORDINATOR_DATA_DIRECTORY instead, pass the --auto-load-history-db flag.`, Args: cobra.NoArgs, Run: func(cmd *cobra.Command, args []string) { doRootFlagValidation(cmd.Flags(), checkFileExistsConst) @@ -205,7 +205,7 @@ func doCleanBackup() { } func cleanBackup() error { - hDB, err := gpbckpconfig.OpenHistoryDB(getHistoryDBPath(rootHistoryDB)) + hDB, err := gpbckpconfig.OpenHistoryDB(getHistoryDBPath(rootHistoryDB, rootAutoLoadHistoryDB)) if err != nil { gplog.Error("%s", textmsg.ErrorTextUnableActionHistoryDB("open", err)) return err diff --git a/gpbackman/cmd/backup_delete.go b/gpbackman/cmd/backup_delete.go index 14fa2795..4eb525a5 100644 --- a/gpbackman/cmd/backup_delete.go +++ b/gpbackman/cmd/backup_delete.go @@ -84,7 +84,7 @@ For non local backups the following logic are applied: The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database will be searched in the current directory.`, +If the --history-db option is not specified, the history database is looked for in the current directory. To resolve it from $COORDINATOR_DATA_DIRECTORY instead, pass the --auto-load-history-db flag.`, Args: cobra.NoArgs, Run: func(cmd *cobra.Command, args []string) { doRootFlagValidation(cmd.Flags(), checkFileExistsConst) @@ -205,7 +205,7 @@ func doDeleteBackup() { } func deleteBackup() error { - hDB, err := gpbckpconfig.OpenHistoryDB(getHistoryDBPath(rootHistoryDB)) + hDB, err := gpbckpconfig.OpenHistoryDB(getHistoryDBPath(rootHistoryDB, rootAutoLoadHistoryDB)) if err != nil { gplog.Error("%s", textmsg.ErrorTextUnableActionHistoryDB("open", err)) return err diff --git a/gpbackman/cmd/backup_info.go b/gpbackman/cmd/backup_info.go index 199311cc..96674b48 100644 --- a/gpbackman/cmd/backup_info.go +++ b/gpbackman/cmd/backup_info.go @@ -98,7 +98,7 @@ To display the "object filtering details" column for all backups without using - The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database will be searched in the current directory.`, +If the --history-db option is not specified, the history database is looked for in the current directory. To resolve it from $COORDINATOR_DATA_DIRECTORY instead, pass the --auto-load-history-db flag.`, Args: cobra.NoArgs, Run: func(cmd *cobra.Command, args []string) { doRootFlagValidation(cmd.Flags(), checkFileExistsConst) @@ -226,7 +226,7 @@ func backupInfo() error { } t := tablewriter.NewWriter(os.Stdout) initTable(t, opts.ShowDetails) - hDB, err := gpbckpconfig.OpenHistoryDB(getHistoryDBPath(rootHistoryDB)) + hDB, err := gpbckpconfig.OpenHistoryDB(getHistoryDBPath(rootHistoryDB, rootAutoLoadHistoryDB)) if err != nil { gplog.Error("%s", textmsg.ErrorTextUnableActionHistoryDB("open", err)) return err diff --git a/gpbackman/cmd/constants.go b/gpbackman/cmd/constants.go index 8259b594..03b13d36 100644 --- a/gpbackman/cmd/constants.go +++ b/gpbackman/cmd/constants.go @@ -35,6 +35,7 @@ const ( // Flags. historyDBFlagName = "history-db" + autoLoadHistoryDBFlagName = "auto-load-history-db" logFileFlagName = "log-file" logLevelConsoleFlagName = "log-level-console" logLevelFileFlagName = "log-level-file" @@ -72,4 +73,11 @@ var ( beforeTimestamp string // Timestamp to delete all backups after. afterTimestamp string + + // historyDBEnvVars lists the environment variables inspected when + // --history-db is not supplied. Exported by the standard Cloudberry + // cluster environment scripts. + historyDBEnvVars = []string{ + "COORDINATOR_DATA_DIRECTORY", + } ) diff --git a/gpbackman/cmd/history_clean.go b/gpbackman/cmd/history_clean.go index d5de643a..42366421 100644 --- a/gpbackman/cmd/history_clean.go +++ b/gpbackman/cmd/history_clean.go @@ -50,7 +50,7 @@ Only --older-than-days or --before-timestamp option must be specified, not both. The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database will be searched in the current directory.`, +If the --history-db option is not specified, the history database is looked for in the current directory. To resolve it from $COORDINATOR_DATA_DIRECTORY instead, pass the --auto-load-history-db flag.`, Args: cobra.NoArgs, Run: func(cmd *cobra.Command, args []string) { doRootFlagValidation(cmd.Flags(), checkFileExistsConst) @@ -106,7 +106,7 @@ func doCleanHistory() { } func cleanHistory() error { - hDB, err := gpbckpconfig.OpenHistoryDB(getHistoryDBPath(rootHistoryDB)) + hDB, err := gpbckpconfig.OpenHistoryDB(getHistoryDBPath(rootHistoryDB, rootAutoLoadHistoryDB)) if err != nil { gplog.Error("%s", textmsg.ErrorTextUnableActionHistoryDB("open", err)) return err diff --git a/gpbackman/cmd/report_info.go b/gpbackman/cmd/report_info.go index 3ac7428f..36bf1207 100644 --- a/gpbackman/cmd/report_info.go +++ b/gpbackman/cmd/report_info.go @@ -78,7 +78,7 @@ It is not necessary to use the --plugin-report-file-path flag for the following The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database will be searched in the current directory.`, +If the --history-db option is not specified, the history database is looked for in the current directory. To resolve it from $COORDINATOR_DATA_DIRECTORY instead, pass the --auto-load-history-db flag.`, Args: cobra.NoArgs, Run: func(cmd *cobra.Command, args []string) { doRootFlagValidation(cmd.Flags(), checkFileExistsConst) @@ -174,7 +174,7 @@ func doReportInfo() { } func reportInfo() error { - hDB, err := gpbckpconfig.OpenHistoryDB(getHistoryDBPath(rootHistoryDB)) + hDB, err := gpbckpconfig.OpenHistoryDB(getHistoryDBPath(rootHistoryDB, rootAutoLoadHistoryDB)) if err != nil { gplog.Error("%s", textmsg.ErrorTextUnableActionHistoryDB("open", err)) return err diff --git a/gpbackman/cmd/root.go b/gpbackman/cmd/root.go index 1c113b37..79156cea 100644 --- a/gpbackman/cmd/root.go +++ b/gpbackman/cmd/root.go @@ -33,10 +33,11 @@ var version string // Flags for the gpbackman command (rootCmd) var ( - rootHistoryDB string - rootLogFile string - rootLogLevelConsole string - rootLogLevelFile string + rootHistoryDB string + rootAutoLoadHistoryDB bool + rootLogFile string + rootLogLevelConsole string + rootLogLevelFile string ) var rootCmd = &cobra.Command{ @@ -52,6 +53,12 @@ func init() { "", "full path to the gpbackup_history.db file", ) + rootCmd.PersistentFlags().BoolVar( + &rootAutoLoadHistoryDB, + autoLoadHistoryDBFlagName, + false, + "resolve gpbackup_history.db from $COORDINATOR_DATA_DIRECTORY when --history-db is unset", + ) rootCmd.PersistentFlags().StringVar( &rootLogFile, logFileFlagName, diff --git a/gpbackman/cmd/wrappers.go b/gpbackman/cmd/wrappers.go index 79e34f38..c36a5256 100644 --- a/gpbackman/cmd/wrappers.go +++ b/gpbackman/cmd/wrappers.go @@ -81,12 +81,25 @@ func setLogLevelFile(level string) error { return nil } -func getHistoryDBPath(historyDBPath string) string { - var historyDBName = historyDBNameConst +// getHistoryDBPath resolves the path to the gpbackup_history.db file. +// An explicit --history-db value always wins. Otherwise, when the caller +// asks for auto-load (--auto-load-history-db), look up the file under the +// COORDINATOR_DATA_DIRECTORY environment variable exported by the standard +// Cloudberry environment scripts. As a final fallback, return the bare +// filename so it is resolved against the current working directory, +// preserving the original behaviour for the default invocation. +func getHistoryDBPath(historyDBPath string, autoLoad bool) string { if historyDBPath != "" { return historyDBPath } - return historyDBName + if autoLoad { + for _, envVar := range historyDBEnvVars { + if dir := os.Getenv(envVar); dir != "" { + return filepath.Join(dir, historyDBNameConst) + } + } + } + return historyDBNameConst } func checkCompatibleFlags(flags *pflag.FlagSet, flagNames ...string) error { diff --git a/gpbackman/cmd/wrappers_test.go b/gpbackman/cmd/wrappers_test.go index e378fbf1..10c0d0c5 100644 --- a/gpbackman/cmd/wrappers_test.go +++ b/gpbackman/cmd/wrappers_test.go @@ -33,12 +33,53 @@ import ( var _ = Describe("wrappers tests", func() { Describe("getHistoryDBPath", func() { - It("returns default path when input is empty", func() { - Expect(getHistoryDBPath("")).To(Equal(historyDBNameConst)) + // Save and restore env vars so these cases don't leak into the rest + // of the suite when run with --randomize-all. + var savedEnv map[string]string + + BeforeEach(func() { + savedEnv = make(map[string]string, len(historyDBEnvVars)) + for _, name := range historyDBEnvVars { + savedEnv[name] = os.Getenv(name) + os.Unsetenv(name) + } + }) + + AfterEach(func() { + for name, val := range savedEnv { + if val == "" { + os.Unsetenv(name) + } else { + os.Setenv(name, val) + } + } + }) + + It("returns default filename when input is empty (auto-load off, no env)", func() { + Expect(getHistoryDBPath("", false)).To(Equal(historyDBNameConst)) }) It("returns input path when not empty", func() { - Expect(getHistoryDBPath("path/to/" + historyDBNameConst)).To(Equal("path/to/" + historyDBNameConst)) + Expect(getHistoryDBPath("path/to/"+historyDBNameConst, false)).To(Equal("path/to/" + historyDBNameConst)) + }) + + It("ignores env vars by default (auto-load off)", func() { + os.Setenv("COORDINATOR_DATA_DIRECTORY", "/coord/data") + Expect(getHistoryDBPath("", false)).To(Equal(historyDBNameConst)) + }) + + It("falls back to COORDINATOR_DATA_DIRECTORY when auto-load is on", func() { + os.Setenv("COORDINATOR_DATA_DIRECTORY", "/coord/data") + Expect(getHistoryDBPath("", true)).To(Equal(filepath.Join("/coord/data", historyDBNameConst))) + }) + + It("returns the cwd default when auto-load is on but no env vars are set", func() { + Expect(getHistoryDBPath("", true)).To(Equal(historyDBNameConst)) + }) + + It("explicit input wins over env vars even when auto-load is on", func() { + os.Setenv("COORDINATOR_DATA_DIRECTORY", "/coord/data") + Expect(getHistoryDBPath("/explicit/path.db", true)).To(Equal("/explicit/path.db")) }) }) diff --git a/gpbackman/gpbckpconfig/utils_db.go b/gpbackman/gpbckpconfig/utils_db.go index 304c0907..48cc28d7 100644 --- a/gpbackman/gpbckpconfig/utils_db.go +++ b/gpbackman/gpbckpconfig/utils_db.go @@ -21,15 +21,38 @@ package gpbckpconfig import ( "database/sql" + "errors" "fmt" + "os" "strings" "github.com/apache/cloudberry-backup/history" ) -// OpenHistoryDB opens the history backup database. +// OpenHistoryDB opens an existing gpbackup_history.db SQLite database. +// +// The path is opened with the SQLite "rw" URI mode so that a missing file +// produces a clear error rather than being silently created as an empty +// database (which would later fail with a confusing "no such table: backups" +// when callers issue queries). Existence is also pre-checked with os.Stat to +// surface a friendly error message that points the caller at the relevant +// flag and environment variables. func OpenHistoryDB(historyDBPath string) (*sql.DB, error) { - db, err := sql.Open("sqlite3", historyDBPath) + if _, err := os.Stat(historyDBPath); err != nil { + if errors.Is(err, os.ErrNotExist) { + return nil, fmt.Errorf( + "gpbackup history database file not found: %s. "+ + "Specify the path via --history-db, run gpbackman from "+ + "the directory that contains gpbackup_history.db, or "+ + "pass --auto-load-history-db to resolve it from "+ + "$COORDINATOR_DATA_DIRECTORY", + historyDBPath, + ) + } + return nil, fmt.Errorf("stat history db %q: %w", historyDBPath, err) + } + // mode=rw opens an existing database for read+write but never creates one. + db, err := sql.Open("sqlite3", "file:"+historyDBPath+"?mode=rw") if err != nil { return nil, err } diff --git a/gpbackman/gpbckpconfig/utils_db_test.go b/gpbackman/gpbckpconfig/utils_db_test.go index 17c6530b..a89e93e9 100644 --- a/gpbackman/gpbckpconfig/utils_db_test.go +++ b/gpbackman/gpbckpconfig/utils_db_test.go @@ -20,7 +20,10 @@ under the License. package gpbckpconfig import ( + "database/sql" "fmt" + "os" + "path/filepath" "github.com/apache/cloudberry-backup/history" . "github.com/onsi/ginkgo/v2" @@ -28,6 +31,41 @@ import ( ) var _ = Describe("utils_db tests", func() { + Describe("OpenHistoryDB", func() { + It("returns a friendly error and does not create a file when the path does not exist", func() { + tempDir := GinkgoT().TempDir() + missing := filepath.Join(tempDir, "does-not-exist.db") + + db, err := OpenHistoryDB(missing) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("not found")) + Expect(err.Error()).To(ContainSubstring("--history-db")) + Expect(db).To(BeNil()) + + // Critical regression: no empty SQLite file must have been created. + _, statErr := os.Stat(missing) + Expect(os.IsNotExist(statErr)).To(BeTrue(), "OpenHistoryDB must not create the file when it is missing") + }) + + It("opens an existing SQLite history database successfully", func() { + tempDir := GinkgoT().TempDir() + path := filepath.Join(tempDir, "gpbackup_history.db") + + // Seed an existing (but empty) SQLite file via the rwc URI mode. + seed, err := sql.Open("sqlite3", "file:"+path+"?mode=rwc") + Expect(err).NotTo(HaveOccurred()) + Expect(seed.Ping()).To(Succeed()) + Expect(seed.Close()).To(Succeed()) + + db, err := OpenHistoryDB(path) + Expect(err).NotTo(HaveOccurred()) + Expect(db).NotTo(BeNil()) + Expect(db.Ping()).To(Succeed()) + Expect(db.Close()).To(Succeed()) + }) + }) + Describe("getBackupNameQuery", func() { It("returns correct query for various flag combinations", func() { tests := []struct {