dilim/containerd
1
0
Fork 0
mirror of https://github.com/containerd/containerd.git synced 2026-08-09 09:33:06 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: close boltdb on metadata and mount plugin close

Browse Source 
Co-authored-by: Rob Murray <rob.murray@docker.com>
Signed-off-by: Austin Vazquez <austin.vazquez@docker.com>
This commit is contained in:
Austin Vazquez
2026-05-05 08:42:17 -05:00
committed by k8s-infra-cherrypick-robot
parent 00f509ed9c
commit 1d601271a7
4 changed files with 73 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
core/metadata/db.go
View File

@@ -21,6 +21,7 @@ import (
"encoding/binary"
"errors"
"fmt"
"io"
"strings"
"sync"
"sync/atomic"
@@ -139,6 +140,17 @@ func NewDB(db Transactor, cs content.Store, ss map[string]snapshots.Snapshotter,
return m
}
// Close closes the underlying bolt database.
// Acquires wlock so no GC cycle is in progress when bolt is closed.
func (m *DB) Close() error {
m.wlock.Lock()
defer m.wlock.Unlock()
if c, ok := m.db.(io.Closer); ok {
return c.Close()
}
return nil
}
// Init ensures the database is at the correct version
// and performs any needed migrations.
func (m *DB) Init(ctx context.Context) error {

18
core/metadata/db_test.go
View File

@@ -36,6 +36,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
bolt "go.etcd.io/bbolt"
bolterr "go.etcd.io/bbolt/errors"
"github.com/containerd/containerd/v2/core/containers"
"github.com/containerd/containerd/v2/core/content"
@@ -103,7 +104,7 @@ func testDB(t *testing.T, opt ...testOpt) (context.Context, *DB) {
require.NoError(t, db.Init(ctx))
t.Cleanup(func() {
assert.NoError(t, bdb.Close())
assert.NoError(t, db.Close())
})
return ctx, db
@@ -904,6 +905,19 @@ func newStores(t testing.TB) (*DB, content.Store, snapshots.Snapshotter, func())
return mdb, mdb.ContentStore(), mdb.Snapshotter("native"), func() {
nsn.Close()
db.Close()
mdb.Close()
}
}
func TestClose(t *testing.T) {
td := t.TempDir()
bdb, err := bolt.Open(filepath.Join(td, "metadata.db"), 0644, nil)
require.NoError(t, err)
db := NewDB(bdb, nil, nil)
require.NoError(t, db.Close())
// Verify the underlying bolt DB is closed: a new transaction should fail.
_, err = bdb.Begin(false)
assert.ErrorIs(t, err, bolterr.ErrDatabaseNotOpen)
}

1
core/mount/manager/manager.go
View File

@@ -125,6 +125,7 @@ func (mm *mountManager) Close() error {
errs = append(errs, err)
}
}
errs = append(errs, mm.db.Close())
return errors.Join(errs...)
}

74
core/mount/manager/manager_test.go
View File

