From 29ae51600627f57132d12fc10215fe03c62ec3cf Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Tue, 10 Oct 2023 14:24:48 +0200 Subject: [PATCH] use sqlite as default database Use sqlite as default but for upgrades it will still use boltdb to avoid breaking anyone. This is done by checking if the boltdb file already exists and if it does then we have to use it. I added a e2e test to check the new logic and removed the system test for it, the problem with the system test is that we share the storage dir there so all following commands without --db-backend would try to use boltdb as a single --db-backend boltdb command will create the file and then all folllwing commands will use it because of the backwards compat. In e2e tests each test uses their own --root so it is not an issue there. Signed-off-by: Paul Holzinger --- libpod/boltdb_state.go | 1 + libpod/runtime.go | 75 +++++++++++++++++++++++---------------- libpod/sqlite_state.go | 1 + test/e2e/info_test.go | 30 ++++++++++++++++ test/system/005-info.bats | 12 ------- 5 files changed, 76 insertions(+), 43 deletions(-) diff --git a/libpod/boltdb_state.go b/libpod/boltdb_state.go index 3edd6e6d47..f1888b6a18 100644 --- a/libpod/boltdb_state.go +++ b/libpod/boltdb_state.go @@ -69,6 +69,7 @@ type BoltState struct { // NewBoltState creates a new bolt-backed state database func NewBoltState(path string, runtime *Runtime) (State, error) { + logrus.Info("Using boltdb as database backend") state := new(BoltState) state.dbPath = path state.runtime = runtime diff --git a/libpod/runtime.go b/libpod/runtime.go index 850c697eb4..27e2e05dfe 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -5,6 +5,7 @@ import ( "context" "errors" "fmt" + "io/fs" "os" "path/filepath" "strings" @@ -294,6 +295,48 @@ func getLockManager(runtime *Runtime) (lock.Manager, error) { return manager, nil } +func getDBState(runtime *Runtime) (State, error) { + // TODO - if we further break out the state implementation into + // libpod/state, the config could take care of the code below. It + // would further allow to move the types and consts into a coherent + // package. + backend, err := config.ParseDBBackend(runtime.config.Engine.DBBackend) + if err != nil { + return nil, err + } + + // get default boltdb path + baseDir := runtime.config.Engine.StaticDir + if runtime.storageConfig.TransientStore { + baseDir = runtime.config.Engine.TmpDir + } + boltDBPath := filepath.Join(baseDir, "bolt_state.db") + + switch backend { + case config.DBBackendDefault: + // for backwards compatibility check if boltdb exists, if it does not we use sqlite + if _, err := os.Stat(boltDBPath); err != nil { + if errors.Is(err, fs.ErrNotExist) { + // need to set DBBackend string so podman info will show the backend name correctly + runtime.config.Engine.DBBackend = config.DBBackendSQLite.String() + return NewSqliteState(runtime) + } + // Return error here some other problem with the boltdb file, rather than silently + // switch to sqlite which would be hard to debug for the user return the error back + // as this likely a real bug. + return nil, err + } + runtime.config.Engine.DBBackend = config.DBBackendBoltDB.String() + fallthrough + case config.DBBackendBoltDB: + return NewBoltState(boltDBPath, runtime) + case config.DBBackendSQLite: + return NewSqliteState(runtime) + default: + return nil, fmt.Errorf("unrecognized database backend passed (%q): %w", backend.String(), define.ErrInvalidArg) + } +} + // Make a new runtime based on the given configuration // Sets up containers/storage, state store, OCI runtime func makeRuntime(runtime *Runtime) (retErr error) { @@ -335,40 +378,10 @@ func makeRuntime(runtime *Runtime) (retErr error) { } // Set up the state. - // - // TODO: We probably need a "default" type that will select BoltDB if - // a DB exists already, and SQLite otherwise. - // - // TODO - if we further break out the state implementation into - // libpod/state, the config could take care of the code below. It - // would further allow to move the types and consts into a coherent - // package. - backend, err := config.ParseDBBackend(runtime.config.Engine.DBBackend) + runtime.state, err = getDBState(runtime) if err != nil { return err } - switch backend { - case config.DBBackendBoltDB: - baseDir := runtime.config.Engine.StaticDir - if runtime.storageConfig.TransientStore { - baseDir = runtime.config.Engine.TmpDir - } - dbPath := filepath.Join(baseDir, "bolt_state.db") - - state, err := NewBoltState(dbPath, runtime) - if err != nil { - return err - } - runtime.state = state - case config.DBBackendSQLite: - state, err := NewSqliteState(runtime) - if err != nil { - return err - } - runtime.state = state - default: - return fmt.Errorf("unrecognized state type passed (%v): %w", backend, define.ErrInvalidArg) - } // Grab config from the database so we can reset some defaults dbConfig, err := runtime.state.GetDBConfig() diff --git a/libpod/sqlite_state.go b/libpod/sqlite_state.go index 344137b23b..8643764a5d 100644 --- a/libpod/sqlite_state.go +++ b/libpod/sqlite_state.go @@ -49,6 +49,7 @@ const ( // NewSqliteState creates a new SQLite-backed state database. func NewSqliteState(runtime *Runtime) (_ State, defErr error) { + logrus.Info("Using sqlite as database backend") state := new(SQLiteState) basePath := runtime.storageConfig.GraphRoot diff --git a/test/e2e/info_test.go b/test/e2e/info_test.go index 24c3de22ab..e221caff7d 100644 --- a/test/e2e/info_test.go +++ b/test/e2e/info_test.go @@ -185,6 +185,36 @@ var _ = Describe("Podman Info", func() { Expect(session.OutputToString()).To(Equal(want)) }) + It("podman --db-backend info basic check", func() { + SkipIfRemote("--db-backend only supported on the local client") + type argWant struct { + arg string + want string + } + backends := []argWant{ + // default should be sqlite + {arg: "", want: "sqlite"}, + {arg: "boltdb", want: "boltdb"}, + // now because a boltdb exists it should use boltdb when default is requested + {arg: "", want: "boltdb"}, + {arg: "sqlite", want: "sqlite"}, + } + + for _, tt := range backends { + session := podmanTest.Podman([]string{"--db-backend", tt.arg, "--log-level=info", "info", "--format", "{{.Host.DatabaseBackend}}"}) + session.WaitWithDefaultTimeout() + Expect(session).To(Exit(0)) + Expect(session.OutputToString()).To(Equal(tt.want)) + Expect(session.ErrorToString()).To(ContainSubstring("Using %s as database backend", tt.want)) + } + + // make sure we get an error for bogus values + session := podmanTest.Podman([]string{"--db-backend", "bogus", "info", "--format", "{{.Host.DatabaseBackend}}"}) + session.WaitWithDefaultTimeout() + Expect(session).To(Exit(125)) + Expect(session.ErrorToString()).To(Equal("Error: unsupported database backend: \"bogus\"")) + }) + It("Podman info: check lock count", Serial, func() { // This should not run on architectures and OSes that use the file locks backend. // Which, for now, is Linux + RISCV and FreeBSD, neither of which are in CI - so diff --git a/test/system/005-info.bats b/test/system/005-info.bats index 54ee820cf0..fa30aad214 100644 --- a/test/system/005-info.bats +++ b/test/system/005-info.bats @@ -171,18 +171,6 @@ host.slirp4netns.executable | $expr_path fi } -@test "podman --db-backend info - basic output" { - # TODO: this tests needs to change once sqlite is being tested in the system tests - skip_if_remote "--db-backend does not work on a remote client" - for backend in boltdb sqlite; do - run_podman --db-backend=$backend info --format "{{ .Host.DatabaseBackend }}" - is "$output" "$backend" - done - - run_podman 125 --db-backend=bogus info --format "{{ .Host.DatabaseBackend }}" - is "$output" "Error: unsupported database backend: \"bogus\"" -} - @test "CONTAINERS_CONF_OVERRIDE" { skip_if_remote "remote does not support CONTAINERS_CONF*"