Merge pull request #17193 from cpuguy83/refactor_store_errors

Refactor volume store's error usage
This commit is contained in:
Alexander Morozov 2015-11-11 10:25:00 -08:00
commit 2bb1c530d7
5 changed files with 80 additions and 30 deletions

View file

@ -7,7 +7,7 @@ import (
"github.com/Sirupsen/logrus"
derr "github.com/docker/docker/errors"
"github.com/docker/docker/volume/store"
volumestore "github.com/docker/docker/volume/store"
)
// ContainerRmConfig is a holder for passing in runtime config.
@ -153,7 +153,7 @@ func (daemon *Daemon) VolumeRm(name string) error {
return err
}
if err := daemon.volumes.Remove(v); err != nil {
if err == store.ErrVolumeInUse {
if volumestore.IsInUse(err) {
return derr.ErrorCodeRmVolumeInUse.WithArgs(err)
}
return derr.ErrorCodeRmVolume.WithArgs(name, err)

View file

@ -4,7 +4,7 @@ import (
"strings"
derr "github.com/docker/docker/errors"
"github.com/docker/docker/volume/store"
volumestore "github.com/docker/docker/volume/store"
)
func (daemon *Daemon) prepareMountPoints(container *Container) error {
@ -34,7 +34,7 @@ func (daemon *Daemon) removeMountPoints(container *Container, rm bool) error {
// not an error, but an implementation detail.
// This prevents docker from logging "ERROR: Volume in use"
// where there is another container using the volume.
if err != nil && err != store.ErrVolumeInUse {
if err != nil && !volumestore.IsInUse(err) {
rmErrors = append(rmErrors, err.Error())
}
}

58
volume/store/errors.go Normal file
View file

@ -0,0 +1,58 @@
package store
import "errors"
var (
// errVolumeInUse is a typed error returned when trying to remove a volume that is currently in use by a container
errVolumeInUse = errors.New("volume is in use")
// errNoSuchVolume is a typed error returned if the requested volume doesn't exist in the volume store
errNoSuchVolume = errors.New("no such volume")
// errInvalidName is a typed error returned when creating a volume with a name that is not valid on the platform
errInvalidName = errors.New("volume name is not valid on this platform")
)
// OpErr is the error type returned by functions in the store package. It describes
// the operation, volume name, and error.
type OpErr struct {
// Err is the error that occurred during the operation.
Err error
// Op is the operation which caused the error, such as "create", or "list".
Op string
// Name is the name of the resource being requested for this op, typically the volume name or the driver name.
Name string
}
// Error satifies the built-in error interface type.
func (e *OpErr) Error() string {
if e == nil {
return "<nil>"
}
s := e.Op
if e.Name != "" {
s = s + " " + e.Name
}
s = s + ": " + e.Err.Error()
return s
}
// IsInUse returns a boolean indicating whether the error indicates that a
// volume is in use
func IsInUse(err error) bool {
return isErr(err, errVolumeInUse)
}
// IsNotExist returns a boolean indicating whether the error indicates that the volume does not exist
func IsNotExist(err error) bool {
return isErr(err, errNoSuchVolume)
}
func isErr(err error, expected error) bool {
switch pe := err.(type) {
case nil:
return false
case *OpErr:
err = pe.Err
}
return err == expected
}

View file

@ -1,7 +1,6 @@
package store
import (
"errors"
"sync"
"github.com/Sirupsen/logrus"
@ -10,15 +9,6 @@ import (
"github.com/docker/docker/volume/drivers"
)
var (
// ErrVolumeInUse is a typed error returned when trying to remove a volume that is currently in use by a container
ErrVolumeInUse = errors.New("volume is in use")
// ErrNoSuchVolume is a typed error returned if the requested volume doesn't exist in the volume store
ErrNoSuchVolume = errors.New("no such volume")
// ErrInvalidName is a typed error returned when creating a volume with a name that is not valid on the platform
ErrInvalidName = errors.New("volume name is not valid on this platform")
)
// New initializes a VolumeStore to keep
// reference counting of volumes in the system.
func New() *VolumeStore {
@ -81,7 +71,7 @@ func (s *VolumeStore) Create(name, driverName string, opts map[string]string) (v
vd, err := volumedrivers.GetDriver(driverName)
if err != nil {
return nil, err
return nil, &OpErr{Err: err, Name: driverName, Op: "create"}
}
// Validate the name in a platform-specific manner
@ -90,12 +80,12 @@ func (s *VolumeStore) Create(name, driverName string, opts map[string]string) (v
return nil, err
}
if !valid {
return nil, ErrInvalidName
return nil, &OpErr{Err: errInvalidName, Name: name, Op: "create"}
}
v, err := vd.Create(name, opts)
if err != nil {
return nil, err
return nil, &OpErr{Op: "create", Name: name, Err: err}
}
s.set(name, &volumeCounter{v, 0})
@ -110,7 +100,7 @@ func (s *VolumeStore) Get(name string) (volume.Volume, error) {
vc, exists := s.get(name)
if !exists {
return nil, ErrNoSuchVolume
return nil, &OpErr{Err: errNoSuchVolume, Name: name, Op: "get"}
}
return vc.Volume, nil
}
@ -124,19 +114,19 @@ func (s *VolumeStore) Remove(v volume.Volume) error {
logrus.Debugf("Removing volume reference: driver %s, name %s", v.DriverName(), name)
vc, exists := s.get(name)
if !exists {
return ErrNoSuchVolume
return &OpErr{Err: errNoSuchVolume, Name: name, Op: "remove"}
}
if vc.count > 0 {
return ErrVolumeInUse
return &OpErr{Err: errVolumeInUse, Name: name, Op: "remove"}
}
vd, err := volumedrivers.GetDriver(vc.DriverName())
if err != nil {
return err
return &OpErr{Err: err, Name: vc.DriverName(), Op: "remove"}
}
if err := vd.Remove(vc.Volume); err != nil {
return err
return &OpErr{Err: err, Name: name, Op: "remove"}
}
s.remove(name)

View file

@ -1,6 +1,7 @@
package store
import (
"errors"
"testing"
"github.com/docker/docker/volume"
@ -30,8 +31,8 @@ func TestGet(t *testing.T) {
t.Fatalf("Expected fake1 volume, got %v", v)
}
if _, err := s.Get("fake4"); err != ErrNoSuchVolume {
t.Fatalf("Expected ErrNoSuchVolume error, got %v", err)
if _, err := s.Get("fake4"); !IsNotExist(err) {
t.Fatalf("Expected IsNotExist error, got %v", err)
}
}
@ -54,24 +55,25 @@ func TestCreate(t *testing.T) {
}
_, err = s.Create("fakeError", "fake", map[string]string{"error": "create error"})
if err == nil || err.Error() != "create error" {
t.Fatalf("Expected create error, got %v", err)
expected := &OpErr{Op: "create", Name: "fakeError", Err: errors.New("create error")}
if err != nil && err.Error() != expected.Error() {
t.Fatalf("Expected create fakeError: create error, got %v", err)
}
}
func TestRemove(t *testing.T) {
volumedrivers.Register(vt.FakeDriver{}, "fake")
s := New()
if err := s.Remove(vt.NoopVolume{}); err != ErrNoSuchVolume {
t.Fatalf("Expected ErrNoSuchVolume error, got %v", err)
if err := s.Remove(vt.NoopVolume{}); !IsNotExist(err) {
t.Fatalf("Expected IsNotExist error, got %v", err)
}
v, err := s.Create("fake1", "fake", nil)
if err != nil {
t.Fatal(err)
}
s.Increment(v)
if err := s.Remove(v); err != ErrVolumeInUse {
t.Fatalf("Expected ErrVolumeInUse error, got %v", err)
if err := s.Remove(v); !IsInUse(err) {
t.Fatalf("Expected IsInUse error, got %v", err)
}
s.Decrement(v)
if err := s.Remove(v); err != nil {