@@ -37,18 +37,12 @@ import (
"github.com/containerd/containerd/v2/pkg/testutil"
bolt "go.etcd.io/bbolt"
bolterr "go.etcd.io/bbolt/errors"
)
func TestManager(t *testing.T) {
testutil.RequiresRoot(t)
td := t.TempDir()
metadb := filepath.Join(td, "mounts.db")
targetdir := filepath.Join(td, "m")
db, err := bolt.Open(metadb, 0600, nil)
if err != nil {
t.Fatal(err)
}
defer db.Close()
ctx := namespaces.WithNamespace(context.Background(), "test")
sourcedir := filepath.Join(td, "source")
@@ -63,26 +57,35 @@ func TestManager(t *testing.T) {
},
}
t.Run("ActivateNoMounts", func(t *testing.T) {
m, err := NewManager(db, targetdir)
// newManager creates a fresh bolt DB and mount manager for each subtest so
// that closing the manager (which now closes the DB) does not affect sibling
// subtests. It returns the manager and the target directory it was given.
newManager := func(t *testing.T, opts ...Opt) (mount.Manager, string) {
t.Helper()
subtd := t.TempDir()
db, err := bolt.Open(filepath.Join(subtd, "mounts.db"), 0600, nil)
require.NoError(t, err)
t.Cleanup(func() { m.(io.Closer).Close() })
_, err = m.Activate(ctx, "id1", []mount.Mount{})
targetdir := filepath.Join(subtd, "m")
m, err := NewManager(db, targetdir, opts...)
require.NoError(t, err)
t.Cleanup(func() { assert.NoError(t, m.(io.Closer).Close()) })
return m, targetdir
}
t.Run("ActivateNoMounts", func(t *testing.T) {
m, _ := newManager(t)
_, err := m.Activate(ctx, "id1", []mount.Mount{})
assert.ErrorIs(t, err, errdefs.ErrNotImplemented)
})
t.Run("SystemOnly", func(t *testing.T) {
m, err := NewManager(db, targetdir)
require.NoError(t, err)
t.Cleanup(func() { m.(io.Closer).Close() })
_, err = m.Activate(ctx, "id1", mounts)
m, _ := newManager(t)
_, err := m.Activate(ctx, "id1", mounts)
assert.ErrorIs(t, err, errdefs.ErrNotImplemented)
})
t.Run("SystemOverride", func(t *testing.T) {
m, err := NewManager(db, targetdir, WithMountHandler("bind", &noopHandler{mounts: &atomic.Int32{}}))
require.NoError(t, err)
t.Cleanup(func() { m.(io.Closer).Close() })
m, targetdir := newManager(t, WithMountHandler("bind", &noopHandler{mounts: &atomic.Int32{}}))
ainfo, err := m.Activate(ctx, "id1", mounts)
require.NoError(t, err)
defer assert.NoError(t, m.Deactivate(ctx, "id1"))
@@ -245,7 +248,6 @@ func TestGC(t *testing.T) {
if err != nil {
t.Fatal(err)
}
defer db.Close()
ctx := namespaces.WithNamespace(context.Background(), "test")
sourcedir := filepath.Join(td, "source")
@@ -255,7 +257,9 @@ func TestGC(t *testing.T) {
mountC := new(atomic.Int32)
m, err := NewManager(db, targetdir, WithMountHandler("noop", &noopHandler{mounts: mountC}), WithMountHandler("error", &errOnceHandler{mounts: mountC, mounted: make(map[string]struct{})}))
require.NoError(t, err)
t.Cleanup(func() { m.(io.Closer).Close() })
t.Cleanup(func() {
assert.NoError(t, m.(io.Closer).Close())
})
for i, run := range tc.gcruns {
for j, mnt := range run.a {
@@ -340,13 +344,12 @@ func TestActivateAlreadyExists(t *testing.T) {
targetdir := filepath.Join(td, "m")
db, err := bolt.Open(metadb, 0600, nil)
require.NoError(t, err)
t.Cleanup(func() { db.Close() })
ctx := namespaces.WithNamespace(context.Background(), "test")
mountC := new(atomic.Int32)
m, err := NewManager(db, targetdir, WithMountHandler("noop", &noopHandler{mounts: mountC}))
require.NoError(t, err)
t.Cleanup(func() { m.(io.Closer).Close() })
t.Cleanup(func() { assert.NoError(t, m.(io.Closer).Close()) })
mounts := []mount.Mount{{Type: "noop"}}
@@ -374,7 +377,6 @@ func TestActivateStaleIncomplete(t *testing.T) {
targetdir := filepath.Join(td, "m")
db, err := bolt.Open(metadb, 0600, nil)
require.NoError(t, err)
t.Cleanup(func() { db.Close() })
ctx := namespaces.WithNamespace(context.Background(), "test")
// Simulate a stale incomplete activation by directly writing a bucket
@@ -415,7 +417,7 @@ func TestActivateStaleIncomplete(t *testing.T) {
mountC := new(atomic.Int32)
m, err := NewManager(db, targetdir, WithMountHandler("noop", &noopHandler{mounts: mountC}))
require.NoError(t, err)
t.Cleanup(func() { m.(io.Closer).Close() })
t.Cleanup(func() { assert.NoError(t, m.(io.Closer).Close()) })
mounts := []mount.Mount{{Type: "noop"}}
@@ -439,13 +441,12 @@ func TestInfo(t *testing.T) {
targetdir := filepath.Join(td, "m")
db, err := bolt.Open(metadb, 0600, nil)
require.NoError(t, err)
t.Cleanup(func() { db.Close() })
ctx := namespaces.WithNamespace(context.Background(), "test")
mountC := new(atomic.Int32)
m, err := NewManager(db, targetdir, WithMountHandler("noop", &noopHandler{mounts: mountC}))
require.NoError(t, err)
t.Cleanup(func() { m.(io.Closer).Close() })
t.Cleanup(func() { assert.NoError(t, m.(io.Closer).Close()) })
// Info on non-existent mount should return ErrNotFound
_, err = m.Info(ctx, "nonexistent")
@@ -506,14 +507,13 @@ func TestInfoSystemMounts(t *testing.T) {
targetdir := filepath.Join(td, "m")
db, err := bolt.Open(metadb, 0600, nil)
require.NoError(t, err)
t.Cleanup(func() { db.Close() })
ctx := namespaces.WithNamespace(context.Background(), "test")
mountC := new(atomic.Int32)
// Only register a handler for "noop"; "bind" will pass through as a system mount
m, err := NewManager(db, targetdir, WithMountHandler("noop", &noopHandler{mounts: mountC}))
require.NoError(t, err)
t.Cleanup(func() { m.(io.Closer).Close() })
t.Cleanup(func() { assert.NoError(t, m.(io.Closer).Close()) })
sourcedir := filepath.Join(td, "source")
require.NoError(t, os.Mkdir(sourcedir, 0700))
@@ -565,13 +565,12 @@ func TestActivateConcurrentSameName(t *testing.T) {
targetdir := filepath.Join(td, "m")
db, err := bolt.Open(metadb, 0600, nil)
require.NoError(t, err)
t.Cleanup(func() { db.Close() })
ctx := namespaces.WithNamespace(context.Background(), "test")
mountC := new(atomic.Int32)
m, err := NewManager(db, targetdir, WithMountHandler("noop", &noopHandler{mounts: mountC}))
require.NoError(t, err)
t.Cleanup(func() { m.(io.Closer).Close() })
t.Cleanup(func() { assert.NoError(t, m.(io.Closer).Close()) })
mounts := []mount.Mount{{Type: "noop"}}
@@ -603,3 +602,18 @@ func TestActivateConcurrentSameName(t *testing.T) {
// TODO: Test deactivate
// TODO: Test Sync
func TestClose(t *testing.T) {
td := t.TempDir()
db, err := bolt.Open(filepath.Join(td, "mounts.db"), 0600, nil)
require.NoError(t, err)
m, err := NewManager(db, filepath.Join(td, "m"))
require.NoError(t, err)
require.NoError(t, m.(io.Closer).Close())
// Verify the underlying bolt DB is closed: a new transaction should fail.
_, err = db.Begin(false)
assert.ErrorIs(t, err, bolterr.ErrDatabaseNotOpen)
}