Skip to content

Commit adf5c64

Browse files
mxpvmxpv
committed
devmapper: don't create or reload thin-pool from snapshotter
Signed-off-by: Maksym Pavlenko <[email protected]>
1 parent 7efda48 commit adf5c64

5 files changed

Lines changed: 33 additions & 161 deletions

File tree

‎snapshots/devmapper/config.go‎

Lines changed: 5 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -23,23 +23,11 @@ import (
2323
"os"
2424

2525
"github.com/BurntSushi/toml"
26-
"github.com/containerd/containerd/snapshots/devmapper/dmsetup"
2726
"github.com/docker/go-units"
2827
"github.com/hashicorp/go-multierror"
2928
"github.com/pkg/errors"
3029
)
3130

32-
const (
33-
// See https://www.kernel.org/doc/Documentation/device-mapper/thin-provisioning.txt for details
34-
dataBlockMinSize = 128
35-
dataBlockMaxSize = 2097152
36-
)
37-
38-
var (
39-
errInvalidBlockSize = errors.Errorf("block size should be between %d and %d", dataBlockMinSize, dataBlockMaxSize)
40-
errInvalidBlockAlignment = errors.Errorf("block size should be multiple of %d sectors", dataBlockMinSize)
41-
)
42-
4331
// Config represents device mapper configuration loaded from file.
4432
// Size units can be specified in human-readable string format (like "32KIB", "32GB", "32Tb")
4533
type Config struct {
@@ -49,19 +37,6 @@ type Config struct {
4937
// Name for 'thin-pool' device to be used by snapshotter (without /dev/mapper/ prefix)
5038
PoolName string `toml:"pool_name"`
5139

52-
// Path to data volume to be used by thin-pool
53-
DataDevice string `toml:"data_device"`
54-
55-
// Path to metadata volume to be used by thin-pool
56-
MetadataDevice string `toml:"meta_device"`
57-
58-
// The size of allocation chunks in data file.
59-
// Must be between 128 sectors (64KB) and 2097152 sectors (1GB) and a multiple of 128 sectors (64KB)
60-
// Block size can't be changed after pool created.
61-
// See https://www.kernel.org/doc/Documentation/device-mapper/thin-provisioning.txt
62-
DataBlockSize string `toml:"data_block_size"`
63-
DataBlockSizeSectors uint32 `toml:"-"`
64-
6540
// Defines how much space to allocate when creating base image for container
6641
BaseImageSize string `toml:"base_image_size"`
6742
BaseImageSizeBytes uint64 `toml:"-"`
@@ -94,23 +69,13 @@ func LoadConfig(path string) (*Config, error) {
9469
}
9570

9671
func (c *Config) parse() error {
97-
var result *multierror.Error
98-
99-
if c.DataBlockSize != "" {
100-
if blockSize, err := units.RAMInBytes(c.DataBlockSize); err != nil {
101-
result = multierror.Append(result, errors.Wrapf(err, "failed to parse data block size: %q", c.DataBlockSize))
102-
} else {
103-
c.DataBlockSizeSectors = uint32(blockSize / dmsetup.SectorSize)
104-
}
105-
}
106-
107-
if baseImageSize, err := units.RAMInBytes(c.BaseImageSize); err != nil {
108-
result = multierror.Append(result, errors.Wrapf(err, "failed to parse base image size: %q", c.BaseImageSize))
109-
} else {
110-
c.BaseImageSizeBytes = uint64(baseImageSize)
72+
baseImageSize, err := units.RAMInBytes(c.BaseImageSize)
73+
if err != nil {
74+
return errors.Wrapf(err, "failed to parse base image size: '%s'", c.BaseImageSize)
11175
}
11276

113-
return result.ErrorOrNil()
77+
c.BaseImageSizeBytes = uint64(baseImageSize)
78+
return nil
11479
}
11580

11681
// Validate makes sure configuration fields are valid
@@ -129,32 +94,5 @@ func (c *Config) Validate() error {
12994
result = multierror.Append(result, fmt.Errorf("base_image_size is required"))
13095
}
13196

132-
// The following fields are required only if we want to create or reload pool.
133-
// Otherwise existing pool with 'PoolName' (prepared in advance) can be used by snapshotter.
134-
if c.DataDevice != "" || c.MetadataDevice != "" || c.DataBlockSize != "" || c.DataBlockSizeSectors != 0 {
135-
strChecks := []struct {
136-
field string
137-
name string
138-
}{
139-
{c.DataDevice, "data_device"},
140-
{c.MetadataDevice, "meta_device"},
141-
{c.DataBlockSize, "data_block_size"},
142-
}
143-
144-
for _, check := range strChecks {
145-
if check.field == "" {
146-
result = multierror.Append(result, errors.Errorf("%s is empty", check.name))
147-
}
148-
}
149-
150-
if c.DataBlockSizeSectors < dataBlockMinSize || c.DataBlockSizeSectors > dataBlockMaxSize {
151-
result = multierror.Append(result, errInvalidBlockSize)
152-
}
153-
154-
if c.DataBlockSizeSectors%dataBlockMinSize != 0 {
155-
result = multierror.Append(result, errInvalidBlockAlignment)
156-
}
157-
}
158-
15997
return result.ErrorOrNil()
16098
}

‎snapshots/devmapper/config_test.go‎

Lines changed: 6 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ package devmapper
2121
import (
2222
"io/ioutil"
2323
"os"
24-
"strings"
2524
"testing"
2625

2726
"github.com/BurntSushi/toml"
@@ -32,12 +31,9 @@ import (
3231

3332
func TestLoadConfig(t *testing.T) {
3433
expected := Config{
35-
RootPath: "/tmp",
36-
PoolName: "test",
37-
DataDevice: "/dev/loop0",
38-
MetadataDevice: "/dev/loop1",
39-
DataBlockSize: "1mb",
40-
BaseImageSize: "128Mb",
34+
RootPath: "/tmp",
35+
PoolName: "test",
36+
BaseImageSize: "128Mb",
4137
}
4238

4339
file, err := ioutil.TempFile("", "devmapper-config-")
@@ -60,12 +56,8 @@ func TestLoadConfig(t *testing.T) {
6056

6157
assert.Equal(t, loaded.RootPath, expected.RootPath)
6258
assert.Equal(t, loaded.PoolName, expected.PoolName)
63-
assert.Equal(t, loaded.DataDevice, expected.DataDevice)
64-
assert.Equal(t, loaded.MetadataDevice, expected.MetadataDevice)
65-
assert.Equal(t, loaded.DataBlockSize, expected.DataBlockSize)
6659
assert.Equal(t, loaded.BaseImageSize, expected.BaseImageSize)
6760

68-
assert.Assert(t, loaded.DataBlockSizeSectors == 1*1024*1024/512)
6961
assert.Assert(t, loaded.BaseImageSizeBytes == 128*1024*1024)
7062
}
7163

@@ -79,37 +71,24 @@ func TestLoadConfigInvalidPath(t *testing.T) {
7971

8072
func TestParseInvalidData(t *testing.T) {
8173
config := Config{
82-
DataBlockSize: "x",
8374
BaseImageSize: "y",
8475
}
8576

8677
err := config.parse()
87-
assert.Assert(t, err != nil)
88-
89-
multErr := (err).(*multierror.Error)
90-
assert.Assert(t, is.Len(multErr.Errors, 2))
91-
92-
assert.Assert(t, strings.Contains(multErr.Errors[0].Error(), "failed to parse data block size: \"x\""))
93-
assert.Assert(t, strings.Contains(multErr.Errors[1].Error(), "failed to parse base image size: \"y\""))
78+
assert.Error(t, err, "failed to parse base image size: 'y': invalid size: 'y'")
9479
}
9580

9681
func TestFieldValidation(t *testing.T) {
97-
config := &Config{DataBlockSizeSectors: 1}
82+
config := &Config{}
9883
err := config.Validate()
9984
assert.Assert(t, err != nil)
10085

10186
multErr := (err).(*multierror.Error)
102-
assert.Assert(t, is.Len(multErr.Errors, 8))
87+
assert.Assert(t, is.Len(multErr.Errors, 3))
10388

10489
assert.Assert(t, multErr.Errors[0] != nil, "pool_name is empty")
10590
assert.Assert(t, multErr.Errors[1] != nil, "root_path is empty")
10691
assert.Assert(t, multErr.Errors[2] != nil, "base_image_size is empty")
107-
assert.Assert(t, multErr.Errors[3] != nil, "data_device is empty")
108-
assert.Assert(t, multErr.Errors[4] != nil, "meta_device is empty")
109-
assert.Assert(t, multErr.Errors[5] != nil, "data_block_size is empty")
110-
111-
assert.Equal(t, multErr.Errors[6], errInvalidBlockSize)
112-
assert.Equal(t, multErr.Errors[7], errInvalidBlockAlignment)
11392
}
11493

11594
func TestExistingPoolFieldValidation(t *testing.T) {

‎snapshots/devmapper/pool_device.go‎

Lines changed: 4 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ package devmapper
2020

2121
import (
2222
"context"
23-
"os"
2423
"path/filepath"
2524

2625
"github.com/containerd/containerd/log"
@@ -54,60 +53,17 @@ func NewPoolDevice(ctx context.Context, config *Config) (*PoolDevice, error) {
5453
return nil, err
5554
}
5655

57-
if err := openPool(ctx, config); err != nil {
58-
return nil, err
56+
// Make sure pool exists and available
57+
poolPath := dmsetup.GetFullDevicePath(config.PoolName)
58+
if _, err := dmsetup.Info(poolPath); err != nil {
59+
return nil, errors.Wrapf(err, "failed to query pool %q", poolPath)
5960
}
60-
6161
return &PoolDevice{
6262
poolName: config.PoolName,
6363
metadata: poolMetaStore,
6464
}, nil
6565
}
6666

67-
func openPool(ctx context.Context, config *Config) error {
68-
if err := config.Validate(); err != nil {
69-
return err
70-
}
71-
72-
var (
73-
poolPath = dmsetup.GetFullDevicePath(config.PoolName)
74-
poolExists = false
75-
)
76-
77-
if _, err := os.Stat(poolPath); err != nil && !os.IsNotExist(err) {
78-
return errors.Wrapf(err, "failed to stat for %q", poolPath)
79-
} else if err == nil {
80-
poolExists = true
81-
}
82-
83-
// Create new pool if not exists
84-
if !poolExists {
85-
log.G(ctx).Debug("creating new pool device")
86-
if err := dmsetup.CreatePool(config.PoolName, config.DataDevice, config.MetadataDevice, config.DataBlockSizeSectors); err != nil {
87-
return errors.Wrapf(err, "failed to create thin-pool with name %q", config.PoolName)
88-
}
89-
90-
return nil
91-
}
92-
93-
// Pool exists, check if it needs to be reloaded
94-
if config.DataDevice != "" && config.MetadataDevice != "" {
95-
log.G(ctx).Debugf("reloading existing pool %q", poolPath)
96-
if err := dmsetup.ReloadPool(config.PoolName, config.DataDevice, config.MetadataDevice, config.DataBlockSizeSectors); err != nil {
97-
return errors.Wrapf(err, "failed to reload pool %q", config.PoolName)
98-
}
99-
100-
return nil
101-
}
102-
103-
// If data and meta devices are not provided, use existing pool. Query info to make sure it's OK.
104-
if _, err := dmsetup.Info(poolPath); err != nil {
105-
return errors.Wrapf(err, "failed to query info for existing pool %q", poolPath)
106-
}
107-
108-
return nil
109-
}
110-
11167
// transition invokes 'updateStateFn' callback to perform devmapper operation and reflects device state changes/errors in meta store.
11268
// 'tryingState' will be set before invoking callback. If callback succeeded 'successState' will be set, otherwise
11369
// error details will be recorded in meta store.

‎snapshots/devmapper/pool_device_test.go‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,13 @@ package devmapper
2020

2121
import (
2222
"context"
23+
"fmt"
2324
"io/ioutil"
2425
"os"
2526
"os/exec"
2627
"path/filepath"
2728
"testing"
29+
"time"
2830

2931
"github.com/containerd/containerd/pkg/testutil"
3032
"github.com/containerd/containerd/snapshots/devmapper/dmsetup"
@@ -65,6 +67,10 @@ func TestPoolDevice(t *testing.T) {
6567
_, loopDataDevice := createLoopbackDevice(t, tempDir)
6668
_, loopMetaDevice := createLoopbackDevice(t, tempDir)
6769

70+
poolName := fmt.Sprintf("test-pool-device-%d", time.Now().Nanosecond())
71+
err = dmsetup.CreatePool(poolName, loopDataDevice, loopMetaDevice, 64*1024/dmsetup.SectorSize)
72+
assert.NilError(t, err, "failed to create pool %q", poolName)
73+
6874
defer func() {
6975
// Detach loop devices and remove images
7076
err := losetup.DetachLoopDevice(loopDataDevice, loopMetaDevice)
@@ -75,14 +81,10 @@ func TestPoolDevice(t *testing.T) {
7581
}()
7682

7783
config := &Config{
78-
PoolName: "test-pool-device-1",
79-
RootPath: tempDir,
80-
DataDevice: loopDataDevice,
81-
MetadataDevice: loopMetaDevice,
82-
DataBlockSize: "65536",
83-
DataBlockSizeSectors: 128,
84-
BaseImageSize: "16mb",
85-
BaseImageSizeBytes: 16 * 1024 * 1024,
84+
PoolName: poolName,
85+
RootPath: tempDir,
86+
BaseImageSize: "16mb",
87+
BaseImageSizeBytes: 16 * 1024 * 1024,
8688
}
8789

8890
pool, err := NewPoolDevice(ctx, config)

‎snapshots/devmapper/snapshotter_test.go‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"github.com/containerd/containerd/snapshots/devmapper/dmsetup"
3131
"github.com/containerd/containerd/snapshots/devmapper/losetup"
3232
"github.com/containerd/containerd/snapshots/testsuite"
33+
"github.com/hashicorp/go-multierror"
3334
"github.com/sirupsen/logrus"
3435
"gotest.tools/assert"
3536
)
@@ -59,22 +60,18 @@ func TestSnapshotterSuite(t *testing.T) {
5960
return nil, nil, err
6061
}
6162

62-
// Remove device mapper pool after test completes
63+
// Remove device mapper pool and detach loop devices after test completes
6364
removePool := func() error {
64-
return snap.pool.RemovePool(ctx)
65+
result := multierror.Append(
66+
snap.pool.RemovePool(ctx),
67+
losetup.DetachLoopDevice(loopDataDevice, loopMetaDevice))
68+
69+
return result.ErrorOrNil()
6570
}
6671

6772
// Pool cleanup should be called before closing metadata store (as we need to retrieve device names)
6873
snap.cleanupFn = append([]closeFunc{removePool}, snap.cleanupFn...)
6974

70-
return snap, func() error {
71-
err := snap.Close()
72-
assert.NilError(t, err, "failed to close snapshotter")
73-
74-
err = losetup.DetachLoopDevice(loopDataDevice, loopMetaDevice)
75-
assert.NilError(t, err, "failed to detach loop devices")
76-
77-
return err
78-
}, nil
75+
return snap, snap.Close, nil
7976
})
8077
}

0 commit comments

Comments
 (0)